[5.6] Update package dependencies to use the 'release/5.6' branch #929

abertelrud · 2021-12-08T16:26:33Z

Update swift-llbuild and swift-tools-support-core dependencies to use the 'release/5.6' branch instead of main. This is needed before we can switch over SwiftPM to depend on 'release/5.6'.

…use the 'release/5.6' branch

abertelrud · 2021-12-08T16:27:40Z

@swift-ci please smoke test

MaxDesiatov · 2021-12-08T16:29:48Z

Package.swift

@@ -121,7 +121,7 @@ let package = Package(
 if ProcessInfo.processInfo.environment["SWIFT_DRIVER_LLBUILD_FWK"] == nil {
    if ProcessInfo.processInfo.environment["SWIFTCI_USE_LOCAL_DEPS"] == nil {
        package.dependencies += [
-            .package(url: "https://github.com/apple/swift-llbuild.git", .branch("main")),
+            .package(url: "https://github.com/apple/swift-llbuild.git", .branch("release/5.6")),


Would it work better to refactor this as a let binding reused for both Driver and TSC dependencies, to be consistent with Package.swift in the SwiftPM repo?

Probably. I'll let the driver folks decide. I wanted to do the minimal change in this case. If so, it's probably better to do the change in main as well to minimize differences between the branches.

MaxDesiatov · 2021-12-08T16:56:21Z

@swift-ci please test

MaxDesiatov · 2021-12-08T17:02:54Z

Not sure if builds for release/5.6 are set up correctly? It fails because corresponding URL on swift.org doesn't exist:

17:56:36 + echo+ sed s/-ubuntu16.04.tar.gz//g
17:56:36 
17:56:36 + DOWNLOAD_DIR=
17:56:36 + /usr/bin/curl https://download.swift.org/swift-5.6-branch/ubuntu1604//

artemcm · 2021-12-08T17:03:20Z

Not sure if builds for release/5.6 are set up correctly? It fails because corresponding URL on swift.org doesn't exist:
17:56:36 + echo+ sed s/-ubuntu16.04.tar.gz//g
17:56:36 
17:56:36 + DOWNLOAD_DIR=
17:56:36 + /usr/bin/curl https://download.swift.org/swift-5.6-branch/ubuntu1604//

cc @shahmishal

abertelrud · 2021-12-08T17:09:38Z

We might not have the first nightly 5.6 toolchain yet. I don't see one on the swift.org Downloads web page.

shahmishal · 2021-12-08T18:03:14Z

Correct, we are not ready yet.

benlangmuir · 2021-12-08T18:04:36Z

@ahoppen we'll need a sourcekit-lsp change to match this. It won't be caught by CI, because CI uses the local checkout of swift-driver instead of the branch dependency.

abertelrud · 2021-12-09T18:09:56Z

@swift-ci please test

abertelrud · 2021-12-09T18:13:29Z

Still don't have a 5.6 toolchain it seems.

MaxDesiatov · 2021-12-09T18:14:34Z

Yeah, I'm checking the list of toolchains here https://www.swift.org/download/, and 5.6 is not in the list yet.

abertelrud · 2021-12-14T20:19:20Z

It looks like @swift-ci please test is actually doing a smoke test, unless I'm reading the log wrong (I see sh swift-ci/jobs/pull-request/pr-swift-driver-smoke.sh). In SwiftPM, the 5.6 release branch tests are full tests, so it doesn't seem to have to download a toolchain.

abertelrud · 2021-12-17T19:04:06Z

Running a cross-repo test in swiftlang/swift#40613 to avoid the need for a 5.6 toolchain.

abertelrud · 2021-12-17T23:05:52Z

So swiftlang/swift#40613 seems to have passed, except for what looks like an unrelated issue with Windows that I have asked @compnerd about. @artemcm what successful tests in swiftlang/swift#40613 would be require in order to be able to merge this?

abertelrud · 2021-12-17T23:07:05Z

Also, why is @swift-ci test appear to be configured the same as @swift-ci smoke test in this repo? This seems to leave no way to do a full build, as we can in the swiftpm repo to get around the lack of an available 5.6 toolchain.

abertelrud · 2021-12-18T04:46:46Z

Looks like the smoke tests for all three platforms in swiftlang/swift#40613 have passed now, so should that be considered equivalent testing to what we would have had if we'd had a downloadable 5.6 toolchain?

abertelrud · 2021-12-21T01:08:52Z

@artemcm @nkcsgexi @benlangmuir @shahmishal Is this something we can merge based on the successful cross-repo test in swiftlang/swift#40613?

ahoppen · 2021-12-21T13:27:15Z

I don’t think the cross-PR test actually covers the change. AFAICT the test invocation ran

/Users/buildnode/jenkins/workspace/swift-PR-macos-smoke-test/branch-release/5.6/swift-driver/Utilities/build-script-helper.py build --package-path /Users/buildnode/jenkins/workspace/swift-PR-macos-smoke-test/branch-release/5.6/swift-driver --build-path /Users/buildnode/jenkins/workspace/swift-PR-macos-smoke-test/branch-release/5.6/buildbot_incremental/earlyswiftdriver-macosx-x86_64 --configuration release --toolchain /Applications/Xcode-beta.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr --ninja-bin /Users/buildnode/jenkins/workspace/swift-PR-macos-smoke-test/branch-release/5.6/buildbot_incremental/ninja-build/ninja --cmake-bin /usr/local/bin/cmake --local_compiler_build --verbose

which doesn’t have --no-local-deps, so it’s setting the SWIFTCI_USE_LOCAL_DEPS environment variable and thus resolving the dependencies using locally checked-out sibling repositories instead of the branch names that were changed in this PR.

FWIW I don’t have any objections to merging this if a local build with --no-local-deps passes.

abertelrud · 2021-12-21T21:33:12Z

I don’t think the cross-PR test actually covers the change. AFAICT the test invocation ran
/Users/buildnode/jenkins/workspace/swift-PR-macos-smoke-test/branch-release/5.6/swift-driver/Utilities/build-script-helper.py build --package-path /Users/buildnode/jenkins/workspace/swift-PR-macos-smoke-test/branch-release/5.6/swift-driver --build-path /Users/buildnode/jenkins/workspace/swift-PR-macos-smoke-test/branch-release/5.6/buildbot_incremental/earlyswiftdriver-macosx-x86_64 --configuration release --toolchain /Applications/Xcode-beta.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr --ninja-bin /Users/buildnode/jenkins/workspace/swift-PR-macos-smoke-test/branch-release/5.6/buildbot_incremental/ninja-build/ninja --cmake-bin /usr/local/bin/cmake --local_compiler_build --verbose
which doesn’t have --no-local-deps, so it’s setting the SWIFTCI_USE_LOCAL_DEPS environment variable and thus resolving the dependencies using locally checked-out sibling repositories instead of the branch names that were changed in this PR.

Interesting, I missed that part! Thanks for noticing that it wasn't a proper test.

FWIW I don’t have any objections to merging this if a local build with --no-local-deps passes.

We might not be able to merge it from a technical perspective since this PR has to have green checks (though I'm sure someone can override it if we really wanted to).

It seems odd that @swift-ci test does just a smoke test for this repo. In SwiftPM we got around the lack of a downloadable toolchain by doing @swift-ci test to build the whole stack (you can still do @swift-ci please smoke test if you really wanted a smoke test). That would have been useful here.

artemcm · 2022-01-04T18:29:13Z

I tried out this PR locally building/testing with SPM directly and by using build-script-helper.py with and without --no-local-deps, and all looks good on my end.

We should force-merge this to unblock SwiftPM testing, while we figure out the downloadable 5.6 toolchain situation.

Update dependencies of swift-llbuild and swift-tools-support-core to …

3cc6ba6

…use the 'release/5.6' branch

abertelrud requested review from benlangmuir and artemcm December 8, 2021 16:26

abertelrud requested a review from nkcsgexi December 8, 2021 16:27

abertelrud mentioned this pull request Dec 8, 2021

[5.6] Set relatedDependenciesBranch to release/5.6 swiftlang/swift-package-manager#3919

Merged

abertelrud changed the title ~~Update swift-llbuild and swift-tools-support-core dependencies to use the 'release/5.6' branch~~ [5.6] Update package dependencies to use the 'release/5.6' branch Dec 8, 2021

MaxDesiatov reviewed Dec 8, 2021

View reviewed changes

artemcm approved these changes Dec 8, 2021

View reviewed changes

abertelrud mentioned this pull request Dec 17, 2021

[Do Not Merge!] Empty commit for cross-repo testing of SwiftDriver PR #929 swiftlang/swift#40613

Closed

abertelrud mentioned this pull request Jan 2, 2022

[5.6] Turn on emit-module-separately for whole-module builds by default #960

Closed

tomerd merged commit a49908d into release/5.6 Jan 4, 2022

artemcm mentioned this pull request Jan 4, 2022

[5.6] Update 5.5-specific manifest dependencies to 5.6 #964

Merged

[5.6] Update package dependencies to use the 'release/5.6' branch #929

[5.6] Update package dependencies to use the 'release/5.6' branch #929

Uh oh!

Conversation

abertelrud commented Dec 8, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

abertelrud commented Dec 8, 2021

Uh oh!

MaxDesiatov Dec 8, 2021

Choose a reason for hiding this comment

Uh oh!

abertelrud Dec 8, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

MaxDesiatov commented Dec 8, 2021

Uh oh!

MaxDesiatov commented Dec 8, 2021

Uh oh!

artemcm commented Dec 8, 2021

Uh oh!

abertelrud commented Dec 8, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

shahmishal commented Dec 8, 2021

Uh oh!

benlangmuir commented Dec 8, 2021

Uh oh!

abertelrud commented Dec 9, 2021

Uh oh!

abertelrud commented Dec 9, 2021

Uh oh!

MaxDesiatov commented Dec 9, 2021

Uh oh!

abertelrud commented Dec 14, 2021

Uh oh!

abertelrud commented Dec 17, 2021

Uh oh!

abertelrud commented Dec 17, 2021

Uh oh!

abertelrud commented Dec 17, 2021

Uh oh!

abertelrud commented Dec 18, 2021

Uh oh!

abertelrud commented Dec 21, 2021

Uh oh!

ahoppen commented Dec 21, 2021

Uh oh!

abertelrud commented Dec 21, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

artemcm commented Jan 4, 2022

Uh oh!

Uh oh!

abertelrud commented Dec 8, 2021 •

edited

Loading

abertelrud Dec 8, 2021 •

edited

Loading

abertelrud commented Dec 8, 2021 •

edited

Loading

abertelrud commented Dec 21, 2021 •

edited

Loading