[5.3][build] Fix --skip-build-llvm so that its minimal targets, like tblgen, are still built #33026
Conversation
|
@swift-ci test |
|
Build failed |
|
Hi @buttaface can you create a Jira for this? We need to track the changes that go onto the release branch. |
|
@swift-ci test macOS |
|
@airspeedswift, will do. |
|
Reported this issue as SR-13267. |
|
Build failed |
|
@swift-ci test macOS |
|
Let me know if I need to add the issue number to the commit info or something. |
|
No need to update the commit. My one question is how do we prevent this regressing again? It’s not the kind of thing you can easily test, but it’s unfortunate that this didn’t get spotted and fixed until this long after we branched, it’d be good if we can avoid that kind of thing systematically. |
|
Ahead of you, a test was added for this in the main branch, so it won't happen again. 🎆 |
|
Oh – why wasn’t that included in this PR? |
|
That test modified a line from a larger set of tests that was added just last month to the main branch, #32256. I didn't think that would normally be brought back to the release branch, should I add it here? |
|
@airspeedswift the test that @buttaface is talking about is a build-system unit test that I added that does FileCheck based build-script dry-runs to make sure build-script doesn't change like this anymore. This specific thing broke before I wrote that test, so I just put into place the behavior at that time until I could find time to get back to fix this. This is a perfect example of why build-system unit tests are so important and we need more of them. |
|
@buttaface Why not just bring over that specific test (validation-test/BuildSystem/skip_cmark_swift_llvm.test). I think it should work just as expected. I would just tweak the command line slightly by replacing --install-all with the relevant individual --install commands. How does that sound? |
|
Will do. |
| # SKIP-CMARK-CHECK: cmake --build {{.*}}swift- | ||
|
|
||
| # SKIP-LLVM-CHECK: cmake --build {{.*}}cmark- | ||
| # SKIP-LLVM-CHECK: cmake --build {{.*}}llvm-tblgen |
There was a problem hiding this comment.
Updated this pull to bring back this test from the main branch, ready for another CI run.
… for cmake This was backported from a313f62 with all the --infer and --install-all work there left out, bringing back a single test and the lit variable for cmake that it used. One note describing that from the original commit: 2. I added %cmake as a lit variable. I did this so I could specify in my build-system unit tests that on Linux they should use the just built cmake (if we built cmake due to an old cmake being on the system). Otherwise, the build system unit tests took way too long. These are meant to be dry-runs, so building this cmake again is just wasteful and doesn't make sense.
…n, are still built Backported from commit c969814, resolves SR-13267
|
@swift-ci please test |
|
Build failed |
|
Build failed |
|
@swift-ci test macOS |
AnthonyLatsis
added
swift 5.3
🍒 release cherry pick
The Swift compiler and stdlib builds always need a few LLVM targets like tblgen built, even if skipping building a full LLVM. This was just corrected in the main branch with #32860, backporting to the 5.3 branch so there's no regression for this flag when Swift 5.3 is released. For example, I use this flag when cross-compiling the Swift stdlib for Android, others may be using it too.
Ping @atrick.