Bump Cargo workspace root dependency requirements - #400
Conversation
Package Changes Through b59db83There are 9 changes which include action with minor, @covector/assemble with minor, @covector/files with minor, covector with minor, @covector/apply with minor, @covector/changelog with minor, @covector/command with minor, @covector/toml with minor, @covector/types with minor Planned Package VersionsThe following package releases are the planned based on the context of changes in this pull request.
Add another change file through the GitHub UI by following this link. Read about change files or the docs at github.com/jbolda/covector |
|
Do we have a use case to support |
b4a3aa6 to
26766eb
Compare
|
Responding here so no one has to do discord forensics!: You're right on the use case. External deps never enter this path at all (it only probes catalog entries whose name matches a covector-managed package), so bump only fires if someone catalogs an internal package with a concrete range instead of using But! Left in the package.json guard, since when using it in a fixture I found a |
jbolda
left a comment
There was a problem hiding this comment.
Trying to imagine all of the edge cases we can hit here. Ideally we are just handling new situations more appropriately and not crashing or failing in an existing setup. I think addressing these two comments a bit more directly and that is everything I can think of.
| if (doc.has("workspace")) { | ||
| roots[rootManifestPath] = { file, doc }; | ||
| break; | ||
| } |
There was a problem hiding this comment.
Can we get into a situation where a single Cargo.toml crate (non-workspace) will loop infinitely?
There was a problem hiding this comment.
Not with the paths covector produces today, but nothing enforced that, so I made termination structural. Every manifest path fed to the walk is cwd-relative (loadFile normalizes them) and dirname bottoms out at . for those, so a standalone crate probes its own manifest, finds no [workspace], and stops at the top of the tree. An absolute path would have spun forever, though (dirname("/") is "/"). The loop now breaks where dirname stops changing (the fixed point at . or /), so it terminates regardless of the path's shape. Tests cover a standalone crate, a nested crate with no workspace above it, and the absolute-path worst case.
| @@ -0,0 +1,11 @@ | |||
| [package] | |||
| name = "rust_root_pkg_a_fixture" | |||
| version = "0.5.0" | |||
There was a problem hiding this comment.
Does this meant the situation where you set the version at the root, would this be like version.workspace = "true"? Have we tested for this versus the version specified here and just a path / features in the root? (Guess that is probably the normal case)
There was a problem hiding this comment.
Hadn't tested it, and writing the test caught a real bug, so good call. With version.workspace = true the covector package points at the root manifest and its version lives at [workspace.package]. The new requirement bump derived fine, but the version bump itself silently never landed: getPackageFileVersion was missing the [workspace.package] fallback that setPackageFileVersion already has, so the read came back empty and the manifest was written back with the old version while [workspace.dependencies] moved to the new requirement. Fixed the read to mirror the write, and added a fixture + apply test for the inherited arrangement: the root ends up with both the bumped [workspace.package] version and the bumped requirement, the path-and-features-only entry stays byte-identical, and the member manifests keep their checked-out bytes. (Your guess on the normal case is right: an entry with just a path and features has no version to bump and is left alone; that's pkg-d in the existing fixture and the helper entry in the new one.)
f08c91a to
9f616e6
Compare
An exact pin (`=0.5`) keeps its comparator through a bump, a member that no root declares leaves every table alone, and each workspace root in a repo is bumped independently. Note why the root walk starts at the member manifest itself: a root holding the version its members inherit at `[workspace.package]` is the package covector bumps.
A dependency in a `[target]` table reads its version straight off the
declaration, so a `{ workspace = true }` or path-only entry there reads back
undefined rather than empty and threw on the first requirement check.
Both paths pulled the comparator off a requirement with the same regex and rebuilt it around the bumped version. Name that rewrite once, and name the two conditions that make a root requirement unbumpable, so the regexes read as what they mean.
A comparator range, a wildcard, and `*` each already cover the bumped version and have no single pin to rewrite, but member declarations were collapsed onto one anyway: `>=1.0 <2` became `=1.1`, `1.x` became `1.1`, `*` became `1`. The workspace root table already refused to narrow these; apply the same guard to member declarations, plain and behind a `workspace:` prefix alike.
|
All six addressed, four commits. Exact pin.
The walk checking its own manifest. Kept, and I tested your suggestion first. Starting at the parent turns the inherited-version test red: a root holding Duplication. One shared Extending that guard to member declarations turned up the same bug, wider: Declared ranges narrowed onto the bumped version. Same class as 172 → 180 tests. |
Follow-up to #397 from the Discord thread: version requirements that live outside member manifests now track member bumps, covering Cargo's root
[workspace.dependencies]table (the tauri-apps/tauri#15412 shape). Default-on: it only fires where a version requirement exists, and a stale one is a silently under-constrained publish.^/~/=prefixes are preserved, other keys are untouched. Path-only,*, comparator-range (>=1.2, <2), and wildcard (1.*) entries stay byte-identical.catalog:references in package.json are now left alone; they previously hit the partial-pin path and were rewritten to a bare major, theworkspace:*corruption again. The catalog tables in pnpm-workspace.yaml stay manually managed (table bumping dropped per the review thread).covector versionapplies bumps;statusand validation are unchanged.From a real
covector versionrun (pkg-b takes a minor, pkg-a cascades a patch):What a bump should do to a comparator range is #184's policy question, so those are recognized and skipped here.
package = "..."renames are not probed, matching member-level behavior today.