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 a --dry-run flag to the install command #14280

Merged
merged 3 commits into from
Sep 21, 2024
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion src/bin/cargo/commands/install.rs
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ pub fn cli() -> Command {
)
.arg(opt("root", "Directory to install packages into").value_name("DIR"))
.arg(flag("force", "Force overwriting existing crates or binaries").short('f'))
.arg_dry_run("Perform all checks without installing (unstable)")
.arg(flag("no-track", "Do not save tracking information"))
.arg(flag(
"list",
Expand Down Expand Up @@ -200,7 +201,9 @@ pub fn exec(gctx: &mut GlobalContext, args: &ArgMatches) -> CliResult {

compile_opts.build_config.requested_profile =
args.get_profile_name("release", ProfileChecking::Custom)?;

if args.dry_run() {
gctx.cli_unstable().fail_if_stable_opt("--dry-run", 11123)?;
}
if args.flag("list") {
ops::install_list(root, gctx)?;
} else {
Expand All @@ -213,6 +216,7 @@ pub fn exec(gctx: &mut GlobalContext, args: &ArgMatches) -> CliResult {
&compile_opts,
args.flag("force"),
args.flag("no-track"),
args.dry_run(),
)?;
}
Ok(())
Expand Down
3 changes: 3 additions & 0 deletions src/cargo/core/compiler/build_config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@ pub struct BuildConfig {
pub build_plan: bool,
/// Output the unit graph to stdout instead of actually compiling.
pub unit_graph: bool,
/// `true` to avoid really compiling.
pub dry_run: bool,
/// An optional override of the rustc process for primary units
pub primary_unit_rustc: Option<ProcessBuilder>,
/// A thread used by `cargo fix` to receive messages on a socket regarding
Expand Down Expand Up @@ -112,6 +114,7 @@ impl BuildConfig {
force_rebuild: false,
build_plan: false,
unit_graph: false,
dry_run: false,
primary_unit_rustc: None,
rustfix_diagnostic_server: Rc::new(RefCell::new(None)),
export_dir: None,
Expand Down
74 changes: 49 additions & 25 deletions src/cargo/core/compiler/build_runner/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,27 @@ impl<'a, 'gctx> BuildRunner<'a, 'gctx> {
})
}

/// Dry-run the compilation without actually running it.
///
/// This is expected to collect information like the location of output artifacts.
/// Please keep in sync with non-compilation part in [`BuildRunner::compile`].
pub fn dry_run(mut self) -> CargoResult<Compilation<'gctx>> {
weihanglo marked this conversation as resolved.
Show resolved Hide resolved
let _lock = self
.bcx
.gctx
.acquire_package_cache_lock(CacheLockMode::Shared)?;
self.lto = super::lto::generate(self.bcx)?;
self.prepare_units()?;
self.prepare()?;
self.check_collisions()?;

for unit in &self.bcx.roots {
self.collect_tests_and_executables(unit)?;
}

Ok(self.compilation)
}

/// Starts compilation, waits for it to finish, and returns information
/// about the result of compilation.
///
Expand Down Expand Up @@ -214,31 +235,7 @@ impl<'a, 'gctx> BuildRunner<'a, 'gctx> {

// Collect the result of the build into `self.compilation`.
for unit in &self.bcx.roots {
// Collect tests and executables.
for output in self.outputs(unit)?.iter() {
if output.flavor == FileFlavor::DebugInfo || output.flavor == FileFlavor::Auxiliary
{
continue;
}

let bindst = output.bin_dst();

if unit.mode == CompileMode::Test {
self.compilation
.tests
.push(self.unit_output(unit, &output.path));
} else if unit.target.is_executable() {
self.compilation
.binaries
.push(self.unit_output(unit, bindst));
} else if unit.target.is_cdylib()
&& !self.compilation.cdylibs.iter().any(|uo| uo.unit == *unit)
{
self.compilation
.cdylibs
.push(self.unit_output(unit, bindst));
}
}
self.collect_tests_and_executables(unit)?;

// Collect information for `rustdoc --test`.
if unit.mode.is_doc_test() {
Expand Down Expand Up @@ -307,6 +304,33 @@ impl<'a, 'gctx> BuildRunner<'a, 'gctx> {
Ok(self.compilation)
}

fn collect_tests_and_executables(&mut self, unit: &Unit) -> CargoResult<()> {
for output in self.outputs(unit)?.iter() {
if output.flavor == FileFlavor::DebugInfo || output.flavor == FileFlavor::Auxiliary {
continue;
}

let bindst = output.bin_dst();

if unit.mode == CompileMode::Test {
self.compilation
.tests
.push(self.unit_output(unit, &output.path));
} else if unit.target.is_executable() {
self.compilation
.binaries
.push(self.unit_output(unit, bindst));
} else if unit.target.is_cdylib()
&& !self.compilation.cdylibs.iter().any(|uo| uo.unit == *unit)
{
self.compilation
.cdylibs
.push(self.unit_output(unit, bindst));
}
}
Ok(())
}

/// Returns the executable for the specified unit (if any).
pub fn get_executable(&mut self, unit: &Unit) -> CargoResult<Option<PathBuf>> {
let is_binary = unit.target.is_executable();
Expand Down
6 changes: 5 additions & 1 deletion src/cargo/ops/cargo_compile/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,11 @@ pub fn compile_ws<'a>(
}
crate::core::gc::auto_gc(bcx.gctx);
let build_runner = BuildRunner::new(&bcx)?;
build_runner.compile(exec)
if options.build_config.dry_run {
build_runner.dry_run()
} else {
build_runner.compile(exec)
}
}

/// Executes `rustc --print <VALUE>`.
Expand Down
67 changes: 43 additions & 24 deletions src/cargo/ops/cargo_install.rs
Original file line number Diff line number Diff line change
Expand Up @@ -297,7 +297,7 @@ impl<'gctx> InstallablePackage<'gctx> {
Ok(duplicates)
}

fn install_one(mut self) -> CargoResult<bool> {
fn install_one(mut self, dry_run: bool) -> CargoResult<bool> {
self.gctx.shell().status("Installing", &self.pkg)?;

let dst = self.root.join("bin").into_path_unlocked();
Expand All @@ -321,6 +321,7 @@ impl<'gctx> InstallablePackage<'gctx> {
self.check_yanked_install()?;

let exec: Arc<dyn Executor> = Arc::new(DefaultExecutor);
self.opts.build_config.dry_run = dry_run;
let compile = ops::compile_ws(&self.ws, &self.opts, &exec).with_context(|| {
if let Some(td) = td_opt.take() {
// preserve the temporary directory, so the user can inspect it
Expand Down Expand Up @@ -419,13 +420,15 @@ impl<'gctx> InstallablePackage<'gctx> {
let staging_dir = TempFileBuilder::new()
.prefix("cargo-install")
.tempdir_in(&dst)?;
for &(bin, src) in binaries.iter() {
let dst = staging_dir.path().join(bin);
// Try to move if `target_dir` is transient.
if !self.source_id.is_path() && fs::rename(src, &dst).is_ok() {
continue;
if !dry_run {
for &(bin, src) in binaries.iter() {
let dst = staging_dir.path().join(bin);
// Try to move if `target_dir` is transient.
if !self.source_id.is_path() && fs::rename(src, &dst).is_ok() {
continue;
}
paths::copy(src, &dst)?;
}
paths::copy(src, &dst)?;
}

let (to_replace, to_install): (Vec<&str>, Vec<&str>) = binaries
Expand All @@ -441,11 +444,13 @@ impl<'gctx> InstallablePackage<'gctx> {
let src = staging_dir.path().join(bin);
let dst = dst.join(bin);
self.gctx.shell().status("Installing", dst.display())?;
fs::rename(&src, &dst).with_context(|| {
format!("failed to move `{}` to `{}`", src.display(), dst.display())
})?;
installed.bins.push(dst);
successful_bins.insert(bin.to_string());
if !dry_run {
fs::rename(&src, &dst).with_context(|| {
format!("failed to move `{}` to `{}`", src.display(), dst.display())
})?;
installed.bins.push(dst);
successful_bins.insert(bin.to_string());
}
}
weihanglo marked this conversation as resolved.
Show resolved Hide resolved

// Repeat for binaries which replace existing ones but don't pop the error
Expand All @@ -456,10 +461,12 @@ impl<'gctx> InstallablePackage<'gctx> {
let src = staging_dir.path().join(bin);
let dst = dst.join(bin);
self.gctx.shell().status("Replacing", dst.display())?;
fs::rename(&src, &dst).with_context(|| {
format!("failed to move `{}` to `{}`", src.display(), dst.display())
})?;
successful_bins.insert(bin.to_string());
if !dry_run {
fs::rename(&src, &dst).with_context(|| {
format!("failed to move `{}` to `{}`", src.display(), dst.display())
})?;
successful_bins.insert(bin.to_string());
}
}
Ok(())
};
Expand All @@ -476,9 +483,14 @@ impl<'gctx> InstallablePackage<'gctx> {
&self.rustc.verbose_version,
);

if let Err(e) =
remove_orphaned_bins(&self.ws, &mut tracker, &duplicates, &self.pkg, &dst)
{
if let Err(e) = remove_orphaned_bins(
&self.ws,
&mut tracker,
&duplicates,
&self.pkg,
&dst,
dry_run,
) {
// Don't hard error on remove.
self.gctx
.shell()
Expand Down Expand Up @@ -515,7 +527,10 @@ impl<'gctx> InstallablePackage<'gctx> {
}
}

if duplicates.is_empty() {
if dry_run {
self.gctx.shell().warn("aborting install due to dry run")?;
Ok(true)
} else if duplicates.is_empty() {
self.gctx.shell().status(
"Installed",
format!(
Expand Down Expand Up @@ -620,6 +635,7 @@ pub fn install(
opts: &ops::CompileOptions,
force: bool,
no_track: bool,
dry_run: bool,
) -> CargoResult<()> {
let root = resolve_root(root, gctx)?;
let dst = root.join("bin").into_path_unlocked();
Expand Down Expand Up @@ -654,7 +670,7 @@ pub fn install(
)?;
let mut installed_anything = true;
if let Some(installable_pkg) = installable_pkg {
installed_anything = installable_pkg.install_one()?;
installed_anything = installable_pkg.install_one(dry_run)?;
}
(installed_anything, false)
} else {
Expand Down Expand Up @@ -705,7 +721,7 @@ pub fn install(

let install_results: Vec<_> = pkgs_to_install
.into_iter()
.map(|(krate, installable_pkg)| (krate, installable_pkg.install_one()))
.map(|(krate, installable_pkg)| (krate, installable_pkg.install_one(dry_run)))
.collect();

for (krate, result) in install_results {
Expand Down Expand Up @@ -857,6 +873,7 @@ fn remove_orphaned_bins(
duplicates: &BTreeMap<String, Option<PackageId>>,
pkg: &Package,
dst: &Path,
dry_run: bool,
) -> CargoResult<()> {
let filter = ops::CompileFilter::new_all_targets();
let all_self_names = exe_names(pkg, &filter);
Expand Down Expand Up @@ -894,8 +911,10 @@ fn remove_orphaned_bins(
old_pkg
),
)?;
paths::remove_file(&full_path)
.with_context(|| format!("failed to remove {:?}", full_path))?;
if !dry_run {
paths::remove_file(&full_path)
.with_context(|| format!("failed to remove {:?}", full_path))?;
}
}
}
}
Expand Down
4 changes: 4 additions & 0 deletions src/doc/man/cargo-install.md
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,10 @@ Filesystem path to local crate to install from.
List all installed packages and their versions.
{{/option}}

{{#option "`-n`" "`--dry-run`" }}
(unstable) Perform all checks without installing.
{{/option}}

{{#option "`-f`" "`--force`" }}
Force overwriting existing crates or binaries. This can be used if a package
has installed a binary with the same name as another package. This is also
Expand Down
3 changes: 3 additions & 0 deletions src/doc/man/generated_txt/cargo-install.txt
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,9 @@ OPTIONS
--list
List all installed packages and their versions.

-n, --dry-run
(unstable) Perform all checks without installing.

-f, --force
Force overwriting existing crates or binaries. This can be used if a
package has installed a binary with the same name as another
Expand Down
5 changes: 5 additions & 0 deletions src/doc/src/commands/cargo-install.md
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,11 @@ treated as a caret requirement like Cargo dependencies are.</dd>
<dd class="option-desc">List all installed packages and their versions.</dd>


<dt class="option-term" id="option-cargo-install--n"><a class="option-anchor" href="#option-cargo-install--n"></a><code>-n</code></dt>
<dt class="option-term" id="option-cargo-install---dry-run"><a class="option-anchor" href="#option-cargo-install---dry-run"></a><code>--dry-run</code></dt>
<dd class="option-desc">(unstable) Perform all checks without installing.</dd>


<dt class="option-term" id="option-cargo-install--f"><a class="option-anchor" href="#option-cargo-install--f"></a><code>-f</code></dt>
<dt class="option-term" id="option-cargo-install---force"><a class="option-anchor" href="#option-cargo-install---force"></a><code>--force</code></dt>
<dd class="option-desc">Force overwriting existing crates or binaries. This can be used if a package
Expand Down
6 changes: 6 additions & 0 deletions src/etc/man/cargo-install.1
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,12 @@ Filesystem path to local crate to install from.
List all installed packages and their versions.
.RE
.sp
\fB\-n\fR,
\fB\-\-dry\-run\fR
.RS 4
(unstable) Perform all checks without installing.
.RE
.sp
\fB\-f\fR,
\fB\-\-force\fR
.RS 4
Expand Down
Loading