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
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion doc/missing.md
Original file line number Diff line number Diff line change
@@ -10,7 +10,6 @@
## Missing flags

- `-l`, load average throttling
- `-n`, dry run

### Missing subcommands

6 changes: 6 additions & 0 deletions src/run.rs
Original file line number Diff line number Diff line change
@@ -19,6 +19,7 @@ enum BuildResult {
struct BuildParams<'a> {
parallelism: usize,
regen: bool,
dry_run: bool,
keep_going: usize,
target_names: &'a [String],
build_filename: &'a String,
@@ -36,6 +37,7 @@ fn build(progress: &mut ConsoleProgress, params: &BuildParams) -> anyhow::Result
&state.hashes,
&mut state.db,
progress,
params.dry_run,
params.keep_going,
state.pools,
params.parallelism,
@@ -125,6 +127,7 @@ fn run_impl() -> anyhow::Result<i32> {
"N",
);
opts.optflag("h", "help", "");
opts.optflag("n", "", "dry run");
opts.optflag("v", "verbose", "print executed command lines");
if fake_ninja_compat {
opts.optflag("", "version", "print fake ninja version");
@@ -195,6 +198,8 @@ fn run_impl() -> anyhow::Result<i32> {
build_filename = name;
}

let dry_run = matches.opt_present("n");

let mut progress = ConsoleProgress::new(matches.opt_present("v"), use_fancy_terminal());

// Build once with regen=true, and if the result says we regenerated the
@@ -203,6 +208,7 @@ fn run_impl() -> anyhow::Result<i32> {
let mut params = BuildParams {
parallelism,
regen: true,
dry_run,
keep_going,
target_names: &matches.free,
build_filename: &build_filename,
21 changes: 14 additions & 7 deletions src/task.rs
Original file line number Diff line number Diff line change
@@ -80,10 +80,18 @@ fn run_task(
cmdline: &str,
depfile: Option<&str>,
rspfile: Option<&RspFile>,
dry_run: bool,
) -> 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.

return Ok(TaskResult {
termination: Termination::Success,
output: vec![],
discovered_deps: None,
});
}
let mut result = run_command(cmdline)?;
if result.termination == Termination::Success {
if let Some(depfile) = depfile {
@@ -308,18 +316,17 @@ impl Runner {
cmdline: String,
depfile: Option<String>,
rspfile: Option<RspFile>,
dry_run: bool,
) {
let tid = self.tids.claim();
let tx = self.finished_send.clone();
std::thread::spawn(move || {
let start = Instant::now();
let result =
run_task(&cmdline, depfile.as_deref(), rspfile.as_ref()).unwrap_or_else(|err| {
TaskResult {
termination: Termination::Failure,
output: err.to_string().into_bytes(),
discovered_deps: None,
}
let result = run_task(&cmdline, depfile.as_deref(), rspfile.as_ref(), dry_run)
.unwrap_or_else(|err| TaskResult {
termination: Termination::Failure,
output: err.to_string().into_bytes(),
discovered_deps: None,
});
let finish = Instant::now();

17 changes: 15 additions & 2 deletions src/work.rs
Original file line number Diff line number Diff line change
@@ -308,6 +308,7 @@ pub struct Work<'a> {
file_state: FileState,
last_hashes: &'a Hashes,
build_states: BuildStates,
dry_run: bool,
runner: task::Runner,
}

@@ -317,6 +318,7 @@ impl<'a> Work<'a> {
last_hashes: &'a Hashes,
db: &'a mut db::Writer,
progress: &'a mut dyn Progress,
dry_run: bool,
keep_going: usize,
pools: Vec<(String, usize)>,
parallelism: usize,
@@ -327,6 +329,7 @@ impl<'a> Work<'a> {
graph,
db,
progress,
dry_run,
keep_going,
file_state,
last_hashes,
@@ -445,6 +448,12 @@ impl<'a> Work<'a> {
}
}

if self.dry_run {
// In dry run mode we didn't actually run the task so we don't
// expect the output files to change.
return Ok(());
}

// Stat all the outputs. This step just finished, so we need to update
// any cached state of the output files to reflect their new state.
let build = self.graph.build(id);
@@ -517,15 +526,15 @@ impl<'a> Work<'a> {

// stat any non-generated inputs if needed.
// Note that generated inputs should already have been stat()ed when
// they were visited as outputs.
// they were visited as outputs unless we're doing a dry run.

// For dirtying_ins, ensure we both have mtimes and that the files are present.
for &id in build.dirtying_ins() {
let file = self.graph.file(id);
let mtime = match self.file_state.get(id) {
Some(mtime) => mtime,
None => {
if file.input.is_some() {
if file.input.is_some() && !self.dry_run {
// This is a logic error in ninja; any generated file should
// already have been visited by this point.
panic!(
@@ -540,6 +549,9 @@ impl<'a> Work<'a> {
if workaround_missing_phony_deps {
continue;
}
if self.dry_run {
return Ok(true);
}
anyhow::bail!("{}: input {} missing", build.location, file.name);
}
}
@@ -670,6 +682,7 @@ impl<'a> Work<'a> {
build.cmdline.clone().unwrap(),
build.depfile.clone(),
build.rspfile.clone(),
self.dry_run,
);
self.progress.task_state(id, build, BuildState::Running);
made_progress = true;