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

feat: adding new packages to workspace Cargo.toml automatically #277

Merged
merged 4 commits into from
Aug 8, 2024
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
1,137 changes: 1,089 additions & 48 deletions Cargo.lock

Large diffs are not rendered by default.

8 changes: 8 additions & 0 deletions crates/pop-cli/src/commands/new/contract.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ use crate::cli::{
traits::{Cli as _, Confirm as _},
Cli,
};
use pop_common::manifest::{add_crate_to_workspace, find_workspace_toml};

use anyhow::Result;
use clap::{
builder::{PossibleValue, PossibleValuesParser, TypedValueParser},
Expand Down Expand Up @@ -75,6 +77,12 @@ impl NewContractCommand {

is_template_supported(contract_type, &template)?;
generate_contract_from_template(name, &path, &template)?;

// If the contract is part of a workspace, add it to that workspace
tsenovilla marked this conversation as resolved.
Show resolved Hide resolved
if let Some(workspace_toml) = find_workspace_toml(&path) {
add_crate_to_workspace(&workspace_toml, &path)?;
}

Ok(())
}
}
Expand Down
13 changes: 12 additions & 1 deletion crates/pop-cli/src/commands/new/pallet.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ use crate::style::Theme;
use clap::Args;
use cliclack::{clear_screen, confirm, intro, outro, outro_cancel, set_theme};
use console::style;
use pop_common::manifest::{add_crate_to_workspace, find_workspace_toml};
use pop_parachains::{create_pallet_template, resolve_pallet_path, TemplatePalletConfig};
use std::fs;

Expand All @@ -30,6 +31,10 @@ impl NewPalletCommand {
))?;
set_theme(Theme);
let target = resolve_pallet_path(self.path.clone())?;

// Determine if the pallet is being created inside a workspace
let workspace_toml = find_workspace_toml(&target);

let pallet_name = self.name.clone();
let pallet_path = target.join(pallet_name.clone());
if pallet_path.exists() {
Expand All @@ -45,7 +50,7 @@ impl NewPalletCommand {
))?;
return Ok(());
}
fs::remove_dir_all(pallet_path)?;
fs::remove_dir_all(pallet_path.clone())?;
}
let spinner = cliclack::spinner();
spinner.start("Generating pallet...");
Expand All @@ -55,9 +60,15 @@ impl NewPalletCommand {
name: self.name.clone(),
authors: self.authors.clone().expect("default values"),
description: self.description.clone().expect("default values"),
pallet_in_workspace: workspace_toml.is_some(),
},
)?;

// If the pallet has been created inside a workspace, add it to that workspace
if let Some(workspace_toml) = workspace_toml {
add_crate_to_workspace(&workspace_toml, &pallet_path)?;
}

spinner.stop("Generation complete");
outro(format!("cd into \"{}\" and enjoy hacking! 🚀", &self.name))?;
Ok(())
Expand Down
1 change: 1 addition & 0 deletions crates/pop-common/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ tar.workspace = true
tempfile.workspace = true
thiserror.workspace = true
tokio.workspace = true
toml_edit.workspace = true
url.workspace = true

[dev-dependencies]
Expand Down
294 changes: 289 additions & 5 deletions crates/pop-common/src/manifest.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,13 @@
// SPDX-License-Identifier: GPL-3.0

use crate::Error;
use anyhow;
pub use cargo_toml::{Dependency, Manifest};
use std::path::{Path, PathBuf};
use std::{
fs::{read_to_string, write},
path::{Path, PathBuf},
};
use toml_edit::{value, DocumentMut, Item, Value};

/// Parses the contents of a `Cargo.toml` manifest.
///
Expand All @@ -23,14 +28,165 @@ pub fn from_path(path: Option<&Path>) -> Result<Manifest, Error> {
Ok(Manifest::from_path(path.canonicalize()?)?)
}

/// This function is used to determine if a Path is contained inside a workspace, and returns a PathBuf to the workspace Cargo.toml if found
///
/// # Arguments
/// * `target_dir` - A directory that may be contained inside a workspace
pub fn find_workspace_toml(target_dir: &Path) -> Option<PathBuf> {
let mut dir = target_dir;
while let Some(parent) = dir.parent() {
let cargo_toml = parent.join("Cargo.toml");
if cargo_toml.exists() {
if let Ok(contents) = read_to_string(&cargo_toml) {
if contents.contains("[workspace]") {
return Some(cargo_toml);
}
}
}
dir = parent;
}
None
}

/// 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<()> {
let toml_contents = read_to_string(workspace_toml)?;
let mut doc = toml_contents.parse::<DocumentMut>()?;

// Find the workspace dir
let workspace_dir = workspace_toml.parent().expect("A file always lives inside a dir; qed");
// Find the relative path to the crate from the workspace root
let crate_relative_path = crate_path.strip_prefix(workspace_dir)?;

if let Some(workspace) = doc.get_mut("workspace") {
if let Item::Table(workspace_table) = workspace {
if let Some(members) = workspace_table.get_mut("members") {
if let Item::Value(members_array) = members {
if let Value::Array(array) = members_array {
array.push(
crate_relative_path
.to_str()
.expect("target's always a valid string; qed"),
);
} else {
return Err(anyhow::anyhow!("Corrupted workspace"));
}
}
} else {
workspace_table["members"] = value(
crate_relative_path.to_str().expect("target's always a valid string; qed"),
);
}
}
} else {
return Err(anyhow::anyhow!("Corrupted workspace"));
}

write(workspace_toml, doc.to_string())?;
Ok(())
}

#[cfg(test)]
mod tests {
use crate::manifest::from_path;
use anyhow::{Error, Result};
use std::path::Path;
use super::*;
use std::fs::{write, File};
use tempfile::TempDir;

struct TestBuilder {
main_tempdir: TempDir,
workspace: Option<TempDir>,
inside_workspace_dir: Option<TempDir>,
workspace_cargo_toml: Option<PathBuf>,
outside_workspace_dir: Option<TempDir>,
}

impl Default for TestBuilder {
fn default() -> Self {
Self {
main_tempdir: TempDir::new().expect("Failed to create tempdir"),
workspace: None,
inside_workspace_dir: None,
workspace_cargo_toml: None,
outside_workspace_dir: None,
}
}
}

impl TestBuilder {
fn add_workspace(self) -> Self {
Self { workspace: TempDir::new_in(self.main_tempdir.as_ref()).ok(), ..self }
}

fn add_inside_workspace_dir(self) -> Self {
Self {
inside_workspace_dir: TempDir::new_in(self.workspace.as_ref().expect(
"add_inside_workspace_dir is only callable if workspace has been created",
))
.ok(),
..self
}
}

fn add_workspace_cargo_toml(self) -> Self {
let workspace_cargo_toml = self
.workspace
.as_ref()
.expect("add_workspace_cargo_toml is only callable if workspace has been created")
.path()
.join("Cargo.toml");
File::create(&workspace_cargo_toml).expect("Failed to create Cargo.toml");
write(
&workspace_cargo_toml,
r#"[workspace]
resolver = "2"
members = ["member1"]
"#,
)
.expect("Failed to write Cargo.toml");
Self { workspace_cargo_toml: Some(workspace_cargo_toml.to_path_buf()), ..self }
}

fn add_workspace_cargo_toml_member_not_array(self) -> Self {
let workspace_cargo_toml = self
.workspace
.as_ref()
.expect("add_workspace_cargo_toml is only callable if workspace has been created")
.path()
.join("Cargo.toml");
File::create(&workspace_cargo_toml).expect("Failed to create Cargo.toml");
write(
&workspace_cargo_toml,
r#"[workspace]
resolver = "2"
members = "member1"
"#,
)
.expect("Failed to write Cargo.toml");
Self { workspace_cargo_toml: Some(workspace_cargo_toml.to_path_buf()), ..self }
}

fn add_workspace_cargo_toml_not_defining_workspace(self) -> Self {
let workspace_cargo_toml = self
.workspace
.as_ref()
.expect("add_workspace_cargo_toml is only callable if workspace has been created")
.path()
.join("Cargo.toml");
File::create(&workspace_cargo_toml).expect("Failed to create Cargo.toml");
write(&workspace_cargo_toml, r#""#).expect("Failed to write Cargo.toml");
Self { workspace_cargo_toml: Some(workspace_cargo_toml.to_path_buf()), ..self }
}

fn add_outside_workspace_dir(self) -> Self {
Self { outside_workspace_dir: TempDir::new_in(self.main_tempdir.as_ref()).ok(), ..self }
}
}

#[test]
fn from_path_works() -> Result<(), Error> {
fn from_path_works() -> anyhow::Result<(), anyhow::Error> {
// Workspace manifest from directory
from_path(Some(Path::new("../../")))?;
// Workspace manifest from path
Expand All @@ -52,4 +208,132 @@ mod tests {
));
Ok(())
}

#[test]
fn find_workspace_toml_works_well() {
let test_builder = TestBuilder::default()
.add_workspace()
.add_inside_workspace_dir()
.add_workspace_cargo_toml()
.add_outside_workspace_dir();
assert!(find_workspace_toml(
test_builder
.inside_workspace_dir
.as_ref()
.expect("Inside workspace dir should exist")
.path()
)
.is_some());
assert_eq!(
find_workspace_toml(
test_builder
.inside_workspace_dir
.as_ref()
.expect("Inside workspace dir should exist")
.path()
)
.expect("The Cargo.toml should exist at this point"),
test_builder.workspace_cargo_toml.expect("Cargo.toml should exist")
);
assert!(find_workspace_toml(
test_builder
.outside_workspace_dir
.as_ref()
.expect("Outside workspace dir should exist")
.path()
)
.is_none());
}

#[test]
fn add_crate_to_workspace_works_well() {
let test_builder = TestBuilder::default()
.add_workspace()
.add_workspace_cargo_toml()
.add_inside_workspace_dir();
let add_crate = add_crate_to_workspace(
test_builder.workspace_cargo_toml.as_ref().expect("Workspace should exist"),
test_builder
.inside_workspace_dir
.as_ref()
.expect("Inside workspace dir should exist")
.path(),
);
assert!(add_crate.is_ok());
let content = read_to_string(
test_builder.workspace_cargo_toml.as_ref().expect("Workspace should exist"),
)
.expect("Cargo.toml should be readable");
let doc = content.parse::<DocumentMut>().expect("This should work");
if let Some(Item::Table(workspace_table)) = doc.get("workspace") {
if let Some(Item::Value(Value::Array(array))) = workspace_table.get("members") {
assert!(array.iter().any(|item| {
if let Value::String(item) = item {
test_builder
.inside_workspace_dir
.as_ref()
.expect("Inside workspace should exist")
.path()
.to_str()
.expect("Dir should be mapped to a str")
.contains(item.value())
} else {
false
}
}));
} else {
panic!("add_crate_to_workspace fails and should work");
}
} else {
panic!("add_crate_to_workspace fails and should work");
}
}

#[test]
fn add_crate_to_workspace_fails_if_crate_path_not_inside_workspace() {
let test_builder = TestBuilder::default()
.add_workspace()
.add_workspace_cargo_toml()
.add_outside_workspace_dir();
let add_crate = add_crate_to_workspace(
test_builder.workspace_cargo_toml.as_ref().expect("Workspace should exist"),
test_builder
.outside_workspace_dir
.expect("Inside workspace dir should exist")
.path(),
);
assert!(add_crate.is_err());
}

#[test]
fn add_crate_to_workspace_fails_if_members_not_an_array() {
let test_builder = TestBuilder::default()
.add_workspace()
.add_workspace_cargo_toml_member_not_array()
.add_inside_workspace_dir();
let add_crate = add_crate_to_workspace(
test_builder.workspace_cargo_toml.as_ref().expect("Workspace should exist"),
test_builder
.inside_workspace_dir
.expect("Inside workspace dir should exist")
.path(),
);
assert!(add_crate.is_err());
}

#[test]
fn add_crate_to_workspace_fails_if_workspace_isnt_workspace() {
let test_builder = TestBuilder::default()
.add_workspace()
.add_workspace_cargo_toml_not_defining_workspace()
.add_inside_workspace_dir();
let add_crate = add_crate_to_workspace(
test_builder.workspace_cargo_toml.as_ref().expect("Workspace should exist"),
test_builder
.inside_workspace_dir
.expect("Inside workspace dir should exist")
.path(),
);
assert!(add_crate.is_err());
}
}
Loading
Loading