Skip to content

[build] Add flags to allow skipping rebuilding the corelibs #38673

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
Aug 30, 2021

Conversation

finagolfin
Copy link
Member

Add three new flags, --skip-clean-libdispatch, --skip-clean-foundation, and --skip-clean-xctest, that leave the previous builds of those products in place.

I've been using a crude version of this patch for years when I'm iterating on one of the later products and don't want the corelibs rebuilt each time.

@edymtt, this is a follow-on to #36686, where you mentioned this is already done for XCTest, but it isn't. Mind reviewing?

@finagolfin
Copy link
Member Author

@CodaFi, interested in this pull?

@finagolfin
Copy link
Member Author

@drexin, would you review?

@finagolfin
Copy link
Member Author

@artemcm, you added some of these other flags in #33563, would you review?

Copy link
Contributor

@artemcm artemcm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@artemcm
Copy link
Contributor

artemcm commented Aug 12, 2021

@swift-ci please test

@swift-ci
Copy link
Contributor

Build failed
Swift Test OS X Platform
Git Sha - 9264823feb3a589828ae88c2ce2a65d888077101

@finagolfin
Copy link
Member Author

finagolfin commented Aug 13, 2021

Oh, I only tested on linux, didn't realize my expanded test would fail on macOS. I will split libdispatch and Foundation off into a non-Darwin test.

# The Swift project might have been changed, but CMake might
# not be aware and will not rebuild.
echo "Cleaning the XCTest build directory"
call rm -rf "${XCTEST_BUILD_DIR}"
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is currently only removed for non-Darwin, mistake? I see llbuild is removed by default for all OSs, I can move this above so it's removed for Darwin too, then this new flag will keep it for both.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moved this up so that this build directory is cleaned by default on macOS too, which is now tested by the updated test.

@finagolfin
Copy link
Member Author

Split the test up and made sure XCTest is cleaned up on macOS too.

@finagolfin
Copy link
Member Author

@drexin, would you run this through the CI again?

@drexin
Copy link
Contributor

drexin commented Aug 15, 2021

@swift-ci test

@swift-ci
Copy link
Contributor

Build failed
Swift Test OS X Platform
Git Sha - 5bdbf35a024c396a66148bccb00cc337bbf3f586

Add three new flags, '--skip-clean-libdispatch', '--skip-clean-foundation', and
'--skip-clean-xctest', that leave the previous builds of those products in place.
@finagolfin
Copy link
Member Author

Rebased because #38885 moved a lot of code to a different file and disabled the new test on all Darwin platforms now.

@finagolfin
Copy link
Member Author

@artemcm, this should pass the CI now.

@finagolfin
Copy link
Member Author

@CodaFi, would you run this through the CI?

@artemcm
Copy link
Contributor

artemcm commented Aug 19, 2021

@swift-ci please test

@finagolfin
Copy link
Member Author

Passed all CI except Windows, whose CI may be stuck.

@finagolfin
Copy link
Member Author

Ping, passed CI, ready for merge.

@finagolfin
Copy link
Member Author

@artemcm, anything holding back getting this merged?

@artemcm
Copy link
Contributor

artemcm commented Aug 30, 2021

@swift-ci please test and merge

@swift-ci swift-ci merged commit 0c122a5 into swiftlang:main Aug 30, 2021
@shahmishal
Copy link
Member

shahmishal commented Aug 31, 2021

We are seeing failure on CI:
https://ci.swift.org/job/oss-swift-package-macos/6135/console

Changes to BuildSystem/skip_clean_xctest_llbuild.test now require us to checkout swift-coreslibs-xctest on macOS platform. This should not be required, can we fix this test to only execute on Linux and Windows?

@finagolfin
Copy link
Member Author

Huh, that passed the macOS CI though. I will disable the XCTest portion for Darwin platforms, pull forthcoming.

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.

5 participants