Skip to content

Commit 154e1f7

Browse files
fix: reconcile lockstep crate bumps to one version
Cargo crates inheriting `version.workspace = true` share a single `[workspace.package] version`, but the version flow computed a target version per package independently. Selecting two such crates with different bumps produced divergent target versions, and `write_version` wrote them to the one shared slot in HashMap-iteration order — so the final workspace version was nondeterministic and the crates' tags did not match what landed on disk. Add `Adapter::version_groups` (default: none) so an adapter can declare packages that must move together; the cargo adapter returns its inherited crates as one group. The version flow then raises every bumped member of a group to the group's strongest bump before computing versions, so they resolve to a single deterministic version. Members that received no bump are left out of the plan — the adapter still moves them via the shared version and `publish` ships them from the bumped tree. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 parent f1a2299 commit 154e1f7

3 files changed

Lines changed: 201 additions & 1 deletion

File tree

crates/adapters/src/cargo.rs

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -358,6 +358,31 @@ impl Adapter for CargoAdapter {
358358
Bump::Patch
359359
}
360360

361+
fn version_groups(&self) -> Result<Vec<Vec<String>>> {
362+
let root = CargoManifest::read(&self.root.join("Cargo.toml"))?;
363+
// Without a shared `[workspace.package] version` nothing is locked together.
364+
if root.workspace_version().is_none() {
365+
return Ok(Vec::new());
366+
}
367+
// Every crate that inherits `version.workspace = true` shares the one workspace version,
368+
// so they form a single lockstep group.
369+
let mut group = Vec::new();
370+
for dir in self.member_dirs()? {
371+
let manifest = CargoManifest::read(&dir.join("Cargo.toml"))?;
372+
if manifest.version_is_inherited() {
373+
if let Some(name) = manifest.package_name() {
374+
group.push(name);
375+
}
376+
}
377+
}
378+
group.sort();
379+
Ok(if group.is_empty() {
380+
Vec::new()
381+
} else {
382+
vec![group]
383+
})
384+
}
385+
361386
fn is_published(&self, pkg: &Pkg, version: &str) -> Result<bool> {
362387
let spec = format!("{}@{}", pkg.name, version);
363388
let out = self.runner.run("cargo", &["info", &spec], &self.root)?;
@@ -547,6 +572,42 @@ mod tests {
547572
);
548573
}
549574

575+
#[test]
576+
fn version_groups_locks_only_inherited_crates_together() {
577+
let tmp = tempfile::tempdir().unwrap();
578+
let root = tmp.path();
579+
write(
580+
root.join("Cargo.toml"),
581+
"[workspace]\nmembers = [\"crates/*\"]\n\n[workspace.package]\nversion = \"1.2.3\"\n",
582+
);
583+
// Two crates inherit the workspace version; one pins its own.
584+
write(
585+
root.join("crates/a/Cargo.toml"),
586+
"[package]\nname = \"a\"\nversion.workspace = true\n",
587+
);
588+
write(
589+
root.join("crates/b/Cargo.toml"),
590+
"[package]\nname = \"b\"\nversion.workspace = true\n",
591+
);
592+
write(
593+
root.join("crates/c/Cargo.toml"),
594+
"[package]\nname = \"c\"\nversion = \"0.4.0\"\n",
595+
);
596+
597+
let groups = CargoAdapter::new(root).version_groups().unwrap();
598+
assert_eq!(groups, vec![vec!["a".to_string(), "b".to_string()]]);
599+
}
600+
601+
#[test]
602+
fn version_groups_empty_without_shared_workspace_version() {
603+
// The default `workspace()` fixture pins concrete versions and has no inheriting crate.
604+
let tmp = workspace();
605+
assert!(CargoAdapter::new(tmp.path())
606+
.version_groups()
607+
.unwrap()
608+
.is_empty());
609+
}
610+
550611
#[test]
551612
fn dependent_bump_is_always_patch_and_range_is_bare() {
552613
let adapter = CargoAdapter::new(".");

crates/core/src/adapter.rs

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,16 @@ pub trait Adapter {
9898
/// given a dependency's bump and the edge kind, what bump does the dependent take?
9999
fn dependent_bump(&self, dep_bump: Bump, kind: &DepKind) -> Bump;
100100

101+
/// Sets of packages that are versioned in lockstep and must always move to the **same**
102+
/// version together — e.g. cargo crates that inherit `version.workspace = true` and share
103+
/// one `[workspace.package] version`. Each inner vec is one such group, named by package.
104+
///
105+
/// Default: no groups (every package is versioned independently). The `version` flow uses
106+
/// this to reconcile a group's members to a single bump, so they cannot diverge.
107+
fn version_groups(&self) -> Result<Vec<Vec<String>>> {
108+
Ok(Vec::new())
109+
}
110+
101111
/// Registry check: is `version` of `pkg` already published? Used to make publish idempotent.
102112
fn is_published(&self, pkg: &Pkg, version: &str) -> Result<bool>;
103113

crates/core/src/version.rs

Lines changed: 130 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -225,7 +225,9 @@ pub fn orchestrate_many(
225225
.map(|(name, bump)| (name.clone(), bump.clone()))
226226
.collect();
227227
let graph = Graph::build(&ctx.packages)?;
228-
let bumps = graph.cascade(ctx.adapter, &selected_for_adapter)?;
228+
let mut bumps = graph.cascade(ctx.adapter, &selected_for_adapter)?;
229+
// Lockstep crates share one version; reconcile their bumps so they can't diverge.
230+
reconcile_version_groups(&mut bumps, &ctx.adapter.version_groups()?);
229231
for (name, bump) in &bumps {
230232
let pkg = by_name[name.as_str()];
231233
new_versions.insert(name.clone(), apply_bump(&pkg.version, bump)?);
@@ -388,6 +390,29 @@ pub fn orchestrate_many(
388390
Ok(())
389391
}
390392

393+
/// Raise every bumped member of each lockstep group to the strongest (max) bump in its group, so
394+
/// packages versioned together (cargo `version.workspace = true`) resolve to one deterministic
395+
/// version instead of diverging by the order their manifests were written. Members that received
396+
/// no bump are left untouched — the adapter still moves them on disk via the shared version, and
397+
/// `publish` ships them from the bumped working tree.
398+
fn reconcile_version_groups(bumps: &mut HashMap<String, Bump>, groups: &[Vec<String>]) {
399+
for group in groups {
400+
let Some(max) = group
401+
.iter()
402+
.filter_map(|name| bumps.get(name))
403+
.max()
404+
.cloned()
405+
else {
406+
continue; // no member of this group was bumped
407+
};
408+
for name in group {
409+
if let Some(slot) = bumps.get_mut(name) {
410+
*slot = max.clone();
411+
}
412+
}
413+
}
414+
}
415+
391416
/// Apply a bump to an `x.y.z` version (pre-release/build metadata is dropped unless entering/iterating prerelease).
392417
fn apply_bump(version: &str, bump: &Bump) -> Result<String> {
393418
let mut parts = version.split('-');
@@ -480,6 +505,7 @@ mod tests {
480505

481506
struct FakeVersionAdapter {
482507
packages: Vec<Pkg>,
508+
groups: Vec<Vec<String>>,
483509
writes: RefCell<Vec<(String, String)>>,
484510
lockfile_updates: RefCell<usize>,
485511
}
@@ -492,10 +518,16 @@ mod tests {
492518
fn with_packages(packages: Vec<Pkg>) -> Self {
493519
Self {
494520
packages,
521+
groups: Vec::new(),
495522
writes: RefCell::new(Vec::new()),
496523
lockfile_updates: RefCell::new(0),
497524
}
498525
}
526+
527+
fn with_lockstep_group(mut self, names: &[&str]) -> Self {
528+
self.groups = vec![names.iter().map(|n| n.to_string()).collect()];
529+
self
530+
}
499531
}
500532

501533
impl Adapter for FakeVersionAdapter {
@@ -531,6 +563,10 @@ mod tests {
531563
Bump::Patch
532564
}
533565

566+
fn version_groups(&self) -> Result<Vec<Vec<String>>> {
567+
Ok(self.groups.clone())
568+
}
569+
534570
fn is_published(&self, _: &Pkg, _: &str) -> Result<bool> {
535571
Ok(false)
536572
}
@@ -628,6 +664,25 @@ mod tests {
628664
}
629665
}
630666

667+
/// Selects every pending package and returns a per-package bump from a scripted map.
668+
struct ScriptedBumpPrompt {
669+
bumps: HashMap<String, Bump>,
670+
}
671+
672+
impl Prompt for ScriptedBumpPrompt {
673+
fn select_packages(&self, pending: &[&Pkg]) -> Result<Vec<String>> {
674+
Ok(pending.iter().map(|pkg| pkg.name.clone()).collect())
675+
}
676+
677+
fn choose_bump(&self, name: &str, _: &str) -> Result<Bump> {
678+
Ok(self.bumps[name].clone())
679+
}
680+
681+
fn confirm(&self, _: &str) -> Result<bool> {
682+
Ok(true)
683+
}
684+
}
685+
631686
struct FakeForge {
632687
prs: RefCell<Vec<String>>,
633688
}
@@ -838,4 +893,78 @@ mod tests {
838893
"the curated notes must survive:\n{after}"
839894
);
840895
}
896+
897+
#[test]
898+
fn reconcile_version_groups_raises_bumped_members_to_max() {
899+
let mut bumps = HashMap::from([
900+
("a".to_string(), Bump::Minor),
901+
("b".to_string(), Bump::Major),
902+
("loner".to_string(), Bump::Patch),
903+
]);
904+
let groups = vec![
905+
vec!["a".to_string(), "b".to_string(), "c".to_string()],
906+
vec!["loner".to_string()],
907+
];
908+
reconcile_version_groups(&mut bumps, &groups);
909+
910+
// a and b were both bumped → both raised to the group max (major).
911+
assert_eq!(bumps["a"], Bump::Major);
912+
assert_eq!(bumps["b"], Bump::Major);
913+
// c got no bump and stays absent (the adapter still moves it via the shared version).
914+
assert!(!bumps.contains_key("c"));
915+
// A singleton group is unaffected.
916+
assert_eq!(bumps["loner"], Bump::Patch);
917+
}
918+
919+
#[test]
920+
fn lockstep_group_selected_with_different_bumps_resolves_to_one_version() {
921+
let tmp = tempfile::tempdir().unwrap();
922+
let root = tmp.path();
923+
// Two lockstep crates at the same shared version, picked with *different* bumps.
924+
let adapter = FakeVersionAdapter::with_packages(vec![
925+
test_pkg(root, "crate-a"),
926+
test_pkg(root, "crate-b"),
927+
])
928+
.with_lockstep_group(&["crate-a", "crate-b"]);
929+
let git = FakeGit::new();
930+
let forge = FakeForge {
931+
prs: RefCell::new(Vec::new()),
932+
};
933+
let prompt = ScriptedBumpPrompt {
934+
bumps: HashMap::from([
935+
("crate-a".to_string(), Bump::Major),
936+
("crate-b".to_string(), Bump::Minor),
937+
]),
938+
};
939+
let config = crate::config::ReleaseConfig {
940+
changelog_strategy: crate::config::ChangelogStrategy::Curated,
941+
..Default::default()
942+
};
943+
944+
orchestrate_many(
945+
&[&adapter],
946+
&FakeRepo,
947+
&git,
948+
&forge,
949+
&prompt,
950+
root,
951+
"2026-06-28",
952+
&VersionOptions::default(),
953+
&config,
954+
&crate::hooks::fakes::FakeHookRunner::new(),
955+
)
956+
.unwrap();
957+
958+
// Both crates are written to the same version — the strongest (major) bump wins — instead
959+
// of diverging (2.0.0 vs 1.1.0) by write order.
960+
let mut writes = adapter.writes.borrow().clone();
961+
writes.sort();
962+
assert_eq!(
963+
writes,
964+
[
965+
("crate-a".to_string(), "2.0.0".to_string()),
966+
("crate-b".to_string(), "2.0.0".to_string()),
967+
]
968+
);
969+
}
841970
}

0 commit comments

Comments
 (0)