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

Only copy and rewrite target contract manifest #1079

Merged
merged 9 commits into from
Apr 24, 2023
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: 3 additions & 3 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

10 changes: 8 additions & 2 deletions crates/build/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -286,7 +286,7 @@ fn exec_cargo_for_wasm_target(
manifest
.with_crate_types(["cdylib"])?
.with_profile_release_defaults(Profile::default_contract_release())?
.with_workspace()?;
.with_empty_workspace();
Ok(())
})?
.using_temp(cargo_build)?;
Expand Down Expand Up @@ -328,7 +328,7 @@ fn exec_cargo_dylint(crate_metadata: &CrateMetadata, verbosity: Verbosity) -> Re

Workspace::new(&crate_metadata.cargo_meta, &crate_metadata.root_package.id)?
.with_root_package_manifest(|manifest| {
manifest.with_dylint()?;
manifest.with_dylint()?.with_empty_workspace();
Ok(())
})?
.using_temp(|manifest_path| {
Expand Down Expand Up @@ -660,6 +660,12 @@ pub fn execute(args: ExecuteArgs) -> Result<BuildResult> {
)
})?;

tracing::debug!(
"Fingerprint before build: {:?}, after build: {:?}",
pre_fingerprint,
post_fingerprint
);

let dest_wasm_path = crate_metadata.dest_wasm.clone();

if pre_fingerprint == Some(post_fingerprint)
Expand Down
7 changes: 3 additions & 4 deletions crates/build/src/metadata.rs
Original file line number Diff line number Diff line change
Expand Up @@ -187,12 +187,11 @@ pub(crate) fn execute(
.with_root_package_manifest(|manifest| {
manifest
.with_added_crate_type("rlib")?
.with_profile_release_lto(false)?;
.with_profile_release_lto(false)?
.with_empty_workspace();
Ok(())
})?
.with_metadata_gen_package(
crate_metadata.manifest_path.absolute_directory()?,
)?
.with_metadata_gen_package()?
.using_temp(generate_metadata)?;
}

Expand Down
49 changes: 18 additions & 31 deletions crates/build/src/workspace/manifest.rs
Original file line number Diff line number Diff line change
Expand Up @@ -142,11 +142,6 @@ impl Manifest {
})
}

/// Get the path of the manifest file
pub(super) fn path(&self) -> &ManifestPath {
&self.path
}

/// Get mutable reference to `[lib] crate-types = []` section
fn get_crate_types_mut(&mut self) -> Result<&mut value::Array> {
let lib = self
Expand Down Expand Up @@ -230,15 +225,15 @@ impl Manifest {
Ok(self)
}

/// Set empty `[workspace]` section if it does not exist.
/// Set `[workspace]` section to an empty table. When building a contract project any workspace
/// members are not copied to the temporary workspace, so need to be removed.
///
/// Ignores the `workspace` from the parent `Cargo.toml`.
/// This can reduce the size of the contract in some cases.
pub fn with_workspace(&mut self) -> Result<&mut Self> {
if let toml::map::Entry::Vacant(value) = self.toml.entry("workspace") {
value.insert(value::Value::Table(Default::default()));
}
Ok(self)
/// Additionally, where no workspace is already specified, this can in some cases reduce the
/// size of the contract.
pub fn with_empty_workspace(&mut self) -> &mut Self {
self.toml
.insert("workspace".into(), value::Value::Table(Default::default()));
self
}

/// Get mutable reference to `[profile.release]` section
Expand Down Expand Up @@ -341,14 +336,9 @@ impl Manifest {
///
/// - `[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<()> {
pub fn rewrite_relative_paths(&mut self) -> Result<()> {
let manifest_dir = self.path.absolute_directory()?;
let path_rewrite = PathRewrite {
exclude_deps,
manifest_dir,
};
let path_rewrite = PathRewrite { manifest_dir };
path_rewrite.rewrite_relative_paths(&mut self.toml)
}

Expand Down Expand Up @@ -413,12 +403,11 @@ impl Manifest {
}

/// Replace relative paths with absolute paths with the working directory.
struct PathRewrite<'a> {
exclude_deps: &'a [String],
struct PathRewrite {
manifest_dir: PathBuf,
}

impl<'a> PathRewrite<'a> {
impl PathRewrite {
/// Replace relative paths with absolute paths with the working directory.
fn rewrite_relative_paths(&self, toml: &mut value::Table) -> Result<()> {
// Rewrite `[lib] path = /path/to/lib.rs`
Expand Down Expand Up @@ -516,14 +505,12 @@ impl<'a> PathRewrite<'a> {
package_name.to_string()
};

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(
format!("dependency {package_name}"),
dep_path,
)?;
}
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,
)?;
}
}
}
Expand Down
175 changes: 44 additions & 131 deletions crates/build/src/workspace/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,59 +34,41 @@ use cargo_metadata::{
PackageId,
};

use std::{
collections::HashMap,
path::{
Path,
PathBuf,
},
use std::path::{
Path,
PathBuf,
};

/// Make a copy of a cargo workspace, maintaining only the directory structure and manifest
/// files. Relative paths to source files and non-workspace dependencies are rewritten to absolute
/// paths to the original locations.
/// Make a copy of a contract project manifest, allow modifications to be made to it, rewrite the
/// paths to point to the original project files, then write to a temporary directory.
///
/// This allows custom amendments to be made to the manifest files without editing the originals
/// directly.
pub struct Workspace {
workspace_root: PathBuf,
root_package: PackageId,
members: HashMap<PackageId, (Package, Manifest)>,
root_package: Package,
root_manifest: Manifest,
}

impl Workspace {
/// Create a new Workspace from the supplied cargo metadata.
/// Create a new Workspace from the supplied cargo metadata and the id of the root contract
/// package.
pub fn new(metadata: &CargoMetadata, root_package: &PackageId) -> Result<Self> {
let member_manifest =
|package_id: &PackageId| -> Result<(PackageId, (Package, Manifest))> {
let package = metadata
.packages
.iter()
.find(|p| p.id == *package_id)
.unwrap_or_else(|| {
panic!(
"Package '{package_id}' is a member and should be in the packages list"
)
});
let manifest_path = ManifestPath::new(&package.manifest_path)?;
let manifest = Manifest::new(manifest_path)?;
Ok((package_id.clone(), (package.clone(), manifest)))
};

let members = metadata
.workspace_members
let root_package = metadata
.packages
.iter()
.map(member_manifest)
.collect::<Result<HashMap<_, _>>>()?;
.find(|p| p.id == *root_package)
.ok_or_else(|| {
anyhow::anyhow!("The root package should be a workspace member")
})?;

if !members.contains_key(root_package) {
anyhow::bail!("The root package should be a workspace member")
}
let manifest_path = ManifestPath::new(&root_package.manifest_path)?;
let root_manifest = Manifest::new(manifest_path)?;

Ok(Workspace {
workspace_root: metadata.workspace_root.clone().into(),
root_package: root_package.clone(),
members,
root_manifest,
})
}

Expand All @@ -100,97 +82,40 @@ impl Workspace {
where
F: FnOnce(&mut Manifest) -> Result<()>,
{
let root_package_manifest = self
.members
.get_mut(&self.root_package)
.map(|(_, m)| m)
.expect("The root package should be a workspace member");
f(root_package_manifest)?;
Ok(self)
}

/// Amend the manifest of the package at `package_path` using the supplied function.
pub fn with_contract_manifest<F>(
&mut self,
package_path: &Path,
f: F,
) -> Result<&mut Self>
where
F: FnOnce(&mut Manifest) -> Result<()>,
{
let manifest = self
.members
.iter_mut()
.find_map(|(_, (_, manifest))| {
// `package_path` is always absolute and canonicalized. Thus we need to
// canonicalize the manifest's directory path as well in order to compare
// both of them.
let manifest_path = manifest.path().directory()?;
let manifest_path = manifest_path.canonicalize().unwrap_or_else(|_| {
panic!("Cannot canonicalize {}", manifest_path.display())
});
if manifest_path == package_path {
Some(manifest)
} else {
None
}
})
.ok_or_else(|| {
anyhow::anyhow!(
"Cannot find package with package path {} in workspace members",
package_path.display(),
)
})?;
f(manifest)?;
f(&mut self.root_manifest)?;
Ok(self)
}

/// Generates a package to invoke for generating contract metadata.
///
/// The contract metadata will be generated for the package found at `package_path`.
pub(super) fn with_metadata_gen_package(
&mut self,
package_path: PathBuf,
) -> Result<&mut Self> {
self.with_contract_manifest(&package_path, |manifest| {
manifest.with_metadata_package()?;
Ok(())
})
pub(super) fn with_metadata_gen_package(&mut self) -> Result<&mut Self> {
self.root_manifest.with_metadata_package()?;
Ok(self)
}

/// Writes the amended manifests to the `target` directory, retaining the workspace directory
/// structure, but only with the `Cargo.toml` files.
/// Writes the amended manifest to the `target` directory. Relative paths will be rewritten to
/// absolute paths from the original project root.
///
/// Relative paths will be rewritten to absolute paths from the original workspace root, except
/// intra-workspace relative dependency paths which will be preserved.
///
/// Returns the paths of the new manifests.
pub fn write<P: AsRef<Path>>(
&mut self,
target: P,
) -> Result<Vec<(PackageId, ManifestPath)>> {
let exclude_member_package_names = self
.members
.iter()
.map(|(_, (p, _))| p.name.clone())
.collect::<Vec<_>>();
let mut new_manifest_paths = Vec::new();
for (package_id, (package, manifest)) in self.members.iter_mut() {
// replace the original workspace root with the temporary directory
let mut new_path: PathBuf = target.as_ref().into();
new_path.push(package.manifest_path.strip_prefix(&self.workspace_root)?);
let new_manifest = ManifestPath::new(new_path)?;

manifest.rewrite_relative_paths(&exclude_member_package_names)?;
manifest.write(&new_manifest)?;

new_manifest_paths.push((package_id.clone(), new_manifest));
}
Ok(new_manifest_paths)
/// Returns the path of the new manifest.
pub fn write<P: AsRef<Path>>(&mut self, target: P) -> Result<ManifestPath> {
// replace the original workspace root with the temporary directory
let mut new_path: PathBuf = target.as_ref().into();
new_path.push(
self.root_package
.manifest_path
.strip_prefix(&self.workspace_root)?,
);
let new_manifest = ManifestPath::new(new_path)?;

self.root_manifest.rewrite_relative_paths()?;
self.root_manifest.write(&new_manifest)?;

Ok(new_manifest)
}

/// Copy the workspace with amended manifest files to a temporary directory, executing the
/// supplied function with the root manifest path before the directory is cleaned up.
/// Write the amended manifest file to a temporary directory, then execute the supplied function
/// with the temporary manifest path before the directory is cleaned up.
pub fn using_temp<F>(&mut self, f: F) -> Result<()>
where
F: FnOnce(&ManifestPath) -> Result<()>,
Expand All @@ -199,23 +124,11 @@ impl Workspace {
.prefix("cargo-contract_")
.tempdir()?;
tracing::debug!("Using temp workspace at '{}'", tmp_dir.path().display());
let new_paths = self.write(&tmp_dir)?;
let tmp_root_manifest_path = new_paths
.iter()
.find_map(|(pid, path)| {
if *pid == self.root_package {
Some(path)
} else {
None
}
})
.expect("root package should be a member of the temp workspace");
let tmp_root_manifest_path = self.write(&tmp_dir)?;

// copy the `Cargo.lock` file
let src_lockfile = self.workspace_root.clone().join("Cargo.lock");
let dest_lockfile = tmp_root_manifest_path
.absolute_directory()?
.join("Cargo.lock");
let dest_lockfile = tmp_dir.path().join("Cargo.lock");
if src_lockfile.exists() {
tracing::debug!(
"Copying '{}' to ' '{}'",
Expand All @@ -225,6 +138,6 @@ impl Workspace {
std::fs::copy(src_lockfile, dest_lockfile)?;
}

f(tmp_root_manifest_path)
f(&tmp_root_manifest_path)
}
}