Skip to content
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

Use libSwiftPM 5.6 to parse manifests #302

Merged
merged 2 commits into from
Mar 28, 2022
Merged

Conversation

yonihemi
Copy link
Member

@yonihemi yonihemi commented Mar 28, 2022

Upgrades libSwiftPM from 5.5 to 5.6 + related dependencies' versions (tools-support-core and swift-argument-parser). Fixes #285.

This changes the parsing strategy to rely on libSwiftPM to directly invoke swiftc and parse a compiled manifests' output. While I expected the output of swift package dump-package to be more stable than PackageDescription's JSON output, the opposite is true, and at least between 5.5 and 5.6 that format is backwards compatible. We may have to revisit this if this internal format breaks.

Previous strategy

sequenceDiagram
carton w/ libSwiftPM->>SwiftWasm SwiftPM: (shell) swift package dump-package
SwiftWasm SwiftPM->>SwiftWasm swiftc: build manifest for host platform with host SDK
SwiftWasm swiftc->>SwiftWasm SwiftPM: compiled host manifest
SwiftWasm SwiftPM->>SwiftWasm SwiftPM: parse manifest output
SwiftWasm SwiftPM->>carton w/ libSwiftPM: JSON output
carton w/ libSwiftPM->>carton w/ libSwiftPM: Parse with libSwiftPM
Loading

New strategy

sequenceDiagram
carton w/ libSwiftPM->>SwiftWasm swiftc: build manifest for host platform with host SDK
SwiftWasm swiftc->>carton w/ libSwiftPM: compiled host manifest
carton w/ libSwiftPM->>carton w/ libSwiftPM: parse manifest output
Loading

Alternatives considered:

  • Using libSwiftPM 5.6 to parse 5.6 dump-package output + copy parts of libSwiftPM 5.5 for parsing older dump-package output: This becomes unwieldy fast due to significant code changes with many different types, plus to avoid naming conflicts would need to be isolated to a separate module.
  • Go back to our own parser: we'd still need to support at least 2 formats, plus we'd be missing a way to instantiate a proper Manifest object.

@yonihemi yonihemi added bug Something isn't working dependencies Updates to the project dependencies labels Mar 28, 2022
@yonihemi yonihemi marked this pull request as draft March 28, 2022 04:20
@yonihemi yonihemi marked this pull request as ready for review March 28, 2022 06:47
@yonihemi yonihemi requested a review from a team March 28, 2022 06:47
Copy link
Collaborator

@MaxDesiatov MaxDesiatov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great, thank you so much! Do I understand correctly that this is compatible with 5.5 toolchains as well?

@yonihemi
Copy link
Member Author

Great, thank you so much! Do I understand correctly that this is compatible with 5.5 toolchains as well?

Correct, tested with both 5.5 and 5.6.

@yonihemi yonihemi merged commit b89d7c7 into swiftwasm:main Mar 28, 2022
@yonihemi yonihemi deleted the swiftpm_5_6 branch March 28, 2022 10:13
@SDGGiesbrecht
Copy link

This strategy is compatible with tools version markers as far back as SwiftPM proper supports, but is only compatible with the corresponding patch version of the actual toolchain inside which it expects to reside. If it happens to work between 5.5 and 5.6 by accident, consider yourself lucky, and go ahead and use it for now. Just be aware that you will likely have to switch back to the dump-package strategy in the future if you intend to continue supporting multiple toolchains with the same installation of carton. (Having used SwiftPM this way myself since 4.1, I can confirm that it regularly breaks when new toolchains are released.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working dependencies Updates to the project dependencies
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add support for Swift 5.6 package description format
3 participants