-
Notifications
You must be signed in to change notification settings - Fork 6.7k
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
[CMake] VCPKG_APPLOCAL_DEPS sometimes causes conflicts when processing multiple files in the same directory #13025
[CMake] VCPKG_APPLOCAL_DEPS sometimes causes conflicts when processing multiple files in the same directory #13025
Conversation
…is run in parallel so make it uses_terminal to allow only 1 job to run simultaneously
In this specific case, it should then be safe to skip over files that are in use (because the script using them will handle their transitive dependencies). However, that may not be good enough to solve the general problem. I'm a bit concerned about USES_TERMINAL potentially making the user's link steps serialized; I haven't checked recently, but I recall this essentally adding an Another approach that might be of interest would be to build on top of the applocal.ps1 changes in #12223 and group all the applocal commands into a single call. This could have some nice performance benefits since invoking powershell is somewhat expensive. |
Hmm that is a good point that I didn't think about. I'll investigate this a bit further and also look into your suggestion for another approach. |
I did some tests to see if USES_TERMINAL affects the amount of targets that are linked in parallel, but just removing the linked output on some simple test targets and this seems to work just fine. But, atleast for Ninja generator, USES_TERMINAL will cause all post build steps to be run single threaded. So if a user adds other steps they might conflict. I've also been looking at the other suggestion of combining and making sure it only runs once. Would you have any pointers as in how to make that happen. Right now we can only know when a target needs this on the Then again, I do not think that the USES_TERMINAL is as bad as we were afraid of. I did change the code slightly to make this an optional setting. |
…S_SERIALIZED This clarifies it as an experimental flag that may change or be removed at any time
Thanks for making it optional -- I think the north star obviously needs to be thread safe all the time (possibly via merging them together), so I named the option accordingly to indicate to users that they shouldn't depend on this behavior. That said, this seems like a good intermediate solution until we're able to do the perfect one. |
Blocked by #13722. |
#13722 seems to be merged so can this get merged too? |
/azp run |
No pipelines are associated with this pull request. |
Looks good, all test passed. |
/azp run |
Azure Pipelines successfully started running 1 pipeline(s). |
Thanks for your contribution! |
In our CMake setup we often have multiple targets output to the same directory keeping lists of plugins together for instance. When these targets run the custom target on windows to gather their depedencies (
VCPKG_APPLOCAL_DEPS
ison
) the custom target script sometimes complains that files are already in use.Adding a
USES_TERMINAL
makes sure these commands do on run in parallel. Maybe there is another (better) way to only do when running within the same directory so in 'normal' situation these can still happen in parallel