Skip to content

Commit 65d2cd8

Browse files
committed
cargo-rail: cleaning up old code, dead commands, flags, etc
1 parent 11e1fdf commit 65d2cd8

File tree

21 files changed

+313
-160
lines changed

21 files changed

+313
-160
lines changed

.gitignore

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ AGENTS.md
2323
# Documentation
2424
docs/*
2525
TODO.md
26+
CLI_AND_CONFIG_AUDIT.md
2627

2728
# Cargo-Rail (Testing)
2829
*rail.toml

README.md

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@ cargo rail test --explain
6464
6565
```bash
6666
# Preview unification changes
67-
cargo rail unify --dry-run
67+
cargo rail unify --check
6868

6969
# Apply unification to workspace.dependencies
7070
cargo rail unify
@@ -218,7 +218,7 @@ cargo rail test --nextest --watch # Watch mode with nextest
218218
### Dependency Unification
219219

220220
```bash
221-
cargo rail unify --dry-run # Preview changes
221+
cargo rail unify --check # Preview changes
222222
cargo rail unify # Apply unification
223223
```
224224

@@ -234,7 +234,7 @@ cargo rail status # Show sync status
234234

235235
```bash
236236
# Plan releases with dependency-aware ordering
237-
cargo rail release --all --bump minor --dry-run
237+
cargo rail release --all --bump minor --check
238238

239239
# Execute: bump versions, generate changelogs, tag, publish
240240
cargo rail release --all --bump patch --execute

src/backup/metadata.rs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ pub struct BackupMetadata {
1616
/// When the backup was created (ISO 8601 format)
1717
pub timestamp: String,
1818

19-
/// Command that created this backup (e.g., "cargo rail unify apply")
19+
/// Command that created this backup (e.g., "cargo rail unify")
2020
pub command: String,
2121

2222
/// Files that were backed up (relative to workspace root)
@@ -140,8 +140,8 @@ mod tests {
140140

141141
#[test]
142142
fn test_backup_metadata_creation() {
143-
let mut metadata = BackupMetadata::new("cargo rail unify apply");
144-
assert_eq!(metadata.command, "cargo rail unify apply");
143+
let mut metadata = BackupMetadata::new("cargo rail unify");
144+
assert_eq!(metadata.command, "cargo rail unify");
145145
assert!(metadata.files_modified.is_empty());
146146
assert!(metadata.config_snapshot.is_none());
147147

@@ -162,7 +162,7 @@ mod tests {
162162
let backup_dir = temp_dir.path();
163163

164164
// Create and save metadata
165-
let mut original = BackupMetadata::new("cargo rail unify apply");
165+
let mut original = BackupMetadata::new("cargo rail unify");
166166
original.add_file("Cargo.toml");
167167
original.add_file("crates/foo/Cargo.toml");
168168
original = original.with_description("Test backup");

src/backup/mod.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@
1919
//! let manager = BackupManager::new(workspace_root);
2020
//!
2121
//! // Create backup before modifications
22-
//! let metadata = BackupMetadata::new("cargo rail unify apply");
22+
//! let metadata = BackupMetadata::new("cargo rail unify");
2323
//! let backup_id = manager.create_backup(&files_to_backup, metadata)?;
2424
//!
2525
//! // Restore if needed

src/cargo/manifest_analyzer.rs

Lines changed: 62 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -418,6 +418,10 @@ impl ManifestAnalyzer {
418418
///
419419
/// Used when mixed default-features are detected or intersection is empty.
420420
/// Includes ALL dep kinds (Normal, Dev, Build) per design doc requirements.
421+
///
422+
/// **IMPORTANT**: Only includes features from UNCONDITIONAL usages (no target constraint).
423+
/// Features from target-specific usages (e.g., `[target.'cfg(linux)'.dependencies]`)
424+
/// are excluded because they may not be valid on all platforms and should stay local.
421425
pub fn compute_union(&self, dep: &DepKey) -> BTreeSet<String> {
422426
let Some(usages) = self.usage_index.get(dep) else {
423427
return BTreeSet::new();
@@ -428,36 +432,69 @@ impl ManifestAnalyzer {
428432
return BTreeSet::new();
429433
}
430434

431-
// Union all features from all usage sites
435+
// Union features from UNCONDITIONAL usages only
436+
// Target-specific features stay local to avoid platform incompatibilities
432437
let mut union = BTreeSet::new();
433438
for usage in usages {
434-
union.extend(usage.unconditional_features.iter().cloned());
439+
if usage.target.is_none() {
440+
union.extend(usage.unconditional_features.iter().cloned());
441+
}
435442
}
436443

437444
union
438445
}
439446

447+
/// Compute features that are ONLY declared with target constraints
448+
///
449+
/// These features should stay local (in member Cargo.toml) because they may
450+
/// have platform-specific requirements (like cfg flags or OS restrictions).
451+
/// Returns the set of features that appear only in target-constrained usages.
452+
pub fn compute_target_local_features(&self, dep: &DepKey) -> BTreeSet<String> {
453+
let Some(usages) = self.usage_index.get(dep) else {
454+
return BTreeSet::new();
455+
};
456+
457+
// Collect features from target-constrained usages
458+
let mut target_features = BTreeSet::new();
459+
for usage in usages {
460+
if usage.target.is_some() {
461+
target_features.extend(usage.unconditional_features.iter().cloned());
462+
}
463+
}
464+
465+
// Subtract features that also appear unconditionally
466+
let unconditional = self.compute_union(dep);
467+
target_features.difference(&unconditional).cloned().collect()
468+
}
469+
440470
/// Compute the intersection of features used by all packages that depend on this dependency
441471
///
442472
/// This is the CORE of the minimal feature approach:
443473
/// Only features that are ALWAYS enabled go into [workspace.dependencies].
444474
/// Members that need more can add local features.
445475
/// Includes ALL dep kinds (Normal, Dev, Build) per design doc requirements.
476+
///
477+
/// **IMPORTANT**: Only considers UNCONDITIONAL usages (no target constraint).
478+
/// Target-specific usages are excluded because their features may have
479+
/// platform-specific requirements.
446480
pub fn compute_intersection(&self, dep: &DepKey) -> BTreeSet<String> {
447481
let Some(usages) = self.usage_index.get(dep) else {
448482
return BTreeSet::new();
449483
};
450484

485+
// Filter to unconditional usages only
486+
let unconditional_usages: Vec<_> = usages.iter().filter(|u| u.target.is_none()).collect();
487+
451488
// Include ALL dep kinds - workspace deps serve all usage contexts
452-
if usages.len() < 2 {
453-
return BTreeSet::new(); // Not enough uses to unify
489+
if unconditional_usages.len() < 2 {
490+
return BTreeSet::new(); // Not enough unconditional uses to unify
454491
}
455492

456-
// Start with the first usage's features
457-
let mut intersection = usages[0].unconditional_features.clone();
493+
// Start with the first unconditional usage's features
494+
let mut intersection = unconditional_usages[0].unconditional_features.clone();
458495

459-
// Intersect with all other usages
460-
for usage in &usages[1..] {
496+
// Intersect with all other unconditional usages
497+
for usage in &unconditional_usages[1..] {
461498
intersection = intersection
462499
.intersection(&usage.unconditional_features)
463500
.cloned()
@@ -484,34 +521,44 @@ impl ManifestAnalyzer {
484521
/// Check if a dependency has mixed default-features settings
485522
///
486523
/// Includes ALL dep kinds (Normal, Dev, Build) per design doc requirements.
524+
///
525+
/// **IMPORTANT**: Only considers UNCONDITIONAL usages (no target constraint).
487526
pub fn has_mixed_defaults(&self, dep: &DepKey) -> bool {
488527
let Some(usages) = self.usage_index.get(dep) else {
489528
return false;
490529
};
491530

531+
// Filter to unconditional usages only
532+
let unconditional_usages: Vec<_> = usages.iter().filter(|u| u.target.is_none()).collect();
533+
492534
// Include ALL dep kinds - workspace deps serve all usage contexts
493-
if usages.len() < 2 {
535+
if unconditional_usages.len() < 2 {
494536
return false;
495537
}
496538

497-
// Check if all have the same default-features setting
498-
let first_default = usages[0].default_features;
499-
!usages.iter().all(|u| u.default_features == first_default)
539+
// Check if all unconditional usages have the same default-features setting
540+
let first_default = unconditional_usages[0].default_features;
541+
!unconditional_usages.iter().all(|u| u.default_features == first_default)
500542
}
501543

502544
/// Determine the default-features policy for a dependency
503545
///
504546
/// Includes ALL dep kinds (Normal, Dev, Build) per design doc requirements.
547+
///
548+
/// **IMPORTANT**: Only considers UNCONDITIONAL usages (no target constraint).
505549
pub fn default_features_policy(&self, dep: &DepKey) -> Option<bool> {
506550
let usages = self.usage_index.get(dep)?;
507551

552+
// Filter to unconditional usages only
553+
let unconditional_usages: Vec<_> = usages.iter().filter(|u| u.target.is_none()).collect();
554+
508555
// Include ALL dep kinds - workspace deps serve all usage contexts
509-
if usages.is_empty() {
556+
if unconditional_usages.is_empty() {
510557
return None;
511558
}
512559

513-
// If any usage has default-features = false, we must use false at root
514-
if usages.iter().any(|u| !u.default_features) {
560+
// If any unconditional usage has default-features = false, we must use false at root
561+
if unconditional_usages.iter().any(|u| !u.default_features) {
515562
Some(false)
516563
} else {
517564
Some(true)

src/cargo/unify_analyzer.rs

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -430,6 +430,26 @@ impl UnifyAnalyzer {
430430
// Get usage sites early - needed for version checks
431431
let usage_sites = self.manifests.get_usage_sites(dep_key);
432432

433+
// === CRITICAL: Check for major version conflicts ===
434+
// Different major versions should NEVER be merged - this is an anti-pattern
435+
// that users must fix manually. Merging features across major versions
436+
// produces invalid configurations (e.g., features that don't exist in the selected version).
437+
let major_versions = find_major_version_conflicts(&usage_sites);
438+
if major_versions.len() > 1 {
439+
let versions_str: Vec<_> = major_versions.iter().map(|v| v.to_string()).collect();
440+
issues.push(UnifyIssue {
441+
dep_name: dep_key.name.clone(),
442+
severity: IssueSeverity::Error,
443+
message: format!(
444+
"Multiple major versions detected (majors: {}) - cannot unify safely. \
445+
This is an anti-pattern in Rust workspaces that causes duplicate compilation \
446+
and wasted build resources. Please consolidate to a single major version.",
447+
versions_str.join(", ")
448+
),
449+
});
450+
continue; // Skip this dependency entirely
451+
}
452+
433453
// === Issue B: Check for exact version pins ===
434454
let has_exact_pin = usage_sites
435455
.iter()
@@ -886,6 +906,32 @@ fn is_exact_pin(version: &str) -> bool {
886906
version.starts_with('=') && !version.starts_with(">=")
887907
}
888908

909+
/// Extract major version from a declared version string
910+
///
911+
/// Returns the major version number, handling various version formats:
912+
/// - "1.0" -> Some(1)
913+
/// - "^2.3.0" -> Some(2)
914+
/// - ">=0.5" -> Some(0)
915+
fn extract_major_version(version: &str) -> Option<u32> {
916+
let cleaned = strip_version_op(version);
917+
let parts: Vec<&str> = cleaned.split('.').collect();
918+
parts.first().and_then(|s| s.parse().ok())
919+
}
920+
921+
/// Check for major version conflicts in declared versions
922+
///
923+
/// Returns the set of unique major versions found. If the set has more than
924+
/// one element, there's a conflict that cannot be safely unified.
925+
fn find_major_version_conflicts(
926+
usages: &[&crate::cargo::manifest_analyzer::DepUsage],
927+
) -> std::collections::HashSet<u32> {
928+
usages
929+
.iter()
930+
.filter_map(|u| u.declared_version.as_ref())
931+
.filter_map(|v| extract_major_version(v))
932+
.collect()
933+
}
934+
889935
/// Check if two version requirements are compatible
890936
///
891937
/// This is a heuristic check - it compares the major.minor portions
@@ -1091,6 +1137,24 @@ impl UnifyReport {
10911137
mod tests {
10921138
use super::*;
10931139

1140+
#[test]
1141+
fn test_extract_major_version() {
1142+
// Simple versions
1143+
assert_eq!(extract_major_version("1.0"), Some(1));
1144+
assert_eq!(extract_major_version("2.3.4"), Some(2));
1145+
assert_eq!(extract_major_version("0.99.3"), Some(0));
1146+
1147+
// With operators
1148+
assert_eq!(extract_major_version("^1.0"), Some(1));
1149+
assert_eq!(extract_major_version("~2.0"), Some(2));
1150+
assert_eq!(extract_major_version(">=3.0"), Some(3));
1151+
assert_eq!(extract_major_version("=4.0.0"), Some(4));
1152+
1153+
// Edge cases
1154+
assert_eq!(extract_major_version(""), None);
1155+
assert_eq!(extract_major_version("invalid"), None);
1156+
}
1157+
10941158
#[test]
10951159
fn test_is_exact_pin() {
10961160
assert!(is_exact_pin("=1.0.0"));

src/commands/init.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,7 @@ pub fn run_init(
8888

8989
println!("✅ Created {}", config_path.display());
9090
println!("\nNext steps:");
91-
println!(" cargo rail unify --dry-run # Preview dependency unification");
91+
println!(" cargo rail unify --check # Preview dependency unification");
9292
}
9393

9494
Ok(())
@@ -232,7 +232,7 @@ pub fn run_init_standalone(
232232
write_config_file(&config_path, &config_toml)?;
233233
println!("✅ Created {}", config_path.display());
234234
println!("\nNext steps:");
235-
println!(" cargo rail unify --dry-run # Preview dependency unification");
235+
println!(" cargo rail unify --check # Preview dependency unification");
236236
}
237237

238238
Ok(())

src/commands/split.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ use rayon::prelude::*;
1111
/// Run the split command
1212
///
1313
/// By default, executes the split operation (with confirmation prompt in interactive mode).
14-
/// Use --dry-run to show the plan without executing.
14+
/// Use --check to show the plan without executing.
1515
pub fn run_split(
1616
ctx: &WorkspaceContext,
1717
crate_name: Option<String>,

src/commands/sync.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ use rayon::prelude::*;
1111
/// Run the sync command
1212
///
1313
/// By default, executes the sync operation (with confirmation prompt in interactive mode).
14-
/// Use --dry-run to show the plan without executing.
14+
/// Use --check to show the plan without executing.
1515
#[allow(clippy::too_many_arguments)]
1616
pub fn run_sync(
1717
ctx: &WorkspaceContext,

src/commands/unify.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -131,13 +131,13 @@ pub fn run_unify_analyze(
131131
let total_edits: usize = plan.member_edits.values().map(|v| v.len()).sum();
132132
if !plan.workspace_deps.is_empty() {
133133
println!(
134-
"\n✅ Ready to unify {} dependencies ({} member edits). Run 'cargo rail unify apply' to apply changes.",
134+
"\n✅ Ready to unify {} dependencies ({} member edits). Run 'cargo rail unify' to apply changes.",
135135
plan.workspace_deps.len(),
136136
total_edits
137137
);
138138
} else {
139139
println!(
140-
"\n✅ Ready to convert {} members to use workspace inheritance ({} edits). Run 'cargo rail unify apply' to apply changes.",
140+
"\n✅ Ready to convert {} members to use workspace inheritance ({} edits). Run 'cargo rail unify' to apply changes.",
141141
plan.member_edits.len(),
142142
total_edits
143143
);
@@ -227,7 +227,7 @@ pub fn run_unify_apply(
227227
}
228228
}
229229

230-
let metadata = BackupMetadata::new("cargo rail unify apply");
230+
let metadata = BackupMetadata::new("cargo rail unify");
231231
let max_backups = ctx.config.as_ref().map(|c| c.unify.max_backups).unwrap_or(3);
232232
let backup_id = backup_manager.create_backup(&files_to_backup, metadata, max_backups)?;
233233
println!(" Backup created: {}", backup_id);

0 commit comments

Comments
 (0)