From 29ef271d47c34eee6aa5025a95f016aa94b8cde0 Mon Sep 17 00:00:00 2001 From: Christian Duerr Date: Mon, 6 Jan 2020 17:04:16 +0100 Subject: [PATCH 1/9] Search for root manifest with ephemeral workspaces Fixes #5495. --- src/cargo/core/workspace.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/cargo/core/workspace.rs b/src/cargo/core/workspace.rs index b34ea9acaf8..7854ed2c71f 100644 --- a/src/cargo/core/workspace.rs +++ b/src/cargo/core/workspace.rs @@ -198,7 +198,9 @@ impl<'cfg> Workspace<'cfg> { target_dir: Option, require_optional_deps: bool, ) -> CargoResult> { - let mut ws = Workspace::new_default(package.manifest_path().to_path_buf(), config); + let manifest_path = package.manifest_path(); + let mut ws = Workspace::new_default(manifest_path.to_path_buf(), config); + ws.root_manifest = ws.find_root(&manifest_path)?; ws.is_ephemeral = true; ws.require_optional_deps = require_optional_deps; let key = ws.current_manifest.parent().unwrap(); From a08fe088b36a0dccb706e654d005a9186a4fe9fd Mon Sep 17 00:00:00 2001 From: Christian Duerr Date: Mon, 6 Jan 2020 22:02:26 +0100 Subject: [PATCH 2/9] Fix failing test --- tests/testsuite/install.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/testsuite/install.rs b/tests/testsuite/install.rs index 98dd12fe26c..3d7ae76e03a 100644 --- a/tests/testsuite/install.rs +++ b/tests/testsuite/install.rs @@ -316,10 +316,10 @@ fn multiple_crates_git_all() { let p = git::repo(&paths::root().join("foo")) .file( "Cargo.toml", - r#"\ -[workspace] -members = ["bin1", "bin2"] -"#, + r#" + [workspace] + members = ["bin1", "bin2"] + "#, ) .file("bin1/Cargo.toml", &basic_manifest("bin1", "0.1.0")) .file("bin2/Cargo.toml", &basic_manifest("bin2", "0.1.0")) From ae8dee513dd8593a189076a706552bc18e102edd Mon Sep 17 00:00:00 2001 From: Christian Duerr Date: Mon, 6 Jan 2020 22:36:33 +0100 Subject: [PATCH 3/9] Test that git install reads virtual manifest --- tests/testsuite/install.rs | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/tests/testsuite/install.rs b/tests/testsuite/install.rs index 3d7ae76e03a..c2777f85acb 100644 --- a/tests/testsuite/install.rs +++ b/tests/testsuite/install.rs @@ -1422,3 +1422,29 @@ fn install_version_req() { .with_stderr_contains("[INSTALLING] foo v0.0.3") .run(); } + +#[cargo_test] +fn git_install_reads_workspace_manifest() { + let p = git::repo(&paths::root().join("foo")) + .file( + "Cargo.toml", + r#" + [workspace] + members = ["bin1"] + + [profile.release] + incremental = 3 + "#, + ) + .file("bin1/Cargo.toml", &basic_manifest("bin1", "0.1.0")) + .file( + "bin1/src/main.rs", + r#"fn main() { println!("Hello, world!"); }"#, + ) + .build(); + + cargo_process(&format!("install --git {}", p.url().to_string())) + .with_status(101) + .with_stderr_contains(" invalid type: integer `3`[..]") + .run(); +} From e137ee6979481aad64e524c037618ed6451a7c75 Mon Sep 17 00:00:00 2001 From: Christian Duerr Date: Tue, 14 Jan 2020 01:05:30 +0100 Subject: [PATCH 4/9] Use non-ephemeral workspace --- src/cargo/core/workspace.rs | 15 ++++++++++++--- src/cargo/ops/cargo_install.rs | 13 ++++++++++++- 2 files changed, 24 insertions(+), 4 deletions(-) diff --git a/src/cargo/core/workspace.rs b/src/cargo/core/workspace.rs index 7854ed2c71f..dc08811f248 100644 --- a/src/cargo/core/workspace.rs +++ b/src/cargo/core/workspace.rs @@ -198,9 +198,7 @@ impl<'cfg> Workspace<'cfg> { target_dir: Option, require_optional_deps: bool, ) -> CargoResult> { - let manifest_path = package.manifest_path(); - let mut ws = Workspace::new_default(manifest_path.to_path_buf(), config); - ws.root_manifest = ws.find_root(&manifest_path)?; + let mut ws = Workspace::new_default(package.manifest_path().to_path_buf(), config); ws.is_ephemeral = true; ws.require_optional_deps = require_optional_deps; let key = ws.current_manifest.parent().unwrap(); @@ -831,6 +829,17 @@ impl<'cfg> Workspace<'cfg> { } Ok(()) } + + pub fn set_target_dir(&mut self, target_dir: Filesystem) { + self.target_dir = Some(target_dir); + } + + // TODO: This seems like the wrong approach + pub fn set_package(&mut self, package: Package) { + let key = self.current_manifest.parent().unwrap(); + let package = MaybePackage::Package(package); + self.packages.packages.insert(key.to_path_buf(), package); + } } impl<'cfg> Packages<'cfg> { diff --git a/src/cargo/ops/cargo_install.rs b/src/cargo/ops/cargo_install.rs index a2c5565e79f..5622c459464 100644 --- a/src/cargo/ops/cargo_install.rs +++ b/src/cargo/ops/cargo_install.rs @@ -202,7 +202,7 @@ fn install_one( let mut td_opt = None; let mut needs_cleanup = false; - let overidden_target_dir = if source_id.is_path() { + let overidden_target_dir = if source_id.is_git() || source_id.is_path() { None } else if let Some(dir) = config.target_dir()? { Some(dir) @@ -220,6 +220,17 @@ fn install_one( None => { let mut ws = Workspace::new(pkg.manifest_path(), config)?; ws.set_require_optional_deps(false); + + // Use tempdir to build git depedencies to prevent bloat in cargo cache + if source_id.is_git() && config.target_dir()?.is_none() { + match TempFileBuilder::new().prefix("cargo-install").tempdir() { + Ok(td) => ws.set_target_dir(Filesystem::new(td.path().to_owned())), + // If tempfile creation fails, write to cargo cache but clean up afterwards + Err(_) => needs_cleanup = true, + } + } + ws.set_package(pkg); + ws } }; From 483ad7c00f2fa74a0a341a5d22546a0df6349dd5 Mon Sep 17 00:00:00 2001 From: Christian Duerr Date: Mon, 20 Jan 2020 00:40:56 +0100 Subject: [PATCH 5/9] Keep existing package with git install --- src/cargo/core/workspace.rs | 7 ------- src/cargo/ops/cargo_install.rs | 19 +++++++++++-------- 2 files changed, 11 insertions(+), 15 deletions(-) diff --git a/src/cargo/core/workspace.rs b/src/cargo/core/workspace.rs index dc08811f248..c2cb23a8fa8 100644 --- a/src/cargo/core/workspace.rs +++ b/src/cargo/core/workspace.rs @@ -833,13 +833,6 @@ impl<'cfg> Workspace<'cfg> { pub fn set_target_dir(&mut self, target_dir: Filesystem) { self.target_dir = Some(target_dir); } - - // TODO: This seems like the wrong approach - pub fn set_package(&mut self, package: Package) { - let key = self.current_manifest.parent().unwrap(); - let package = MaybePackage::Package(package); - self.packages.packages.insert(key.to_path_buf(), package); - } } impl<'cfg> Packages<'cfg> { diff --git a/src/cargo/ops/cargo_install.rs b/src/cargo/ops/cargo_install.rs index 5622c459464..9252256e1e3 100644 --- a/src/cargo/ops/cargo_install.rs +++ b/src/cargo/ops/cargo_install.rs @@ -215,6 +215,7 @@ fn install_one( Some(Filesystem::new(config.cwd().join("target-install"))) }; + let mut git_package = None; let mut ws = match overidden_target_dir { Some(dir) => Workspace::ephemeral(pkg, config, Some(dir), false)?, None => { @@ -222,20 +223,22 @@ fn install_one( ws.set_require_optional_deps(false); // Use tempdir to build git depedencies to prevent bloat in cargo cache - if source_id.is_git() && config.target_dir()?.is_none() { - match TempFileBuilder::new().prefix("cargo-install").tempdir() { - Ok(td) => ws.set_target_dir(Filesystem::new(td.path().to_owned())), - // If tempfile creation fails, write to cargo cache but clean up afterwards - Err(_) => needs_cleanup = true, + if source_id.is_git() { + if config.target_dir()?.is_none() { + match TempFileBuilder::new().prefix("cargo-install").tempdir() { + Ok(td) => ws.set_target_dir(Filesystem::new(td.path().to_owned())), + // If tempfile creation fails, write to cargo cache but clean up afterwards + Err(_) => needs_cleanup = true, + } } + git_package = Some(&pkg); } - ws.set_package(pkg); ws - } + }, }; ws.set_ignore_lock(config.lock_update_allowed()); - let pkg = ws.current()?; + let pkg = git_package.map_or_else(|| ws.current(), |pkg| Ok(pkg))?; if from_cwd { if pkg.manifest().edition() == Edition::Edition2015 { From 085e4c719df920bac3f9217388971a15fb10be2a Mon Sep 17 00:00:00 2001 From: Christian Duerr Date: Mon, 20 Jan 2020 01:15:02 +0100 Subject: [PATCH 6/9] Remove tempdir after install --- src/cargo/ops/cargo_install.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/cargo/ops/cargo_install.rs b/src/cargo/ops/cargo_install.rs index 9252256e1e3..6438fdc898b 100644 --- a/src/cargo/ops/cargo_install.rs +++ b/src/cargo/ops/cargo_install.rs @@ -226,7 +226,11 @@ fn install_one( if source_id.is_git() { if config.target_dir()?.is_none() { match TempFileBuilder::new().prefix("cargo-install").tempdir() { - Ok(td) => ws.set_target_dir(Filesystem::new(td.path().to_owned())), + Ok(td) => { + let p = td.path().to_owned(); + td_opt = Some(td); + ws.set_target_dir(Filesystem::new(p)); + } // If tempfile creation fails, write to cargo cache but clean up afterwards Err(_) => needs_cleanup = true, } From b791fcfc212cb3b49eabe64119daa024c0c619f3 Mon Sep 17 00:00:00 2001 From: Christian Duerr Date: Mon, 20 Jan 2020 01:17:21 +0100 Subject: [PATCH 7/9] Format code --- src/cargo/ops/cargo_install.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/cargo/ops/cargo_install.rs b/src/cargo/ops/cargo_install.rs index 6438fdc898b..d965873fa6d 100644 --- a/src/cargo/ops/cargo_install.rs +++ b/src/cargo/ops/cargo_install.rs @@ -239,7 +239,7 @@ fn install_one( } ws - }, + } }; ws.set_ignore_lock(config.lock_update_allowed()); let pkg = git_package.map_or_else(|| ws.current(), |pkg| Ok(pkg))?; From 8786dec1ba0a2ce2147de76e2a7a28e1bd3890ec Mon Sep 17 00:00:00 2001 From: Christian Duerr Date: Fri, 24 Jan 2020 22:08:32 +0100 Subject: [PATCH 8/9] Refactor code --- src/cargo/ops/cargo_install.rs | 62 +++++++++++++--------------------- 1 file changed, 24 insertions(+), 38 deletions(-) diff --git a/src/cargo/ops/cargo_install.rs b/src/cargo/ops/cargo_install.rs index d965873fa6d..ca1993772ee 100644 --- a/src/cargo/ops/cargo_install.rs +++ b/src/cargo/ops/cargo_install.rs @@ -200,48 +200,34 @@ fn install_one( )? }; - let mut td_opt = None; - let mut needs_cleanup = false; - let overidden_target_dir = if source_id.is_git() || source_id.is_path() { - None - } else if let Some(dir) = config.target_dir()? { - Some(dir) - } else if let Ok(td) = TempFileBuilder::new().prefix("cargo-install").tempdir() { - let p = td.path().to_owned(); - td_opt = Some(td); - Some(Filesystem::new(p)) + let (mut ws, git_package) = if source_id.is_git() { + // Don't use ws.current() in order to keep the package source as a git source so that + // install tracking uses the correct sourc. + (Workspace::new(pkg.manifest_path(), config)?, Some(&pkg)) + } else if source_id.is_path() { + (Workspace::new(pkg.manifest_path(), config)?, None) } else { - needs_cleanup = true; - Some(Filesystem::new(config.cwd().join("target-install"))) + (Workspace::ephemeral(pkg, config, None, false)?, None) }; + ws.set_ignore_lock(config.lock_update_allowed()); + ws.set_require_optional_deps(false); - let mut git_package = None; - let mut ws = match overidden_target_dir { - Some(dir) => Workspace::ephemeral(pkg, config, Some(dir), false)?, - None => { - let mut ws = Workspace::new(pkg.manifest_path(), config)?; - ws.set_require_optional_deps(false); - - // Use tempdir to build git depedencies to prevent bloat in cargo cache - if source_id.is_git() { - if config.target_dir()?.is_none() { - match TempFileBuilder::new().prefix("cargo-install").tempdir() { - Ok(td) => { - let p = td.path().to_owned(); - td_opt = Some(td); - ws.set_target_dir(Filesystem::new(p)); - } - // If tempfile creation fails, write to cargo cache but clean up afterwards - Err(_) => needs_cleanup = true, - } - } - git_package = Some(&pkg); - } + let mut td_opt = None; + let mut needs_cleanup = false; + if !source_id.is_path() { + let target_dir = if let Some(dir) = config.target_dir()? { + dir + } else if let Ok(td) = TempFileBuilder::new().prefix("cargo-install").tempdir() { + let p = td.path().to_owned(); + td_opt = Some(td); + Filesystem::new(p) + } else { + needs_cleanup = true; + Filesystem::new(config.cwd().join("target-install")) + }; + ws.set_target_dir(target_dir); + } - ws - } - }; - ws.set_ignore_lock(config.lock_update_allowed()); let pkg = git_package.map_or_else(|| ws.current(), |pkg| Ok(pkg))?; if from_cwd { From 601d04c2ce218e68f4f30fdc49a76819b725a0a7 Mon Sep 17 00:00:00 2001 From: Eric Huss Date: Mon, 27 Jan 2020 10:12:22 -0800 Subject: [PATCH 9/9] Fix typo. --- src/cargo/ops/cargo_install.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/cargo/ops/cargo_install.rs b/src/cargo/ops/cargo_install.rs index ca1993772ee..505b940e8d4 100644 --- a/src/cargo/ops/cargo_install.rs +++ b/src/cargo/ops/cargo_install.rs @@ -202,7 +202,7 @@ fn install_one( let (mut ws, git_package) = if source_id.is_git() { // Don't use ws.current() in order to keep the package source as a git source so that - // install tracking uses the correct sourc. + // install tracking uses the correct source. (Workspace::new(pkg.manifest_path(), config)?, Some(&pkg)) } else if source_id.is_path() { (Workspace::new(pkg.manifest_path(), config)?, None)