cargo-rail: fixed CI issue; added testing and gitnotes robustness. cleaned some todo items up

Author: loadingalias

src/cargo/metadata.rs

@@ -119,6 +119,9 @@ impl WorkspaceMetadata {
   /// - Feature dependency analysis: Understand feature activation chains
   /// - Feature optimization: Identify minimal feature sets for build times
   ///
+  /// TODO: Use this for `cargo rail audit` or `cargo rail graph` to inspect specific packages.
+  /// For global analysis (like unification), use `cargo_metadata` directly to avoid O(N) lookups.
+  ///
   /// # Example
   /// ```ignore
   /// let features = metadata.package_features("serde")?;
@@ -138,6 +141,8 @@ impl WorkspaceMetadata {
   /// - `cargo rail build --lib`: Build only library targets
   /// - Target coverage: Ensure all crates have appropriate targets
   ///
+  /// TODO: Use this for `cargo rail test` filtering or `cargo rail check` to validate targets.
+  ///
   /// # Example
   /// ```ignore
   /// let targets = metadata.package_targets("cargo-rail");
@@ -159,6 +164,8 @@ impl WorkspaceMetadata {
   /// - Migration planning: Identify crates stuck on old editions
   /// - Compatibility matrix: Track edition usage across workspace
   ///
+  /// TODO: Integrate into `unify` pre-checks or `cargo rail lint` to ensure consistent editions.
+  ///
   /// # Example
   /// ```ignore
   /// if metadata.package_edition("lib-core")? == "2021" {
@@ -178,6 +185,8 @@ impl WorkspaceMetadata {
   /// - Workspace MSRV policy: Enforce minimum Rust version across crates
   /// - CI matrix: Generate test matrix based on supported Rust versions
   ///
+  /// TODO: Integrate into `unify` pre-checks or `cargo rail lint` to enforce MSRV policy.
+  ///
   /// # Example
   /// ```ignore
   /// let msrv = metadata.package_rust_version("lib-core")?;
@@ -201,6 +210,9 @@ impl WorkspaceMetadata {
   /// - License compliance: Check all dependency licenses
   /// - Duplicate detection: Find dependencies listed multiple times
   ///
+  /// TODO: Use this for `cargo rail graph` or `cargo rail audit` to inspect dependencies of a specific crate.
+  /// For global analysis, iterate `metadata.packages` directly.
+  ///
   /// # Example
   /// ```ignore
   /// let deps = metadata.package_dependencies("cargo-rail");

src/commands/config_sync.rs

@@ -113,6 +113,12 @@ pub struct SyncRegistry {
   syncers: Vec<Box<dyn ConfigSyncer>>,
 }
 
+impl Default for SyncRegistry {
+  fn default() -> Self {
+    Self::new()
+  }
+}
+
 impl SyncRegistry {
   pub fn new() -> Self {
     Self { syncers: Vec::new() }

src/commands/sync.rs

@@ -1,4 +1,5 @@
 use std::io::IsTerminal;
+use std::str::FromStr;
 use std::sync::Arc;
 
 use crate::commands::common::SplitSyncConfigBuilder;

src/config.rs

@@ -389,13 +389,22 @@ impl RailConfig {
     // We do this by checking if a de-canonicalized version exists.
     #[cfg(target_os = "windows")]
     {
-      // Try to find the config by reconstructing paths from the parent
-      // This handles cases where:
-      // 1. cargo metadata returns \\?\C:\Users\RUNNER~1\AppData\Local\Temp\...
-      // 2. But test wrote file to C:\Users\RUNNER~1\AppData\Local\Temp\...
-      // Or the short name (8.3) vs long name issue
+      // 1. Try canonicalizing the path and searching there
+      // This handles 8.3 short paths vs long paths issues (RUNNER~1 vs runneradmin)
+      if let Ok(canonical) = path.canonicalize() {
+        let canonical_candidates = [
+          canonical.join("rail.toml"),
+          canonical.join(".rail.toml"),
+          canonical.join(".cargo").join("rail.toml"),
+          canonical.join(".config").join("rail.toml"),
+        ];
+        if let Some(found) = canonical_candidates.iter().find(|p| p.exists()) {
+          return Some(found.to_path_buf());
+        }
+      }
 
-      // Check if we can read the directory to find config files
+      // 2. Try to find the config by reading the directory entries
+      // This handles cases where exact path string matching fails but the file is in the directory
       if let Ok(entries) = std::fs::read_dir(path) {
         for entry in entries.flatten() {
           let file_name = entry.file_name();
@@ -408,7 +417,7 @@ impl RailConfig {
         }
       }
 
-      // Also check subdirectories .cargo and .config
+      // Also check subdirectories .cargo and .config via read_dir
       for subdir in &[".cargo", ".config"] {
         let subdir_path = path.join(subdir);
         if let Ok(entries) = std::fs::read_dir(&subdir_path) {

src/git/mappings.rs

@@ -1,6 +1,8 @@
 use crate::error::{GitError, RailError, RailResult, ResultExt};
 use std::collections::HashMap;
 use std::path::Path;
+use std::thread;
+use std::time::Duration;
 
 /// Commit mapping store using git-notes
 /// Maps commits between monorepo and split repos
@@ -163,19 +165,22 @@ impl MappingStore {
 
     println!("   Pushing git-notes to remote '{}'...", remote);
 
-    let output = Command::new("git")
-      .current_dir(repo_path)
-      .args(["push", remote, &notes_ref])
-      .output()
-      .context("Failed to push git-notes")?;
-
-    if !output.status.success() {
-      let stderr = String::from_utf8_lossy(&output.stderr);
-      return Err(RailError::Git(GitError::CommandFailed {
-        command: "git push notes".to_string(),
-        stderr: stderr.to_string(),
-      }));
-    }
+    retry_operation(|| {
+      let output = Command::new("git")
+        .current_dir(repo_path)
+        .args(["push", remote, &notes_ref])
+        .output()
+        .context("Failed to push git-notes")?;
+
+      if !output.status.success() {
+        let stderr = String::from_utf8_lossy(&output.stderr);
+        return Err(RailError::Git(GitError::CommandFailed {
+          command: "git push notes".to_string(),
+          stderr: stderr.to_string(),
+        }));
+      }
+      Ok(())
+    })?;
 
     println!("   ✅ Pushed git-notes");
     Ok(())
@@ -190,91 +195,154 @@ impl MappingStore {
 
     println!("   Fetching git-notes from remote '{}'...", remote);
 
-    let output = Command::new("git")
-      .current_dir(repo_path)
-      .args(["fetch", remote, &refspec])
-      .output()
-      .context("Failed to fetch git-notes")?;
+    // Retry the fetch operation
+    let result = retry_operation(|| {
+      let output = Command::new("git")
+        .current_dir(repo_path)
+        .args(["fetch", remote, &refspec])
+        .output()
+        .context("Failed to fetch git-notes")?;
+
+      if !output.status.success() {
+        let stderr = String::from_utf8_lossy(&output.stderr);
 
-    if !output.status.success() {
-      let stderr = String::from_utf8_lossy(&output.stderr);
+        // Ignore "couldn't find remote ref" - notes may not exist yet
+        if stderr.contains("couldn't find remote ref") {
+          return Ok(()); // Treat as success (no notes yet)
+        }
 
-      // Ignore "couldn't find remote ref" - notes may not exist yet
-      if stderr.contains("couldn't find remote ref") {
-        println!("   ℹ️  No remote git-notes found yet (this is normal for first sync)");
-        return Ok(());
+        return Err(RailError::Git(GitError::CommandFailed {
+          command: "git fetch notes".to_string(),
+          stderr: stderr.to_string(),
+        }));
       }
+      Ok(())
+    });
 
-      // Handle non-fast-forward (notes conflict)
-      if stderr.contains("non-fast-forward") || stderr.contains("rejected") {
-        println!("   ⚠️  Git-notes conflict detected (local and remote notes diverged)");
-        println!("   🔄 Attempting automatic merge with union strategy...");
-
-        // Fetch to FETCH_HEAD without updating the ref
-        let fetch_output = Command::new("git")
-          .current_dir(repo_path)
-          .args(["fetch", remote, &notes_ref])
-          .output()
-          .context("Failed to fetch notes to FETCH_HEAD")?;
-
-        if !fetch_output.status.success() {
-          let fetch_stderr = String::from_utf8_lossy(&fetch_output.stderr);
-          if !fetch_stderr.contains("couldn't find remote ref") {
-            return Err(RailError::Git(GitError::CommandFailed {
-              command: "git fetch notes to FETCH_HEAD".to_string(),
-              stderr: fetch_stderr.to_string(),
-            }));
+    match result {
+      Ok(_) => {
+        println!("   ✅ Fetched git-notes");
+        Ok(())
+      }
+      Err(e) => {
+        // Check if it's a conflict (non-fast-forward)
+        let is_conflict = if let RailError::Git(GitError::CommandFailed { stderr, .. }) = &e {
+          stderr.contains("non-fast-forward") || stderr.contains("rejected")
+        } else {
+          false
+        };
+
+        if is_conflict {
+          println!("   ⚠️  Git-notes conflict detected (local and remote notes diverged)");
+          println!("   🔄 Attempting automatic merge with union strategy...");
+
+          // Fetch to FETCH_HEAD without updating the ref
+          let fetch_output = Command::new("git")
+            .current_dir(repo_path)
+            .args(["fetch", remote, &notes_ref])
+            .output()
+            .context("Failed to fetch notes to FETCH_HEAD")?;
+
+          if !fetch_output.status.success() {
+            let fetch_stderr = String::from_utf8_lossy(&fetch_output.stderr);
+            if !fetch_stderr.contains("couldn't find remote ref") {
+              return Err(RailError::Git(GitError::CommandFailed {
+                command: "git fetch notes to FETCH_HEAD".to_string(),
+                stderr: fetch_stderr.to_string(),
+              }));
+            }
+            return Ok(()); // No remote notes
           }
-          return Ok(()); // No remote notes
+
+          // Merge notes using union strategy (combines both without conflict)
+          let merge_output = Command::new("git")
+            .current_dir(repo_path)
+            .args(["notes", "--ref", &notes_ref, "merge", "--strategy=union", "FETCH_HEAD"])
+            .output()
+            .context("Failed to merge git-notes")?;
+
+          if !merge_output.status.success() {
+            let merge_stderr = String::from_utf8_lossy(&merge_output.stderr);
+
+            // If union merge fails, provide clear guidance
+            eprintln!("   ❌ Automatic git-notes merge failed");
+            eprintln!("   📋 Manual resolution required:");
+            eprintln!("      1. cd {}", repo_path.display());
+            eprintln!("      2. git notes --ref={} merge FETCH_HEAD", notes_ref);
+            eprintln!("      3. Resolve conflicts manually");
+            eprintln!("      4. git notes --ref={} merge --commit", notes_ref);
+            eprintln!();
+            return Err(RailError::with_help(
+              format!("git notes merge failed: {}", merge_stderr),
+              format!(
+                "This usually happens when the same commit has different mappings on different machines.\n\
+                 Manual resolution steps:\n  \
+                 1. cd {}\n  \
+                 2. git notes --ref={} merge FETCH_HEAD\n  \
+                 3. Resolve conflicts manually\n  \
+                 4. git notes --ref={} merge --commit",
+                repo_path.display(),
+                notes_ref,
+                notes_ref
+              ),
+            ));
+          }
+
+          println!("   ✅ Git-notes merged successfully (union strategy)");
+          Ok(())
+        } else {
+          // Propagate other errors
+          Err(e)
+        }
+      }
+    }
+  }
+}
+
+/// Retry an operation with exponential backoff
+fn retry_operation<F, T>(mut operation: F) -> RailResult<T>
+where
+  F: FnMut() -> RailResult<T>,
+{
+  let max_retries = 3;
+  let mut attempt = 0;
+  let mut delay = Duration::from_millis(500);
+
+  loop {
+    match operation() {
+      Ok(result) => return Ok(result),
+      Err(e) => {
+        attempt += 1;
+        if attempt > max_retries {
+          return Err(e);
         }
 
-        // Merge notes using union strategy (combines both without conflict)
-        let merge_output = Command::new("git")
-          .current_dir(repo_path)
-          .args(["notes", "--ref", &notes_ref, "merge", "--strategy=union", "FETCH_HEAD"])
-          .output()
-          .context("Failed to merge git-notes")?;
-
-        if !merge_output.status.success() {
-          let merge_stderr = String::from_utf8_lossy(&merge_output.stderr);
-
-          // If union merge fails, provide clear guidance
-          eprintln!("   ❌ Automatic git-notes merge failed");
-          eprintln!("   📋 Manual resolution required:");
-          eprintln!("      1. cd {}", repo_path.display());
-          eprintln!("      2. git notes --ref={} merge FETCH_HEAD", notes_ref);
-          eprintln!("      3. Resolve conflicts manually");
-          eprintln!("      4. git notes --ref={} merge --commit", notes_ref);
-          eprintln!();
-          return Err(RailError::with_help(
-            format!("git notes merge failed: {}", merge_stderr),
-            format!(
-              "This usually happens when the same commit has different mappings on different machines.\n\
-               Manual resolution steps:\n  \
-               1. cd {}\n  \
-               2. git notes --ref={} merge FETCH_HEAD\n  \
-               3. Resolve conflicts manually\n  \
-               4. git notes --ref={} merge --commit",
-              repo_path.display(),
-              notes_ref,
-              notes_ref
-            ),
-          ));
+        // Only retry on network/lock errors, not logical errors
+        // For now, we assume most git command failures in this context are transient or lock-related
+        // unless it's a conflict which we handle separately in fetch_notes
+        let should_retry = match &e {
+          RailError::Git(GitError::CommandFailed { stderr, .. }) => {
+            stderr.contains("lock")
+              || stderr.contains("temporarily unavailable")
+              || stderr.contains("Connection timed out")
+              || stderr.contains("Could not resolve host")
+              || stderr.contains("Failed to connect")
+          }
+          _ => false,
+        };
+
+        if !should_retry {
+          return Err(e);
         }
 
-        println!("   ✅ Git-notes merged successfully (union strategy)");
-        return Ok(());
+        println!(
+          "   ⚠️  Operation failed. Retrying in {:?}... (Attempt {}/{})",
+          delay, attempt, max_retries
+        );
+        thread::sleep(delay);
+        delay *= 2;
       }
-
-      // Unknown error
-      return Err(RailError::Git(GitError::CommandFailed {
-        command: "git fetch notes".to_string(),
-        stderr: stderr.to_string(),
-      }));
     }
-
-    println!("   ✅ Fetched git-notes");
-    Ok(())
   }
 }