-
Notifications
You must be signed in to change notification settings - Fork 10.5k
DO NOT MERGE [build-script] Flip the default value of legacy_impl #21020
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
Conversation
@swift-ci please smoke test |
@swift-ci please test |
@swift-ci please build toolchain |
@swift-ci please build toolchain macOS |
Build failed |
Looks like the libicu config is trying to add to the swift cmake config in build-script-impl, which doesn't work if we don't configure them at the same time. |
Build failed |
Looks like lldb is also leaking configuration variables between steps. |
@swift-ci please smoke test |
@swift-ci please build toolchain macOS |
@swift-ci please test macOS |
Build failed |
@swift-ci please smoke test Linux |
1 similar comment
@swift-ci please smoke test Linux |
@swift-ci please build toolchain macOS |
@swift-ci please smoke test Linux |
1 similar comment
@swift-ci please smoke test Linux |
@swift-ci please build toolchain macOS |
@swift-ci please smoke test Linux |
@swift-ci please build toolchain macOS |
@swift-ci please smoke test Linux |
@swift-ci please build toolchain macOS |
@swift-ci please smoke test Linux |
@swift-ci please build toolchain macOS |
@swift-ci please test Linux |
Build failed |
447ec09
to
ecc8601
Compare
@swift-ci please test |
Build failed |
Build failed |
@swift-ci please build toolchain macOS |
@swift-ci please test Linux |
Build failed |
@swift-ci please test Linux |
@swift-ci please build toolchain macOS |
Build failed |
@swift-ci please build toolchain macOS |
@swift-ci please test |
@swift-ci please build toolchain Linux |
@swift-ci please test |
@swift-ci please build toolchain |
(don't know why the status isn't updated, but the toolchain builds passed too) |
Turns on the `--no-legacy-impl` option to build-script by default; the old behaviour is temporarily still available as `--legacy-impl`. This causes build-script to invoke build-script-impl for every individual build/test/install/etc. action rather than a single global invocation. For example, a single invocation might be for `macosx-swift-install`. This will enable the python code in build-script to drive the overall process and add additional steps in between actions without the involvement of build-script-impl. It also provides a path to refactoring the existing actions out of build-script-impl individually. Discussed as part of https://forums.swift.org/t/rfc-building-swift-packages-in-build-script/18920 The --no-legacy-impl flag was originally disabled by default because of concerns about the performance of null builds due to the increased number of script invocations. There is a small optimization in this commit to use `tr` when processing command-line options instead of bash's builtin substitution, which eliminates most of the overhead. After this change, a null build of llvm+swift changes from 1.6 s to 2.1 s on Linux, and from 5 s to 6 s on macOS. Non-null builds and builds that involve more build products than just llvm+swift (e.g. corelibs) are basically unaffected since they are not correctly incremental to begin with. The changes to build-script-impl in this commit are to fix the behaviour of --no-legacy-impl, which had bitrotted since it was introduced. These changes are to make various parts of the script not rely on variables defined in "earlier" parts of the script, which is good hygiene in general.
65d501c
to
ce4c85a
Compare
@swift-ci please test |
@swift-ci please build toolchain |
Closing in favour of #21772 |
Experimenting with whether this code still works.