From af78d8d086c6c509ca7b353a040b7c924d08dee9 Mon Sep 17 00:00:00 2001 From: Chad Ostrowski <221614+chadoh@users.noreply.github.com> Date: Tue, 30 Apr 2024 16:08:30 -0400 Subject: [PATCH] fix(init): don't add two `dev_dependencies` (#1296) When initializing a new project with `soroban contract init`, we need to clean up the `Cargo.toml` of example projects a little. We need to make sure `soroban-sdk` is always inherited from the workspace, and we need to make sure there's no `profile` section. We had a couple issues with the old logic. 1. It was adding a `dev_dependencies` section, with an underscore, which is deprecated. If the Cargo.toml already had a `dev-dependencies` section, it would add a second `dev_dependencies`, which cargo would complain about. 2. The new setting would only set `{ workspace = true }`, even if the old one included other details like `features = ['testutils']`. This adds a new crate, `cargo_toml`, which allows us to parse and manipulate Cargo.toml files in a more sophisticated way, which allows us to make sure we get the `dev-dependencies` section even if it's called `dev_dependencies`. It also makes it straightforward to add the correct inherited dependency. --- Cargo.lock | 11 ++ cmd/soroban-cli/Cargo.toml | 14 +-- cmd/soroban-cli/src/commands/contract/init.rs | 113 ++++++++++++++---- 3 files changed, 105 insertions(+), 33 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index c245e03fc..a44af436f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -363,6 +363,16 @@ dependencies = [ "thiserror", ] +[[package]] +name = "cargo_toml" +version = "0.20.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "c8cb1d556b8b8f36e5ca74938008be3ac102f5dcb5b68a0477e4249ae2291cd3" +dependencies = [ + "serde", + "toml 0.8.10", +] + [[package]] name = "cc" version = "1.0.95" @@ -3631,6 +3641,7 @@ dependencies = [ "base64 0.21.7", "bollard", "cargo_metadata", + "cargo_toml", "chrono", "clap", "clap-markdown", diff --git a/cmd/soroban-cli/Cargo.toml b/cmd/soroban-cli/Cargo.toml index b20d22301..8c0aa3e8c 100644 --- a/cmd/soroban-cli/Cargo.toml +++ b/cmd/soroban-cli/Cargo.toml @@ -46,12 +46,12 @@ soroban-ledger-snapshot = { workspace = true } stellar-strkey = { workspace = true } soroban-sdk = { workspace = true } soroban-rpc = { workspace = true } - +cargo_toml = "0.20.1" clap = { workspace = true, features = [ - "derive", - "env", - "deprecated", - "string", + "derive", + "env", + "deprecated", + "string", ] } clap_complete = { workspace = true } async-trait = { workspace = true } @@ -98,8 +98,8 @@ dotenvy = "0.15.7" strum = "0.17.1" strum_macros = "0.17.1" gix = { version = "0.58.0", default-features = false, features = [ - "blocking-http-transport-reqwest-rust-tls", - "worktree-mutation", + "blocking-http-transport-reqwest-rust-tls", + "worktree-mutation", ] } ureq = { version = "2.9.1", features = ["json"] } diff --git a/cmd/soroban-cli/src/commands/contract/init.rs b/cmd/soroban-cli/src/commands/contract/init.rs index 725a9a9da..2db495854 100644 --- a/cmd/soroban-cli/src/commands/contract/init.rs +++ b/cmd/soroban-cli/src/commands/contract/init.rs @@ -1,3 +1,11 @@ +use cargo_toml; +use clap::{ + builder::{PossibleValue, PossibleValuesParser, ValueParser}, + Parser, ValueEnum, +}; +use gix::{clone, create, open, progress, remote}; +use rust_embed::RustEmbed; +use serde_json::{from_str, json, to_string_pretty, Error as JsonError, Value as JsonValue}; use std::{ env, ffi::OsStr, @@ -11,15 +19,6 @@ use std::{ str, sync::atomic::AtomicBool, }; - -use clap::{ - builder::{PossibleValue, PossibleValuesParser, ValueParser}, - Parser, ValueEnum, -}; -use gix::{clone, create, open, progress, remote}; -use rust_embed::RustEmbed; -use serde_json::{from_str, json, to_string_pretty, Error as JsonError, Value as JsonValue}; -use toml_edit::{Document, Formatted, InlineTable, Item, TomlError, Value as TomlValue}; use ureq::get; const SOROBAN_EXAMPLES_URL: &str = "https://github.com/stellar/soroban-examples.git"; @@ -73,7 +72,7 @@ pub enum Error { CheckoutError(#[from] clone::checkout::main_worktree::Error), #[error("Failed to parse toml file: {0}")] - TomlParseError(#[from] TomlError), + CargoTomlParseError(#[from] cargo_toml::Error), #[error("Failed to parse package.json file: {0}")] JsonParseError(#[from] JsonError), @@ -314,27 +313,49 @@ fn copy_example_contracts(from: &Path, to: &Path, contracts: &[String]) -> Resul Ok(()) } +fn inherit_sdk(mut deps: cargo_toml::DepsSet) -> cargo_toml::DepsSet { + if let Some(sdk_dep) = deps.get("soroban-sdk") { + match sdk_dep { + cargo_toml::Dependency::Simple(_) => { + deps.insert( + "soroban-sdk".to_string(), + cargo_toml::Dependency::Inherited(cargo_toml::InheritedDependencyDetail { + workspace: true, + ..Default::default() + }), + ); + } + + cargo_toml::Dependency::Detailed(details) => { + deps.insert( + "soroban-sdk".to_string(), + cargo_toml::Dependency::Inherited(cargo_toml::InheritedDependencyDetail { + features: details.features.clone(), + optional: details.optional, + workspace: true, + }), + ); + } + + // we don't need to do anything, it already has `workspace = true` + cargo_toml::Dependency::Inherited(_) => (), + } + } + deps +} + fn edit_contract_cargo_file(contract_path: &Path) -> Result<(), Error> { let cargo_path = contract_path.join("Cargo.toml"); - let cargo_toml_str = read_to_string(&cargo_path).map_err(|e| { - eprint!("Error reading Cargo.toml file in: {contract_path:?}"); - e - })?; - let mut doc = cargo_toml_str.parse::().map_err(|e| { + let mut doc = cargo_toml::Manifest::from_path(cargo_path.clone()).map_err(|e| { eprintln!("Error parsing Cargo.toml file in: {contract_path:?}"); e })?; - let mut workspace_table = InlineTable::new(); - workspace_table.insert("workspace", TomlValue::Boolean(Formatted::new(true))); - - doc["dependencies"]["soroban-sdk"] = - Item::Value(TomlValue::InlineTable(workspace_table.clone())); - doc["dev_dependencies"]["soroban-sdk"] = Item::Value(TomlValue::InlineTable(workspace_table)); - - doc.remove("profile"); + doc.dependencies = inherit_sdk(doc.dependencies); + doc.dev_dependencies = inherit_sdk(doc.dev_dependencies); + doc.profile = cargo_toml::Profiles::default(); - write(&cargo_path, doc.to_string()).map_err(|e| { + write(&cargo_path, toml::to_string(&doc).unwrap()).map_err(|e| { eprintln!("Error writing to Cargo.toml file in: {contract_path:?}"); e })?; @@ -637,8 +658,48 @@ mod tests { fn assert_contract_cargo_file_uses_workspace(project_dir: &Path, contract_name: &str) { let contract_dir = project_dir.join("contracts").join(contract_name); let cargo_toml_path = contract_dir.as_path().join("Cargo.toml"); - let cargo_toml_str = read_to_string(cargo_toml_path).unwrap(); - assert!(cargo_toml_str.contains("soroban-sdk = { workspace = true }")); + let cargo_toml_str = read_to_string(cargo_toml_path.clone()).unwrap(); + let doc = cargo_toml_str.parse::().unwrap(); + println!("{cargo_toml_path:?} contents:\n{cargo_toml_str}"); + assert!( + doc.get("dependencies") + .unwrap() + .get("soroban-sdk") + .unwrap() + .get("workspace") + .unwrap() + .as_bool() + .unwrap(), + "expected [dependencies.soroban-sdk] to be a workspace dependency" + ); + assert!( + doc.get("dev-dependencies") + .unwrap() + .get("soroban-sdk") + .unwrap() + .get("workspace") + .unwrap() + .as_bool() + .unwrap(), + "expected [dev-dependencies.soroban-sdk] to be a workspace dependency" + ); + assert_ne!( + 0, + doc.get("dev-dependencies") + .unwrap() + .get("soroban-sdk") + .unwrap() + .get("features") + .unwrap() + .as_array() + .unwrap() + .len(), + "expected [dev-dependencies.soroban-sdk] to have a features list" + ); + assert!( + doc.get("dev_dependencies").is_none(), + "erroneous 'dev_dependencies' section" + ); } fn assert_example_contract_excluded_files_do_not_exist(