Add PID to .ninja_lock, warn if another instance is running in builddir - #2770
Open
jhasse wants to merge 2 commits into
Open
Add PID to .ninja_lock, warn if another instance is running in builddir#2770jhasse wants to merge 2 commits into
jhasse wants to merge 2 commits into
Conversation
The warning might seem like a regression, but technically the use case of running two ninjas in the same build directory was never really supported - it works accidentally and we might break it at any time. Although not the intention of this, this should fix ninja-build#2762, because now the lock file isn't empty anymore, which was a problem on some network filesystems where the mtime wasn't updated in that case.
digit-google
approved these changes
May 11, 2026
| IsProcessRunning(static_cast<int>(pid))) { | ||
| Warning("another ninja process (pid %ld) seems to be running in this " | ||
| "build directory; if it is not, delete %s", | ||
| pid, lock_file_path_.c_str()); |
Contributor
There was a problem hiding this comment.
I think this should be a Fatal() error instead. Two concurrent Ninjas are likely to break everything, from the .ninja_deps and .ninja_log files, and the content of output files when both launch the same command that want to write to the same output using a tool that doesn't support that well (e.g. without atomically renaming the output file).
Collaborator
Author
There was a problem hiding this comment.
Unfortunately ninja calling ninja again on the same build dir is used in the wild. And it "works" right now, a fatal error would be a hard regression.
When thinking about it even the warning is too much. We need to correctly fix this (the second ninja process communicates to the first process which targets it wants to run).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The warning might seem like a regression, but technically the use case of running two ninjas in the same build directory was never really supported - it works accidentally and we might break it at any time.
Although not the intention of this, this should fix #2762, because now the lock file isn't empty anymore, which was a problem on some network filesystems where the mtime wasn't updated in that case.
TODO