Skip to content

Commit 3234319

Browse files
committed
cargo-rail: cleaning up dead_code blocks; fixing CI for Windows ARM64 runner (wrong label) and Windows escaping in TOML parsing.
1 parent a40aff9 commit 3234319

File tree

15 files changed

+593
-263
lines changed

15 files changed

+593
-263
lines changed

.github/workflows/commit.yaml

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -47,16 +47,16 @@ jobs:
4747
runner: windows-latest
4848
cache-key: commit-windows-x64
4949

50-
# Windows ARM64 (GitHub Teams/Enterprise)
50+
# Windows ARM64 (GitHub Free in public repos)
5151
- name: aarch64-pc-windows-msvc
52-
runner: windows-11-arm64
52+
runner: windows-11-arm
5353
cache-key: commit-windows-arm64
5454

5555
# macOS tested locally (not in CI)
5656

5757
steps:
5858
- name: Checkout
59-
uses: actions/checkout@08c6903cd8c0fde910a37f88322edcfb5dd907a8 # v5.0.0
59+
uses: actions/checkout@08c6903cd8c0fde910a37f88322edcfb5dd907a8 # v5.0.0
6060

6161
- name: Setup
6262
uses: ./.github/actions/setup
@@ -74,7 +74,7 @@ jobs:
7474

7575
- name: Upload Test Results (on failure)
7676
if: failure()
77-
uses: actions/upload-artifact@330a01c490aca151604b8cf639adc76d48f6c5d4 # v5.0.0
77+
uses: actions/upload-artifact@330a01c490aca151604b8cf639adc76d48f6c5d4 # v5.0.0
7878
with:
7979
name: test-results-${{ matrix.target.name }}
8080
path: target/nextest/

src/cargo/files.rs

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,3 @@
1-
#![allow(dead_code)]
2-
31
use crate::core::error::{RailResult, ResultExt};
42
use crate::ui::progress::FileProgress;
53
use std::fs;

src/cargo/metadata.rs

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,3 @@
1-
#![allow(dead_code)]
2-
31
use crate::core::error::RailResult;
42
use cargo_metadata::{MetadataCommand, Package};
53
use std::path::Path;
@@ -22,6 +20,8 @@ impl WorkspaceMetadata {
2220
self.metadata.workspace_packages()
2321
}
2422

23+
/// Get package by name - public API for external use
24+
#[allow(dead_code)]
2525
pub fn get_package(&self, name: &str) -> Option<&Package> {
2626
self
2727
.metadata
@@ -34,7 +34,8 @@ impl WorkspaceMetadata {
3434
self.metadata.workspace_root.as_std_path()
3535
}
3636

37-
/// Get raw JSON string for external tools
37+
/// Get raw JSON string for external tools - public API for debugging/logging
38+
#[allow(dead_code)]
3839
pub fn to_json_string(&self) -> RailResult<String> {
3940
serde_json::to_string(&self.metadata)
4041
.map_err(|e| crate::core::error::RailError::message(format!("Failed to serialize metadata: {}", e)))

src/cargo/transform.rs

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,16 @@
1-
#![allow(dead_code)]
2-
31
use crate::cargo::metadata::WorkspaceMetadata;
42
use crate::core::error::{RailError, RailResult, ResultExt};
53
use std::collections::HashMap;
64
use std::path::PathBuf;
75
use toml_edit::{DocumentMut, Item, Value};
86

9-
/// Context for transforming Cargo.toml
7+
/// Context for transformation operations
108
pub struct TransformContext {
9+
/// Crate name being transformed - public API field for future use
10+
#[allow(dead_code)]
1111
pub crate_name: String,
12+
/// Workspace root path - public API field for future use
13+
#[allow(dead_code)]
1214
pub workspace_root: PathBuf,
1315
}
1416

src/commands/release.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,8 @@ pub fn run_release_plan(name: Option<String>, all: bool, json: bool) -> RailResu
6363
pub fn run_release_apply(
6464
name: String,
6565
dry_run: bool,
66-
_skip_sync: bool, // TODO: Implement split sync
66+
// Skip syncing to split repos - deferred to post-v1 (see TODO.md Post-v1: Auto-sync to split repos)
67+
#[allow(unused_variables)] skip_sync: bool,
6768
) -> RailResult<()> {
6869
let workspace_root = env::current_dir()?;
6970

src/commands/split.rs

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,13 @@ fn prompt_for_confirmation(message: &str) -> RailResult<bool> {
2222
}
2323

2424
/// Run the split command
25-
pub fn run_split(crate_name: Option<String>, all: bool, apply: bool, json: bool) -> RailResult<()> {
25+
pub fn run_split(
26+
crate_name: Option<String>,
27+
all: bool,
28+
remote: Option<String>,
29+
apply: bool,
30+
json: bool,
31+
) -> RailResult<()> {
2632
let current_dir = env::current_dir()?;
2733

2834
// Load configuration
@@ -35,8 +41,8 @@ pub fn run_split(crate_name: Option<String>, all: bool, apply: bool, json: bool)
3541
let config = RailConfig::load(&current_dir)?;
3642
println!("📦 Loaded configuration from .rail/config.toml");
3743

38-
// Determine which crates to split (need this to check if they're all local)
39-
let crates_to_split_check: Vec<_> = if all {
44+
// Determine which crates to split
45+
let mut crates_to_split_check: Vec<_> = if all {
4046
config.splits.clone()
4147
} else if let Some(ref name) = crate_name {
4248
let split_config = config
@@ -52,6 +58,13 @@ pub fn run_split(crate_name: Option<String>, all: bool, apply: bool, json: bool)
5258
));
5359
};
5460

61+
// Apply remote override if provided (before all_local check)
62+
if let Some(ref remote_override) = remote {
63+
for split_config in &mut crates_to_split_check {
64+
split_config.remote = remote_override.clone();
65+
}
66+
}
67+
5568
// Check if all remotes are local paths (skip SSH checks for local testing)
5669
let all_local = crates_to_split_check
5770
.iter()

src/commands/sync.rs

Lines changed: 60 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,16 +9,60 @@ use crate::core::sync::{SyncConfig, SyncDirection, SyncEngine, SyncResult};
99
use crate::ui::progress::{FileProgress, MultiProgress};
1010
use rayon::prelude::*;
1111

12+
/// Sync command parameters
13+
pub struct SyncParams {
14+
pub crate_name: Option<String>,
15+
pub all: bool,
16+
pub remote: Option<String>,
17+
pub from_remote: bool,
18+
pub to_remote: bool,
19+
pub strategy_str: String,
20+
pub no_protected_branches: bool,
21+
pub apply: bool,
22+
pub json: bool,
23+
}
24+
1225
/// Run the sync command
26+
#[allow(clippy::too_many_arguments)]
1327
pub fn run_sync(
1428
crate_name: Option<String>,
1529
all: bool,
30+
remote: Option<String>,
1631
from_remote: bool,
1732
to_remote: bool,
1833
strategy_str: String,
34+
no_protected_branches: bool,
1935
apply: bool,
2036
json: bool,
2137
) -> RailResult<()> {
38+
// Convert to struct for internal use
39+
let params = SyncParams {
40+
crate_name,
41+
all,
42+
remote,
43+
from_remote,
44+
to_remote,
45+
strategy_str,
46+
no_protected_branches,
47+
apply,
48+
json,
49+
};
50+
run_sync_impl(params)
51+
}
52+
53+
/// Internal implementation of sync command
54+
fn run_sync_impl(params: SyncParams) -> RailResult<()> {
55+
let SyncParams {
56+
crate_name,
57+
all,
58+
remote,
59+
from_remote,
60+
to_remote,
61+
strategy_str,
62+
no_protected_branches,
63+
apply,
64+
json,
65+
} = params;
2266
// Parse conflict strategy
2367
let strategy = ConflictStrategy::from_str(&strategy_str)?;
2468
let current_dir = env::current_dir()?;
@@ -30,11 +74,17 @@ pub fn run_sync(
3074
}));
3175
}
3276

33-
let config = RailConfig::load(&current_dir)?;
77+
let mut config = RailConfig::load(&current_dir)?;
78+
79+
// Apply CLI overrides to security config if provided
80+
if no_protected_branches {
81+
config.security.protected_branches.clear();
82+
}
83+
3484
println!("📦 Loaded configuration from .rail/config.toml");
3585

36-
// Determine which crates to sync (need this to check if they're all local)
37-
let crates_to_sync_check: Vec<_> = if all {
86+
// Determine which crates to sync
87+
let mut crates_to_sync_check: Vec<_> = if all {
3888
config.splits.clone()
3989
} else if let Some(ref name) = crate_name {
4090
let split_config = config
@@ -50,6 +100,13 @@ pub fn run_sync(
50100
));
51101
};
52102

103+
// Apply remote override if provided (before all_local check)
104+
if let Some(ref remote_override) = remote {
105+
for split_config in &mut crates_to_sync_check {
106+
split_config.remote = remote_override.clone();
107+
}
108+
}
109+
53110
// Check if all remotes are local paths (skip SSH checks for local testing)
54111
let all_local = crates_to_sync_check
55112
.iter()

src/core/config.rs

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,3 @@
1-
#![allow(dead_code)]
2-
31
use crate::core::error::{ConfigError, RailError, RailResult, ResultExt};
42
use serde::{Deserialize, Serialize};
53
use std::fs;
@@ -147,7 +145,8 @@ impl PolicyConfig {
147145
Ok(())
148146
}
149147

150-
/// Check if policy is enabled (any field is set)
148+
/// Check if policy is enabled (any field is set) - public API for conditional logic
149+
#[allow(dead_code)]
151150
pub fn is_enabled(&self) -> bool {
152151
self.resolver.is_some()
153152
|| self.msrv.is_some()

src/core/split.rs

Lines changed: 38 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,6 @@ struct RecreateCommitParams<'a> {
2626
commit: &'a CommitInfo,
2727
crate_paths: &'a [PathBuf],
2828
target_repo_path: &'a Path,
29-
transform: &'a CargoTransform,
3029
workspace_root: &'a Path,
3130
crate_name: &'a str,
3231
mode: &'a SplitMode,
@@ -76,22 +75,8 @@ impl Splitter {
7675
fn walk_filtered_history(&self, paths: &[PathBuf]) -> RailResult<Vec<CommitInfo>> {
7776
println!(" Walking commit history to find commits touching crate...");
7877

79-
// Collect commits that touch any of the paths
80-
// Use HashSet + Vec to deduplicate while preserving insertion order from git log
81-
let mut seen_shas = std::collections::HashSet::new();
82-
let mut filtered_commits = Vec::new();
83-
84-
for path in paths {
85-
// Get all commits that touch this path (already in chronological order from git log --reverse)
86-
let path_commits = self.git.get_commits_touching_path(path, None, "HEAD")?;
87-
88-
// Add to our deduplication list (skip if already seen)
89-
for commit in path_commits {
90-
if seen_shas.insert(commit.sha.clone()) {
91-
filtered_commits.push(commit);
92-
}
93-
}
94-
}
78+
// Use batched git command for all paths at once (much faster than N separate calls)
79+
let filtered_commits = self.git.get_commits_touching_paths(paths, None, "HEAD")?;
9580

9681
println!(
9782
" Found {} total commits that touch the crate paths",
@@ -101,6 +86,23 @@ impl Splitter {
10186
Ok(filtered_commits)
10287
}
10388

89+
/// Apply Cargo.toml transformation to a manifest file
90+
/// Returns Ok(()) if transform succeeded or file doesn't exist
91+
fn apply_manifest_transform(&self, manifest_path: &Path, crate_name: &str) -> RailResult<()> {
92+
if !manifest_path.exists() {
93+
return Ok(());
94+
}
95+
96+
let content = std::fs::read_to_string(manifest_path)?;
97+
let context = TransformContext {
98+
crate_name: crate_name.to_string(),
99+
workspace_root: self.workspace_root.clone(),
100+
};
101+
let transformed = self.transform.transform_to_split(&content, &context)?;
102+
std::fs::write(manifest_path, transformed)?;
103+
Ok(())
104+
}
105+
104106
/// Recreate a commit in the target repository with transforms applied
105107
/// Returns the new commit SHA
106108
fn recreate_commit_in_target(&self, params: &RecreateCommitParams) -> RailResult<String> {
@@ -143,19 +145,12 @@ impl Splitter {
143145
std::fs::create_dir_all(parent)?;
144146
}
145147

148+
// Write file content
149+
std::fs::write(&target_path, content_bytes)?;
150+
146151
// Apply Cargo.toml transformation if applicable
147152
if file_path.file_name() == Some(std::ffi::OsStr::new("Cargo.toml")) {
148-
// Transform the manifest
149-
let content = String::from_utf8(content_bytes.clone())
150-
.map_err(|_| RailError::message(format!("File '{}' is not valid UTF-8", file_path.display())))?;
151-
let context = TransformContext {
152-
crate_name: params.crate_name.to_string(),
153-
workspace_root: params.workspace_root.to_path_buf(),
154-
};
155-
let transformed = params.transform.transform_to_split(&content, &context)?;
156-
std::fs::write(&target_path, transformed)?;
157-
} else {
158-
std::fs::write(&target_path, content_bytes)?;
153+
self.apply_manifest_transform(&target_path, params.crate_name)?;
159154
}
160155
}
161156

@@ -391,7 +386,6 @@ impl Splitter {
391386
commit,
392387
crate_paths: &config.crate_paths,
393388
target_repo_path: &config.target_repo_path,
394-
transform: &self.transform,
395389
workspace_root: &self.workspace_root,
396390
crate_name: &config.crate_name,
397391
mode: &config.mode,
@@ -417,17 +411,21 @@ impl Splitter {
417411
project_files.copy_to_split(&self.workspace_root, &config.target_repo_path)?;
418412

419413
// Create a final commit if any files were added
420-
let status = std::process::Command::new("git")
414+
// git add -A is safe to run unconditionally (no-op if no changes)
415+
std::process::Command::new("git")
421416
.current_dir(&config.target_repo_path)
422-
.args(["status", "--porcelain"])
423-
.output()?;
417+
.args(["add", "-A"])
418+
.status()?;
424419

425-
if !status.stdout.is_empty() {
420+
// Check if there are staged changes before committing
421+
let diff_cached = std::process::Command::new("git")
422+
.current_dir(&config.target_repo_path)
423+
.args(["diff", "--cached", "--quiet"])
424+
.status()?;
425+
426+
if !diff_cached.success() {
427+
// Exit code 1 means there are differences (i.e., staged changes)
426428
println!(" Creating commit for auxiliary files");
427-
std::process::Command::new("git")
428-
.current_dir(&config.target_repo_path)
429-
.args(["add", "-A"])
430-
.status()?;
431429
std::process::Command::new("git")
432430
.current_dir(&config.target_repo_path)
433431
.args(["commit", "-m", "Add workspace configs and project files"])
@@ -609,17 +607,9 @@ impl Splitter {
609607
self.copy_directory_recursive(&source_path, target_repo_path)?;
610608

611609
// Transform Cargo.toml manifest
610+
println!(" Transforming Cargo.toml");
612611
let manifest_path = target_repo_path.join("Cargo.toml");
613-
if manifest_path.exists() {
614-
println!(" Transforming Cargo.toml");
615-
let content = std::fs::read_to_string(&manifest_path)?;
616-
let context = TransformContext {
617-
crate_name: crate_name.to_string(),
618-
workspace_root: self.workspace_root.clone(),
619-
};
620-
let transformed = self.transform.transform_to_split(&content, &context)?;
621-
std::fs::write(&manifest_path, transformed)?;
622-
}
612+
self.apply_manifest_transform(&manifest_path, crate_name)?;
623613

624614
// Copy auxiliary files
625615
if !aux_files.is_empty() {
@@ -653,15 +643,7 @@ impl Splitter {
653643

654644
// Transform Cargo.toml manifest
655645
let manifest_path = target_path.join("Cargo.toml");
656-
if manifest_path.exists() {
657-
let content = std::fs::read_to_string(&manifest_path)?;
658-
let context = TransformContext {
659-
crate_name: crate_name.to_string(),
660-
workspace_root: self.workspace_root.clone(),
661-
};
662-
let transformed = self.transform.transform_to_split(&content, &context)?;
663-
std::fs::write(&manifest_path, transformed)?;
664-
}
646+
self.apply_manifest_transform(&manifest_path, crate_name)?;
665647
}
666648

667649
// Copy auxiliary files

0 commit comments

Comments
 (0)