Skip to content

[build-script-impl] Require --install-llvm to be passed in to install llvm. #32250

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

gottesmm
Copy link
Contributor

@gottesmm gottesmm commented Jun 8, 2020

Before this patch, we assumed that if LLVM_INSTALL_COMPONENTS was non-empty that
we wanted to install llvm. This is different than /every other/ product in
build-script-impl where INSTALL_$PRODUCT controls if an install is attempted. If
a components sort of thing is required, we force it to only be a configuration
of an option that is a no-op if INSTALL_$PRODUCT is set to true.

I went through build-presets and looked at every place we had an install
statement and added install-llvm when appropriate. This was generally when we
did an install-swift. The one special case is with the tsan_libdsipatch
test. That being said, if I was too aggressive with where I put these
install-llvm, no harm will result since in those cases LLVM_INSTALL_COMPONENTS
had to be empty previously anyways (since otherwise we would have attempted an
install).

… llvm.

Before this patch, we assumed that if LLVM_INSTALL_COMPONENTS was non-empty that
we wanted to install llvm. This is different than /every other/ product in
build-script-impl where INSTALL_$PRODUCT controls if an install is attempted. If
a components sort of thing is required, we force it to only be a configuration
of an option that is a no-op if INSTALL_$PRODUCT is set to true.

I went through build-presets and looked at every place we had an install
statement and added install-llvm when appropriate. This was generally when we
did an install-swift. The one special case is with the tsan_libdsipatch
test. That being said, if I was too aggressive with where I put these
install-llvm, no harm will result since in those cases LLVM_INSTALL_COMPONENTS
had to be empty previously anyways (since otherwise we would have attempted an
install).
@gottesmm gottesmm requested a review from shahmishal June 8, 2020 21:13
@gottesmm
Copy link
Contributor Author

gottesmm commented Jun 8, 2020

@swift-ci test

@gottesmm gottesmm requested a review from benlangmuir June 8, 2020 21:15
@swift-ci
Copy link
Contributor

swift-ci commented Jun 8, 2020

Build failed
Swift Test Linux Platform
Git Sha - 452b6dd

@gottesmm
Copy link
Contributor Author

gottesmm commented Jun 8, 2020

@swift-ci smoke test linux platform

@gottesmm
Copy link
Contributor Author

gottesmm commented Jun 8, 2020

@swift-ci test linux platform

@gottesmm gottesmm merged commit 3531852 into swiftlang:master Jun 9, 2020
@gottesmm gottesmm deleted the pr-962a7fcfad9a0049512c51e6da2e032c8a4a65e0 branch June 9, 2020 01:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants