build: add support to vendor dependencies #158
Conversation
|
CC: @yim-lee |
|
@swift-server-bot please test |
|
@swift-server-bot please test |
| message("-- Vending swift-asn1") | ||
| FetchContent_Declare(ASN1 | ||
| GIT_REPOSITORY https://github.com/apple/swift-asn1 | ||
| GIT_TAG 1.1.0) |
There was a problem hiding this comment.
It is highly likely that we will forget to keep this in-sync with Package.swift. Would it be better to require users of this feature to set a version environment variable so they don't use a wrong version by accident?
There was a problem hiding this comment.
That does make it a lot more complicated to use. The intention here is to enable the use of the CMake based build to debug LSP, which requires SPM, which requires this package.
There was a problem hiding this comment.
@yim-lee Can we amend the soundness script to validate that these two match?
There was a problem hiding this comment.
If we look at the rule for swift-crypto:
.package(url: "https://github.com/apple/swift-crypto.git", "2.5.0" ..< "4.0.0"),
What's the swift-crypto version that we would extract from this for comparison/validation?
If we extract from Package.resolved instead, the resolved version may not necessarily match the one in this file. This might be annoying but could work I suppose? I don't think we do swift build or swift package resolve in the soundness check though.
And it's possible for the resolved dependency version to be different depending on how the dependency is included right? i.e., the swift-crypto version when building swift-certificates vs. SwiftPM is probably different, so which Package.resolved should be looked at (and when)?
There was a problem hiding this comment.
Yeah, these are all good questions. I'm just wondering if we can get to a local maximum of "least likely to be outside the band of reasonableness".
When building swift-crypto outside of the toolchain (e.g. to build LSP), it is convenient to support vendoring dependencies. Add support for falling back to local builds of the dependencies when not provided as an existing build.
|
@yim-lee ping? This has been sitting for a while, and is blocking other work. Can we please merge it and address the feedback for tooling in a follow up? |
|
@compnerd Frankly I still prefer that we don't hardcode dependency versions, but I don't have time to look into alternatives. If we must merge this PR as-is, then I feel strongly that at the minimum the owners of this repository should not be held responsible for updating the versions. Can we agree to that? |
|
@yim-lee I think we can agree to that - I expect that the occasional patch to bring this back into sync isn't a big deal, but having it track precisely is not a requirement. |
|
@swift-server-bot test this please |
Co-authored-by: Yim Lee <yim_lee@apple.com>
|
@swift-server-bot test this please |
|
@compnerd OK. I have a question about keeping the versions in-sync with update-checkout and whether we want to use the latest, otherwise I can merge this if you think it's ready. |
|
I think that we could use the latest, but I feel more comfortable with trying to match the update-checkout because it is going to be the closest to what is actually in the toolchain. The primary use of this is for ease of debugging projects like the LSP which cannot be built with SPM on Windows due to SPM limitations. |
|
Yeah, update-checkout doesn't use the latest: https://github.com/apple/swift/blob/main/utils/update_checkout/update-checkout-config.json#L116-L118 |
|
BTW, FTR, this is not the mechanism for building the toolchain - the toolchain builds rely on update-checkout and builds everything from that checkout, which is why drift here is not a major concern - this is a developer productivity solution. |
When building swift-crypto outside of the toolchain (e.g. to build LSP), it is convenient to support vendoring dependencies. Add support for falling back to local builds of the dependencies when not provided as an existing build.