diff --git a/CHANGELOG.md b/CHANGELOG.md index 4fd62ad31..d8055819a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed +- `apm update` now converges for git-source semver dependencies already at + their locked tag instead of reporting a spurious update on every run. Branch + dependencies remain unaffected. (by @srobroek, #2165) - `apm update` now re-checks a transitive dependency's own semver range against the remote at any depth, not just for direct dependencies. Previously, `download_callback` only ran for a dependency whose install diff --git a/src/apm_cli/install/plan.py b/src/apm_cli/install/plan.py index e02a9880f..1d3a8fe5d 100644 --- a/src/apm_cli/install/plan.py +++ b/src/apm_cli/install/plan.py @@ -220,6 +220,20 @@ def build_update_plan( ): new_ref = old.resolved_ref + # Cached git-semver dep at its locked tag: the resolver rewrote + # ``dep.reference`` but did not attach the resolved SHA to + # ``dep.resolved_reference``, so ``new_commit`` is None. If the ref still + # matches a tag-pinned lock entry, borrow its locked SHA, which remains + # the integrity anchor. ``resolved_tag`` distinguishes tag resolutions + # from movable branches, whose freshly resolved SHAs must remain visible. + if ( + new_commit is None + and old is not None + and getattr(old, "resolved_tag", None) + and new_ref == old.resolved_ref + ): + new_commit = old.resolved_commit + if old is None: plan_entries.append( PlanEntry( diff --git a/tests/unit/install/test_plan.py b/tests/unit/install/test_plan.py index e6fd6fc09..0ed2efa6d 100644 --- a/tests/unit/install/test_plan.py +++ b/tests/unit/install/test_plan.py @@ -235,6 +235,80 @@ def test_source_transition_to_registry_is_not_masked_as_unchanged(self): assert entry.new_resolved_ref != "v1.0.0" assert entry.action == "update" + def test_git_semver_tag_dep_unchanged_when_cached_and_ref_matches(self): + """A cached git-semver dep already at its locked tag must stay 'unchanged'. + + Regression (git-source parity with #1908's registry fix): on ``apm update`` + the git-semver resolver rewrites ``dep.reference`` to the concrete tag and + computes its SHA, but that SHA is stashed in ``ctx.git_semver_resolutions`` + and never attached back to ``dep.resolved_reference``. So at plan time the + dep carries ``reference='eli5--v2.0.0'`` with ``resolved_reference=None``. + Without the locked-commit fallback, ``build_update_plan`` compares the + locked SHA against ``None`` and emits a spurious UPDATE on every run -- the + lockfile then rewrites the identical SHA, so ``apm update`` never converges. + + The locked entry carries the concrete ``resolved_tag`` produced by semver + resolution, so the matching-ref case is safe to treat as unchanged. + """ + lock = _new_lockfile() + lock.add_dependency( + LockedDependency( + repo_url="srobroek/agentic-packages", + resolved_ref="eli5--v2.0.0", + resolved_commit="9" * 40, + depth=1, + is_virtual=True, + virtual_path="packages/eli5", + constraint=">=2.0.0 <3.0.0", + resolved_tag="eli5--v2.0.0", + ) + ) + # Cached git-semver dep: ref rewritten to the concrete tag by the resolver, + # but resolved_reference never populated (SHA not plumbed to the plan). + dep = DependencyReference( + repo_url="srobroek/agentic-packages", + reference="eli5--v2.0.0", + is_virtual=True, + virtual_path="packages/eli5", + ) + assert getattr(dep, "resolved_reference", None) is None + + plan = build_update_plan(lock, [dep]) + + assert plan.has_changes is False + entry = plan.entries[0] + assert entry.action == "unchanged" + assert entry.new_resolved_ref == "eli5--v2.0.0" + # The locked commit is borrowed so the display shows a real SHA, not '-'. + assert entry.new_resolved_commit == "9" * 40 + + def test_git_branch_dep_tip_advance_still_shows_update(self): + """A branch dep whose tip advanced must NOT be masked as unchanged. + + Guards the locked-commit fallback: it applies only when no fresh SHA exists + and the locked entry carries ``resolved_tag``. A branch dep has no + ``resolved_tag`` and its tip can move under a stable ref name, so a freshly + resolved commit that differs from the lockfile must still surface as an + update. + """ + lock = _new_lockfile() + lock.add_dependency( + LockedDependency( + repo_url="https://github.com/o/r", + resolved_ref="main", + resolved_commit="a" * 40, + depth=1, + ) + ) + # Branch dep, freshly resolved to a new tip SHA (no resolved_tag on lock). + deps = [_resolved_dep("https://github.com/o/r", "main", "b" * 40)] + + plan = build_update_plan(lock, deps) + + assert plan.has_changes is True + assert plan.entries[0].action == "update" + assert plan.entries[0].new_resolved_commit == "b" * 40 + # ----------------------------------------------------------------------------- # render_plan_text