Skip to content

Commit 93904cd

Browse files
committed
cargo-rail: fixing the weird 'transitive' issue in virtual workspaces; fixing the version bump on 'x.y' versioning layouts.
1 parent 25d181b commit 93904cd

File tree

11 files changed

+136
-48
lines changed

11 files changed

+136
-48
lines changed

.gitignore

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,10 +22,10 @@ AGENTS.md
2222

2323
# Documentation
2424
docs/*
25-
commands.md
26-
config.md
25+
final_testing.md
2726
rules.md
2827

28+
2929
# Cargo-Rail (Testing)
3030
*rail.toml
3131
.config/rail.toml.example

src/cargo/manifest_ops.rs

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -184,6 +184,35 @@ pub fn build_transitive_entry(features: &[String]) -> Item {
184184
Item::Value(value)
185185
}
186186

187+
/// Build a versioned dependency entry for [workspace.dependencies]
188+
///
189+
/// Used when adding transitive dependencies to workspace.dependencies.
190+
/// Creates entries like: `dep = { version = "1.0", features = ["foo"] }`
191+
///
192+
/// # Arguments
193+
///
194+
/// * `version` - The semver version to use
195+
/// * `features` - Features to enable
196+
pub fn build_versioned_dep_entry(version: &semver::Version, features: &[String]) -> Item {
197+
// Simple case: just version, no features
198+
if features.is_empty() {
199+
let mut value = Value::from(format!("^{}", version));
200+
value.decor_mut().set_suffix(" #unified");
201+
return Item::Value(value);
202+
}
203+
204+
// Complex case: version + features
205+
let mut table = InlineTable::new();
206+
table.insert("version", Value::from(format!("^{}", version)));
207+
table.insert("features", build_feature_array(features));
208+
209+
// Add #unified comment marker
210+
let mut value = Value::InlineTable(table);
211+
value.decor_mut().set_suffix(" #unified");
212+
213+
Item::Value(value)
214+
}
215+
187216
// ============================================================================
188217
// SECTION 2: Workspace Reference Detection
189218
// ============================================================================

src/cargo/manifest_writer.rs

Lines changed: 41 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
55
use crate::cargo::manifest_analyzer::DepKind;
66
use crate::cargo::manifest_ops;
7-
use crate::cargo::unify_analyzer::UnifiedDep;
7+
use crate::cargo::unify_analyzer::{TransitivePin, UnifiedDep};
88
use crate::error::{RailResult, ResultExt};
99
use crate::toml::format::TomlFormatter;
1010
use std::path::Path;
@@ -137,22 +137,22 @@ impl ManifestWriter {
137137
}
138138

139139
/// Add transitive dependencies for pinning (workspace-hack replacement)
140-
pub fn add_transitive_pins(
141-
&self,
142-
host_toml_path: &Path,
143-
transitives: &[(String, Vec<String>)], // (dep_name, features)
144-
) -> RailResult<()> {
140+
///
141+
/// This adds entries with `workspace = true` to the host's dev-dependencies.
142+
/// IMPORTANT: The caller must ensure these deps are already in [workspace.dependencies]
143+
/// before calling this function. Use `write_transitive_workspace_deps` first.
144+
pub fn add_transitive_pins(&self, host_toml_path: &Path, transitives: &[TransitivePin]) -> RailResult<()> {
145145
// Read host Cargo.toml (usually workspace root)
146146
let mut doc = manifest_ops::read_toml_file(host_toml_path)?;
147147

148148
// Ensure [dev-dependencies] exists
149149
let dev_deps =
150150
manifest_ops::get_or_create_table(&mut doc, "dev-dependencies").context("Failed to create [dev-dependencies]")?;
151151

152-
// Add each transitive as a dev dependency
153-
for (dep_name, features) in transitives {
154-
let entry = manifest_ops::build_transitive_entry(features);
155-
manifest_ops::insert_dependency(dev_deps, dep_name, entry).context("Failed to insert transitive dependency")?;
152+
// Add each transitive as a dev dependency with workspace = true
153+
for pin in transitives {
154+
let entry = manifest_ops::build_transitive_entry(&pin.features);
155+
manifest_ops::insert_dependency(dev_deps, &pin.name, entry).context("Failed to insert transitive dependency")?;
156156
}
157157

158158
// Format and write
@@ -162,6 +162,37 @@ impl ManifestWriter {
162162
Ok(())
163163
}
164164

165+
/// Write transitive dependencies to [workspace.dependencies]
166+
///
167+
/// This must be called BEFORE `add_transitive_pins` so that the deps exist
168+
/// in workspace.dependencies when referenced with `workspace = true`.
169+
pub fn write_transitive_workspace_deps(
170+
&self,
171+
workspace_toml_path: &Path,
172+
transitives: &[TransitivePin],
173+
) -> RailResult<()> {
174+
// Read workspace Cargo.toml
175+
let mut doc = manifest_ops::read_toml_file(workspace_toml_path)?;
176+
177+
// Ensure [workspace.dependencies] exists
178+
manifest_ops::ensure_section(&mut doc, "workspace").context("Failed to create [workspace] section")?;
179+
let deps_table = manifest_ops::get_or_create_table(&mut doc, "workspace.dependencies")
180+
.context("Failed to create [workspace.dependencies]")?;
181+
182+
// Add each transitive dependency with version and features
183+
for pin in transitives {
184+
let entry = manifest_ops::build_versioned_dep_entry(&pin.version, &pin.features);
185+
manifest_ops::insert_dependency(deps_table, &pin.name, entry)
186+
.context("Failed to insert transitive to workspace.dependencies")?;
187+
}
188+
189+
// Format and write
190+
self.formatter.format_manifest(&mut doc)?;
191+
manifest_ops::write_toml_file(workspace_toml_path, &doc)?;
192+
193+
Ok(())
194+
}
195+
165196
/// Convert DepKind to Cargo.toml section name
166197
fn dep_kind_to_section(&self, dep_kind: DepKind) -> &'static str {
167198
manifest_ops::dep_kind_to_section(dep_kind)

src/cargo/mod.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,6 @@ pub use manifest_analyzer::{DepKey, DepKind, DepUsage, ManifestAnalyzer};
2121
pub use manifest_writer::ManifestWriter;
2222
pub use multi_target_metadata::{ComputedMsrv, FragmentedTransitive, MultiTargetMetadata};
2323
pub use unify_analyzer::{
24-
DuplicateCleanup, IssueSeverity, MemberEdit, PrunedFeature, UnificationPlan, UnifiedDep, UnifyAnalyzer, UnifyIssue,
25-
UnifyReport, ValidationResult,
24+
DuplicateCleanup, IssueSeverity, MemberEdit, PrunedFeature, TransitivePin, UnificationPlan, UnifiedDep,
25+
UnifyAnalyzer, UnifyIssue, UnifyReport, ValidationResult,
2626
};

src/cargo/multi_target_metadata.rs

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -295,8 +295,16 @@ impl MultiTargetMetadata {
295295
// This dep has different features across builds = fragmented
296296
let all_features: HashSet<String> = features.values().flat_map(|s| s.iter().cloned()).collect();
297297

298+
// Get the resolved version (use highest across all targets)
299+
let versions = self.all_versions(&dep_name);
300+
let version = match versions.values().max() {
301+
Some(v) => v.clone(),
302+
None => continue, // Skip if we can't determine version
303+
};
304+
298305
transitives.push(FragmentedTransitive {
299306
name: dep_name.to_string(),
307+
version,
300308
feature_sets: features,
301309
unified_features: all_features.into_iter().collect(),
302310
});
@@ -363,6 +371,8 @@ impl MultiTargetMetadata {
363371
pub struct FragmentedTransitive {
364372
/// Dependency name
365373
pub name: String,
374+
/// Resolved version (highest across all targets)
375+
pub version: Version,
366376
/// Features per target
367377
pub feature_sets: HashMap<String, HashSet<String>>,
368378
/// Union of all features (for pinning)

src/cargo/unify_analyzer.rs

Lines changed: 21 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -155,6 +155,17 @@ pub enum UnusedReason {
155155
},
156156
}
157157

158+
/// A transitive dependency to pin for workspace-hack replacement
159+
#[derive(Debug, Clone)]
160+
pub struct TransitivePin {
161+
/// Dependency name
162+
pub name: String,
163+
/// Resolved version
164+
pub version: semver::Version,
165+
/// Features to enable
166+
pub features: Vec<String>,
167+
}
168+
158169
/// Complete unification plan
159170
#[derive(Debug)]
160171
pub struct UnificationPlan {
@@ -164,8 +175,8 @@ pub struct UnificationPlan {
164175
pub member_edits: HashMap<String, Vec<MemberEdit>>,
165176
/// Mapping from package name to manifest path (relative to workspace root)
166177
pub member_paths: HashMap<String, PathBuf>,
167-
/// Transitive dependencies to pin (dep_name, features)
168-
pub transitive_pins: Vec<(String, Vec<String>)>,
178+
/// Transitive dependencies to pin (with version info)
179+
pub transitive_pins: Vec<TransitivePin>,
169180
/// Results from validating across target platforms
170181
pub validation_results: Vec<ValidationResult>,
171182
/// Issues detected during analysis
@@ -783,7 +794,14 @@ impl UnifyAnalyzer {
783794
println!("Analyzing transitive dependencies...");
784795
let fragmented = self.metadata.find_fragmented_transitives();
785796

786-
fragmented.into_iter().map(|f| (f.name, f.unified_features)).collect()
797+
fragmented
798+
.into_iter()
799+
.map(|f| TransitivePin {
800+
name: f.name,
801+
version: f.version,
802+
features: f.unified_features,
803+
})
804+
.collect()
787805
} else {
788806
Vec::new()
789807
};

src/commands/init.rs

Lines changed: 8 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -23,16 +23,15 @@ pub fn run_init(
2323
let config_path = workspace_root.join(output_path);
2424

2525
// Check if target path already exists
26-
if config_path.exists() {
26+
// In --check mode, we just preview - so existence doesn't matter
27+
if config_path.exists() && !check {
2728
if !force {
2829
return Err(RailError::with_help(
2930
format!("configuration exists: {}", config_path.display()),
30-
"use --force to overwrite",
31+
"use --force to overwrite, or use --check to preview what would be generated",
3132
));
3233
}
33-
if !check {
34-
eprintln!("overwriting: {}", config_path.display());
35-
}
34+
eprintln!("overwriting: {}", config_path.display());
3635
} else if let Some(existing) = check_existing_config(workspace_root) {
3736
// A config exists elsewhere - warn but allow writing to different path
3837
if !check {
@@ -150,16 +149,15 @@ pub fn run_init_standalone(
150149
let config_path = workspace_root.join(output_path);
151150

152151
// Check if target path already exists
153-
if config_path.exists() {
152+
// In --check mode, we just preview - so existence doesn't matter
153+
if config_path.exists() && !check {
154154
if !force {
155155
return Err(RailError::with_help(
156156
format!("configuration exists: {}", config_path.display()),
157-
"use --force to overwrite",
157+
"use --force to overwrite, or use --check to preview what would be generated",
158158
));
159159
}
160-
if !check {
161-
eprintln!("overwriting: {}", config_path.display());
162-
}
160+
eprintln!("overwriting: {}", config_path.display());
163161
} else if let Some(existing) = check_existing_config(workspace_root) {
164162
// A config exists elsewhere - warn but allow writing to different path
165163
if !check {

src/commands/unify.rs

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -315,6 +315,13 @@ pub fn run_unify_apply(
315315

316316
if !plan.transitive_pins.is_empty() {
317317
eprintln!("pinning {} transitives...", plan.transitive_pins.len());
318+
319+
// STEP 1: Add transitive deps to [workspace.dependencies] first
320+
// This is required before we can reference them with `workspace = true`
321+
eprintln!(" adding to [workspace.dependencies]...");
322+
writer.write_transitive_workspace_deps(&ctx.workspace_root().join("Cargo.toml"), &plan.transitive_pins)?;
323+
324+
// STEP 2: Add to host's [dev-dependencies] with workspace = true
318325
let transitive_host_setting = ctx.config.as_ref().map(|c| &c.unify.transitive_host);
319326
let is_root_host = matches!(
320327
transitive_host_setting,

src/release/version.rs

Lines changed: 7 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -66,15 +66,11 @@ impl BumpType {
6666
}
6767
Self::Major => {
6868
let mut next = current.clone();
69-
// For 0.x versions, bump minor instead (Cargo convention)
70-
if current.major == 0 {
71-
next.minor += 1;
72-
next.patch = 0;
73-
} else {
74-
next.major += 1;
75-
next.minor = 0;
76-
next.patch = 0;
77-
}
69+
// Always bump major version - including 0.x -> 1.0.0
70+
// Users who want semver-compatible "breaking change" on 0.x can use --bump minor
71+
next.major += 1;
72+
next.minor = 0;
73+
next.patch = 0;
7874
next.pre = semver::Prerelease::EMPTY;
7975
next.build = semver::BuildMetadata::EMPTY;
8076
next
@@ -257,10 +253,10 @@ mod tests {
257253

258254
#[test]
259255
fn test_bump_major_pre_1() {
260-
// For 0.x versions, major bump increments minor
256+
// For 0.x versions, major bump goes to 1.0.0
261257
let v = Version::parse("0.2.3").unwrap();
262258
let next = BumpType::Major.apply(&v);
263-
assert_eq!(next, Version::parse("0.3.0").unwrap());
259+
assert_eq!(next, Version::parse("1.0.0").unwrap());
264260
}
265261

266262
#[test]

src/toml/builder.rs

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -170,15 +170,15 @@ impl RailConfigBuilder {
170170
/// Add splits template
171171
pub fn splits_template(&mut self) -> &mut Self {
172172
let content = r#"# Split/Sync: Use 'cargo rail split init <crate>' to configure individual crates
173-
# [[splits]]
174-
# name = "my-crate"
173+
# Per-crate configuration uses [crates.<name>] sections:
174+
#
175+
# [crates.my-crate.split]
175176
# remote = "[email protected]:org/my-crate.git"
176177
# branch = "main"
177-
# mode = "single"
178-
# publish = true
178+
# mode = "single" # or "combined" for multi-crate repos
179179
#
180-
# [[splits.paths]]
181-
# crate = "crates/my-crate"
180+
# [crates.my-crate.release]
181+
# publish = true
182182
"#;
183183
self.sections.push(content.to_string());
184184
self
@@ -324,7 +324,7 @@ mod tests {
324324
assert!(output.contains("targets"));
325325
assert!(output.contains("[unify]"));
326326
assert!(output.contains("[release]"));
327-
assert!(output.contains("# [[splits]]"));
327+
assert!(output.contains("# [crates.my-crate.split]"));
328328
}
329329

330330
#[test]

0 commit comments

Comments
 (0)