Skip to content

generator: The target toolchain is still needed to build a package-based SDK #182

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

Merged
merged 1 commit into from
Feb 14, 2025

Conversation

euanh
Copy link
Contributor

@euanh euanh commented Feb 13, 2025

PR #177 was intended to skip unnecessary toolchain downloads, making it possible to build a container-based SDK offline if all its requirements had already been dowloaded. The change was too broad and also broke building package-based SDKs.

PR #177 skips calling generator.downloadArtifacts() when building
an SDK without an embedded host toolchain (the default). In
addition to downloading the host toolchain (and LLVM, if needed),
generator.downloadArtifacts() is also responsible for downloading
the target toolchain. This is not needed when building a container-based
SDK but is required when building a package-based SDK, which combines
an SDK from swift.org with supporting libraries extracted from
Debian packages.

In fact, generator.downloadArtifacts() already avoids downloading toolchains when building a container-based SDK without an embedded
toolchain. The only network call which caused offline builds to
fail was an unconditional check for a suitable LLVM binary from
GitHub.

This PR restores the call to generator.downloadArtifacts() and only makes the LLVM check if LLVM is in the list of required downloads. This allows the EndToEnd tests to pass again (with PR #170 temporarily reverted because of issue #181).

…sed SDK

PR swiftlang#177 was intended to skip unnecessary toolchain downloads, making it
possible to build a container-based SDK offline if all its requirements
had already been dowloaded.  The change was too broad and also broke
building package-based SDKs.

PR swiftlang#177 skips calling generator.downloadArtifacts() when building
an SDK without an embedded host toolchain (the default).   In
addition to downloading the host toolchain (and LLVM, if needed),
generator.downloadArtifacts() is also responsible for downloading
the target toolchain.  This is not needed when building a container-based
SDK but is required when building a package-based SDK, which combines
an SDK from swift.org with supporting libraries extracted from
Debian packages.

In fact, generator.downloadArtifacts() already avoids downloading
toolchains when building a container-based SDK without an embedded
toolchain.   The only network call which caused offline builds to
fail was an unconditional check for a suitable LLVM binary from
GitHub.

This PR restores the call to generator.downloadArtifacts() and only
makes the LLVM check if LLVM is in the list of required downloads.
This allows the EndToEnd tests to pass again (with PR swiftlang#170 temporarily
reverted because of issue swiftlang#181).
@euanh euanh added the bug Something isn't working label Feb 13, 2025
@euanh euanh requested a review from MaxDesiatov as a code owner February 13, 2025 15:40
@euanh euanh changed the title generator: The target toolchain is still needed to build a package-ba… generator: The target toolchain is still needed to build a package-based SDK Feb 13, 2025
@euanh
Copy link
Contributor Author

euanh commented Feb 13, 2025

@swift-ci test

@euanh
Copy link
Contributor Author

euanh commented Feb 13, 2025

It might be neater to incorporate an idea of 'fallback URLs' in the DownloadableArtifacts struct, so that LLVM sources could be fetched automatically if a binary is not available. However downloading LLVM is not required from Swift 6.0 onwards, as it is included in upstream Swift toolchain builds. If we eventually deprecate 5.9 and 5.10 SDKs all this code can be removed, so redesigning DownloadableArtifacts just for this case would be a wasted effort.

@euanh euanh merged commit a9f8e2a into swiftlang:main Feb 14, 2025
3 checks passed
@euanh euanh deleted the skip-unnecessary-downloads branch February 14, 2025 08:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants