-
-
Notifications
You must be signed in to change notification settings - Fork 320
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Linux support #585
Linux support #585
Conversation
7329195
to
5204ce4
Compare
62d6bec
to
a5c8cbe
Compare
Codecov Report
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
First of all, thank you very much for taking your time for bringing support for Linux to the project @elliottwilliams 🙏 🚀. It's something that has been in our backlog for a long time and it finally happened.
I think the performance trade-offs that you are making make total sense for now. We can certainly go further with optimizations but we can do that in follow-up PRs if it turns out we need them.
One minor suggestion that I'd make before merging the PR is including a GitHub action that ensures changes compile in Linux. We can also update the README of the project to indicate that the library has support for both, Linux and macOS. Also, would you mind updating the CHANGELOG?
After that, we can merge and I'll release a new major version of the tool. I think this improvement really deserves a major bump :).
Again, thanks a lot for taking your time, I appreciate it a lot and I think the users of the library will do it too.
@all-contributors add @elliottwilliams for code |
@pepibumur I've put up a pull request to add @elliottwilliams! 🎉 |
@pepibumur thanks so much!! Yeah, totally agree with your suggestions. Should I do anything about the swiftlint violations? I can try to fix any this PR adds, but it looks like there are a number of failures from existing code. |
@pepibumur
qq: Would you actually be open to making this a minor version? I don't think this introduces any API breaks, and I think it'd be easier to advocate for upgrading downstream projects if there isn't a major version bump :) |
b871f5b
to
66e446b
Compare
66e446b
to
e56c0da
Compare
I take it back, they were mine! Fixed. |
2989ece
to
c05e0c1
Compare
Swift 5.4 changed something that caused the linux build to (correctly) fail to compile an @objc declaration. We can do the same thing we did to the PBXObject hierarchy in tuist#585, and reimplement the dynamic equality checking using memberwise equality functions generated by sourcery, and a dynamic method on each XCScheme object. Tested the generated interface with these changes: swift build -Xswiftc -emit-module-interface -Xswiftc -swift-version -Xswiftc 5 The only change to the interface is that RemoteRunnable now exposes an `==` method: ```diff + public static func == (lhs: XcodeProj.XCScheme.RemoteRunnable, rhs: XcodeProj.XCScheme.RemoteRunnable) -> Swift.Bool ```
I've been interested in getting XcodeProj compiling and running on Linux. It turns out that it's possible!
Linux support is obviously a little esoteric, but I think it's valuable for users that want to integrate this tool into automated systems which are linux-based. For example: at Yelp, we have a ton of Linux CI capacity and lots of infra that makes it easy to run git-based workflows on linux servers, and being able to use XcodeProj and XcodeGen from them would simplify our tools.
Implementation 👩💻👨💻
Removes XcodeProjCExt, which was added in Optimize bottlenecks #529. This library was based on CommonCrypto which is macOS-only. I've tried to move things back to Swift while retaining the performance gains of the C library:
CommentedString.validString
is a new implementation that operates in a single pass of the string in the optimal case. In my testing I've been able to get this 30% of the performance of the C implementation. If you'd like, I can go further, but it probably means switching to pointer APIs to avoid ARC overhead.String.md5
includes the old implementation, but uses CryptoKit when it's available.Changes the sourcery template to generate non-overriding functions. Swift's language model doesn't support overriding in extensions, only allowing it for backwards compatibility with objc. When compiling on linux, objc support is not present and it fails.
Small fixups:
PBXProjIntegrationTests
on a system with no git author (so that tests pass from a Docker image)Verification
You can build and test this branch on a linux system using Swift's official docker images: