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

Always use lib.name for Wasm output file name #277

Merged
merged 4 commits into from
May 25, 2021
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
39 changes: 39 additions & 0 deletions src/cmd/build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -606,6 +606,7 @@ mod tests_ci_only {
#[cfg(unix)]
use std::os::unix::fs::PermissionsExt;
use std::{
ffi::OsStr,
io::Write,
path::{Path, PathBuf},
};
Expand Down Expand Up @@ -902,4 +903,42 @@ mod tests_ci_only {
Ok(())
})
}

#[test]
fn contract_lib_name_different_from_package_name_must_build() {
with_new_contract_project(|manifest_path| {
// given
let mut manifest =
Manifest::new(manifest_path.clone()).expect("manifest creation failed");
let _ = manifest
.set_lib_name("some_lib_name")
.expect("setting lib name failed");
let _ = manifest
.set_package_name("some_package_name")
.expect("setting pacakge name failed");
manifest
.write(&manifest_path)
.expect("writing manifest failed");

// when
let cmd = BuildCommand {
manifest_path: Some(manifest_path.into()),
build_artifact: BuildArtifacts::All,
verbosity: VerbosityFlags::default(),
unstable_options: UnstableOptions::default(),
optimization_passes: None,
};
let res = cmd.exec().expect("build failed");

// then
assert_eq!(
res.dest_wasm
.expect("`dest_wasm` does not exist")
.file_name(),
Some(OsStr::new("some_lib_name.wasm"))
);

Ok(())
})
}
}
13 changes: 10 additions & 3 deletions src/crate_metadata.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,15 @@ impl CrateMetadata {
let (metadata, root_package) = get_cargo_metadata(manifest_path)?;
let mut target_directory = metadata.target_directory.as_path().join("ink");

// Normalize the package name.
// Normalize the package and lib name.
let package_name = root_package.name.replace("-", "_");
let lib_name = &root_package
.targets
.iter()
.find(|target| target.kind.iter().any(|t| t == "cdylib"))
.expect("lib name not found")
.name
.replace("-", "_");

let absolute_manifest_path = manifest_path.absolute_directory()?;
let absolute_workspace_root = metadata.workspace_root.canonicalize()?;
Expand All @@ -58,12 +65,12 @@ impl CrateMetadata {
let mut original_wasm = target_directory.clone();
original_wasm.push("wasm32-unknown-unknown");
original_wasm.push("release");
original_wasm.push(package_name.clone());
original_wasm.push(lib_name.clone());
original_wasm.set_extension("wasm");

// {target_dir}/{package_name}.wasm
let mut dest_wasm = target_directory.clone();
dest_wasm.push(package_name.clone());
dest_wasm.push(lib_name.clone());
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that package_name will still be used for the contract bundle, which could be confusing when viewing the target dir if they are different.

We could just replace the package_name field CrateMetadata with something like contract_artifact_name, and populate with the lib name.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point, I updated the PR.

dest_wasm.set_extension("wasm");

let ink_version = metadata
Expand Down
26 changes: 26 additions & 0 deletions src/workspace/manifest.rs
Original file line number Diff line number Diff line change
Expand Up @@ -224,6 +224,32 @@ impl Manifest {
.insert("version".into(), value::Value::String(version.into())))
}

/// Set the `lib` name to `name`.
#[cfg(feature = "test-ci-only")]
#[cfg(test)]
pub fn set_lib_name(&mut self, name: &str) -> Result<Option<toml::Value>> {
Ok(self
.toml
.get_mut("lib")
.ok_or_else(|| anyhow::anyhow!("[lib] section not found"))?
.as_table_mut()
.ok_or_else(|| anyhow::anyhow!("[lib] should be a table"))?
.insert("name".into(), value::Value::String(name.into())))
}

/// Set the `package` name to `name`.
#[cfg(feature = "test-ci-only")]
#[cfg(test)]
pub fn set_package_name(&mut self, name: &str) -> Result<Option<toml::Value>> {
Ok(self
.toml
.get_mut("package")
.ok_or_else(|| anyhow::anyhow!("[package] section not found"))?
.as_table_mut()
.ok_or_else(|| anyhow::anyhow!("[package] should be a table"))?
.insert("name".into(), value::Value::String(name.into())))
}

/// Set `[profile.release]` lto flag
pub fn with_profile_release_lto(&mut self, enabled: bool) -> Result<&mut Self> {
let lto = self
Expand Down