From 5e644df1fffc5726b2bf4ce8e29990602208ac51 Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Sun, 21 Aug 2016 00:27:06 +0300 Subject: [PATCH] Forward compatibility with rootless lockfiles --- src/cargo/core/resolver/encode.rs | 30 ++++++++++++++-------- src/cargo/ops/lockfile.rs | 38 +++++++++++++++++----------- tests/bad-config.rs | 4 +-- tests/lockfile-compat.rs | 41 +++++++++++++++++++++++++++++++ 4 files changed, 86 insertions(+), 27 deletions(-) diff --git a/src/cargo/core/resolver/encode.rs b/src/cargo/core/resolver/encode.rs index 97623b718ab..1f260761aec 100644 --- a/src/cargo/core/resolver/encode.rs +++ b/src/cargo/core/resolver/encode.rs @@ -13,7 +13,8 @@ use super::Resolve; #[derive(RustcEncodable, RustcDecodable, Debug)] pub struct EncodableResolve { package: Option>, - root: EncodableDependency, + /// `root` is optional to allow forward compatibility. + root: Option, metadata: Option, } @@ -28,9 +29,6 @@ impl EncodableResolve { let mut tmp = HashMap::new(); let mut replacements = HashMap::new(); - let packages = Vec::new(); - let packages = self.package.as_ref().unwrap_or(&packages); - let id2pkgid = |id: &EncodablePackageId| { to_package_id(&id.name, &id.version, id.source.as_ref(), default, &path_deps) @@ -40,7 +38,14 @@ impl EncodableResolve { default, &path_deps) }; - let root = try!(dep2pkgid(&self.root)); + let packages = { + let mut packages = self.package.unwrap_or(Vec::new()); + if let Some(root) = self.root { + packages.insert(0, root); + } + packages + }; + let ids = try!(packages.iter().map(&dep2pkgid) .collect::>>()); @@ -56,7 +61,6 @@ impl EncodableResolve { Ok(()) }; - try!(register_pkg(&root)); for id in ids.iter() { try!(register_pkg(id)); } @@ -90,8 +94,7 @@ impl EncodableResolve { Ok(()) }; - try!(add_dependencies(&root, &self.root)); - for (id, pkg) in ids.iter().zip(packages) { + for (id, ref pkg) in ids.iter().zip(packages) { try!(add_dependencies(id, pkg)); } } @@ -268,6 +271,7 @@ impl Decodable for EncodablePackageId { pub struct WorkspaceResolve<'a, 'cfg: 'a> { pub ws: &'a Workspace<'cfg>, pub resolve: &'a Resolve, + pub use_root_key: bool, } impl<'a, 'cfg> Encodable for WorkspaceResolve<'a, 'cfg> { @@ -280,7 +284,7 @@ impl<'a, 'cfg> Encodable for WorkspaceResolve<'a, 'cfg> { }).unwrap().package_id(); let encodable = ids.iter().filter_map(|&id| { - if root == id { + if self.use_root_key && root == id { return None } @@ -300,9 +304,15 @@ impl<'a, 'cfg> Encodable for WorkspaceResolve<'a, 'cfg> { } let metadata = if metadata.len() == 0 {None} else {Some(metadata)}; + + let root = if self.use_root_key { + Some(encodable_resolve_node(&root, self.resolve)) + } else { + None + }; EncodableResolve { package: Some(encodable), - root: encodable_resolve_node(&root, self.resolve), + root: root, metadata: metadata, }.encode(s) } diff --git a/src/cargo/ops/lockfile.rs b/src/cargo/ops/lockfile.rs index 2afb4d534db..ac9fcbddf31 100644 --- a/src/cargo/ops/lockfile.rs +++ b/src/cargo/ops/lockfile.rs @@ -33,10 +33,28 @@ pub fn load_pkg_lockfile(ws: &Workspace) -> CargoResult> { } pub fn write_pkg_lockfile(ws: &Workspace, resolve: &Resolve) -> CargoResult<()> { + // Load the original lockfile if it exists. + let ws_root = Filesystem::new(ws.root().to_path_buf()); + let orig = ws_root.open_ro("Cargo.lock", ws.config(), "Cargo.lock file"); + let orig = orig.and_then(|mut f| { + let mut s = String::new(); + try!(f.read_to_string(&mut s)); + Ok(s) + }); + + // Forward compatibility: if `orig` uses rootless format + // from the future, do the same. + let use_root_key = if let Ok(ref orig) = orig { + !orig.starts_with("[[package]]") + } else { + true + }; + let mut e = Encoder::new(); WorkspaceResolve { ws: ws, resolve: resolve, + use_root_key: use_root_key, }.encode(&mut e).unwrap(); let mut out = String::new(); @@ -44,10 +62,10 @@ pub fn write_pkg_lockfile(ws: &Workspace, resolve: &Resolve) -> CargoResult<()> // Note that we do not use e.toml.to_string() as we want to control the // exact format the toml is in to ensure pretty diffs between updates to the // lockfile. - let root = e.toml.get(&"root".to_string()).unwrap(); - - out.push_str("[root]\n"); - emit_package(root.as_table().unwrap(), &mut out); + if let Some(root) = e.toml.get(&"root".to_string()) { + out.push_str("[root]\n"); + emit_package(root.as_table().unwrap(), &mut out); + } let deps = e.toml.get(&"package".to_string()).unwrap().as_slice().unwrap(); for dep in deps.iter() { @@ -65,18 +83,8 @@ pub fn write_pkg_lockfile(ws: &Workspace, resolve: &Resolve) -> CargoResult<()> None => {} } - let root = Filesystem::new(ws.root().to_path_buf()); - - // Load the original lockfile if it exists. - // // If the lockfile contents haven't changed so don't rewrite it. This is // helpful on read-only filesystems. - let orig = root.open_ro("Cargo.lock", ws.config(), "Cargo.lock file"); - let orig = orig.and_then(|mut f| { - let mut s = String::new(); - try!(f.read_to_string(&mut s)); - Ok(s) - }); if let Ok(orig) = orig { if has_crlf_line_endings(&orig) { out = out.replace("\n", "\r\n"); @@ -93,7 +101,7 @@ pub fn write_pkg_lockfile(ws: &Workspace, resolve: &Resolve) -> CargoResult<()> } // Ok, if that didn't work just write it out - root.open_rw("Cargo.lock", ws.config(), "Cargo.lock file").and_then(|mut f| { + ws_root.open_rw("Cargo.lock", ws.config(), "Cargo.lock file").and_then(|mut f| { try!(f.file().set_len(0)); try!(f.write_all(out.as_bytes())); Ok(()) diff --git a/tests/bad-config.rs b/tests/bad-config.rs index 9e6aad274cc..b62939c847b 100644 --- a/tests/bad-config.rs +++ b/tests/bad-config.rs @@ -224,7 +224,7 @@ fn bad_cargo_lock() { version = "0.0.0" authors = [] "#) - .file("Cargo.lock", "") + .file("Cargo.lock", "[[package]]\nfoo = 92") .file("src/lib.rs", ""); assert_that(foo.cargo_process("build").arg("-v"), @@ -232,7 +232,7 @@ fn bad_cargo_lock() { [ERROR] failed to parse lock file at: [..]Cargo.lock Caused by: - expected a section for the key `root` + expected a value of type `string` for the key `package.name` ")); } diff --git a/tests/lockfile-compat.rs b/tests/lockfile-compat.rs index 5462339fd91..4ebba15ea18 100644 --- a/tests/lockfile-compat.rs +++ b/tests/lockfile-compat.rs @@ -276,3 +276,44 @@ unable to verify that `foo v0.1.0 ([..])` is the same as when the lockfile was g ")); } + +#[test] +fn lockfile_without_root() { + Package::new("foo", "0.1.0").publish(); + + let p = project("bar") + .file("Cargo.toml", r#" + [package] + name = "bar" + version = "0.0.1" + authors = [] + + [dependencies] + foo = "0.1.0" + "#) + .file("src/lib.rs", ""); + p.build(); + + let lockfile = r#"[[package]] +name = "bar" +version = "0.0.1" +dependencies = [ + "foo 0.1.0 (registry+https://github.com/rust-lang/crates.io-index)", +] + +[[package]] +name = "foo" +version = "0.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +"#; + File::create(p.root().join("Cargo.lock")).unwrap() + .write_all(lockfile.as_bytes()).unwrap(); + + assert_that(p.cargo("build"), + execs().with_status(0)); + + let mut lock = String::new(); + File::open(p.root().join("Cargo.lock")).unwrap() + .read_to_string(&mut lock).unwrap(); + assert!(lock.starts_with(lockfile.trim())); +}