Skip to content

Commit 5647d6b

Browse files
committed
cargo-rail: fixing the release pipe; Issues #6 & 7 are addressed. added the improved macros for cleaner outputs w/o the log/tracing.
1 parent 45ee4ef commit 5647d6b

38 files changed

+676
-303
lines changed

src/backup/manager.rs

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -85,15 +85,15 @@ impl BackupManager {
8585

8686
let metadata = BackupMetadata::load(&backup_dir)?;
8787

88-
eprintln!("restoring backup: {}", metadata.timestamp);
89-
eprintln!(" {} files", metadata.files_modified.len());
88+
crate::status!("restoring backup: {}", metadata.timestamp);
89+
crate::status!(" {} files", metadata.files_modified.len());
9090

9191
for file in &metadata.files_modified {
9292
let src = backup_dir.join(file);
9393
let dest = self.workspace_root.join(file);
9494

9595
if !src.exists() {
96-
eprintln!(" skipped (missing): {}", file.display());
96+
crate::status!(" skipped (missing): {}", file.display());
9797
continue;
9898
}
9999

@@ -104,7 +104,7 @@ impl BackupManager {
104104

105105
fs::copy(&src, &dest).map_err(|e| RailError::message(format!("failed to restore {}: {}", file.display(), e)))?;
106106

107-
eprintln!(" restored: {}", file.display());
107+
crate::status!(" restored: {}", file.display());
108108
}
109109

110110
println!("backup restored");
@@ -141,7 +141,7 @@ impl BackupManager {
141141
backups.push(BackupRecord::new(backup_id, metadata, path));
142142
}
143143
Err(e) => {
144-
eprintln!("warning: skipping corrupted backup '{}': {}", backup_id, e);
144+
crate::warn!("skipping corrupted backup '{}': {}", backup_id, e);
145145
continue;
146146
}
147147
}

src/cargo/manifest_analyzer.rs

Lines changed: 9 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
//! Unified manifest analysis for feature classification
22
//!
3-
//! This module handles ALL manifest parsing and analysis, replacing the duplicate
4-
//! handling between manifest.rs and unify/manifest_parser.rs
3+
//! This module handles ALL manifest parsing and analysis
54
65
use crate::error::{RailResult, ResultExt};
76
use cargo_metadata::DependencyKind as MetadataDepKind;
@@ -11,11 +10,9 @@ use std::path::{Path, PathBuf};
1110
use std::sync::Arc;
1211
use toml_edit::{DocumentMut, Item, Value};
1312

14-
// ============================================================================
1513
// Core Types
16-
// ============================================================================
1714

18-
/// Unique identifier for a dependency
15+
/// Unique identifier for a dep
1916
///
2017
/// Uses `Arc<str>` for the name fields to avoid expensive clones in hot loops.
2118
/// Cloning a DepKey is cheap (just Arc refcount bumps).
@@ -161,9 +158,7 @@ pub struct ParsedManifest {
161158
pub dependencies: HashMap<DepKey, DepUsage>,
162159
}
163160

164-
// ============================================================================
165161
// Parse Context
166-
// ============================================================================
167162

168163
/// Context for parsing dependency sections within a single manifest.
169164
/// Bundles common parameters to reduce function argument count.
@@ -176,9 +171,7 @@ struct ParseContext<'a> {
176171
dependencies: &'a mut HashMap<DepKey, DepUsage>,
177172
}
178173

179-
// ============================================================================
180174
// Main Analyzer
181-
// ============================================================================
182175

183176
/// Analyzes all workspace manifests for dependency usage patterns
184177
pub struct ManifestAnalyzer {
@@ -616,7 +609,7 @@ impl ManifestAnalyzer {
616609

617610
/// Count how many crates use a package (aggregated across renamed and non-renamed deps)
618611
///
619-
/// Issue #6: When include_renamed = true, count all usages of the package
612+
/// When include_renamed = true, count all usages of the package
620613
/// Uses package_index for O(1) key lookup instead of O(n) scan.
621614
pub fn package_usage_count(&self, package_name: &str) -> usize {
622615
let Some(dep_keys) = self.package_index.get(package_name) else {
@@ -637,7 +630,7 @@ impl ManifestAnalyzer {
637630

638631
/// Get all dep keys that refer to a specific package (including renamed)
639632
///
640-
/// Issue #6: Used to find all renamed variants of a package
633+
/// Used to find all renamed variants of a package
641634
/// Uses package_index for O(1) lookup instead of O(n) scan.
642635
pub fn dep_keys_for_package(&self, package_name: &str) -> Vec<&DepKey> {
643636
self
@@ -649,7 +642,7 @@ impl ManifestAnalyzer {
649642

650643
/// Get aggregated usage sites for a package (all renamed and non-renamed usages)
651644
///
652-
/// Issue #6: Used when include_renamed = true to merge features across all usages
645+
/// Used when include_renamed = true to merge features across all usages
653646
/// Uses package_index for O(1) key lookup instead of O(n) scan.
654647
pub fn get_package_usage_sites(&self, package_name: &str) -> Vec<&DepUsage> {
655648
let Some(dep_keys) = self.package_index.get(package_name) else {
@@ -668,7 +661,7 @@ impl ManifestAnalyzer {
668661

669662
/// Compute union of features across all usages of a package (including renamed)
670663
///
671-
/// Issue #6: When include_renamed = true, aggregate features from all variants
664+
/// When include_renamed = true, aggregate features from all variants
672665
pub fn compute_package_union(&self, package_name: &str) -> BTreeSet<String> {
673666
let mut union = BTreeSet::new();
674667

@@ -683,7 +676,7 @@ impl ManifestAnalyzer {
683676

684677
/// Check if a package has mixed default-features across all usages (including renamed)
685678
///
686-
/// Issue #6: When include_renamed = true, check across all variants
679+
/// When include_renamed = true, check across all variants
687680
pub fn package_has_mixed_defaults(&self, package_name: &str) -> bool {
688681
let usages: Vec<_> = self
689682
.get_package_usage_sites(package_name)
@@ -701,7 +694,7 @@ impl ManifestAnalyzer {
701694

702695
/// Get default-features policy across all usages of a package (including renamed)
703696
///
704-
/// Issue #6: When include_renamed = true, use conservative policy across all variants
697+
/// When include_renamed = true, use conservative policy across all variants
705698
pub fn package_default_features_policy(&self, package_name: &str) -> Option<bool> {
706699
let usages: Vec<_> = self
707700
.get_package_usage_sites(package_name)
@@ -723,15 +716,13 @@ impl ManifestAnalyzer {
723716

724717
/// Get unique package names from all dependencies
725718
///
726-
/// Issue #6: Used to iterate by package rather than by dep key
719+
/// Used to iterate by package rather than by dep key
727720
pub fn unique_packages(&self) -> HashSet<Arc<str>> {
728721
self.usage_index.keys().map(|k| Arc::clone(&k.name)).collect()
729722
}
730723
}
731724

732-
// ============================================================================
733725
// Workspace Dependencies Parser
734-
// ============================================================================
735726

736727
/// Information about an existing workspace dependency
737728
#[derive(Debug, Clone)]
@@ -843,9 +834,7 @@ fn parse_workspace_dep_entry(name: &str, value: &Item) -> ExistingWorkspaceDep {
843834
dep
844835
}
845836

846-
// ============================================================================
847837
// Unit Tests
848-
// ============================================================================
849838

850839
#[cfg(test)]
851840
mod tests {

src/cargo/manifest_ops/fields.rs

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,9 +3,7 @@
33
use crate::error::{RailError, RailResult};
44
use toml_edit::{Array, InlineTable, Item, Table, Value};
55

6-
// ============================================================================
76
// Feature Array Operations
8-
// ============================================================================
97

108
/// Build feature array from strings
119
pub fn build_feature_array(features: &[String]) -> Value {
@@ -55,9 +53,7 @@ pub fn set_features(item: &mut Item, features: Vec<String>) -> RailResult<()> {
5553
}
5654
}
5755

58-
// ============================================================================
5956
// Path Dependency Operations
60-
// ============================================================================
6157

6258
/// Remove path field from dependency
6359
pub fn remove_path(item: &mut Item) -> bool {

src/cargo/manifest_ops/navigation.rs

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,9 +3,7 @@
33
use crate::error::{RailError, RailResult};
44
use toml_edit::{DocumentMut, Item, Table};
55

6-
// ============================================================================
76
// TOML Section Navigation
8-
// ============================================================================
97

108
/// Ensure a section exists, creating if necessary
119
pub fn ensure_section(doc: &mut DocumentMut, path: &str) -> RailResult<()> {
@@ -70,9 +68,7 @@ pub fn insert_dependency(section: &mut Table, name: &str, entry: Item) -> RailRe
7068
Ok(())
7169
}
7270

73-
// ============================================================================
7471
// Target-Specific Dependencies
75-
// ============================================================================
7672

7773
/// Add target-specific dependency
7874
pub fn insert_target_dependency(

src/cargo/manifest_ops/transform.rs

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,9 +6,7 @@ use toml_edit::{DocumentMut, Item, Table, Value};
66

77
use super::workspace_ref::is_package_workspace_inherited;
88

9-
// ============================================================================
109
// Batch Transformation Operations
11-
// ============================================================================
1210

1311
/// Transform all dependencies in a section
1412
pub fn transform_dependencies_in_section<F>(doc: &mut DocumentMut, section: &str, mut transform_fn: F) -> RailResult<()>

src/cargo/multi_target_metadata.rs

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -749,9 +749,7 @@ mod tests {
749749
assert_ne!(MsrvSourceUsed::MaxWorkspace, MsrvSourceUsed::MaxDeps);
750750
}
751751

752-
// ============================================================================
753752
// Determinism Regression Tests
754-
// ============================================================================
755753
// These tests verify that outputs are deterministic (sorted) to prevent
756754
// non-deterministic behavior from HashMap/HashSet iteration order.
757755

src/cargo/unify_analyzer.rs

Lines changed: 12 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
//!
33
//! This is the core of the unify implementation that properly integrates
44
//! with WorkspaceContext and uses multi-target metadata + manifest analysis.
5+
//! The undeclared features detection lives here.
56
67
use crate::cargo::{
78
manifest_analyzer::{ExistingWorkspaceDep, ManifestAnalyzer, parse_existing_workspace_deps},
@@ -21,11 +22,7 @@ use semver::VersionReq;
2122
use std::collections::{HashMap, HashSet};
2223
use std::path::{Path, PathBuf};
2324

24-
// ============================================================================
25-
// Main Analyzer
26-
// ============================================================================
27-
28-
/// Analyzes workspace for dependency unification opportunities
25+
/// Analyzes workspace for dependency unifications
2926
pub struct UnifyAnalyzer {
3027
metadata: MultiTargetMetadata,
3128
manifests: ManifestAnalyzer,
@@ -129,7 +126,7 @@ impl UnifyAnalyzer {
129126
let dep_key = candidate.dep_key;
130127
let usage_sites = candidate.usages;
131128

132-
// === Check for major version conflicts ===
129+
// Check for major version conflicts
133130
// Different major versions cannot be safely merged - behavior depends on config.
134131
let major_versions = find_major_version_conflicts(&usage_sites);
135132
if major_versions.len() > 1 {
@@ -165,7 +162,7 @@ impl UnifyAnalyzer {
165162
}
166163
}
167164

168-
// === Issue B: Check for exact version pins ===
165+
// Check for exact version pins
169166
let has_exact_pin = usage_sites
170167
.iter()
171168
.any(|u| u.declared_version.as_ref().map(|v| is_exact_pin(v)).unwrap_or(false));
@@ -235,7 +232,7 @@ impl UnifyAnalyzer {
235232
VersionReq::parse(&format!("^{}", version)).unwrap_or_else(|_| VersionReq::parse(&version.to_string()).unwrap())
236233
};
237234

238-
// === Issue A: Check for version mismatches with existing workspace.dependencies ===
235+
// Check for version mismatches with existing workspace.dependencies
239236
if let Some(existing_ws_dep) = self.existing_workspace_deps.get(&*dep_key.name)
240237
&& let Some(ref ws_version) = existing_ws_dep.version
241238
{
@@ -326,7 +323,7 @@ impl UnifyAnalyzer {
326323
// Get users - convert Arc<str> to String for output
327324
let users: HashSet<String> = usage_sites.iter().map(|u| u.used_by.to_string()).collect();
328325

329-
// === Issue C: Check include_paths config before processing path dependencies ===
326+
// Check include_paths config before processing path dependencies
330327
let dep_path: Option<PathBuf> = if self.config.include_paths {
331328
// Check if any usage has a path (path dependencies)
332329
// If so, check if the dep is a workspace member to include path in workspace.dependencies
@@ -442,7 +439,7 @@ impl UnifyAnalyzer {
442439
(Vec::new(), Vec::new())
443440
};
444441

445-
// === Issue D: Detect unused dependencies ===
442+
// Detect unused dependencies
446443
let unused_finder = UnusedDepFinder::new(&self.metadata, &self.manifests);
447444
let unused_deps = if self.config.detect_unused {
448445
progress!("Detecting unused dependencies...");
@@ -467,7 +464,7 @@ impl UnifyAnalyzer {
467464
member_edits.entry(crate_name).or_default().extend(edits);
468465
}
469466

470-
// === Detect undeclared features ===
467+
// Detect undeclared features
471468
// Find cases where a crate uses features that it didn't declare in Cargo.toml
472469
// This happens when Cargo's feature unification "borrows" features from other members
473470
let undeclared_features = if self.config.detect_undeclared_features {
@@ -605,7 +602,7 @@ impl UnifyAnalyzer {
605602
fn detect_undeclared_features(&self) -> Vec<UndeclaredFeature> {
606603
let mut undeclared = Vec::new();
607604

608-
// Step 1: Get workspace baseline (features declared in [workspace.dependencies])
605+
// Get workspace baseline (features declared in [workspace.dependencies])
609606
// These are workspace policy - members don't need to re-declare them
610607
let workspace_baseline: HashMap<String, HashSet<String>> = self
611608
.existing_workspace_deps
@@ -619,7 +616,7 @@ impl UnifyAnalyzer {
619616
})
620617
.collect();
621618

622-
// Step 2: Build a map of what each member explicitly declares for each dependency
619+
// Build a map of what each member explicitly declares for each dependency
623620
// Include both unconditional_features AND conditional_features (from [features] table)
624621
// Also track which features each member contributes (for borrowed_from)
625622
let mut member_declared_features: HashMap<String, HashMap<String, HashSet<String>>> = HashMap::new();
@@ -640,7 +637,7 @@ impl UnifyAnalyzer {
640637
member_declared_features.insert(member.package_name.clone(), dep_features);
641638
}
642639

643-
// Step 3: Track which members contribute which features (beyond workspace baseline)
640+
// Track which members contribute which features (beyond workspace baseline)
644641
// This powers the `borrowed_from` field for transparency
645642
// Structure: dep_name -> feature -> set of member names
646643
let mut feature_sources: HashMap<String, HashMap<String, HashSet<String>>> = HashMap::new();
@@ -665,7 +662,7 @@ impl UnifyAnalyzer {
665662
}
666663
}
667664

668-
// Step 4: Find borrowed features for each member
665+
// Find borrowed features for each member
669666
// A feature is borrowed if:
670667
// - It's contributed by OTHER members (not this member)
671668
// - It's NOT in workspace baseline (workspace policy doesn't count as borrowing)

src/cargo/unify_types.rs

Lines changed: 4 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -12,11 +12,9 @@ use semver::VersionReq;
1212
use std::collections::{BTreeMap, HashMap, HashSet};
1313
use std::path::PathBuf;
1414

15-
// ============================================================================
1615
// Core Types
17-
// ============================================================================
1816

19-
/// A dependency that will be unified in [workspace.dependencies]
17+
/// A dep that will be unified in [workspace.dependencies]
2018
#[derive(Debug, Clone)]
2119
pub struct UnifiedDep {
2220
/// Dependency package name
@@ -35,10 +33,10 @@ pub struct UnifiedDep {
3533
pub path: Option<PathBuf>,
3634
}
3735

38-
/// An edit to apply to a member's Cargo.toml
36+
/// An edit to apply to a member's `Cargo.toml`
3937
#[derive(Debug, Clone)]
4038
pub enum MemberEdit {
41-
/// Replace dependency with workspace inheritance
39+
/// Replace dep with workspace inheritance
4240
UseWorkspace {
4341
/// Name of the dependency to replace
4442
dep_name: String,
@@ -51,7 +49,7 @@ pub enum MemberEdit {
5149
/// Whether the dependency is optional
5250
is_optional: bool,
5351
},
54-
/// Remove an unused dependency
52+
/// Remove an unused dep
5553
RemoveDep {
5654
/// Name of the dependency to remove
5755
dep_name: String,
@@ -217,9 +215,7 @@ pub struct UndeclaredFeature {
217215
pub borrowed_from: Vec<String>,
218216
}
219217

220-
// ============================================================================
221218
// UnificationPlan
222-
// ============================================================================
223219

224220
/// Complete unification plan
225221
#[derive(Debug)]
@@ -591,9 +587,7 @@ mod tests {
591587
}
592588
}
593589

594-
// ============================================================================
595590
// UndeclaredFeature Tests
596-
// ============================================================================
597591

598592
#[test]
599593
fn test_undeclared_feature_struct() {
@@ -650,9 +644,7 @@ mod tests {
650644
assert_eq!(uf.dep_kind, DepKind::Dev);
651645
}
652646

653-
// ============================================================================
654647
// UnificationPlan Summary Tests
655-
// ============================================================================
656648

657649
#[test]
658650
fn test_summary_with_add_features_edit() {

0 commit comments

Comments
 (0)