diff --git a/Cargo.lock b/Cargo.lock index ea45ef27..b6d96eda 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4716,6 +4716,7 @@ dependencies = [ "tempfile", "thiserror", "tokio", + "toml 0.5.11", "toml_edit 0.22.20", "url", ] @@ -7292,6 +7293,15 @@ dependencies = [ "tokio", ] +[[package]] +name = "toml" +version = "0.5.11" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f4f7f0dd8d50a853a531c426359045b1998f04219d88799810762cd4ad314234" +dependencies = [ + "serde", +] + [[package]] name = "toml" version = "0.7.8" diff --git a/Cargo.toml b/Cargo.toml index 40121860..97c911b4 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -36,6 +36,7 @@ tar = "0.4.40" tempfile = "3.10" thiserror = "1.0.58" tokio-test = "0.4.4" +toml = "0.5.0" # networking reqwest = { version = "0.12", features = ["json"] } diff --git a/crates/pop-cli/src/commands/build/mod.rs b/crates/pop-cli/src/commands/build/mod.rs index 08af079b..f1a69c9c 100644 --- a/crates/pop-cli/src/commands/build/mod.rs +++ b/crates/pop-cli/src/commands/build/mod.rs @@ -5,6 +5,7 @@ use clap::{Args, Subcommand}; #[cfg(feature = "contract")] use contract::BuildContractCommand; use duct::cmd; +use pop_common::Profile; use std::path::PathBuf; #[cfg(feature = "parachain")] use {parachain::BuildParachainCommand, spec::BuildSpecCommand}; @@ -29,8 +30,11 @@ pub(crate) struct BuildArgs { #[arg(short = 'p', long)] pub(crate) package: Option, /// For production, always build in release mode to exclude debug features. - #[clap(short, long)] + #[clap(short, long, conflicts_with = "profile")] pub(crate) release: bool, + /// Build profile [default: debug]. + #[clap(long, value_enum)] + pub(crate) profile: Option, /// Parachain ID to be used when generating the chain spec files. #[arg(short = 'i', long = "id")] #[cfg(feature = "parachain")] @@ -62,19 +66,26 @@ impl Command { #[cfg(feature = "contract")] if pop_contracts::is_supported(args.path.as_deref())? { // All commands originating from root command are valid - BuildContractCommand { path: args.path, release: args.release, valid: true } - .execute()?; + let release = match args.profile { + Some(profile) => profile.into(), + None => args.release, + }; + BuildContractCommand { path: args.path, release, valid: true }.execute()?; return Ok("contract"); } // If only parachain feature enabled, build as parachain #[cfg(feature = "parachain")] if pop_parachains::is_supported(args.path.as_deref())? { + let profile = match args.profile { + Some(profile) => profile, + None => args.release.into(), + }; // All commands originating from root command are valid BuildParachainCommand { path: args.path, package: args.package, - release: args.release, + profile: Some(profile), id: args.id, valid: true, } @@ -101,13 +112,15 @@ impl Command { _args.push("--package"); _args.push(package) } - if args.release { + let profile = args.profile.unwrap_or(Profile::Debug); + if profile == Profile::Release { _args.push("--release"); + } else if profile == Profile::Production { + _args.push("--profile=production"); } cmd("cargo", _args).dir(args.path.unwrap_or_else(|| "./".into())).run()?; - let mode = if args.release { "RELEASE" } else { "DEBUG" }; - cli.info(format!("The {project} was built in {mode} mode."))?; + cli.info(format!("The {project} was built in {} mode.", profile))?; cli.outro("Build completed successfully!")?; Ok(project) } @@ -117,41 +130,46 @@ impl Command { mod tests { use super::*; use cli::MockCli; + use pop_common::manifest::add_production_profile; + use strum::VariantArray; #[test] fn build_works() -> anyhow::Result<()> { let name = "hello_world"; let temp_dir = tempfile::tempdir()?; let path = temp_dir.path(); + let project_path = path.join(name); cmd("cargo", ["new", name, "--bin"]).dir(&path).run()?; + add_production_profile(&project_path)?; for package in [None, Some(name.to_string())] { - for release in [false, true] { - let project = if package.is_some() { "package" } else { "project" }; - let mode = if release { "RELEASE" } else { "DEBUG" }; - let mut cli = MockCli::new() - .expect_intro(format!("Building your {project}")) - .expect_info(format!("The {project} was built in {mode} mode.")) - .expect_outro("Build completed successfully!"); - - assert_eq!( - Command::build( - BuildArgs { - command: None, - path: Some(path.join(name)), - package: package.clone(), - release, - id: None, - }, - &mut cli, - )?, - project - ); - - cli.verify()?; + for release in [true, false] { + for profile in Profile::VARIANTS { + let profile = if release { Profile::Release } else { profile.clone() }; + let project = if package.is_some() { "package" } else { "project" }; + let mut cli = MockCli::new() + .expect_intro(format!("Building your {project}")) + .expect_info(format!("The {project} was built in {profile} mode.")) + .expect_outro("Build completed successfully!"); + + assert_eq!( + Command::build( + BuildArgs { + command: None, + path: Some(project_path.clone()), + package: package.clone(), + release, + profile: Some(profile.clone()), + id: None, + }, + &mut cli, + )?, + project + ); + cli.verify()?; + } } } - Ok(()) } } diff --git a/crates/pop-cli/src/commands/build/parachain.rs b/crates/pop-cli/src/commands/build/parachain.rs index a6e47c67..67000e62 100644 --- a/crates/pop-cli/src/commands/build/parachain.rs +++ b/crates/pop-cli/src/commands/build/parachain.rs @@ -16,9 +16,9 @@ pub struct BuildParachainCommand { /// The package to be built. #[arg(short = 'p', long)] pub(crate) package: Option, - /// For production, always build in release mode to exclude debug features. - #[clap(short, long, default_value = "true")] - pub(crate) release: bool, + /// Build profile [default: debug]. + #[clap(long, value_enum)] + pub(crate) profile: Option, /// Parachain ID to be used when generating the chain spec files. #[arg(short = 'i', long = "id")] pub(crate) id: Option, @@ -41,12 +41,13 @@ impl BuildParachainCommand { let project = if self.package.is_some() { "package" } else { "parachain" }; cli.intro(format!("Building your {project}"))?; + let profile = self.profile.unwrap_or(Profile::Debug); // Show warning if specified as deprecated. if !self.valid { cli.warning("NOTE: this command is deprecated. Please use `pop build` (or simply `pop b`) in future...")?; #[cfg(not(test))] sleep(Duration::from_secs(3)) - } else if !self.release { + } else if profile == Profile::Debug { cli.warning("NOTE: this command now defaults to DEBUG builds. Please use `--release` (or simply `-r`) for a release build...")?; #[cfg(not(test))] sleep(Duration::from_secs(3)) @@ -55,9 +56,8 @@ impl BuildParachainCommand { // Build parachain. cli.warning("NOTE: this may take some time...")?; let project_path = self.path.unwrap_or_else(|| PathBuf::from("./")); - let mode: Profile = self.release.into(); - let binary = build_parachain(&project_path, self.package, &mode, None)?; - cli.info(format!("The {project} was built in {mode} mode."))?; + let binary = build_parachain(&project_path, self.package, &profile, None)?; + cli.info(format!("The {project} was built in {} mode.", profile))?; cli.outro("Build completed successfully!")?; let generated_files = [format!("Binary generated at: {}", binary.display())]; let generated_files: Vec<_> = generated_files @@ -79,7 +79,9 @@ mod tests { use super::*; use cli::MockCli; use duct::cmd; + use pop_common::manifest::add_production_profile; use std::{fs, io::Write, path::Path}; + use strum::VariantArray; // Function that generates a Cargo.toml inside node directory for testing. fn generate_mock_node(temp_dir: &Path) -> anyhow::Result<()> { @@ -106,33 +108,34 @@ mod tests { let name = "hello_world"; let temp_dir = tempfile::tempdir()?; let path = temp_dir.path(); + let project_path = path.join(name); cmd("cargo", ["new", name, "--bin"]).dir(&path).run()?; - generate_mock_node(&temp_dir.path().join(name))?; + add_production_profile(&project_path)?; + generate_mock_node(&project_path)?; for package in [None, Some(name.to_string())] { - for release in [false, true] { + for profile in Profile::VARIANTS { for valid in [false, true] { let project = if package.is_some() { "package" } else { "parachain" }; - let mode = if release { Profile::Release } else { Profile::Debug }; let mut cli = MockCli::new() .expect_intro(format!("Building your {project}")) .expect_warning("NOTE: this may take some time...") - .expect_info(format!("The {project} was built in {mode} mode.")) + .expect_info(format!("The {project} was built in {profile} mode.")) .expect_outro("Build completed successfully!"); if !valid { cli = cli.expect_warning("NOTE: this command is deprecated. Please use `pop build` (or simply `pop b`) in future..."); } else { - if !release { + if profile == &Profile::Debug { cli = cli.expect_warning("NOTE: this command now defaults to DEBUG builds. Please use `--release` (or simply `-r`) for a release build..."); } } assert_eq!( BuildParachainCommand { - path: Some(path.join(name)), + path: Some(project_path.clone()), package: package.clone(), - release, + profile: Some(profile.clone()), id: None, valid, } diff --git a/crates/pop-cli/src/commands/build/spec.rs b/crates/pop-cli/src/commands/build/spec.rs index 32ff7425..541e6624 100644 --- a/crates/pop-cli/src/commands/build/spec.rs +++ b/crates/pop-cli/src/commands/build/spec.rs @@ -138,10 +138,13 @@ pub struct BuildSpecCommand { /// the necessary directories will be created #[arg(short = 'o', long = "output")] pub(crate) output_file: Option, - /// This command builds the node if it has not been built already. - /// For production, always build in release mode to exclude debug features. - #[arg(short = 'r', long)] + // TODO: remove at release ... + /// [DEPRECATED] use `profile`. + #[arg(short = 'r', long, conflicts_with = "profile")] pub(crate) release: bool, + /// Build profile for the binary to generate the chain specification. + #[arg(long, value_enum)] + pub(crate) profile: Option, /// Parachain ID to be used when generating the chain spec files. #[arg(short = 'i', long)] pub(crate) id: Option, @@ -197,6 +200,7 @@ impl BuildSpecCommand { ) -> anyhow::Result { let BuildSpecCommand { output_file, + profile, release, id, default_bootnode, @@ -334,6 +338,33 @@ impl BuildSpecCommand { }, }; + // Prompt user for build profile. + let mut profile = match profile { + Some(profile) => profile, + None => { + let default = Profile::Release; + if prompt && !release { + // Prompt for build profile. + let mut prompt = cli + .select( + "Choose the build profile of the binary that should be used: " + .to_string(), + ) + .initial_value(&default); + for profile in Profile::VARIANTS { + prompt = prompt.item( + profile, + profile.get_message().unwrap_or(profile.as_ref()), + profile.get_detailed_message().unwrap_or_default(), + ); + } + prompt.interact()?.clone() + } else { + default + } + }, + }; + // Protocol id. let protocol_id = match protocol_id { Some(protocol_id) => protocol_id, @@ -385,18 +416,16 @@ impl BuildSpecCommand { true }; - // Only prompt user for build profile if a live spec is being built on debug mode. - let release = if !release && matches!(chain_type, ChainType::Live) { - cli.confirm("Using Debug profile to build a Live specification. Should Release be used instead ?") - .initial_value(true) - .interact()? - } else { - release - }; + if release { + cli.warning("NOTE: release flag is deprecated. Use `--profile` instead.")?; + #[cfg(not(test))] + sleep(Duration::from_secs(3)); + profile = Profile::Release; + } Ok(BuildSpec { output_file, - release, + profile, id, default_bootnode, chain_type, @@ -413,7 +442,7 @@ impl BuildSpecCommand { #[derive(Debug)] struct BuildSpec { output_file: PathBuf, - release: bool, + profile: Profile, id: u32, default_bootnode: bool, chain_type: ChainType, @@ -430,64 +459,58 @@ impl BuildSpec { // This function generates plain and raw chain spec files based on the provided configuration, // optionally including genesis state and runtime artifacts. If the node binary is missing, // it triggers a build process. - fn build(&self, cli: &mut impl cli::traits::Cli) -> anyhow::Result<&'static str> { + fn build(self, cli: &mut impl cli::traits::Cli) -> anyhow::Result<&'static str> { cli.intro("Building your chain spec")?; let mut generated_files = vec![]; - + let BuildSpec { + ref output_file, + ref profile, + id, + default_bootnode, + ref chain, + genesis_state, + genesis_code, + .. + } = self; // Ensure binary is built. - let mode: Profile = self.release.into(); - let binary_path = ensure_binary_exists(cli, &mode)?; - if !self.release { - cli.warning( - "NOTE: this command defaults to DEBUG builds for development chain types. Please use `--release` (or simply `-r` for a release build...)", - )?; - #[cfg(not(test))] - sleep(Duration::from_secs(3)) - } + let binary_path = ensure_binary_exists(cli, profile)?; let spinner = spinner(); spinner.start("Generating chain specification..."); // Generate chain spec. - generate_plain_chain_spec( - &binary_path, - &self.output_file, - self.default_bootnode, - &self.chain, - )?; + generate_plain_chain_spec(&binary_path, output_file, default_bootnode, chain)?; // Customize spec based on input. self.customize()?; generated_files.push(format!( "Plain text chain specification file generated at: {}", - self.output_file.display() + &output_file.display() )); // Generate raw spec. spinner.set_message("Generating raw chain specification..."); - let spec_name = self - .output_file + let spec_name = &output_file .file_name() .and_then(|s| s.to_str()) .unwrap_or(DEFAULT_SPEC_NAME) .trim_end_matches(".json"); let raw_spec_name = format!("{spec_name}-raw.json"); - let raw_chain_spec = - generate_raw_chain_spec(&binary_path, &self.output_file, &raw_spec_name)?; + let raw_chain_spec = generate_raw_chain_spec(&binary_path, output_file, &raw_spec_name)?; generated_files.push(format!( "Raw chain specification file generated at: {}", raw_chain_spec.display() )); // Generate genesis artifacts. - if self.genesis_code { + if genesis_code { spinner.set_message("Generating genesis code..."); - let wasm_file_name = format!("para-{}.wasm", self.id); + let wasm_file_name = format!("para-{}.wasm", id); let wasm_file = export_wasm_file(&binary_path, &raw_chain_spec, &wasm_file_name)?; generated_files .push(format!("WebAssembly runtime file exported at: {}", wasm_file.display())); } - if self.genesis_state { + if genesis_state { spinner.set_message("Generating genesis state..."); - let genesis_file_name = format!("para-{}-genesis-state", self.id); + let genesis_file_name = format!("para-{}-genesis-state", id); let genesis_state_file = generate_genesis_state_file(&binary_path, &raw_chain_spec, &genesis_file_name)?; generated_files @@ -511,7 +534,7 @@ impl BuildSpec { fn customize(&self) -> anyhow::Result<()> { let mut chain_spec = ChainSpec::from(&self.output_file)?; chain_spec.replace_para_id(self.id)?; - chain_spec.replace_relay_chain(&self.relay.to_string())?; + chain_spec.replace_relay_chain(&self.relay.as_ref())?; chain_spec.replace_chain_type(&self.chain_type.to_string())?; chain_spec.replace_protocol_id(&self.protocol_id)?; chain_spec.to_file(&self.output_file)?; @@ -538,36 +561,25 @@ fn ensure_binary_exists( // Prepare the output path provided. fn prepare_output_path(output_path: impl AsRef) -> anyhow::Result { let mut output_path = output_path.as_ref().to_path_buf(); - // Convert to string to check for trailing slash. - let output_path_str = output_path.to_string_lossy(); - let ends_with_slash = output_path_str.ends_with(std::path::MAIN_SEPARATOR); - // Check if the path has an extension. - let has_extension = output_path.extension().is_some(); // Check if the path ends with '.json' let is_json_file = output_path .extension() .and_then(|ext| ext.to_str()) .map(|ext| ext.eq_ignore_ascii_case("json")) .unwrap_or(false); - if ends_with_slash { + + if !is_json_file { // Treat as directory. if !output_path.exists() { create_dir_all(&output_path)?; } output_path.push(DEFAULT_SPEC_NAME); - } else if !has_extension { - // No extension, treat as file, set extension to '.json'. - output_path.set_extension("json"); - } else if !is_json_file { - // Has an extension but not '.json', change extension to '.json'. - output_path.set_extension("json"); - } - // Else: Ends with '.json', treat as file (no changes needed). - - // After modifications, create the parent directory if it doesn't exist. - if let Some(parent_dir) = output_path.parent() { - if !parent_dir.exists() { - create_dir_all(parent_dir)?; + } else { + // Treat as file. + if let Some(parent_dir) = output_path.parent() { + if !parent_dir.exists() { + create_dir_all(parent_dir)?; + } } } Ok(output_path) @@ -591,7 +603,8 @@ mod tests { let para_id = 4242; let protocol_id = "pop"; let relay = Polkadot; - let release = true; + let release = false; + let profile = Profile::Production; for build_spec_cmd in [ // No flags used. @@ -599,6 +612,7 @@ mod tests { // All flags used. BuildSpecCommand { output_file: Some(PathBuf::from(output_file)), + profile: Some(profile.clone()), release, id: Some(para_id), default_bootnode, @@ -616,33 +630,38 @@ mod tests { cli = cli .expect_input("Provide the chain specification to use (e.g. dev, local, custom or a path to an existing file)", chain.to_string()) .expect_input( - "Name or path for the plain chain spec file:", output_file.to_string()) + "Name or path for the plain chain spec file:", output_file.to_string()) .expect_input( - "What parachain ID should be used?", para_id.to_string()) + "What parachain ID should be used?", para_id.to_string()) .expect_input( - "Enter the protocol ID that will identify your network:", protocol_id.to_string()) + "Enter the protocol ID that will identify your network:", protocol_id.to_string()) .expect_select( - "Choose the chain type: ", - Some(false), - true, - Some(chain_types()), - chain_type.clone() as usize, - ).expect_select( + "Choose the chain type: ", + Some(false), + true, + Some(chain_types()), + chain_type.clone() as usize, + ).expect_select( "Choose the relay your chain will be connecting to: ", Some(false), true, Some(relays()), relay.clone() as usize, - ).expect_confirm("Would you like to use local host as a bootnode ?", default_bootnode).expect_confirm("Should the genesis state file be generated ?", genesis_state).expect_confirm("Should the genesis code file be generated ?", genesis_code).expect_confirm( - "Using Debug profile to build a Live specification. Should Release be used instead ?", - release, - ); + ).expect_select( + "Choose the build profile of the binary that should be used: ", + Some(false), + true, + Some(profiles()), + profile.clone() as usize + ).expect_confirm("Would you like to use local host as a bootnode ?", default_bootnode + ).expect_confirm("Should the genesis state file be generated ?", genesis_state + ).expect_confirm("Should the genesis code file be generated ?", genesis_code); } let build_spec = build_spec_cmd.configure_build_spec(&mut cli).await?; assert_eq!(build_spec.chain, chain); assert_eq!(build_spec.output_file, PathBuf::from(output_file)); assert_eq!(build_spec.id, para_id); - assert_eq!(build_spec.release, release); + assert_eq!(build_spec.profile, profile); assert_eq!(build_spec.default_bootnode, default_bootnode); assert_eq!(build_spec.chain_type, chain_type); assert_eq!(build_spec.relay, relay); @@ -664,12 +683,13 @@ mod tests { let para_id = 4242; let protocol_id = "pop"; let relay = Polkadot; - let release = true; + let release = false; + let profile = Profile::Production; // Create a temporary file to act as the existing chain spec file. let temp_dir = tempdir()?; let chain_spec_path = temp_dir.path().join("existing-chain-spec.json"); - std::fs::write(&chain_spec_path, "{}")?; // Write a dummy JSON to the file + std::fs::write(&chain_spec_path, "{}")?; // Write a dummy JSON to the file. // Whether to make changes to the provided chain spec file. for changes in [true, false] { @@ -682,6 +702,7 @@ mod tests { // All flags used. BuildSpecCommand { output_file: Some(PathBuf::from(output_file)), + profile: Some(profile.clone()), release, id: Some(para_id), default_bootnode, @@ -729,6 +750,15 @@ mod tests { relay.clone() as usize, ); } + if build_spec_cmd.profile.is_none() { + cli = cli.expect_select( + "Choose the build profile of the binary that should be used: ", + Some(false), + true, + Some(profiles()), + profile.clone() as usize, + ); + } if !build_spec_cmd.default_bootnode { cli = cli.expect_confirm( "Would you like to use local host as a bootnode ?", @@ -747,17 +777,11 @@ mod tests { genesis_code, ); } - if !build_spec_cmd.release { - cli = cli.expect_confirm( - "Using Debug profile to build a Live specification. Should Release be used instead ?", - release, - ); - } } let build_spec = build_spec_cmd.configure_build_spec(&mut cli).await?; if changes && no_flags_used { assert_eq!(build_spec.id, para_id); - assert_eq!(build_spec.release, release); + assert_eq!(build_spec.profile, profile); assert_eq!(build_spec.default_bootnode, default_bootnode); assert_eq!(build_spec.chain_type, chain_type); assert_eq!(build_spec.relay, relay); @@ -765,7 +789,7 @@ mod tests { assert_eq!(build_spec.genesis_state, genesis_state); assert_eq!(build_spec.genesis_code, genesis_code); } - // Assert that the chain spec file is correctly detected and used + // Assert that the chain spec file is correctly detected and used. assert_eq!(build_spec.chain, chain_spec_path.to_string_lossy()); assert_eq!(build_spec.output_file, chain_spec_path); cli.verify()?; @@ -774,40 +798,61 @@ mod tests { Ok(()) } + #[tokio::test] + async fn configure_build_spec_release_deprecated_works() -> anyhow::Result<()> { + // Create a temporary file to act as the existing chain spec file. + let temp_dir = tempdir()?; + let chain_spec_path = temp_dir.path().join("existing-chain-spec.json"); + std::fs::write(&chain_spec_path, "{}")?; + // Use the deprcrated release flag. + let release = true; + let build_spec_cmd = BuildSpecCommand { + release, + chain: Some(chain_spec_path.to_string_lossy().to_string()), + ..Default::default() + }; + let mut cli = + MockCli::new().expect_confirm( + "An existing chain spec file is provided. Do you want to make additional changes to it?", + false, + ).expect_warning("NOTE: release flag is deprecated. Use `--profile` instead."); + let build_spec = build_spec_cmd.configure_build_spec(&mut cli).await?; + assert_eq!(build_spec.profile, release.into()); + cli.verify()?; + Ok(()) + } + #[test] fn prepare_output_path_works() -> anyhow::Result<()> { - // Create a temporary directory for testing + // Create a temporary directory for testing. let temp_dir = TempDir::new()?; let temp_dir_path = temp_dir.path(); - let test_cases = [ - ("chain-spec.json", "chain-spec.json"), - ("existing_dir", "existing_dir.json"), - ("existing_dir/", "existing_dir/chain-spec.json"), - ("non_existing_dir", "non_existing_dir.json"), - ("non_existing_dir/", "non_existing_dir/chain-spec.json"), - ("some_file", "some_file.json"), - ("some_dir/some_file", "some_dir/some_file.json"), - ("existing_dir/subdir/", "existing_dir/subdir/chain-spec.json"), - ]; - for (input_str, expected_str) in &test_cases { - let input_path = temp_dir_path.join(input_str); - let expected_path = temp_dir_path.join(expected_str); - // Create directories for existing paths - if input_str.starts_with("existing_dir") { - let dir_to_create = if input_str.ends_with('/') { - input_path.clone() - } else { - input_path.parent().unwrap_or(&input_path).to_path_buf() - }; - create_dir_all(&dir_to_create)?; - } - let result = prepare_output_path(&input_path)?; + // No directory path. + let file = temp_dir_path.join("chain-spec.json"); + let result = prepare_output_path(&file)?; + // Expected path: chain-spec.json + assert_eq!(result, file); + + // Existing directory Path. + for dir in ["existing_dir", "existing_dir/", "existing_dir_json"] { + let existing_dir = temp_dir_path.join(dir); + create_dir_all(&existing_dir)?; + let result = prepare_output_path(&existing_dir)?; + // Expected path: existing_dir/chain-spec.json + let expected_path = existing_dir.join(DEFAULT_SPEC_NAME); assert_eq!(result, expected_path); - // Ensure the parent directory exists - if let Some(parent_dir) = result.parent() { - assert!(parent_dir.exists()); - } + } + + // Non-existing directory Path. + for dir in ["non_existing_dir", "non_existing_dir/", "non_existing_dir_json"] { + let non_existing_dir = temp_dir_path.join(dir); + let result = prepare_output_path(&non_existing_dir)?; + // Expected path: non_existing_dir/chain-spec.json + let expected_path = non_existing_dir.join(DEFAULT_SPEC_NAME); + assert_eq!(result, expected_path); + // The directory should now exist. + assert!(result.parent().unwrap().exists()); } Ok(()) @@ -836,4 +881,16 @@ mod tests { }) .collect() } + + fn profiles() -> Vec<(String, String)> { + Profile::VARIANTS + .iter() + .map(|variant| { + ( + variant.get_message().unwrap_or(variant.as_ref()).into(), + variant.get_detailed_message().unwrap_or_default().into(), + ) + }) + .collect() + } } diff --git a/crates/pop-cli/tests/parachain.rs b/crates/pop-cli/tests/parachain.rs index 57a49187..b9baaac5 100644 --- a/crates/pop-cli/tests/parachain.rs +++ b/crates/pop-cli/tests/parachain.rs @@ -65,6 +65,8 @@ async fn parachain_lifecycle() -> Result<()> { "local", "--relay", "paseo-local", + "--profile", + "release", "--genesis-state", "--genesis-code", "--protocol-id", diff --git a/crates/pop-common/Cargo.toml b/crates/pop-common/Cargo.toml index a0b533a1..87074a60 100644 --- a/crates/pop-common/Cargo.toml +++ b/crates/pop-common/Cargo.toml @@ -19,14 +19,15 @@ reqwest.workspace = true serde_json.workspace = true serde.workspace = true strum.workspace = true +strum_macros.workspace = true tar.workspace = true tempfile.workspace = true thiserror.workspace = true tokio.workspace = true toml_edit.workspace = true url.workspace = true +toml.workspace = true [dev-dependencies] mockito.workspace = true -strum_macros.workspace = true -tempfile.workspace = true \ No newline at end of file +tempfile.workspace = true diff --git a/crates/pop-common/src/build.rs b/crates/pop-common/src/build.rs index bfe658b9..f871078a 100644 --- a/crates/pop-common/src/build.rs +++ b/crates/pop-common/src/build.rs @@ -2,14 +2,29 @@ use std::{ fmt, path::{Path, PathBuf}, }; +use strum_macros::{AsRefStr, EnumMessage, EnumString, VariantArray}; /// Enum representing a build profile. -#[derive(Debug, PartialEq)] +#[derive(AsRefStr, Clone, Default, Debug, EnumString, EnumMessage, VariantArray, Eq, PartialEq)] pub enum Profile { /// Debug profile, optimized for debugging. + #[strum(serialize = "debug", message = "Debug", detailed_message = "Optimized for debugging.")] Debug, /// Release profile, optimized without any debugging functionality. + #[default] + #[strum( + serialize = "release", + message = "Release", + detailed_message = "Optimized without any debugging functionality." + )] Release, + /// Production profile, optimized for ultimate performance. + #[strum( + serialize = "production", + message = "Production", + detailed_message = "Optimized for ultimate performance." + )] + Production, } impl Profile { @@ -18,13 +33,20 @@ impl Profile { match self { Profile::Release => path.join("target/release"), Profile::Debug => path.join("target/debug"), + Profile::Production => path.join("target/production"), } } } +impl From for bool { + fn from(value: Profile) -> Self { + value != Profile::Debug + } +} + impl From for Profile { - fn from(release: bool) -> Self { - if release { + fn from(value: bool) -> Self { + if value { Profile::Release } else { Profile::Debug @@ -35,8 +57,73 @@ impl From for Profile { impl fmt::Display for Profile { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { match self { - Self::Debug => write!(f, "DEBUG"), - Self::Release => write!(f, "RELEASE"), + Self::Debug => write!(f, "debug"), + Self::Release => write!(f, "release"), + Self::Production => write!(f, "production"), } } } + +#[cfg(test)] +mod tests { + use super::*; + use std::path::Path; + use strum::EnumMessage; + + #[test] + fn profile_from_string() { + assert_eq!("debug".parse::().unwrap(), Profile::Debug); + assert_eq!("release".parse::().unwrap(), Profile::Release); + assert_eq!("production".parse::().unwrap(), Profile::Production); + } + + #[test] + fn profile_detailed_message() { + assert_eq!(Profile::Debug.get_detailed_message(), Some("Optimized for debugging.")); + assert_eq!( + Profile::Release.get_detailed_message(), + Some("Optimized without any debugging functionality.") + ); + assert_eq!( + Profile::Production.get_detailed_message(), + Some("Optimized for ultimate performance.") + ); + } + + #[test] + fn profile_target_directory() { + let base_path = Path::new("/example/path"); + + assert_eq!( + Profile::Debug.target_directory(base_path), + Path::new("/example/path/target/debug") + ); + assert_eq!( + Profile::Release.target_directory(base_path), + Path::new("/example/path/target/release") + ); + assert_eq!( + Profile::Production.target_directory(base_path), + Path::new("/example/path/target/production") + ); + } + + #[test] + fn profile_default() { + let default_profile = Profile::default(); + assert_eq!(default_profile, Profile::Release); + } + + #[test] + fn profile_from_bool() { + assert_eq!(Profile::from(true), Profile::Release); + assert_eq!(Profile::from(false), Profile::Debug); + } + + #[test] + fn profile_into_bool() { + assert_eq!(bool::from(Profile::Debug), false); + assert_eq!(bool::from(Profile::Release), true); + assert_eq!(bool::from(Profile::Production), true); + } +} diff --git a/crates/pop-common/src/manifest.rs b/crates/pop-common/src/manifest.rs index ff84c1be..16db4f0d 100644 --- a/crates/pop-common/src/manifest.rs +++ b/crates/pop-common/src/manifest.rs @@ -2,7 +2,7 @@ use crate::Error; use anyhow; -pub use cargo_toml::{Dependency, Manifest}; +pub use cargo_toml::{Dependency, LtoSetting, Manifest, Profile, Profiles}; use std::{ fs::{read_to_string, write}, path::{Path, PathBuf}, @@ -58,6 +58,7 @@ pub fn find_workspace_toml(target_dir: &Path) -> Option { /// This function is used to add a crate to a workspace. /// # Arguments +/// /// * `workspace_toml` - The path to the workspace `Cargo.toml` /// * `crate_path`: The path to the crate that should be added to the workspace pub fn add_crate_to_workspace(workspace_toml: &Path, crate_path: &Path) -> anyhow::Result<()> { @@ -97,6 +98,44 @@ pub fn add_crate_to_workspace(workspace_toml: &Path, crate_path: &Path) -> anyho Ok(()) } +/// Adds a "production" profile to the Cargo.toml manifest if it doesn't already exist. +/// +/// # Arguments +/// * `project` - The path to the root of the Cargo project containing the Cargo.toml. +pub fn add_production_profile(project: &Path) -> anyhow::Result<()> { + let root_toml_path = project.join("Cargo.toml"); + let mut manifest = Manifest::from_path(&root_toml_path)?; + // Check if the `production` profile already exists. + if manifest.profile.custom.contains_key("production") { + return Ok(()); + } + // Create the production profile with required fields. + let production_profile = Profile { + opt_level: None, + debug: None, + split_debuginfo: None, + rpath: None, + lto: Some(LtoSetting::Fat), + debug_assertions: None, + codegen_units: Some(1), + panic: None, + incremental: None, + overflow_checks: None, + strip: None, + package: std::collections::BTreeMap::new(), + build_override: None, + inherits: Some("release".to_string()), + }; + // Insert the new profile into the custom profiles + manifest.profile.custom.insert("production".to_string(), production_profile); + + // Serialize the updated manifest and write it back to the file + let toml_string = toml::to_string(&manifest)?; + write(&root_toml_path, toml_string)?; + + Ok(()) +} + #[cfg(test)] mod tests { use super::*; @@ -419,4 +458,42 @@ mod tests { ); assert!(add_crate.is_err()); } + + #[test] + fn add_production_profile_works() { + let test_builder = TestBuilder::default().add_workspace().add_workspace_cargo_toml( + r#"[profile.release] + opt-level = 3 + "#, + ); + + let binding = test_builder.workspace.expect("Workspace should exist"); + let project_path = binding.path(); + let cargo_toml_path = project_path.join("Cargo.toml"); + + // Call the function to add the production profile + let result = add_production_profile(project_path); + assert!(result.is_ok()); + + // Verify the production profile is added + let manifest = + Manifest::from_path(&cargo_toml_path).expect("Should parse updated Cargo.toml"); + let production_profile = manifest + .profile + .custom + .get("production") + .expect("Production profile should exist"); + assert_eq!(production_profile.codegen_units, Some(1)); + assert_eq!(production_profile.inherits.as_deref(), Some("release")); + assert_eq!(production_profile.lto, Some(LtoSetting::Fat)); + + // Test idempotency: Running the function again should not modify the manifest + let initial_toml_content = + read_to_string(&cargo_toml_path).expect("Cargo.toml should be readable"); + let second_result = add_production_profile(project_path); + assert!(second_result.is_ok()); + let final_toml_content = + read_to_string(&cargo_toml_path).expect("Cargo.toml should be readable"); + assert_eq!(initial_toml_content, final_toml_content); + } } diff --git a/crates/pop-parachains/src/build.rs b/crates/pop-parachains/src/build.rs index 9bcb890b..69fa2d2a 100644 --- a/crates/pop-parachains/src/build.rs +++ b/crates/pop-parachains/src/build.rs @@ -31,8 +31,10 @@ pub fn build_parachain( args.push("--package"); args.push(package) } - if matches!(profile, &Profile::Release) { + if profile == &Profile::Release { args.push("--release"); + } else if profile == &Profile::Production { + args.push("--profile=production"); } cmd("cargo", args).dir(path).run()?; binary_path(&profile.target_directory(path), node_path.unwrap_or(&path.join("node"))) @@ -76,7 +78,7 @@ pub fn binary_path(target_path: &Path, node_path: &Path) -> Result Result { + fn setup_template_and_instantiate() -> Result { let temp_dir = tempdir().expect("Failed to create temp dir"); let config = Config { symbol: "DOT".to_string(), @@ -367,9 +370,9 @@ mod tests { } // Function that generates a Cargo.toml inside node directory for testing. - fn generate_mock_node(temp_dir: &Path) -> Result<(), Error> { + fn generate_mock_node(temp_dir: &Path, name: Option<&str>) -> Result { // Create a node directory - let target_dir = temp_dir.join("node"); + let target_dir = temp_dir.join(name.unwrap_or("node")); fs::create_dir(&target_dir)?; // Create a Cargo.toml file let mut toml_file = fs::File::create(target_dir.join("Cargo.toml"))?; @@ -384,7 +387,7 @@ mod tests { "# )?; - Ok(()) + Ok(target_dir) } // Function that fetch a binary from pop network @@ -393,13 +396,13 @@ mod tests { writeln!( config.as_file(), r#" -[relaychain] -chain = "rococo-local" + [relaychain] + chain = "rococo-local" -[[parachains]] -id = 4385 -default_command = "pop-node" -"# + [[parachains]] + id = 4385 + default_command = "pop-node" + "# )?; let mut zombienet = Zombienet::new(&cache, config.path().to_str().unwrap(), None, None, None, None, None) @@ -425,20 +428,45 @@ default_command = "pop-node" Ok(binary_path) } + fn add_production_profile(project: &Path) -> Result<()> { + let root_toml_path = project.join("Cargo.toml"); + let mut root_toml_content = fs::read_to_string(&root_toml_path)?; + root_toml_content.push_str( + r#" + [profile.production] + codegen-units = 1 + inherits = "release" + lto = true + "#, + ); + // Write the updated content back to the file + write(&root_toml_path, root_toml_content)?; + Ok(()) + } + #[test] fn build_parachain_works() -> Result<()> { - let temp_dir = tempdir()?; let name = "parachain_template_node"; + let temp_dir = tempdir()?; cmd("cargo", ["new", name, "--bin"]).dir(temp_dir.path()).run()?; - generate_mock_node(&temp_dir.path().join(name))?; - let binary = build_parachain(&temp_dir.path().join(name), None, &Profile::Release, None)?; - let target_directory = temp_dir.path().join(name).join("target/release"); - assert!(target_directory.exists()); - assert!(target_directory.join("parachain_template_node").exists()); - assert_eq!( - binary.display().to_string(), - target_directory.join("parachain_template_node").display().to_string() - ); + let project = temp_dir.path().join(name); + add_production_profile(&project)?; + for node in vec![None, Some("custom_node")] { + let node_path = generate_mock_node(&project, node)?; + for package in vec![None, Some(String::from("parachain_template_node"))] { + for profile in Profile::VARIANTS { + let node_path = node.map(|_| node_path.as_path()); + let binary = build_parachain(&project, package.clone(), &profile, node_path)?; + let target_directory = profile.target_directory(&project); + assert!(target_directory.exists()); + assert!(target_directory.join("parachain_template_node").exists()); + assert_eq!( + binary.display().to_string(), + target_directory.join("parachain_template_node").display().to_string() + ); + } + } + } Ok(()) }