Skip to content

Conversation

@JoeMatt
Copy link

@JoeMatt JoeMatt commented Nov 10, 2022

Initial support for Swift PM/Package.swift

Changes:

  1. Some files needed to be renamed to .mm from .cpp or .m for Obj-C to work correctly
  2. reference counting code wrapped in if !arc
  3. Added some missing explicit imports since SwiftPM doesn't do implicits for Cocoa etc
  4. Moved files around to make SwiftPM happy (every target has a folder, updated in xodeproj too)

Not working:
1. xdelta is failing, added a #if HAVE_XDELTA marco to temp remove xdelta
2. Main macOS app not building, swiftpm dies for some reason

What does work:

  1. macOS commnand line app
  2. macOS app (though can't launch from cmdline, not sure how to make .app's yet)
  3. Shared library for tvOS, iOS and macOS

Test:

swift build
.build/debug/cmdMultiPatch

Why?

I plan on adding automatic patching into my app Provenance https://github.com/Provenance-EMU/Provenance
and now I can include as a Swift Package

Signed-off-by: Joseph Mattello <mail@joemattiello.com>
Signed-off-by: Joseph Mattello <mail@joemattiello.com>
needed for swift pm to work, no implicit imports

Signed-off-by: Joseph Mattello <mail@joemattiello.com>
Signed-off-by: Joseph Mattello <mail@joemattiello.com>
Signed-off-by: Joseph Mattello <mail@joemattiello.com>
Signed-off-by: Joseph Mattello <mail@joemattiello.com>
Signed-off-by: Joseph Mattello <mail@joemattiello.com>
Fixes issues with main app building

Signed-off-by: Joseph Mattello <mail@joemattiello.com>
Signed-off-by: Joseph Mattello <mail@joemattiello.com>
@JoeMatt
Copy link
Author

JoeMatt commented Nov 10, 2022

Just pushed an update that fixes XDelta by moving some files around and making a symlink to the header in a way swift pm prefers.

I updated the paths in the xcodeproj too. Tested that both targets build and run.

@JoeMatt JoeMatt marked this pull request as ready for review November 10, 2022 08:29
Signed-off-by: Joseph Mattello <mail@joemattiello.com>
Signed-off-by: Joseph Mattello <mail@joemattiello.com>
Signed-off-by: Joseph Mattello <mail@joemattiello.com>
Signed-off-by: Joseph Mattello <mail@joemattiello.com>
Signed-off-by: Joseph Mattello <mail@joemattiello.com>
Signed-off-by: Joseph Mattello <mail@joemattiello.com>
Cocoa isn’t used in these files, Foundation is and works cross platform

Signed-off-by: Joseph Mattello <mail@joemattiello.com>
@JoeMatt
Copy link
Author

JoeMatt commented Nov 10, 2022

A few more updates so Package.swift works in XCode.

I'm able to build the cmd line app for iOS target. Kind of useless, but it works.

The main app could work with iOS UI files. It gets all the way to the app target before failing on AppKit classes.

I saw someone mention linux support. I threw in some conditionals for Linux, doubt it would work as-is but it's a starting point if anyone wants to get it to build.

@Sappharad
Copy link
Owner

Sappharad commented Nov 11, 2022

I'm not understanding the reason for some of the file renames.
Only Objective C files that use C++ code need to have the .mm extension. Do tools other than Xcode not handle that properly? If no C++ code is called, it doesn't need .mm in Xcode.

@Sappharad
Copy link
Owner

Never mind, I saw number 1 in the list and I didn't pay close enough attention to the review and see you really only renamed two files.

@Sappharad
Copy link
Owner

I will probably approve this tomorrow or over the weekend.

@JoeMatt Since the new xdelta.m is just an exact copy of the .c version, I don't see a reason to maintain two copies of the same file. While it may make sense just to rename it instead of having two, I also think it's beneficial to keep the original name in the event I ever have to pull in a newer version of the original code since I did end up upgrading XDelta last year for additional file support.

Do you want to make it symlink instead, or should I do it myself after accepting the PR? (Git does track them properly in my experience, as long as someone doesn't use Windows to pull the repo down)

Still wondering if it was even necessary to make the two renames, but I haven't used Swift Package Manager before nor did I realize that it supported languages other than Swift.

@JoeMatt
Copy link
Author

JoeMatt commented Nov 12, 2022

I can test renaming back, but in my experience, the issue comes to how swift pm imports foundation, and Cocoa classes in C/C++.

I have my own project for wrapping xdelta in its own package, maybe I can get that to work.

In the meantime, I'm making a swiftui branch because I want to Port to mobile and it's probably easier just to have one user interface with swiftui, but leaving the old AppKit code for Legacy purposes

@JoeMatt
Copy link
Author

JoeMatt commented Nov 13, 2022

Just checked, using .cpp instead of .mm results in this,

image

@JoeMatt
Copy link
Author

JoeMatt commented Nov 13, 2022

I think you may have a build issue with flips since it's a submodule.

I need a header path that's a subpath without other root folders.
To fix this mkdir flips/include
There's probably a better solution but I'm pressed for time, trying to get my own app release out.

If this is a blocker, lmk, I'll get back to it next week.

@Sappharad
Copy link
Owner

I tried running "swift build" for the first time and I see the include directory problem you mentioned. I'm going to look into it further this week myself; I guess if the package doesn't build out of the box it would be nice for it to work. I could pull the PR into a branch for now but I'm just playing with it locally until there's a satisfactory solution or I've given up and decide not to care and just throw a script in to do the prep work.

I also messed with it for an hour trying to get it to not want that include folder, or create it, but it looks like access to FileManager has been explicitly marked unavailable in Package.swift. I'm open to additional changes / ideas but once I understand it better I'm hoping the best course of action will be more obvious.

@JoeMatt
Copy link
Author

JoeMatt commented Nov 14, 2022

No worries,

There's probably a more "proper" way of putting everything into Sources/Target name and doing things "the apple / swift way" as well.

I have to do some travelling but I want to return to this asap on my own as well, so don't sweat too much on this unless you just want to try it out for fun.

I too am trying to get better with Swift PM now that it's becoming usable.

@JoeMatt JoeMatt marked this pull request as draft November 14, 2022 15:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants