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

Add support for -n "dry run" feature #12

Closed
wants to merge 1 commit into from
Closed

Conversation

jamesr
Copy link
Contributor

@jamesr jamesr commented Mar 26, 2022

This executes the normal build logic up until the point of spawning a
subprocess at which point it pretends the command succeeded. Internal
command such as generating directories and regeneration still happen.
Missing input files are permitted in dry run mode.

Copy link
Owner

@evmar evmar left a comment

Choose a reason for hiding this comment

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

I think this also needs some bailing logic in Work::record_finished, which does stuff like stat expected output files and record hashes of modified files.

This mostly looks good, let me know if you just want me to pick it up from here.

src/work.rs Outdated Show resolved Hide resolved
@jamesr jamesr force-pushed the dry_run branch 2 times, most recently from 248bccb to 17e31a2 Compare March 29, 2022 00:28
@jamesr
Copy link
Contributor Author

jamesr commented Mar 29, 2022

I took a crack at fixing up the record_finished path and the inputs-that-are-generated-outputs check to match - PTAL.

) -> anyhow::Result<TaskResult> {
if let Some(rspfile) = rspfile {
write_rspfile(rspfile)?;
}
if dry_run {
Copy link
Owner

Choose a reason for hiding this comment

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

Hm, is it intentional here to write the rspfile in the dry run case? I am not sure about it, but my first impression is we shouldn't write any files, and perhaps even not start any tasks.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ninja does write rsp files in dry run mode. I know that some people use -n to generate rsp files.

Then again, n2 doesn't delete rsp files by default, so maybe that's less needed here.

It is weird if -n mode has visible side effects, and using it for writing rsp files feels like a bit of a hack.

Copy link
Owner

Choose a reason for hiding this comment

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

What's the use case for generating rsp files with -n? Debugging?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

My intent here was to match ninja - but upon reflection I agree that the ninja behavior is strange. Is the goal of n2 in general to match behaviors or to rethink them? I.e. do you anticipate having a keeprsp control as well?

I've never intentionally used the -n flag to examine rspfiles but I've certainly used and seen folks deeply confused by ninja's keeprsp behehavior. Perhaps there's a better way to approach that entirely.

Copy link
Contributor

Choose a reason for hiding this comment

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

What's the use case for generating rsp files with -n? Debugging?

I don't use it for this myself, so I'm not 100% sure, but I think the answer is "yes". These are concrete examples:

  1. Being able to manually run commands. You might want to run with -v to get a commandline, but if that uses a rsp file and it's not on disk and you forgot to pass -d keeprsp, you can re-run with -n to create the rsp file.
  2. You just might want to read the rsp file to look at its contents.

@evmar
Copy link
Owner

evmar commented Mar 30, 2022

@jamesr can you talk about your use case for this? Would help understand whether we want rspfiles or not, or what exactly should happen in general. E.g. if you had foo.c compiles to foo.o links to foo, one possible interpretation of a dry-run build is to say "I would start by compiling foo.o, and I can't go any farther than that because I don't have foo.o for the next step".

@jamesr
Copy link
Contributor Author

jamesr commented Mar 31, 2022

The way that I've usually used "-n" is to ask the tool what steps it thinks it should run given the currently available information. In that use case I would consider the rspfile's contents as part of the tool's description of the step that it wants to run and would like access to that information. Another way to factor it would be to have the dry run simply go through the steps it would like to run and have a different query to ask for a fuller description of a particular step or set of steps, including rspfile file contents.

This executes the normal build logic up until the point of spawning a
subprocess at which point it pretends the command succeeded. Internal
command such as generating directories and regeneration still happen.
Missing input files are permitted in dry run mode.
@evmar evmar force-pushed the main branch 2 times, most recently from 03de3da to afe500f Compare December 21, 2022 19:27
@evmar evmar force-pushed the main branch 2 times, most recently from 95e6f5f to 1b31c58 Compare June 14, 2023 19:04
@evmar evmar force-pushed the main branch 2 times, most recently from 8cafbda to d43e8ca Compare January 8, 2024 17:38
@evmar
Copy link
Owner

evmar commented Jan 25, 2024

I had kept this open for the use case notes, but they'd be better as a bug,
#111

@evmar evmar closed this Jan 25, 2024
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.

3 participants