From f8634a16253598422832cc5f95844ca88dc0d69e Mon Sep 17 00:00:00 2001 From: Andrew Jones Date: Fri, 30 Sep 2022 10:46:45 +0100 Subject: [PATCH 1/3] Extract path rewrite type --- .../cargo-contract/src/workspace/manifest.rs | 256 ++++++++++-------- 1 file changed, 139 insertions(+), 117 deletions(-) diff --git a/crates/cargo-contract/src/workspace/manifest.rs b/crates/cargo-contract/src/workspace/manifest.rs index c3b2cec4b..6195935f4 100644 --- a/crates/cargo-contract/src/workspace/manifest.rs +++ b/crates/cargo-contract/src/workspace/manifest.rs @@ -316,124 +316,13 @@ impl Manifest { Ok(self) } - /// Replace relative paths with absolute paths with the working directory. - /// - /// Enables the use of a temporary amended copy of the manifest. - /// - /// # Rewrites - /// - /// - `[lib]/path` - /// - `[dependencies]` - /// - /// Dependencies with package names specified in `exclude_deps` will not be rewritten. - pub(super) fn rewrite_relative_paths( - &mut self, - exclude_deps: I, - ) -> Result<&mut Self> - where - I: IntoIterator, - S: AsRef, - { - let abs_path = self.path.as_ref().canonicalize()?; - let abs_dir = abs_path - .parent() - .expect("The manifest path is a file path so has a parent; qed"); - - let to_absolute = |value_id: String, - existing_path: &mut value::Value| - -> Result<()> { - let path_str = existing_path - .as_str() - .ok_or_else(|| anyhow::anyhow!("{} should be a string", value_id))?; - #[cfg(windows)] - // On Windows path separators are `\`, hence we need to replace the `/` in - // e.g. `src/lib.rs`. - let path_str = &path_str.replace("/", "\\"); - let path = PathBuf::from(path_str); - if path.is_relative() { - let lib_abs = abs_dir.join(path); - tracing::debug!("Rewriting {} to '{}'", value_id, lib_abs.display()); - *existing_path = value::Value::String(lib_abs.to_string_lossy().into()) - } - Ok(()) + pub fn rewrite_relative_paths(&mut self, exclude_deps: &[String]) -> Result<()> { + let manifest_dir = self.path.absolute_directory()?; + let path_rewrite = PathRewrite { + exclude_deps, + manifest_dir, }; - - let rewrite_path = - |table_value: &mut value::Value, table_section: &str, default: &str| { - let table = table_value.as_table_mut().ok_or_else(|| { - anyhow::anyhow!("'[{}]' section should be a table", table_section) - })?; - - match table.get_mut("path") { - Some(existing_path) => { - to_absolute(format!("[{}]/path", table_section), existing_path) - } - None => { - let default_path = PathBuf::from(default); - if !default_path.exists() { - anyhow::bail!( - "No path specified, and the default `{}` was not found", - default - ) - } - let path = abs_dir.join(default_path); - tracing::debug!("Adding default path '{}'", path.display()); - table.insert( - "path".into(), - value::Value::String(path.to_string_lossy().into()), - ); - Ok(()) - } - } - }; - - // Rewrite `[lib] path = /path/to/lib.rs` - if let Some(lib) = self.toml.get_mut("lib") { - rewrite_path(lib, "lib", "src/lib.rs")?; - } - - // Rewrite `[[bin]] path = /path/to/main.rs` - if let Some(bin) = self.toml.get_mut("bin") { - let bins = bin.as_array_mut().ok_or_else(|| { - anyhow::anyhow!("'[[bin]]' section should be a table array") - })?; - - // Rewrite `[[bin]] path =` value to an absolute path. - for bin in bins { - rewrite_path(bin, "[bin]", "src/main.rs")?; - } - } - - // Rewrite any dependency relative paths - if let Some(dependencies) = self.toml.get_mut("dependencies") { - let exclude = exclude_deps - .into_iter() - .map(|s| s.as_ref().to_string()) - .collect::>(); - let table = dependencies - .as_table_mut() - .ok_or_else(|| anyhow::anyhow!("dependencies should be a table"))?; - for (name, value) in table { - let package_name = { - let package = value.get("package"); - let package_name = package.and_then(|p| p.as_str()).unwrap_or(name); - package_name.to_string() - }; - - if !exclude.contains(&package_name) { - if let Some(dependency) = value.as_table_mut() { - if let Some(dep_path) = dependency.get_mut("path") { - to_absolute( - format!("dependency {}", package_name), - dep_path, - )?; - } - } - } - } - } - - Ok(self) + path_rewrite.rewrite_relative_paths(&mut self.toml) } /// Writes the amended manifest to the given path. @@ -484,6 +373,139 @@ impl Manifest { } } +struct PathRewrite<'a> { + exclude_deps: &'a [String], + manifest_dir: PathBuf, +} + +impl<'a> PathRewrite<'a> { + /// Replace relative paths with absolute paths with the working directory. + /// + /// Enables the use of a temporary amended copy of the manifest. + /// + /// # Rewrites + /// + /// - `[lib]/path` + /// - `[dependencies]` + /// + /// Dependencies with package names specified in `exclude_deps` will not be rewritten. + fn rewrite_relative_paths(&self, toml: &mut value::Table) -> Result<()> { + // Rewrite `[lib] path = /path/to/lib.rs` + if let Some(lib) = toml.get_mut("lib") { + self.rewrite_path(lib, "lib", "src/lib.rs")?; + } + + // Rewrite `[[bin]] path = /path/to/main.rs` + if let Some(bin) = toml.get_mut("bin") { + let bins = bin.as_array_mut().ok_or_else(|| { + anyhow::anyhow!("'[[bin]]' section should be a table array") + })?; + + // Rewrite `[[bin]] path =` value to an absolute path. + for bin in bins { + self.rewrite_path(bin, "[bin]", "src/main.rs")?; + } + } + + self.rewrite_dependencies_relative_paths(toml, "dependencies")?; + self.rewrite_dependencies_relative_paths(toml, "dev-dependencies")?; + + Ok(()) + } + + fn rewrite_path( + &self, + table_value: &mut value::Value, + table_section: &str, + default: &str, + ) -> Result<()> { + let table = table_value.as_table_mut().ok_or_else(|| { + anyhow::anyhow!("'[{}]' section should be a table", table_section) + })?; + + match table.get_mut("path") { + Some(existing_path) => { + self.to_absolute_path(format!("[{}]/path", table_section), existing_path) + } + None => { + let default_path = PathBuf::from(default); + if !default_path.exists() { + anyhow::bail!( + "No path specified, and the default `{}` was not found", + default + ) + } + let path = self.manifest_dir.join(default_path); + tracing::debug!("Adding default path '{}'", path.display()); + table.insert( + "path".into(), + value::Value::String(path.to_string_lossy().into()), + ); + Ok(()) + } + } + } + + /// Expand a relative path to an absolute path. + fn to_absolute_path( + &self, + value_id: String, + existing_path: &mut value::Value, + ) -> Result<()> { + let path_str = existing_path + .as_str() + .ok_or_else(|| anyhow::anyhow!("{} should be a string", value_id))?; + #[cfg(windows)] + // On Windows path separators are `\`, hence we need to replace the `/` in + // e.g. `src/lib.rs`. + let path_str = &path_str.replace("/", "\\"); + let path = PathBuf::from(path_str); + if path.is_relative() { + let lib_abs = self.manifest_dir.join(path); + tracing::debug!("Rewriting {} to '{}'", value_id, lib_abs.display()); + *existing_path = value::Value::String(lib_abs.to_string_lossy().into()) + } + Ok(()) + } + + /// Rewrite the relative paths of dependencies. + fn rewrite_dependencies_relative_paths( + &self, + toml: &mut value::Table, + section_name: &str, + ) -> Result<()> { + if let Some(dependencies) = toml.get_mut(section_name) { + let exclude = self + .exclude_deps + .into_iter() + .map(|s| s.clone()) + .collect::>(); + let table = dependencies + .as_table_mut() + .ok_or_else(|| anyhow::anyhow!("dependencies should be a table"))?; + for (name, value) in table { + let package_name = { + let package = value.get("package"); + let package_name = package.and_then(|p| p.as_str()).unwrap_or(name); + package_name.to_string() + }; + + if !exclude.contains(&package_name) { + if let Some(dependency) = value.as_table_mut() { + if let Some(dep_path) = dependency.get_mut("path") { + self.to_absolute_path( + format!("dependency {}", package_name), + dep_path, + )?; + } + } + } + } + } + Ok(()) + } +} + fn crate_type_exists(crate_type: &str, crate_types: &[value::Value]) -> bool { crate_types .iter() From ee67ebae8e708426d4de7b24b56e6b3e74589cc8 Mon Sep 17 00:00:00 2001 From: Andrew Jones Date: Fri, 30 Sep 2022 11:05:06 +0100 Subject: [PATCH 2/3] Add comment --- .../cargo-contract/src/workspace/manifest.rs | 20 ++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/crates/cargo-contract/src/workspace/manifest.rs b/crates/cargo-contract/src/workspace/manifest.rs index 6195935f4..ee9d17431 100644 --- a/crates/cargo-contract/src/workspace/manifest.rs +++ b/crates/cargo-contract/src/workspace/manifest.rs @@ -316,6 +316,16 @@ impl Manifest { Ok(self) } + /// Replace relative paths with absolute paths with the working directory. + /// + /// Enables the use of a temporary amended copy of the manifest. + /// + /// # Rewrites + /// + /// - `[lib]/path` + /// - `[dependencies]` + /// + /// Dependencies with package names specified in `exclude_deps` will not be rewritten. pub fn rewrite_relative_paths(&mut self, exclude_deps: &[String]) -> Result<()> { let manifest_dir = self.path.absolute_directory()?; let path_rewrite = PathRewrite { @@ -373,6 +383,7 @@ impl Manifest { } } +/// Replace relative paths with absolute paths with the working directory. struct PathRewrite<'a> { exclude_deps: &'a [String], manifest_dir: PathBuf, @@ -380,15 +391,6 @@ struct PathRewrite<'a> { impl<'a> PathRewrite<'a> { /// Replace relative paths with absolute paths with the working directory. - /// - /// Enables the use of a temporary amended copy of the manifest. - /// - /// # Rewrites - /// - /// - `[lib]/path` - /// - `[dependencies]` - /// - /// Dependencies with package names specified in `exclude_deps` will not be rewritten. fn rewrite_relative_paths(&self, toml: &mut value::Table) -> Result<()> { // Rewrite `[lib] path = /path/to/lib.rs` if let Some(lib) = toml.get_mut("lib") { From a0cfb3747c7594a152210567bd7c6c7e8d150906 Mon Sep 17 00:00:00 2001 From: Andrew Jones Date: Fri, 30 Sep 2022 12:38:18 +0100 Subject: [PATCH 3/3] Satisfy clippy --- crates/cargo-contract/src/workspace/manifest.rs | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/crates/cargo-contract/src/workspace/manifest.rs b/crates/cargo-contract/src/workspace/manifest.rs index ee9d17431..3bc72514f 100644 --- a/crates/cargo-contract/src/workspace/manifest.rs +++ b/crates/cargo-contract/src/workspace/manifest.rs @@ -26,7 +26,6 @@ use super::{ use crate::OptimizationPasses; use std::{ - collections::HashSet, convert::TryFrom, fs, path::{ @@ -477,11 +476,6 @@ impl<'a> PathRewrite<'a> { section_name: &str, ) -> Result<()> { if let Some(dependencies) = toml.get_mut(section_name) { - let exclude = self - .exclude_deps - .into_iter() - .map(|s| s.clone()) - .collect::>(); let table = dependencies .as_table_mut() .ok_or_else(|| anyhow::anyhow!("dependencies should be a table"))?; @@ -492,7 +486,7 @@ impl<'a> PathRewrite<'a> { package_name.to_string() }; - if !exclude.contains(&package_name) { + if !self.exclude_deps.contains(&package_name) { if let Some(dependency) = value.as_table_mut() { if let Some(dep_path) = dependency.get_mut("path") { self.to_absolute_path(