Skip to content
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

Added dependency check after post_build step. #3174

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

cemeceme
Copy link
Contributor

Fixes the race condition for dependencies created within post_build steps #3168.

The issue is that a target will mark itself complete, even though the post_build step could have added additional dependencies to it. This meant that any dependents would start running, without waiting for the post_build dependencies to complete, resulting in a race.

The added code will now re-check all the dependencies of a target, after its post_build step has completed. This way, additional dependencies will properly get waited on, avoiding the race.

I would like to note, however, that the addition is essentially a copy of some code in queueTargetAsync, where the initial dependency checks occur. In this case, this is somewhat duplicating the dependency checks and also re-checking already resolved dependencies, even if a post_build step doesn't change the dependencies.

@goddenrich
Copy link
Contributor

Thanks for this. Im taking a look at it now and while it initially seems reasonable there are a few things I want to check before approving.

@cemeceme
Copy link
Contributor Author

Any updates on this? I would love to see the issue this fixes get resolved.

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.

2 participants