Fix lift-to-workspace

Signed-off-by: Oliver Tale-Yazdi <[email protected]>

.github/workflows/downstream.yml

@@ -48,15 +48,9 @@ jobs:
     - if: matrix.repo == 'polkadot-sdk'
       name: Zepter passes
       run: |
-        echo "Checking features #1"
-        zepter lint propagate-feature --feature try-runtime --left-side-feature-missing=ignore --workspace --fix --feature-enables-dep="try-runtime:frame-try-runtime"
-        echo "Checking features #2"
-        zepter lint propagate-feature --feature runtime-benchmarks --left-side-feature-missing=ignore --workspace --feature-enables-dep="runtime-benchmarks:frame-benchmarking"
-        echo "Checking features #3"
-        zepter lint propagate-feature --feature std --left-side-feature-missing=ignore --workspace
-        echo "Checking formatting #1"
-        zepter f f
-    - if: matrix.repo != 'substrate' && matrix.repo != 'polkadot-sdk'
+        echo "Checking..."
+        zepter run check
+    - if: matrix.repo == 'cumulus'
       name: Zepter doesnt panic
       # Polkadot and Cumulus can be red, but should not panic. Hence the `--exit-code-zero`.
       run: |

README.md

@@ -26,6 +26,10 @@ zepter
   - never-implies *(⚠️ unstable)*: A feature should never transitively imply another one.
   - only-enables *(⚠️ unstable)*: A features should exclusively enable another one.
   - why-enables *(⚠️ unstable)*: Find out why a specific feature is enables.
+- debug: *(⚠️ unstable)* just for quick debugging some stuff.
+- transpose (⚠️ unstable)*
+  - dependency
+    - lift-to-workspace: Lifts crate dependencies to the workspace.
 
 ## Example - Feature Formatting
 

src/autofix.rs

@@ -471,11 +471,11 @@ impl AutoFixer {
 		if let Some(as_str) = dep.as_str() {
 			cargo_metadata::semver::VersionReq::parse(as_str).expect("Is semver");
 			let mut table = InlineTable::new();
-			table.insert("workspace", Value::Boolean(Formatted::new(true)));
+			table.remove("default-features");// We also remove it to get the order right.
 
-			if default_feats {
-				table.insert("default-features", Value::Boolean(Formatted::new(true)));
-			}
+			table.insert("workspace", Value::Boolean(Formatted::new(true)));
+			table.insert("default-features", Value::Boolean(Formatted::new(default_feats)));
+
 			table.set_dotted(false);
 
 			*dep = Item::Value(Value::InlineTable(table));
@@ -484,10 +484,10 @@ impl AutoFixer {
 				return Err("'git' or 'path' dependency are currently not supported".into())
 			}
 			as_table.remove("version");
+			as_table.remove("default-features");// We also remove it to get the order right.
+
 			as_table.insert("workspace", Value::Boolean(Formatted::new(true)));
-			if default_feats {
-				as_table.insert("default-features", Value::Boolean(Formatted::new(true)));
-			}
+			as_table.insert("default-features", Value::Boolean(Formatted::new(default_feats)));			
 		} else {
 			unreachable!("Unknown kind of dependency: {:?}", dep);
 		}
@@ -496,10 +496,33 @@ impl AutoFixer {
 
 	pub fn add_workspace_dep(
 		&mut self,
-		_dep: &Dependency,
-		_default_feats: bool,
+		dep: &Dependency,
+		default_feats: bool,
 	) -> Result<(), String> {
-		panic!("todo");
+		let doc: &mut Document = self.doc.as_mut().unwrap();
+
+		if !doc.contains_table("workspace") {
+			return Err("No workspace table".into())
+		}
+		let workspace = doc["workspace"].as_table_mut().unwrap();
+
+		if !workspace.contains_table("dependencies") {
+			workspace.insert("dependencies", table());
+		}
+
+		let deps = workspace["dependencies"].as_table_mut().unwrap();
+
+		if deps.contains_key(&dep.name) {
+			return Err("Dependency already exists in the workspace".into())
+		}
+
+		let mut t = InlineTable::new();
+		t.insert("version", Value::String(Formatted::new(dep.req.to_string())));
+		t.insert("default-features", Value::Boolean(Formatted::new(default_feats)));
+
+		deps.insert(&dep.name, Item::Value(Value::InlineTable(t)));
+
+		Ok(())
 	}
 
 	pub fn modified(&self) -> bool {

src/cmd/debug.rs

@@ -2,7 +2,7 @@
 // SPDX-FileCopyrightText: Oliver Tale-Yazdi <[email protected]>
 
 use super::{lint::CrateAndFeature, GlobalArgs};
-use crate::{cmd::lint::build_feature_dag, log, prelude::Dag};
+use crate::{cmd::lint::build_feature_dag, prelude::Dag};
 
 use cargo_metadata::Metadata;
 use histo::Histogram;
@@ -22,11 +22,11 @@ pub struct DebugCmd {
 }
 
 impl DebugCmd {
-	pub fn run(&self, _g: &GlobalArgs) {
+	pub fn run(&self, g: &GlobalArgs) {
+		g.warn_unstable();
 		let meta = self.cargo_args.load_metadata().expect("Loads metadata");
 		let dag = build_feature_dag(&meta, &meta.packages);
-
-		log::warn!("Unstable feature - do not rely on this!");
+
 		if !self.no_root {
 			println!("Root: {}", meta.workspace_root.to_string());
 		}

src/cmd/mod.rs

@@ -92,6 +92,10 @@ impl GlobalArgs {
 		}
 	}
 
+	pub fn warn_unstable(&self) {
+		log::warn!("Unstable feature - do not rely on this!");
+	}
+
 	pub fn error_code(&self) -> i32 {
 		if self.exit_code_zero {
 			0

src/cmd/transpose.rs

@@ -6,6 +6,7 @@ use crate::{autofix::*, grammar::*, log};
 
 use cargo_metadata::{Dependency as Dep, Package};
 use itertools::Itertools;
+use semver::{Version, VersionReq, Op};
 use std::{
 	collections::{BTreeMap as Map, HashMap},
 	fs::canonicalize,
@@ -74,6 +75,8 @@ pub enum DefaultFeatureMode {
 
 impl LiftToWorkspaceCmd {
 	pub fn run(&self, g: &GlobalArgs) {
+		g.warn_unstable();
+
 		let mut args = self.cargo_args.clone();
 		args.workspace = true;
 		let meta = args.load_metadata().expect("Loads metadata");
@@ -93,7 +96,7 @@ impl LiftToWorkspaceCmd {
 
 		let versions = by_version.keys().collect::<Vec<_>>();
 		if versions.len() > 1 {
-			let longest = versions.iter().map(|v| v.to_string().len()).max().unwrap();
+			let str_width = versions.iter().map(|v| v.to_string().len()).max().unwrap();
 			let mut err = String::new();
 			// iter by descending frequence
 			for (version, pkgs) in by_version.iter().sorted_by_key(|(_, pkgs)| pkgs.len()).rev() {
@@ -109,15 +112,23 @@ impl LiftToWorkspaceCmd {
 						.take(3)
 						.collect::<Vec<_>>()
 						.join(", "),
-					width = longest
+					width = str_width
 				));
 			}
 
-			let _hint = format!("cargo upgrade -p {}@version", &self.dependency);
+			let version_hint = match try_find_latest(by_version.keys()) {
+				Ok(latest) => latest.to_string(),
+				Err(e) => {
+					log::warn!("Could not find determine latest common version: {}", e);
+					"version".to_string()
+				},
+			};
+			let hint = format!("cargo upgrade -p {}@{version_hint}", &self.dependency);
 			panic!(
-				"\nFound {} different versions of '{}' in the workspace:\n{err}",
+				"\nFound {} different versions of '{}' in the workspace:\n\n{err}\nHint: {}\n",
 				versions.len(),
 				&self.dependency,
+				g.bold(&hint),
 			);
 		}
 
@@ -133,9 +144,6 @@ impl LiftToWorkspaceCmd {
 			&version,
 			found,
 			crate::grammar::plural(found),
-			//by_kind.get(&DepKind::Normal).unwrap_or(&0),
-			//by_kind.get(&DepKind::Development).unwrap_or(&0),
-			//by_kind.get(&DepKind::Build).unwrap_or(&0)
 		);
 
 		let mut fixers = Map::new();
@@ -154,9 +162,40 @@ impl LiftToWorkspaceCmd {
 		}
 
 		// Now create fixer for the root package
-		//let mut fixer =
-		// AutoFixer::from_manifest(&meta.workspace_root.into_std_path_buf()).unwrap();
-		// fixer.add_workspace_dep(&found_dep.unwrap(), false);
-		//fixer.save().unwrap();
+		let root_manifest_path = meta.workspace_root.join("Cargo.toml");
+		let mut fixer =
+		 AutoFixer::from_manifest(&root_manifest_path.into_std_path_buf()).unwrap();
+		let dep = by_version.values().next().unwrap().first().unwrap().1.clone();
+		fixer.add_workspace_dep(&dep, false).unwrap();
+		fixer.save().unwrap();
+	}
+}
+
+fn try_find_latest<'a, I: Iterator<Item = &'a VersionReq>>(reqs: I) -> Result<Version, String> {
+	let mut versions = Vec::<Version>::new();
+
+	// Try to convert each to a version. This is done as best-effort:
+	for req in reqs {
+		if req.comparators.len() != 1 {
+			return Err(format!("Invalid version requirement: '{}'", req));
+		}
+		let comp = req.comparators.first().unwrap();
+		if comp.op != Op::Caret {
+			return Err(format!("Only caret is supported, but got: '{}'", req));
+		}
+		if !comp.pre.is_empty() {
+			return Err(format!("Pre-release versions are not supported: '{}'", req));
+		}
+
+		versions.push(Version {
+			major: comp.major,
+			minor: comp.minor.unwrap_or(0),
+			patch: comp.patch.unwrap_or(0),
+			pre: Default::default(),
+			build: Default::default(),
+		});
 	}
+
+	let latest = versions.iter().max().ok_or_else(|| "No versions found".to_string())?;
+	Ok(latest.clone())
 }

src/mock/mod.rs

@@ -29,6 +29,10 @@ pub struct Case {
 	#[serde(default)]
 	pub stdout: String,
 
+	#[serde(skip_serializing_if = "String::is_empty")]
+	#[serde(default)]
+	pub stderr: String,
+
 	#[serde(skip_serializing_if = "Option::is_none")]
 	pub code: Option<i32>,
 
@@ -95,7 +99,7 @@ impl CaseFile {
 	pub fn default_args(&self) -> bool {
 		match self {
 			CaseFile::Ui(ui) => !ui.no_default_args.unwrap_or_default(),
-			CaseFile::Integration(_) => true,
+			CaseFile::Integration(ig) => !ig.no_default_args.unwrap_or_default(),
 		}
 	}
 
@@ -169,6 +173,8 @@ impl UiCaseFile {
 pub struct IntegrationCaseFile {
 	pub repo: Repo,
 	pub cases: Vec<Case>,
+	#[serde(skip_serializing_if = "Option::is_none")]
+	pub no_default_args: Option<bool>,
 }
 
 impl IntegrationCaseFile {

tests/integration/sdk/transpose.yaml

@@ -0,0 +1,85 @@
+repo:
+  name: polkadot-sdk
+  ref: c86b633695299ed27053940d5ea5c5a2392964b3
+cases:
+- cmd: transpose dependency lift-to-workspace parity-scale-codec
+  stderr: |
+    [WARN] Unstable feature - do not rely on this!
+    thread 'main' panicked at '
+    Found 6 different versions of 'parity-scale-codec' in the workspace:
+
+      ^3.6.1: 244 times (frame-support, sp-api, sp-core, …)
+      ^3.0.0:  51 times (pallet-broker, cumulus-client-cli, cumulus-client-collator, …)
+      ^3.1.5:  19 times (bridge-runtime-common, bp-header-chain, bp-runtime, …)
+      ^3.2.2:   5 times (pallet-asset-conversion-tx-payment, pallet-safe-mode, pallet-tx-pause, …)
+      ^3.4.0:   4 times (asset-hub-rococo-integration-tests, integration-tests-common, asset-hub-westend-integration-tests, …)
+      ^3.6.4:   4 times (cumulus-relay-chain-interface, polkadot-node-core-prospective-parachains, cumulus-relay-chain-rpc-interface, …)
+
+    Hint: cargo upgrade -p [email protected]
+    ', src/cmd/transpose.rs:127:13
+    note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
+  code: 101
+- cmd: transpose dependency lift-to-workspace log
+  stderr: |
+    [WARN] Unstable feature - do not rely on this!
+    thread 'main' panicked at '
+    Found 8 different versions of 'log' in the workspace:
+
+      ^0.4.17: 135 times (frame-support, sp-api, sp-core, …)
+      ^0.4.20:  28 times (bridge-runtime-common, pallet-bridge-grandpa, pallet-bridge-messages, …)
+      ^0.4   :   7 times (sc-utils, pallet-contracts, substrate-rpc-client, …)
+      ^0.4.16:   4 times (sc-network-light, pallet-core-fellowship, pallet-ranked-collective, …)
+      ^0.4.14:   4 times (pallet-alliance, pallet-elections-phragmen, pallet-glutton, …)
+      ^0.4.0 :   2 times (pallet-nomination-pools, pallet-nomination-pools-test-staking)
+      ^0.4.19:   2 times (bp-runtime, parachains-common)
+      ^0.4.8 :   1 time  (sc-consensus-grandpa-rpc)
+
+    Hint: cargo upgrade -p [email protected]
+    ', src/cmd/transpose.rs:127:13
+    note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
+  code: 101
+- cmd: zepter transpose dependency lift-to-workspace macro_magic
+  stderr: |
+    [WARN] Unstable feature - do not rely on this!
+  diff: |
+    diff --git Cargo.toml Cargo.toml
+    index c98fe6d1a3..229870c266 100644
+    --- Cargo.toml
+    +++ Cargo.toml
+    @@ -6,0 +7,3 @@ license = "GPL-3.0-only"
+    +[workspace.dependencies]
+    +macro_magic = { version = "^0.5.0", default-features = false }
+    +
+    diff --git substrate/frame/support/Cargo.toml substrate/frame/support/Cargo.toml
+    index 5caf993bb3..8259257314 100644
+    --- substrate/frame/support/Cargo.toml
+    +++ substrate/frame/support/Cargo.toml
+    @@ -33 +33 @@ tt-call = "1.0.8"
+    -macro_magic = "0.5.0"
+    +macro_magic = { workspace = true, default-features = true }
+    diff --git substrate/frame/support/procedural/Cargo.toml substrate/frame/support/procedural/Cargo.toml
+    index 45ed1750a5..0e5400b0ac 100644
+    --- substrate/frame/support/procedural/Cargo.toml
+    +++ substrate/frame/support/procedural/Cargo.toml
+    @@ -26 +26 @@ frame-support-procedural-tools = { path = "tools" }
+    -macro_magic = { version = "0.5.0", features = ["proc_support"] }
+    +macro_magic = { features = ["proc_support"] , workspace = true, default-features = true }
+- cmd: zepter transpose dependency lift-to-workspace tt-call
+  stderr: |
+    [WARN] Unstable feature - do not rely on this!
+  diff: |
+    diff --git Cargo.toml Cargo.toml
+    index c98fe6d1a3..42db4817d6 100644
+    --- Cargo.toml
+    +++ Cargo.toml
+    @@ -6,0 +7,3 @@ license = "GPL-3.0-only"
+    +[workspace.dependencies]
+    +tt-call = { version = "^1.0.8", default-features = false }
+    +
+    diff --git substrate/frame/support/Cargo.toml substrate/frame/support/Cargo.toml
+    index 5caf993bb3..eb2562ff86 100644
+    --- substrate/frame/support/Cargo.toml
+    +++ substrate/frame/support/Cargo.toml
+    @@ -32 +32 @@ sp-metadata-ir = { path = "../../primitives/metadata-ir", default-features = fal
+    -tt-call = "1.0.8"
+    +tt-call = { workspace = true, default-features = true }

tests/integration/sdk/workflows.yaml

@@ -0,0 +1,15 @@
+repo:
+  name: polkadot-sdk
+  ref: c86b633695299ed27053940d5ea5c5a2392964b3
+cases:
+- cmd: run check
+  stderr: |
+    [INFO] Running workflow 'check'
+    [INFO] 1/2 lint propagate-feature
+    [INFO] 2/2 format features
+- cmd: run default
+  stderr: |
+    [INFO] Running workflow 'default'
+    [INFO] 1/2 lint propagate-feature
+    [INFO] 2/2 format features
+no_default_args: true