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

Add option to build with unmodified original manifest #51

Merged
merged 9 commits into from
May 22, 2020
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
51 changes: 38 additions & 13 deletions src/cmd/build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ use std::{
use crate::{
util,
workspace::{ManifestPath, Workspace},
Verbosity,
UnstableFlags, Verbosity,
};
use anyhow::{Context, Result};
use cargo_metadata::Package;
Expand Down Expand Up @@ -94,7 +94,18 @@ pub fn collect_crate_metadata(manifest_path: &ManifestPath) -> Result<CrateMetad
///
/// Uses [`cargo-xbuild`](https://github.com/rust-osdev/cargo-xbuild) for maximum optimization of
/// the resulting Wasm binary.
fn build_cargo_project(crate_metadata: &CrateMetadata, verbosity: Option<Verbosity>) -> Result<()> {
///
/// # Cargo.toml optimizations
///
/// The original Cargo.toml will be amended to remove the `rlib` crate type in order to minimize
/// the final Wasm binary size.
///
/// To disable this and use the `Cargo.toml` as is then pass the `-Z original_manifest` flag.
fn build_cargo_project(
crate_metadata: &CrateMetadata,
verbosity: Option<Verbosity>,
unstable_options: UnstableFlags,
) -> Result<()> {
util::assert_channel()?;

// set RUSTFLAGS, read from environment var by cargo-xbuild
Expand Down Expand Up @@ -136,14 +147,24 @@ fn build_cargo_project(crate_metadata: &CrateMetadata, verbosity: Option<Verbosi
Ok(())
};

Workspace::new(&crate_metadata.cargo_meta, &crate_metadata.root_package.id)?
.with_root_package_manifest(|manifest| {
manifest
.with_removed_crate_type("rlib")?
.with_profile_release_lto(true)?;
Ok(())
})?
.using_temp(xbuild)?;
if unstable_options.original_manifest {
println!(
"{} {}",
"warning:".yellow().bold(),
"with 'original-manifest' enabled, the contract binary may not be of optimal size."
.bold()
);
xbuild(&crate_metadata.manifest_path)?;
} else {
Workspace::new(&crate_metadata.cargo_meta, &crate_metadata.root_package.id)?
.with_root_package_manifest(|manifest| {
manifest
.with_removed_crate_type("rlib")?
.with_profile_release_lto(true)?;
Ok(())
})?
.using_temp(xbuild)?;
}

Ok(())
}
Expand Down Expand Up @@ -273,6 +294,7 @@ fn optimize_wasm(crate_metadata: &CrateMetadata) -> Result<()> {
pub(crate) fn execute_build(
manifest_path: ManifestPath,
verbosity: Option<Verbosity>,
unstable_options: UnstableFlags,
) -> Result<String> {
println!(
" {} {}",
Expand All @@ -285,7 +307,7 @@ pub(crate) fn execute_build(
"[2/4]".bold(),
"Building cargo project".bright_green().bold()
);
build_cargo_project(&crate_metadata, verbosity)?;
build_cargo_project(&crate_metadata, verbosity, unstable_options)?;
println!(
" {} {}",
"[3/4]".bold(),
Expand All @@ -308,15 +330,18 @@ pub(crate) fn execute_build(
#[cfg(feature = "test-ci-only")]
#[cfg(test)]
mod tests {
use crate::{cmd::execute_new, util::tests::with_tmp_dir, workspace::ManifestPath};
use crate::{
cmd::execute_new, util::tests::with_tmp_dir, workspace::ManifestPath, UnstableFlags,
};

#[test]
fn build_template() {
with_tmp_dir(|path| {
execute_new("new_project", Some(path)).expect("new project creation failed");
let manifest_path =
ManifestPath::new(&path.join("new_project").join("Cargo.toml")).unwrap();
super::execute_build(manifest_path, None).expect("build failed");
super::execute_build(manifest_path, None, UnstableFlags::default())
.expect("build failed");
});
}
}
38 changes: 22 additions & 16 deletions src/cmd/metadata.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
use crate::{
util,
workspace::{ManifestPath, Workspace},
Verbosity,
UnstableFlags, Verbosity,
};
use anyhow::Result;

Expand All @@ -27,44 +27,49 @@ const METADATA_FILE: &str = "metadata.json";
///
/// It does so by invoking build by cargo and then post processing the final binary.
pub(crate) fn execute_generate_metadata(
manifest_path: ManifestPath,
original_manifest_path: ManifestPath,
verbosity: Option<Verbosity>,
unstable_options: UnstableFlags,
) -> Result<String> {
util::assert_channel()?;
println!(" Generating metadata");

let (metadata, root_package_id) = crate::util::get_cargo_metadata(&manifest_path)?;
let (metadata, root_package_id) = crate::util::get_cargo_metadata(&original_manifest_path)?;

let out_path = metadata.target_directory.join(METADATA_FILE);
let out_path_display = format!("{}", out_path.display());

let target_dir = metadata.target_directory.clone();

let generate_metadata = move |tmp_manifest_path: &ManifestPath| -> Result<()> {
let generate_metadata = |manifest_path: &ManifestPath| -> Result<()> {
let target_dir_arg = format!("--target-dir={}", target_dir.to_string_lossy());
util::invoke_cargo(
"run",
&[
"--package",
"abi-gen",
&tmp_manifest_path.cargo_arg(),
&manifest_path.cargo_arg(),
&target_dir_arg,
"--release",
// "--no-default-features", // Breaks builds for MacOS (linker errors), we should investigate this issue asap!
],
manifest_path.directory(),
original_manifest_path.directory(),
verbosity,
)
};

Workspace::new(&metadata, &root_package_id)?
.with_root_package_manifest(|manifest| {
manifest
.with_added_crate_type("rlib")?
.with_profile_release_lto(false)?;
Ok(())
})?
.using_temp(generate_metadata)?;
if unstable_options.original_manifest {
generate_metadata(&original_manifest_path)?;
} else {
Workspace::new(&metadata, &root_package_id)?
.with_root_package_manifest(|manifest| {
manifest
.with_added_crate_type("rlib")?
.with_profile_release_lto(false)?;
Ok(())
})?
.using_temp(generate_metadata)?;
}

Ok(format!(
"Your metadata file is ready.\nYou can find it here:\n{}",
Expand All @@ -79,6 +84,7 @@ mod tests {
cmd::{execute_generate_metadata, execute_new},
util::tests::with_tmp_dir,
workspace::ManifestPath,
UnstableFlags,
};

#[test]
Expand All @@ -88,8 +94,8 @@ mod tests {
execute_new("new_project", Some(path)).expect("new project creation failed");
let working_dir = path.join("new_project");
let manifest_path = ManifestPath::new(working_dir.join("Cargo.toml")).unwrap();
let message =
execute_generate_metadata(manifest_path, None).expect("generate metadata failed");
let message = execute_generate_metadata(manifest_path, None, UnstableFlags::default())
.expect("generate metadata failed");
println!("{}", message);

let mut abi_file = working_dir;
Expand Down
57 changes: 51 additions & 6 deletions src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,37 @@ impl TryFrom<&VerbosityFlags> for Option<Verbosity> {
}
}

#[derive(Debug, StructOpt)]
struct UnstableOptions {
/// Use the original manifest (Cargo.toml), do not modify for build optimizations
#[structopt(long = "unstable-options", short = "Z", number_of_values = 1)]
options: Vec<String>,
}
Comment on lines +115 to +120
Copy link
Contributor

Choose a reason for hiding this comment

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

How would this look like if we specify multiple unstable options? Each having their own -Z or one -Z for all? I guess we should mimic rustc here that requires one -Z per unstable feature usage.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Indeed that should work i.e. 1 -Z per option, I took this pattern from https://github.com/TeXitoi/structopt/blob/master/examples/keyvalue.rs#L22


#[derive(Default)]
struct UnstableFlags {
original_manifest: bool,
}

impl TryFrom<&UnstableOptions> for UnstableFlags {
type Error = Error;

fn try_from(value: &UnstableOptions) -> Result<Self, Self::Error> {
let valid_flags = ["original-manifest"];
let invalid_flags = value
.options
.iter()
.filter(|o| !valid_flags.contains(&o.as_str()))
.collect::<Vec<_>>();
if !invalid_flags.is_empty() {
anyhow::bail!("Unknown unstable-options {:?}", invalid_flags)
}
Ok(UnstableFlags {
original_manifest: value.options.contains(&"original-manifest".to_owned()),
})
}
}

#[derive(Debug, StructOpt)]
enum Command {
/// Setup and create a new smart contract project
Expand All @@ -128,12 +159,16 @@ enum Command {
Build {
#[structopt(flatten)]
verbosity: VerbosityFlags,
#[structopt(flatten)]
unstable_options: UnstableOptions,
},
/// Generate contract metadata artifacts
#[structopt(name = "generate-metadata")]
GenerateMetadata {
#[structopt(flatten)]
verbosity: VerbosityFlags,
#[structopt(flatten)]
unstable_options: UnstableOptions,
},
/// Test the smart contract off-chain
#[structopt(name = "test")]
Expand Down Expand Up @@ -197,12 +232,22 @@ fn main() {
fn exec(cmd: Command) -> Result<String> {
match &cmd {
Command::New { name, target_dir } => cmd::execute_new(name, target_dir.as_ref()),
Command::Build { verbosity } => {
cmd::execute_build(Default::default(), verbosity.try_into()?)
}
Command::GenerateMetadata { verbosity } => {
cmd::execute_generate_metadata(Default::default(), verbosity.try_into()?)
}
Command::Build {
verbosity,
unstable_options,
} => cmd::execute_build(
Default::default(),
verbosity.try_into()?,
unstable_options.try_into()?,
),
Command::GenerateMetadata {
verbosity,
unstable_options,
} => cmd::execute_generate_metadata(
Default::default(),
verbosity.try_into()?,
unstable_options.try_into()?,
),
Command::Test {} => Err(anyhow::anyhow!("Command unimplemented")),
#[cfg(feature = "extrinsics")]
Command::Deploy {
Expand Down