From a0eaca946b1b3790aa5fe76fe16e1fdf309d29dd Mon Sep 17 00:00:00 2001 From: dkijania Date: Wed, 22 Jul 2026 22:35:40 +0200 Subject: [PATCH] session reversion: rewrite all version operators unconditionally (match bash) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes #12. deb-session-reversion.sh (whose mina callers invoke it with no flag) rewrites the old version to the new one across every versioned dependency constraint on every reversion. deb-toolkit did this only under --update-deps, and even then only for exact `=` pins, leaving `>=`/`<<`/etc. alone on the theory that a looser constraint is still satisfied by a higher version. That theory fails for mina: reversions are not monotonic (manager.sh reversions to an arbitrary target, e.g. a fork build 3.4.0-rc2-company - down to a clean 3.4.0-rc2). On a downward reversion a stale `(>= old)` is unsatisfiable and the package will not install. Changes: - replace_in_constraint rewrites all Debian relation operators (=, <<, <=, >=, >>), matching the operator as a prefix so both `(<< X)` and `(=X)` — mina emits both — are handled. - reversion performs the dependency rewrite unconditionally; the --update-deps CLI flag and update_deps manifest field are kept, accepted, and ignored (deprecated) so existing callers/manifests keep working. The flag now warns. - rewrite stays constraint-scoped (inside `(...)`), so a version-like substring in a package name is never mangled — an improvement over the bash bare-substring pass. Verified against the bash scripts on a realistic downward reversion with mixed `(=X)`/`(<< X)` formats: control fields are now identical. --- README.md | 2 +- docs/session-manifest.md | 25 +++++++----- src/cli.rs | 5 ++- src/main.rs | 8 +++- src/session/apply.rs | 37 ++++++----------- src/session/control.rs | 82 +++++++++++++++++++++++++++----------- src/session/mod.rs | 15 +++++-- tests/session_roundtrip.rs | 7 +++- 8 files changed, 114 insertions(+), 67 deletions(-) diff --git a/README.md b/README.md index 2923dca..6b0b971 100644 --- a/README.md +++ b/README.md @@ -32,7 +32,7 @@ session save [--verify] session read-field session rename-package session replace-suite -session reversion [--update-deps] +session reversion session insert [-d] … session remove session move diff --git a/docs/session-manifest.md b/docs/session-manifest.md index 4ec0d20..73e20ca 100644 --- a/docs/session-manifest.md +++ b/docs/session-manifest.md @@ -95,23 +95,28 @@ CLI equivalent: `session replace-suite ` --- -### `reversion` — set `Version:`, optionally rewrite dep `=` pins +### `reversion` — set `Version:` and rewrite versioned dep constraints ```json -{ "op": "reversion", "new_version": "2.0.0", "update_deps": true } +{ "op": "reversion", "new_version": "2.0.0" } ``` | Field | Type | Required | Description | | ------------- | ------ | -------- | ------------------------------------------------------------------------------------------------------------------------------------------ | | `new_version` | string | yes | New value for the `Version:` field. | -| `update_deps` | bool | no | When `true`, rewrite `= ` to `= ` in `Depends`, `Pre-Depends`, `Recommends`, `Suggests`, `Enhances`, `Breaks`, `Conflicts`, `Replaces`, `Provides`. Defaults to `false`. | - -`update_deps` only rewrites exact-equality pins. Loose constraints -(`>=`, `<<`, `<=`, `>>`) are left alone because they are still satisfied -by the bumped version. This matches the `--update-deps` semantics of the -bash `deb-session-reversion.sh` script. - -CLI equivalent: `session reversion [--update-deps]` +| `update_deps` | bool | no | Deprecated and ignored — the dependency rewrite below is now unconditional. Retained so older manifests keep parsing. | + +Reversion rewrites every dependency constraint that pinned the old version — +for **any** relation operator (`=`, `<<`, `<=`, `>=`, `>>`) — across `Depends`, +`Pre-Depends`, `Recommends`, `Suggests`, `Enhances`, `Breaks`, `Conflicts`, +`Replaces`, `Provides`. All operators are rewritten, not just `=`, because a +reversion can *lower* the version: a left-alone `(>= old)` would then be +unsatisfiable and the package uninstallable. This matches the unconditional +dependency rewrite the bash `deb-session-reversion.sh` performs on every call. +The rewrite is constraint-scoped (only inside `(...)`), so a version-like +substring in a package name is never mangled. + +CLI equivalent: `session reversion ` --- diff --git a/src/cli.rs b/src/cli.rs index cd244f0..b9302bb 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -144,8 +144,9 @@ pub struct SessionReplaceSuiteArgs { pub struct SessionReversionArgs { pub session_dir: String, pub new_version: String, - /// Also rewrite version constraints in Depends / Pre-Depends / etc. - #[arg(long = "update-deps", default_value_t = false)] + /// Deprecated and ignored: reversion now always rewrites versioned + /// dependency constraints. Accepted so existing invocations keep working. + #[arg(long = "update-deps", default_value_t = false, hide = true)] pub update_deps: bool, } diff --git a/src/main.rs b/src/main.rs index fb95c09..ab463ab 100644 --- a/src/main.rs +++ b/src/main.rs @@ -128,8 +128,14 @@ fn dispatch_session(cmd: SessionCommand) -> Result<()> { session.replace_suite(&args.new_suite) } SessionCommand::Reversion(args) => { + if args.update_deps { + log::warn!( + "--update-deps is deprecated and ignored: reversion now always rewrites \ + versioned dependency constraints" + ); + } let session = session::Session::load(&args.session_dir)?; - session.reversion(&args.new_version, args.update_deps) + session.reversion(&args.new_version) } SessionCommand::Apply(args) => { let session = session::Session::load(&args.session_dir)?; diff --git a/src/session/apply.rs b/src/session/apply.rs index 5b72252..d7accc7 100644 --- a/src/session/apply.rs +++ b/src/session/apply.rs @@ -137,17 +137,14 @@ pub enum Step { new_suite: String, }, - /// `session reversion [--update-deps]` — set the - /// `Version:` field, optionally rewriting `=` pins in dependency - /// fields that previously matched the *old* Version. + /// `session reversion ` — set the `Version:` field and rewrite + /// every versioned dependency constraint that pinned the old version, for + /// any operator (`=`, `<<`, `<=`, `>=`, `>>`). Reversion { /// New value for the `Version:` control field. new_version: String, - /// When true, rewrite `= ` to `= ` in - /// `Depends`, `Pre-Depends`, `Recommends`, `Suggests`, `Enhances`, - /// `Breaks`, `Conflicts`, `Replaces`, and `Provides`. Loose - /// constraints (`>=`, `<<`, …) are left alone since the bumped - /// version still satisfies the range. + /// Deprecated and ignored: the dependency rewrite is now unconditional. + /// Retained so manifests that still carry `update_deps` keep parsing. #[serde(default)] update_deps: bool, }, @@ -355,12 +352,8 @@ impl Step { Step::ReplaceSuite { new_suite } => format!("replace-suite → {}", new_suite), Step::Reversion { new_version, - update_deps, - } => format!( - "reversion → {}{}", - new_version, - if *update_deps { " (--update-deps)" } else { "" } - ), + update_deps: _, + } => format!("reversion → {}", new_version), Step::Insert { dest, sources, @@ -397,8 +390,8 @@ fn apply_step(session: &Session, step: &Step, manifest_dir: &Path) -> Result<()> Step::Reversion { new_version, - update_deps, - } => session.reversion(new_version, *update_deps), + update_deps: _, + } => session.reversion(new_version), Step::Insert { dest, @@ -491,19 +484,15 @@ mod tests { } #[test] - fn reversion_defaults_to_no_update_deps() { + fn reversion_parses_without_update_deps() { + // The field is deprecated and ignored, but a manifest may omit it + // (serde default) or still carry it — both must parse. let json = r#"{ "steps": [ { "op": "reversion", "new_version": "2.0" } ] }"#; let plan: Plan = serde_json::from_str(json).unwrap(); match &plan.steps[0] { - Step::Reversion { - new_version, - update_deps, - } => { - assert_eq!(new_version, "2.0"); - assert!(!*update_deps); - } + Step::Reversion { new_version, .. } => assert_eq!(new_version, "2.0"), _ => panic!("wrong variant"), } } diff --git a/src/session/control.rs b/src/session/control.rs index 661bcda..ecc236e 100644 --- a/src/session/control.rs +++ b/src/session/control.rs @@ -76,14 +76,19 @@ pub fn set_field(path: &Path, field: &str, value: &str) -> Result<()> { } /// Rewrite every dependency version constraint that pins exactly the -/// `old_version` to instead reference `new_version`. Operates on the -/// `Depends`, `Pre-Depends`, `Recommends`, `Suggests`, `Enhances`, -/// `Breaks`, `Conflicts`, `Replaces`, and `Provides` fields. Matches the -/// `--update-deps` flag in `deb-session-reversion.sh`. +/// `old_version` to instead reference `new_version`, for any relation operator +/// (`=`, `<<`, `<=`, `>=`, `>>`). Operates on the `Depends`, `Pre-Depends`, +/// `Recommends`, `Suggests`, `Enhances`, `Breaks`, `Conflicts`, `Replaces`, and +/// `Provides` fields, including multi-line continuations. +/// +/// This is the unconditional dependency rewrite that `deb-session-reversion.sh` +/// performs on every reversion (not only under its `--update-deps` flag): a +/// reversion may lower the version, and a stale `(>= old)` would then be +/// unsatisfiable. Constraint-scoped (only inside `(...)`), so a version-like +/// substring in a package name is never mangled. pub fn update_deps(path: &Path, old_version: &str, new_version: &str) -> Result<()> { let text = fs::read_to_string(path).with_context(|| format!("Reading {}", path.display()))?; - // Operators recognized in a Debian version constraint: =, <<, <=, >=, >> let dep_fields = [ "Depends", "Pre-Depends", @@ -148,22 +153,33 @@ fn replace_version_in_constraints(line: &str, old: &str, new: &str) -> String { out } +/// Debian version-relation operators, longest first so `<=`/`>=`/`<<`/`>>` +/// are matched before a bare `=` could be mistaken for a prefix. +const VERSION_OPERATORS: [&str; 5] = ["<<", "<=", ">=", ">>", "="]; + fn replace_in_constraint(inside: &str, old: &str, new: &str) -> String { - // Only exact `=` pins are rewritten — loose constraints like `>=` are - // still satisfied by the bumped version and intentionally left alone. + // Rewrite the pinned version for ANY relation operator (`=`, `<<`, `<=`, + // `>=`, `>>`) when it pins exactly `old`. This mirrors + // deb-session-reversion.sh, which rewrites the old version across every + // versioned constraint regardless of operator — necessary because a + // reversion may *lower* the version, and a left-alone `(>= old)` would then + // be unsatisfiable, making the package uninstallable. + // + // The operator and version may be separated by a space or not — mina emits + // both `(<< X)` and `(=X)` — so match the operator as a prefix rather than + // splitting on whitespace. let trimmed = inside.trim(); - let Some((op, rest)) = trimmed.split_once(char::is_whitespace) else { - return inside.to_string(); - }; - let op = op.trim(); - let ver = rest.trim(); - if op != "=" { - return inside.to_string(); - } - if ver != old { - return inside.to_string(); + for op in VERSION_OPERATORS { + if let Some(rest) = trimmed.strip_prefix(op) { + if rest.trim() == old { + return format!("{} {}", op, new); + } + // Operator matched but a different version is pinned — leave it. + return inside.to_string(); + } } - format!("{} {}", op, new) + // Not a version constraint (e.g. an architecture qualifier) — leave it. + inside.to_string() } #[cfg(test)] @@ -229,16 +245,34 @@ mod tests { } #[test] - fn update_deps_rewrites_equality_only() { + fn update_deps_rewrites_all_operators() { + // Every relation operator pinning the old version is rewritten — a + // reversion may lower the version, so `>=`/`<<` cannot be left alone. let f = write_tmp( - "Package: foo\nVersion: 2.0\nDepends: libfoo (= 1.0), libbar (>= 1.0), libbaz\n", + "Package: foo\nVersion: 2.0\nDepends: a (= 1.0), b (>= 1.0), c (<< 1.0), d (<= 1.0), e (>> 1.0), f\n", ); update_deps(f.path(), "1.0", "2.0").unwrap(); let out = fs::read_to_string(f.path()).unwrap(); - assert!(out.contains("libfoo (= 2.0)"), "got: {}", out); - // Loose constraint must NOT be rewritten — `>= 1.0` is still satisfied by 2.0. - assert!(out.contains("libbar (>= 1.0)"), "got: {}", out); - assert!(out.contains("libbaz")); + for expect in [ + "a (= 2.0)", + "b (>= 2.0)", + "c (<< 2.0)", + "d (<= 2.0)", + "e (>> 2.0)", + ] { + assert!(out.contains(expect), "missing {expect} in: {out}"); + } + assert!(out.contains(", f\n"), "unversioned dep dropped: {out}"); + } + + #[test] + fn update_deps_handles_operator_without_space() { + // mina emits `(=X)` with no space as well as `(<< X)`. + let f = write_tmp("Depends: cfg (=1.0), other (>=1.0)\n"); + update_deps(f.path(), "1.0", "2.0.0-rc1").unwrap(); + let out = fs::read_to_string(f.path()).unwrap(); + assert!(out.contains("cfg (= 2.0.0-rc1)"), "got: {}", out); + assert!(out.contains("other (>= 2.0.0-rc1)"), "got: {}", out); } #[test] diff --git a/src/session/mod.rs b/src/session/mod.rs index 9578c66..123a407 100644 --- a/src/session/mod.rs +++ b/src/session/mod.rs @@ -116,11 +116,20 @@ impl Session { self.set_field("Suite", new_suite) } - pub fn reversion(&self, new_version: &str, update_deps: bool) -> Result<()> { + /// Set the `Version:` field and rewrite every versioned dependency + /// constraint that pinned the old version to the new one. + /// + /// The dependency rewrite is unconditional, matching + /// `deb-session-reversion.sh` (whose callers rely on it with no flag): a + /// reversion may lower the version, and a stale `(>= old)` constraint would + /// otherwise make the package uninstallable. See [`control::update_deps`]. + pub fn reversion(&self, new_version: &str) -> Result<()> { let old_version = self.read_field("Version").ok(); self.set_field("Version", new_version)?; - if update_deps { - if let Some(old) = old_version.as_deref() { + if let Some(old) = old_version.as_deref() { + // A no-op reversion (old == new) would rewrite nothing anyway, but + // skip the file rewrite entirely in that case. + if old != new_version { control::update_deps(&self.control_file(), old, new_version)?; } } diff --git a/tests/session_roundtrip.rs b/tests/session_roundtrip.rs index 498f261..e89cff2 100644 --- a/tests/session_roundtrip.rs +++ b/tests/session_roundtrip.rs @@ -135,14 +135,17 @@ fn session_full_roundtrip() { info ); assert!(info.contains("Suite: stable"), "Suite not set:\n{}", info); + // Reversion rewrites the pinned version for EVERY relation operator, not + // just `=`: a reversion can lower the version, so a stale `(>= old)` would + // be unsatisfiable. Both constraints track the new version. assert!( info.contains("libfoo (= 2.0.0)"), "= pin not rewritten:\n{}", info ); assert!( - info.contains("libbar (>= 1.0.0)"), - ">= constraint should have been left alone:\n{}", + info.contains("libbar (>= 2.0.0)"), + ">= constraint not rewritten:\n{}", info );