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

rustfmt check more reliably works #6172

Merged
merged 1 commit into from Jul 18, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
25 changes: 11 additions & 14 deletions build-support/bin/check_rust_formatting.sh
Expand Up @@ -15,7 +15,7 @@ function usage() {
fi
}

write_mode=diff
write_mode=check

while getopts "hf" opt; do
case ${opt} in
Expand Down Expand Up @@ -54,24 +54,21 @@ bad_files=(
exit ${PIPESTATUS[0]}
)
)
case $? in
4)
exit_code=$?

if [[ ${exit_code} -ne 0 ]]; then
if [[ "${write_mode}" == "check" ]]; then
echo >&2 "The following rust files were incorrectly formatted, run \`$0 -f\` to reformat them:"
for bad_file in ${bad_files[*]}; do
echo >&2 ${bad_file}
done
exit 1
;;
0)
exit 0
;;
*)
else
cat << EOF >&2
An error occurred while checking the formatting of rust files.
Try running \`(cd "${NATIVE_ROOT}" && ${cmd[*]} --write-mode=diff)\` to investigate.
Try running \`(cd "${NATIVE_ROOT}" && ${cmd[*]} --write-mode=${write_mode})\` to investigate.
Its error is:
EOF
cd "${NATIVE_ROOT}" && ${cmd[*]} --write-mode=diff >/dev/null
exit 1
;;
esac
cd "${NATIVE_ROOT}" && ${cmd[*]} --write-mode=${write_mode} >/dev/null
fi
exit 1
fi
25 changes: 18 additions & 7 deletions src/rust/engine/graph/src/lib.rs
Expand Up @@ -608,16 +608,27 @@ impl<N: Node> Graph<N> {
// Get the Generations of all dependencies of the Node. We can trust that these have not changed
// since we began executing, as long as we are not currently marked dirty (see the method doc).
let dep_generations = inner
.pg
.neighbors_directed(entry_id, Direction::Outgoing)
.filter_map(|dep_id| inner.entry_for_id(dep_id))
.map(|entry| entry.generation())
.collect();
(inner.entry_for_id(entry_id).cloned(), entry_id, dep_generations)
.pg
.neighbors_directed(entry_id, Direction::Outgoing)
.filter_map(|dep_id| inner.entry_for_id(dep_id))
.map(|entry| entry.generation())
.collect();
(
inner.entry_for_id(entry_id).cloned(),
entry_id,
dep_generations,
)
};
if let Some(mut entry) = entry {
let mut inner = self.inner.lock().unwrap();
entry.complete(context, entry_id, run_token, dep_generations, result, &mut inner);
entry.complete(
context,
entry_id,
run_token,
dep_generations,
result,
&mut inner,
);
}
}

Expand Down
69 changes: 31 additions & 38 deletions src/rust/engine/process_execution/src/local.rs
Expand Up @@ -157,47 +157,40 @@ impl super::CommandRunner for CommandRunner {
BytesMut::with_capacity(8192),
None,
);
Self::outputs_stream_for_child(child)
.fold(
init,
|(mut stdout, mut stderr, mut exit_code), child_output| {
match child_output {
ChildOutput::Stdout(bytes) => stdout.extend_from_slice(&bytes),
ChildOutput::Stderr(bytes) => stderr.extend_from_slice(&bytes),
ChildOutput::Exit(code) => exit_code = code,
};
Ok((stdout, stderr, exit_code)) as Result<_, String>
},
)
Self::outputs_stream_for_child(child).fold(
init,
|(mut stdout, mut stderr, mut exit_code), child_output| {
match child_output {
ChildOutput::Stdout(bytes) => stdout.extend_from_slice(&bytes),
ChildOutput::Stderr(bytes) => stderr.extend_from_slice(&bytes),
ChildOutput::Exit(code) => exit_code = code,
};
Ok((stdout, stderr, exit_code)) as Result<_, String>
},
)
})
.and_then(move |(stdout, stderr, exit_code)| {
let output_snapshot = if output_file_paths.is_empty() && output_dir_paths.is_empty() {
future::ok(fs::Snapshot::empty()).to_boxed()
} else {
// Use no ignore patterns, because we are looking for explicitly listed paths.
future::done(
fs::PosixFS::new(
workdir_path2,
fs_pool,
&[],
)
)
.map_err(|err| {
format!(
"Error making posix_fs to fetch local process execution output files: {}",
err
)
})
.map(Arc::new)
.and_then(|posix_fs| {
CommandRunner::construct_output_snapshot(
store,
posix_fs,
output_file_paths,
output_dir_paths
)
})
.to_boxed()
future::done(fs::PosixFS::new(workdir_path2, fs_pool, &[]))
.map_err(|err| {
format!(
"Error making posix_fs to fetch local process execution output files: {}",
err
)
})
.map(Arc::new)
.and_then(|posix_fs| {
CommandRunner::construct_output_snapshot(
store,
posix_fs,
output_file_paths,
output_dir_paths,
)
})
.to_boxed()
};

output_snapshot
Expand All @@ -208,16 +201,16 @@ impl super::CommandRunner for CommandRunner {
output_directory: snapshot.digest,
})
.to_boxed()
}).then(move |result| {
})
.then(move |result| {
// Force workdir not to get dropped until after we've ingested the outputs
if !cleanup_local_dirs {
// This consumes the `TempDir` without deleting directory on the filesystem, meaning
// that the temporary directory will no longer be automatically deleted when dropped.
let preserved_path = workdir.into_path();
info!(
"preserved local process execution dir `{:?}` for {:?}",
preserved_path,
req_description
preserved_path, req_description
);
} // Else, workdir gets dropped here
result
Expand Down