-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Fix #9052 #12558
base: master
Are you sure you want to change the base?
Fix #9052 #12558
Conversation
It's hard to reproduce this race condition going forward (e.g. it depends on the precise order that goals are scheduled, but https://github.com/roberth/nix-9052 current reproduces it, and this fixes that.
380b16e
to
0607a2e
Compare
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.
Didn't spot a problem. Wdyt @edolstra?
@@ -369,6 +369,18 @@ public: | |||
*/ | |||
Done amDone(ExitCode result, std::optional<Error> ex = {}); | |||
|
|||
/** | |||
* @return true just for those `ExitCode`s that are avalid argument |
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.
super tiny nit:
* @return true just for those `ExitCode`s that are avalid argument | |
* @return true just for those `ExitCode`s that are a valid argument |
This pull request has been mentioned on NixOS Discourse. There might be relevant details there: https://discourse.nixos.org/t/2025-02-24-nix-team-meeting-minutes-215-216/60920/1 |
@roberth found a problem with this, so let's not do this. The proper fix is to remove |
Motivation
Fix #9052
Context
It's hard to reproduce this race condition going forward (e.g. it depends on the precise order that goals are scheduled, but https://github.com/roberth/nix-9052 current reproduces it, and this fixes that.
I don't really like how the fix just piles on more complexity, but I have some ideas on how a larger re-work can clean things up.
Add 👍 to pull requests you find important.
The Nix maintainer team uses a GitHub project board to schedule and track reviews.