Repository navigation
An allow[hash-drift] that adopts nothing is not reported as redundant #408
Description
Summary
Update-time reports a directive as redundant when it decides nothing for the reference it sits on. It does so for allow[hash-drift] on one kind of reference alone: a GitHub Action or pre-commit hook that names a commit SHA. On every other reference where nothing can drift, or where Update-time never adopts drift, it accepts the directive without a warning. The user is left believing the marker does something.
Update-time reports the sibling opt-in, allow[floating-pin], as redundant wherever it keeps nothing floating. The two directives have the same shape, so the silence about allow[hash-drift] is an inconsistency rather than a decision.
Steps to reproduce
In a git repository, create these files:
requirements.txt:
humanize==4.9.0 # update-time: allow[hash-drift]
.python-version:
3.12 # update-time: allow[hash-drift] ignore[yanked]
docs/conf.py:
html_js_files = [
("https://cdn.jsdelivr.net/npm/clipboard@2.0.11/dist/clipboard.min.js", {"integrity": "sha256-AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA="}), # update-time: allow[hash-drift]
].github/workflows/ci.yml:
jobs:
build:
steps:
- uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # update-time: allow[hash-drift]Run update-time --log-level WARNING.
Expected
A redundancy warning for the allow[hash-drift] in requirements.txt, .python-version, and docs/conf.py, as for the one in the workflow.
Actual
Only the workflow's allow[hash-drift] and the .python-version entry's ignore[yanked] are reported:
WARNING Redundant update-time directive allow[hash-drift] for actions/checkout in .github/workflows/ci.yml:4: this reference pins a commit rather than a tag or branch, so it cannot drift
WARNING Redundant update-time directive ignore[yanked] for python in .python-version:1: this dependency's source does not record a yankUpdate-time does not warn about the allow[hash-drift] in requirements.txt, .python-version, and docs/conf.py.
Affected references
references/github.py:175 is the only place that reports allow[hash-drift] as redundant. So the directive goes unreported on each of these references:
| Reference | Why allow[hash-drift] adopts nothing |
Checked by |
|---|---|---|
| Python dependencies | a pin names a version and carries no hash | the run above, for requirements.txt; the code, for pyproject.toml and inline script metadata |
| Maven dependencies | a pom names a version and carries no hash | the code: _report_redundant_directives in updaters/update_pom_xml.py has no hash-drift case |
| Node engine version | it names a Node version, not an artefact | the code |
| Python version | it names a Python version, not an artefact | the run above |
| jsDelivr npm URLs | Update-time never adopts an integrity hash mismatch, whatever the user opts in to | the run above |
A Docker image a file accounts for itself: an image a Docker Compose file builds, a CircleCI machine-executor image, FROM scratch, and a FROM naming one of the Dockerfile's own build stages |
Update-time does not ask a registry about the image, so it reads no digest that could drift | the code |
Any reference whose update a bare ignore or an ignore[update] holds back |
the held-back update wins over allow[hash-drift], so the pin is never rewritten |
the code |
A package.json dependency takes no marker, so it needs no warning. An image whose tag comes from a ${VAR} or {{ ... }} template does not match as a reference, so Update-time reads no marker for it.
A GitHub Action or pre-commit hook that names a branch, or a tag such as stable-2024, is not affected. Update-time pins it to a commit and reports branch or ref drift, so allow[hash-drift] adopts something there.
What to report
Follow allow[floating-pin], which #256 settled:
- Report the
allowalone.ignore[hash-drift]spells the default, and Update-time reportsignore[floating-pin]nowhere either. - Let the held-back update win as the reason, as
floating_pin_redundancyinreferences/resolve.pydoes. A reader who froze a reference deliberately is better told that than told about the pin. - Report an image a file accounts for itself with a reason of its own, as
floating_pin_redundancydoes withReason.NO_REGISTRY_ASKED.
| Directive | Reason reported |
|---|---|
an allow[hash-drift] on a reference that carries no hash pin |
this reference carries no hash pin that could drift |
an allow[hash-drift] on a jsDelivr URL |
to be decided: the URL carries a hash, but Update-time never adopts a mismatch |
an allow[hash-drift] on an image a file accounts for itself |
to be decided, beside Reason.NO_REGISTRY_ASKED |
an allow[hash-drift] on a reference whose update is held back |
to be decided, beside Reason.UPDATE_HELD_BACK |
Reason.UPDATE_HELD_BACK reads "this reference's update is held back, so it is never pinned", and Reason.NO_REGISTRY_ASKED reads "Update-time does not ask a registry about this reference, so its tag is never pinned". allow[floating-pin] is the only directive that uses them. Widening their words, or adding a member per directive, is a decision this issue has to make.
Where the code is
markers/drift.py holds report_drift, the rule every kind of hash pin follows. It is called only where a hash pin exists, which is why a reference that carries none reaches no reporting at all. references/resolve.py holds floating_pin_redundancy and the call that reports it, which is the shape to follow. The dependencies uv resolves report their redundant directives from references/delegated.py instead, and Maven dependencies from _report_redundant_directives in updaters/update_pom_xml.py.
Out of scope
- Adding a hash pin to a reference type that carries none.
- Reporting
ignore[hash-drift], which spells the default the reference already has.
Documentation
In the README, the subsection on the hash-drift scope under "Redundant markers" names the commit-SHA case alone. Once the fix lands, it gets the cases above. It could take a table per dependency type, as the stale, cooldown, yanked, vulnerable, and archived subsections have. "Adopting hash drift" gains a sentence pointing at the warning. The fix also gets a changelog entry.
Activity
allow[hash-drift]#344allow[hash-drift]#344 as a duplicate of this issueReport an `allow[hash-drift]` as redundant wherever it adopts nothing…
Report an `allow[hash-drift]` as redundant wherever it adopts nothing…