-
Notifications
You must be signed in to change notification settings - Fork 6.4k
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
[vcpkg] Pass CMAKE_DISABLE_SOURCE_CHANGES to all ports by default #10999
[vcpkg] Pass CMAKE_DISABLE_SOURCE_CHANGES to all ports by default #10999
Conversation
Just a Note about this: https://gitlab.kitware.com/cmake/cmake/issues/18403 |
@@ -263,6 +265,7 @@ function(vcpkg_configure_cmake) | |||
-DCMAKE_INSTALL_PREFIX=${CURRENT_PACKAGES_DIR}/debug) | |||
|
|||
if(NINJA_HOST AND CMAKE_HOST_WIN32 AND NOT _csc_DISABLE_PARALLEL_CONFIGURE) | |||
list(APPEND _csc_OPTIONS "-DCMAKE_DISABLE_SOURCE_CHANGES=ON") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
may I ask you why only for windows hosts?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For now, I'm specifically attaching this to parallel configure (which is where we are getting flakiness in our CI system) to minimize the initial pain. We might roll this out for everyone, always, but my main objective for now is to reduce race conditions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Understood thanks.
I like also the future possibility of “imposing” to not touch the source tree, ever, by the configure/build scripts, independently of parallel configure. But again, I understand your pain with flackiness due to Windows hosts using ninja to parallelize configure step.
/azp run |
Azure Pipelines successfully started running 1 pipeline(s). |
/azp run |
Azure Pipelines successfully started running 1 pipeline(s). |
/azp run |
Azure Pipelines successfully started running 1 pipeline(s). |
/azp run |
Azure Pipelines successfully started running 1 pipeline(s). |
/azp run |
Commenter does not have sufficient privileges for PR 10999 in repo microsoft/vcpkg |
…disable-source-changes # Conflicts: # toolsrc/src/vcpkg/build.cpp
/azp run |
Azure Pipelines successfully started running 1 pipeline(s). |
Closed as duplicate. |
This PR passes
CMAKE_DISABLE_SOURCE_CHANGES
by default to allvcpkg_configure_cmake()
calls, which prevents any race conditions around parallel configure steps. It is disabled when passingDISABLE_PARALLEL_CONFIGURE
, since this also removes the race condition.