cargo-rail: a few fixes to the 'unify' command; MSRV feature integrated. Extensive testing

Author: loadingalias

.gitignore

@@ -22,10 +22,7 @@ AGENTS.md
 
 # Documentation
 docs/*
-audit.md
-fixes.md
-fixes_split.md
-fixes_cdtwm.md
+TODO.md
 
 # Cargo-Rail (Testing)
 *rail.toml

src/cargo/manifest_analyzer.rs

@@ -377,10 +377,13 @@ impl ManifestAnalyzer {
       ..Default::default()
     };
 
-    // Check for renamed package
+    // Check for renamed package (only when package name differs from dependency key)
     if let Some(pkg) = table.get("package").and_then(|v| v.as_str()) {
-      parsed.renamed_from = Some(dep_name.to_string());
-      parsed.actual_name = Some(pkg.to_string());
+      // Only flag as renamed when there's actual renaming, not redundant package fields
+      if pkg != dep_name {
+        parsed.renamed_from = Some(dep_name.to_string());
+        parsed.actual_name = Some(pkg.to_string());
+      }
     }
 
     // Parse version - this is the DECLARED version from the manifest

src/cargo/manifest_ops.rs

@@ -90,7 +90,9 @@ pub fn write_toml_file(path: &Path, doc: &DocumentMut) -> RailResult<()> {
 pub fn build_dep_entry(dep: &UnifiedDep) -> Item {
   // Simple case: just version, no features, defaults enabled, no path
   if dep.features.is_empty() && dep.default_features && dep.path.is_none() {
-    return Item::Value(Value::from(dep.version_req.to_string()));
+    let mut value = Value::from(dep.version_req.to_string());
+    value.decor_mut().set_suffix(" #unified");
+    return Item::Value(value);
   }
 
   // Complex case: use inline table
@@ -118,7 +120,11 @@ pub fn build_dep_entry(dep: &UnifiedDep) -> Item {
     table.insert("features", build_feature_array(&dep.features));
   }
 
-  Item::Value(Value::InlineTable(table))
+  // Add #unified comment marker
+  let mut value = Value::InlineTable(table);
+  value.decor_mut().set_suffix(" #unified");
+
+  Item::Value(value)
 }
 
 /// Build a workspace-inherited dependency entry
@@ -132,7 +138,7 @@ pub fn build_dep_entry(dep: &UnifiedDep) -> Item {
 ///
 /// # Returns
 ///
-/// `{ workspace = true }` with optional fields
+/// `{ workspace = true }` with optional fields and `#unified` comment marker
 pub fn build_workspace_dep_entry(local_features: Option<Vec<String>>, is_optional: bool) -> Item {
   let mut table = InlineTable::new();
   table.insert("workspace", Value::from(true));
@@ -149,7 +155,11 @@ pub fn build_workspace_dep_entry(local_features: Option<Vec<String>>, is_optiona
     table.insert("optional", Value::from(true));
   }
 
-  Item::Value(Value::InlineTable(table))
+  // Add #unified comment marker to track what was modified by unify
+  let mut value = Value::InlineTable(table);
+  value.decor_mut().set_suffix(" #unified");
+
+  Item::Value(value)
 }
 
 /// Build a transitive dependency entry for pinning
@@ -167,7 +177,11 @@ pub fn build_transitive_entry(features: &[String]) -> Item {
     table.insert("features", build_feature_array(features));
   }
 
-  Item::Value(Value::InlineTable(table))
+  // Add #unified comment marker
+  let mut value = Value::InlineTable(table);
+  value.decor_mut().set_suffix(" #unified");
+
+  Item::Value(value)
 }
 
 // ============================================================================

src/cargo/manifest_writer.rs

@@ -166,4 +166,32 @@ impl ManifestWriter {
   fn dep_kind_to_section(&self, dep_kind: DepKind) -> &'static str {
     manifest_ops::dep_kind_to_section(dep_kind)
   }
+
+  /// Write MSRV (rust-version) to workspace manifest
+  ///
+  /// Writes to [workspace.package].rust-version so that members can inherit it
+  /// via `rust-version.workspace = true`
+  pub fn write_workspace_msrv(&self, workspace_toml_path: &Path, msrv: &semver::Version) -> RailResult<()> {
+    // Read workspace Cargo.toml
+    let mut doc = manifest_ops::read_toml_file(workspace_toml_path)?;
+
+    // Ensure [workspace] section exists
+    manifest_ops::ensure_section(&mut doc, "workspace").context("Failed to create [workspace] section")?;
+
+    // Get or create [workspace.package] section
+    let ws_package = manifest_ops::get_or_create_table(&mut doc, "workspace.package")
+      .context("Failed to create [workspace.package]")?;
+
+    // Format MSRV as "major.minor" (standard rust-version format)
+    let msrv_str = format!("{}.{}", msrv.major, msrv.minor);
+
+    // Insert or update rust-version
+    ws_package.insert("rust-version", toml_edit::value(&msrv_str));
+
+    // Format and write
+    self.formatter.format_manifest(&mut doc)?;
+    manifest_ops::write_toml_file(workspace_toml_path, &doc)?;
+
+    Ok(())
+  }
 }

src/cargo/mod.rs

@@ -17,7 +17,7 @@ pub mod unify_analyzer;
 pub use cargo_transform::{CargoTransform, TransformContext};
 pub use manifest_analyzer::{DepKey, DepKind, DepUsage, ManifestAnalyzer};
 pub use manifest_writer::ManifestWriter;
-pub use multi_target_metadata::{FragmentedTransitive, MultiTargetMetadata};
+pub use multi_target_metadata::{ComputedMsrv, FragmentedTransitive, MultiTargetMetadata};
 pub use unify_analyzer::{
   IssueSeverity, MemberEdit, UnificationPlan, UnifiedDep, UnifyAnalyzer, UnifyIssue, UnifyReport, ValidationResult,
 };

src/cargo/multi_target_metadata.rs

@@ -285,6 +285,57 @@ impl MultiTargetMetadata {
 
     transitives
   }
+
+  /// Compute the workspace MSRV from all resolved dependencies
+  ///
+  /// The MSRV is the maximum `rust-version` across all dependencies in the
+  /// resolved graph. This ensures the workspace can build all its deps.
+  ///
+  /// Returns `None` if no dependencies specify rust-version.
+  pub fn compute_msrv(&self) -> Option<ComputedMsrv> {
+    let mut max_version: Option<Version> = None;
+    let mut contributors: Vec<String> = Vec::new();
+    let mut deps_with_msrv = 0;
+    let mut seen_packages: HashSet<String> = HashSet::new();
+
+    // Iterate through all packages in the resolved graph
+    for metadata in self.cache.values() {
+      for pkg in &metadata.packages {
+        // Skip if we've already processed this package (may appear in multiple targets)
+        let pkg_key = format!("{}@{}", pkg.name, pkg.version);
+        if seen_packages.contains(&pkg_key) {
+          continue;
+        }
+        seen_packages.insert(pkg_key);
+
+        // Check if this package has rust-version specified
+        if let Some(ref rust_version) = pkg.rust_version {
+          deps_with_msrv += 1;
+
+          match &max_version {
+            None => {
+              max_version = Some(rust_version.clone());
+              contributors = vec![pkg.name.to_string()];
+            }
+            Some(current_max) => {
+              if rust_version > current_max {
+                max_version = Some(rust_version.clone());
+                contributors = vec![pkg.name.to_string()];
+              } else if rust_version == current_max {
+                contributors.push(pkg.name.to_string());
+              }
+            }
+          }
+        }
+      }
+    }
+
+    max_version.map(|version| ComputedMsrv {
+      version,
+      contributors,
+      deps_with_msrv,
+    })
+  }
 }
 
 /// A transitive dependency with fragmented features across targets
@@ -304,3 +355,14 @@ impl FragmentedTransitive {
     self.feature_sets.len()
   }
 }
+
+/// Result of MSRV computation from dependency graph
+#[derive(Debug, Clone)]
+pub struct ComputedMsrv {
+  /// The computed MSRV (maximum of all deps' rust-version)
+  pub version: Version,
+  /// Dependencies that contributed to the MSRV (those with the highest rust-version)
+  pub contributors: Vec<String>,
+  /// Total number of deps with rust-version specified
+  pub deps_with_msrv: usize,
+}

src/cargo/unify_analyzer.rs

@@ -5,7 +5,7 @@
 
 use crate::cargo::{
   manifest_analyzer::{DepKind, ExistingWorkspaceDep, ManifestAnalyzer, parse_existing_workspace_deps},
-  multi_target_metadata::MultiTargetMetadata,
+  multi_target_metadata::{ComputedMsrv, MultiTargetMetadata},
 };
 use crate::config::UnifyConfig;
 use crate::error::RailResult;
@@ -101,6 +101,8 @@ pub struct UnificationPlan {
   pub validation_results: Vec<ValidationResult>,
   /// Issues detected during analysis
   pub issues: Vec<UnifyIssue>,
+  /// Computed MSRV from dependency graph (if msrv = true in config)
+  pub computed_msrv: Option<ComputedMsrv>,
 }
 
 impl UnificationPlan {
@@ -189,6 +191,26 @@ impl UnificationPlan {
       }
     }
 
+    // Show computed MSRV if available
+    if let Some(ref msrv) = self.computed_msrv {
+      s.push_str(&format!(
+        "\nComputed MSRV: {} (from {} deps with rust-version)\n",
+        msrv.version, msrv.deps_with_msrv
+      ));
+      if !msrv.contributors.is_empty() {
+        let contributors_str = if msrv.contributors.len() > 3 {
+          format!(
+            "{}, ... ({} total)",
+            msrv.contributors[..3].join(", "),
+            msrv.contributors.len()
+          )
+        } else {
+          msrv.contributors.join(", ")
+        };
+        s.push_str(&format!("  Contributors: {}\n", contributors_str));
+      }
+    }
+
     s
   }
 }
@@ -486,6 +508,14 @@ impl UnifyAnalyzer {
       Vec::new()
     };
 
+    // Compute MSRV if enabled
+    let computed_msrv = if self.config.msrv {
+      println!("Computing MSRV from dependency graph...");
+      self.metadata.compute_msrv()
+    } else {
+      None
+    };
+
     // Run validation
     let validation_results = self.validate_targets()?;
 
@@ -496,6 +526,7 @@ impl UnifyAnalyzer {
       transitive_pins,
       validation_results,
       issues,
+      computed_msrv,
     })
   }
 

src/commands/unify.rs

@@ -121,10 +121,17 @@ pub fn run_unify_apply(
     return Ok(());
   }
 
-  // Create backup if requested
-  if backup {
-    println!("📦 Creating backup...");
-    let backup_manager = BackupManager::new(ctx.workspace_root());
+  // Create backup if requested OR if this is the first unify run (safety feature)
+  let backup_manager = BackupManager::new(ctx.workspace_root());
+  let is_first_run = !backup_manager.has_backups();
+  let should_backup = backup || is_first_run;
+
+  if should_backup {
+    if is_first_run && !backup {
+      println!("📦 Creating safety backup (first unify run in this workspace)...");
+    } else {
+      println!("📦 Creating backup...");
+    }
 
     // Collect all files that will be modified
     let mut files_to_backup = Vec::new();
@@ -207,9 +214,22 @@ pub fn run_unify_apply(
     writer.add_transitive_pins(&host_path, &plan.transitive_pins)?;
   }
 
+  // Write computed MSRV to workspace manifest if enabled
+  if let Some(ref msrv) = plan.computed_msrv {
+    println!(
+      "📋 Writing rust-version = \"{}.{}\" to [workspace.package]...",
+      msrv.version.major, msrv.version.minor
+    );
+    writer.write_workspace_msrv(&ctx.workspace_root().join("Cargo.toml"), &msrv.version)?;
+  }
+
   // Generate report
   println!("📄 Generating report...");
-  let report_path = ctx.workspace_root().join(".cargo-rail").join("unify-report.md");
+  let report_path = ctx
+    .workspace_root()
+    .join("target")
+    .join("cargo-rail")
+    .join("unify-report.md");
   UnifyReport::write_to_file(&plan, &report_path)?;
   println!("   Report saved to: {}", report_path.display());
 
@@ -220,6 +240,14 @@ pub fn run_unify_apply(
   if !plan.transitive_pins.is_empty() {
     println!("   - {} transitives pinned", plan.transitive_pins.len());
   }
+  if let Some(ref msrv) = plan.computed_msrv {
+    println!(
+      "   - MSRV set to {}.{} (from {})",
+      msrv.version.major,
+      msrv.version.minor,
+      msrv.contributors.first().unwrap_or(&"unknown".to_string())
+    );
+  }
 
   println!("\nNext steps:");
   println!("  1. Run 'cargo check' to verify everything compiles");

src/config.rs

@@ -71,6 +71,12 @@ pub struct UnifyConfig {
   /// Older backups are automatically cleaned up after successful unify operations
   #[serde(default = "default_max_backups")]
   pub max_backups: usize,
+
+  /// Compute and write MSRV to workspace manifest? (default: false)
+  /// When enabled, cargo-rail computes the maximum rust-version from all
+  /// resolved dependencies and writes it to [workspace.package].rust-version
+  #[serde(default)]
+  pub msrv: bool,
 }
 
 fn default_max_backups() -> usize {
@@ -173,6 +179,7 @@ impl Default for UnifyConfig {
       exclude: Vec::new(),
       include: Vec::new(),
       max_backups: default_max_backups(),
+      msrv: false,
     }
   }
 }

src/graph/core.rs

@@ -195,6 +195,8 @@ impl WorkspaceGraph {
     }
 
     // Add edges between workspace members only
+    // IMPORTANT: Skip dev-dependencies as they don't affect publish order
+    // (dev-deps can have cycles, including self-references for feature-gated test utils)
     for (from_name, &from_subgraph_idx) in &name_to_subgraph_idx {
       let from_graph_idx = self.name_to_node[from_name];
 
@@ -208,7 +210,11 @@ impl WorkspaceGraph {
           && let Some(edge) = self.graph.find_edge(from_graph_idx, neighbor_graph_idx)
         {
           let kind = *self.graph.edge_weight(edge).unwrap();
-          subgraph.add_edge(from_subgraph_idx, to_subgraph_idx, kind);
+          // Skip dev-dependencies - they don't affect publish order and can have cycles
+          // Also skip self-references (crate depending on itself for test features)
+          if kind != DependencyKind::Development && from_name != &neighbor_node.name {
+            subgraph.add_edge(from_subgraph_idx, to_subgraph_idx, kind);
+          }
         }
       }
     }