Skip to content

Commit 238f548

Browse files
srobroekdanielmeppielCopilot
authored
fix(deps): git-semver update idempotency in build_update_plan (#2165)
* fix(deps): git-semver update idempotency in build_update_plan `apm update` reported a spurious UPDATE on every run for git-source semver dependencies already at their locked tag, and never converged: the lockfile was rewritten with the identical SHA each time (only timestamps changed). Root cause: the git-semver resolver rewrites `dep.reference` to the concrete tag and computes its SHA, but stashes the result in `ctx.git_semver_resolutions` (and the download result) without attaching it back to `dep.resolved_reference`. At plan time `resolved_reference` is None, so `build_update_plan` compares the locked commit against `None` and classifies every git-semver dep as `update`. Fix mirrors the registry idempotency fix in #1908, applied to the git-source path: when a dep has no freshly-resolved commit, the locked entry pins an immutable tag (`resolved_tag` set), and the ref is unchanged, borrow the locked commit so the comparison sees `unchanged`. Scoped to tags -- a branch tip can advance under a stable ref name, so branch deps (no `resolved_tag`) still surface real updates. Adds two regression tests: cached git-semver tag stays `unchanged`; branch-tip advance still shows `update`. * docs: clarify git-semver idempotency fix Focus the release note on user-visible convergence and describe the locked SHA as the integrity anchor rather than treating git tags as immutable. Addresses panel follow-ups from OSS growth, docs, and supply-chain review. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Sjors Robroek <51440732+danielmeppiel@users.noreply.github.com> Co-authored-by: danielmeppiel <danielmeppiel@users.noreply.github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent a29c049 commit 238f548

3 files changed

Lines changed: 91 additions & 0 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
99

1010
### Fixed
1111

12+
- `apm update` now converges for git-source semver dependencies already at
13+
their locked tag instead of reporting a spurious update on every run. Branch
14+
dependencies remain unaffected. (by @srobroek, #2165)
1215
- `apm update` now re-checks a transitive dependency's own semver range
1316
against the remote at any depth, not just for direct dependencies.
1417
Previously, `download_callback` only ran for a dependency whose install

‎src/apm_cli/install/plan.py‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -220,6 +220,20 @@ def build_update_plan(
220220
):
221221
new_ref = old.resolved_ref
222222

223+
# Cached git-semver dep at its locked tag: the resolver rewrote
224+
# ``dep.reference`` but did not attach the resolved SHA to
225+
# ``dep.resolved_reference``, so ``new_commit`` is None. If the ref still
226+
# matches a tag-pinned lock entry, borrow its locked SHA, which remains
227+
# the integrity anchor. ``resolved_tag`` distinguishes tag resolutions
228+
# from movable branches, whose freshly resolved SHAs must remain visible.
229+
if (
230+
new_commit is None
231+
and old is not None
232+
and getattr(old, "resolved_tag", None)
233+
and new_ref == old.resolved_ref
234+
):
235+
new_commit = old.resolved_commit
236+
223237
if old is None:
224238
plan_entries.append(
225239
PlanEntry(

‎tests/unit/install/test_plan.py‎

Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -235,6 +235,80 @@ def test_source_transition_to_registry_is_not_masked_as_unchanged(self):
235235
assert entry.new_resolved_ref != "v1.0.0"
236236
assert entry.action == "update"
237237

238+
def test_git_semver_tag_dep_unchanged_when_cached_and_ref_matches(self):
239+
"""A cached git-semver dep already at its locked tag must stay 'unchanged'.
240+
241+
Regression (git-source parity with #1908's registry fix): on ``apm update``
242+
the git-semver resolver rewrites ``dep.reference`` to the concrete tag and
243+
computes its SHA, but that SHA is stashed in ``ctx.git_semver_resolutions``
244+
and never attached back to ``dep.resolved_reference``. So at plan time the
245+
dep carries ``reference='eli5--v2.0.0'`` with ``resolved_reference=None``.
246+
Without the locked-commit fallback, ``build_update_plan`` compares the
247+
locked SHA against ``None`` and emits a spurious UPDATE on every run -- the
248+
lockfile then rewrites the identical SHA, so ``apm update`` never converges.
249+
250+
The locked entry carries the concrete ``resolved_tag`` produced by semver
251+
resolution, so the matching-ref case is safe to treat as unchanged.
252+
"""
253+
lock = _new_lockfile()
254+
lock.add_dependency(
255+
LockedDependency(
256+
repo_url="srobroek/agentic-packages",
257+
resolved_ref="eli5--v2.0.0",
258+
resolved_commit="9" * 40,
259+
depth=1,
260+
is_virtual=True,
261+
virtual_path="packages/eli5",
262+
constraint=">=2.0.0 <3.0.0",
263+
resolved_tag="eli5--v2.0.0",
264+
)
265+
)
266+
# Cached git-semver dep: ref rewritten to the concrete tag by the resolver,
267+
# but resolved_reference never populated (SHA not plumbed to the plan).
268+
dep = DependencyReference(
269+
repo_url="srobroek/agentic-packages",
270+
reference="eli5--v2.0.0",
271+
is_virtual=True,
272+
virtual_path="packages/eli5",
273+
)
274+
assert getattr(dep, "resolved_reference", None) is None
275+
276+
plan = build_update_plan(lock, [dep])
277+
278+
assert plan.has_changes is False
279+
entry = plan.entries[0]
280+
assert entry.action == "unchanged"
281+
assert entry.new_resolved_ref == "eli5--v2.0.0"
282+
# The locked commit is borrowed so the display shows a real SHA, not '-'.
283+
assert entry.new_resolved_commit == "9" * 40
284+
285+
def test_git_branch_dep_tip_advance_still_shows_update(self):
286+
"""A branch dep whose tip advanced must NOT be masked as unchanged.
287+
288+
Guards the locked-commit fallback: it applies only when no fresh SHA exists
289+
and the locked entry carries ``resolved_tag``. A branch dep has no
290+
``resolved_tag`` and its tip can move under a stable ref name, so a freshly
291+
resolved commit that differs from the lockfile must still surface as an
292+
update.
293+
"""
294+
lock = _new_lockfile()
295+
lock.add_dependency(
296+
LockedDependency(
297+
repo_url="https://github.com/o/r",
298+
resolved_ref="main",
299+
resolved_commit="a" * 40,
300+
depth=1,
301+
)
302+
)
303+
# Branch dep, freshly resolved to a new tip SHA (no resolved_tag on lock).
304+
deps = [_resolved_dep("https://github.com/o/r", "main", "b" * 40)]
305+
306+
plan = build_update_plan(lock, deps)
307+
308+
assert plan.has_changes is True
309+
assert plan.entries[0].action == "update"
310+
assert plan.entries[0].new_resolved_commit == "b" * 40
311+
238312

239313
# -----------------------------------------------------------------------------
240314
# render_plan_text

0 commit comments

Comments
 (0)