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

Call stack update once #1199

Merged
merged 4 commits into from
Jan 2, 2020
Merged

Call stack update once #1199

merged 4 commits into from
Jan 2, 2020

Conversation

aherrmann
Copy link
Member

Closes #1090

As suggested in #1090 (see #1085 (comment) for related discussion) this factors out the call to stack update into a separate repository rule _stack_update. Every instance of _stack_snapshot depends on the same instance of _stack_update, so that stack update is only called once.

This removes the need to call stack update leniently within _stack_snapshot and the need to preinstall stack and run stack update manually on Windows CI.

I have used Bazel's --experimental_workspace_rules_log_file to verify the following.

  • stack update is executed on every fetch.
  • stack update is run before calls to stack in _stack_snapshot.
  • _stack_snapshot is not invalidated by _stack_update.

This was a workaround for concurrent execution of `stack update`.
See #1085
Copy link
Member

@mboes mboes left a comment

Choose a reason for hiding this comment

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

Great work! Some subtle issues addressed very carefully.

@mboes mboes added the merge-queue merge on green CI label Dec 31, 2019
Ensures that `stack update` is always executed once before `stack
unpack`.
skydoc does not support `doc` attributes to `repository_rule`.
@aherrmann
Copy link
Member Author

Merging manually as mergify is not picking this up.

@aherrmann aherrmann merged commit 0c6ded2 into master Jan 2, 2020
@aherrmann aherrmann deleted the stack-update branch January 2, 2020 18:42
@mergify mergify bot removed the merge-queue merge on green CI label Jan 2, 2020
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.

Work around stack update race condition on Windows
2 participants