update_checkout: add boringssl to the checkout set#86823
update_checkout: add boringssl to the checkout set#86823compnerd merged 1 commit intoswiftlang:mainfrom
Conversation
|
@swift-ci please smoke test |
|
@swift-ci please smoke test |
|
@marcprux, won't this break Android CI, since we've been checking out this repo separately, as discussed in the linked issue? |
|
It seems like it ought to break the Android and Static Linux SDK builds, and yet Static Linux also checks out its own libxml2 and doesn't break despite that dependency being listed in update-checkout-config.json. WDYT, @al45tair? |
|
Oh, I didn't read that script source carefully enough, 🫠 looks like the
|
finagolfin
left a comment
There was a problem hiding this comment.
The static linux and Android SDKs were already downloading this source separately, so good to pull this repo in in one place. Can you add it for the 6.3 branch also, as we already use it there?
Note that Static Linux just bumped its BoringSSL dependency to a hash aligned with Swift NIO: See swiftlang/swift-docker#488 (comment) and apple/swift-nio-ssl#506. I had been wondering if we should do the same for Android, but for the time being we are still on |
|
Okay, I think that aligning on the same version of boringssl is a good idea. I wonder if we can also somehow manage to not build boringssl twice (or rather thrice). It does take a while to build. |
|
@swift-ci please smoke test |
|
No, this will need to be integrated into build.ps1 before we can restore that change. |
|
Hmm, it appears that 817ab07ebb53da35afea409ab9328f578492832d is not on the GH mirror. I don't know how to add the googlesource repo to the update-checkout. Perhaps @charles-zablit knows how to setup update-checkout to use https://boringssl.googlesource.com/boringssl for the checkout. |
|
AFAIK swift/utils/update_checkout/update-checkout-config.json Lines 2 to 3 in 001e421 |
|
swift/utils/update_checkout/update_checkout/update_checkout.py Lines 631 to 639 in 001e421 looks like it can be overriden with {
"boringssl": {
"remote": {
"id": "google/boringssl",
"url": "https://boringssl.googlesource.com/boringssl"
}
}
} |
This library dependency is already present in the Android SDK builds. Replicate the dependency in the checkouts to allow building for the Android SDK on non-Linux platforms.
|
@swift-ci please smoke test |
|
@swift-ci please smoke test Windows platform |
|
The issue is not the repo used, but that the Windows version of This implies that the source checkout is being done differently with git on Windows, ie there is something different or wrong about the Windows execution of this checkout script. |
On Linux and macOS, there is a warning: Linux and macOS are using Git 2.32, Windows is using Git 2.47. Maybe this warning became an error in more recent versions of Git? The commit hash should be changed to a branch or tag: https://github.com/swiftlang/swift/pull/86823/changes#r2758582368 |
| "mimalloc": "v3.0.3", | ||
| "swift-subprocess": "0.2.1" | ||
| "swift-subprocess": "0.2.1", | ||
| "boringssl": "817ab07ebb53da35afea409ab9328f578492832d" |
There was a problem hiding this comment.
This should be a branch or a tag, not a commit hash.
| "mimalloc": "v3.0.3", | ||
| "swift-subprocess": "0.2.1" | ||
| "swift-subprocess": "0.2.1", | ||
| "boringssl": "817ab07ebb53da35afea409ab9328f578492832d" |
There was a problem hiding this comment.
This should be a branch or a tag, not a commit hash.
Same here.
|
@charles-zablit we cannot use a branch or a tag, the NIO folks discuss the state of boringssl with upstream and choose specific commits, so this must remain a commit hash. I know that I can do this with repo, how do we do this with update-checkout? Alternatively, I would be fine with floating this to a branch if we can float NIO as well. |
|
Looking into this closer: Once the patch above is merged, this patch will work. |
|
@swift-ci please smoke test macOS platform |
|
@swift-ci please test Windows platform |
|
@swift-ci please smoke test macOS platform |
|
@compnerd, can we cherry-pick these two pulls to 6.3 also? |
This library dependency is already present in the Android SDK builds. Replicate the dependency in the checkouts to allow building for the Android SDK on non-Linux platforms.