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

Make outputs modified outside of the build system considered dirty #1945

Closed
wants to merge 2 commits into from

Conversation

Dragnalith
Copy link

@Dragnalith Dragnalith commented Mar 29, 2021

This pull request is a fix for #1946.

Today if an output of the build is modified from outside of the build system (manually or by another process) ninja will not consider it to be dirty.

I think if an output state is different from the state it would have if the rule command was executed again, then it should be considered dirty. (Edit: Since in some scenario like when you consciously modified outputs for quick testing you may not want ninja to override your change, my implementation makes it optional. For backward compatibility default ninja keep the same behavior, but if run with -m flag, it will considered modified outputs as dirty).

Situation where it is a problem:

  • You have an action registered as order-only dependency which generate some C++ header.
  • You include one of generated file for the time to one of your .cpp, so ninja does not consider it yet as implicit dependency.
  • You open the generated file with your favorite text editor, and by mistyping on your keyboard you modify the generated header to an invalid C++ state.
  • As consequence, ninja will not try to re-generate the header, because it is not yet a dependency of the build, and ninja will fail the build because the header is not valid C++.

My modification considered an output is dirty if its mtime is more recent than the one in the build log. The mtime of the output can be older than the one on the build log in the case its build edge is mark with 'restat = 1'. In that case if the rule command did not modified the output, its build log entry will inherit the mtime of its most recent inputs or depfile.

@Dragnalith Dragnalith changed the title Make modified outputs considered as dirty Make modified outputs considered dirty Mar 29, 2021
In the case an output mtime is more recent than its log entry, the
output should be considered dirty since its state is not equal to
the state it would have if the rule command was executed again.
@Dragnalith Dragnalith changed the title Make modified outputs considered dirty Make outputs modified outside of the build system considered dirty Mar 30, 2021
@Dragnalith
Copy link
Author

I have updated the commit to fix the test, improve comments and commit description.

@jhasse
Copy link
Collaborator

jhasse commented Mar 31, 2021

This becomes annoying when the output is newer, because you actually reran the edge's command outside of ninja. Or let's say you don't mistype on your keyboard, but want to test something quick and dirty.

@Dragnalith
Copy link
Author

@jhasse I understand the situation you are pointing out.

I think both situations are valid:

  • You may not want to re-run some edges because you consciously override their output. Example: you modified some outputs for quick and dirty test
  • After running ninja you want the build output state to match the state the would have if their edge has be executed. Example: after your quick and dirty test, you want to revert the build to its correct state without having to rebuild everything from scratch.

We could use a command line option to select between the two behavior. Let me implement that...

@Dragnalith
Copy link
Author

I have updated my implementation to add a '-m' option which enabled the behavior of ninja considering modified output as dirty.

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