Skip to content

Restore missing-entry guard in CoursierResolvedLockfile.dependencies() (regression from #22906) (Cherry-pick of #23341)#23359

Merged
tdyas merged 1 commit into
2.31.xfrom
cherry-pick-23341-to-2.31.x
May 20, 2026
Merged

Restore missing-entry guard in CoursierResolvedLockfile.dependencies() (regression from #22906) (Cherry-pick of #23341)#23359
tdyas merged 1 commit into
2.31.xfrom
cherry-pick-23341-to-2.31.x

Conversation

@WorkerPants
Copy link
Copy Markdown
Member

Disclaimer: I used Claude Code to evaluate and confirm comments from #23328. Basically, part of #22906 needs to be reverted.

Okay, now here's Claude:

Summary

Fixes a KeyError regression introduced by #22906 that breaks pants check (and other JVM goals) on lockfiles whose entries reference a transitive dependency that has no top-level coord entry of its own.

#22906 bumped Coursier to v2.1.24 to pick up the fix for coursier#2884 and, on the assumption that the only remaining "missing entry" cases would be parent POMs (classifier="pom"), simplified CoursierResolvedLockfile.dependencies() to do a direct dict lookup instead of an entries.get(...) guard. That assumption turns out to be wrong: even fresh resolves with the latest Coursier (verified against v2.1.25-M19) still emit transitive deps without a coord entry — e.g. ("org.apache.curator", "apache-curator", None) when resolving org.apache.hive:hive-exec:1.1.0, or ("org.apache.arrow", "arrow-memory", None) in the user-reported repro on hive-exec:3.1.3. Both have classifier=None, so the if d.classifier != "pom" filter doesn't catch them, and the unguarded lookup raises KeyError.

This restores the pre-#22906 entries.get(...) skip and adds the bug-report repro as a regression test.

Changes

  • src/python/pants/jvm/resolve/coursier_fetch.py: restore the entries.get(...) guard in dependencies(); update the comment to reflect that [trigger ci] some ivy work in progress #2884-style missing entries are still observable on current Coursier.
  • src/python/pants/jvm/resolve/coursier_fetch_test.py: add test_dependencies_skips_transitive_entries_missing_from_lockfile — the exact fixture from the bug report. Without the fix it reproduces KeyError: ('org.apache.arrow', 'arrow-memory', None); with the fix it passes.

Out of scope

  • CoursierResolvedLockfile.direct_dependencies() has the same unguarded pattern but predates Update Coursier default version to v2.1.24 #22906; leaving for a follow-up PR.
  • The longer-term transitive-closure rewrite drafted in plans/20251117_jvm_transitive_dep_bug.md is also a follow-up.

Test plan

  • New unit test test_dependencies_skips_transitive_entries_missing_from_lockfile passes with the fix and fails (with the original KeyError) without it.
  • ./pants test src/python/pants/jvm/resolve/coursier_fetch_test.py src/python/pants/jvm/resolve/coursier_fetch_integration_test.py — all pass.
  • ./pants fmt lint check clean on both changed files.

…) (regression from #22906) (#23341)

Disclaimer: I used Claude Code to evaluate and confirm comments from

Okay, now here's Claude:

Fixes a `KeyError` regression introduced by #22906 that breaks `pants
check` (and other JVM goals) on lockfiles whose entries reference a
transitive dependency that has no top-level coord entry of its own.

[coursier#2884](coursier/coursier#2884) and,
on the assumption that the only remaining "missing entry" cases would be
parent POMs (`classifier="pom"`), simplified
`CoursierResolvedLockfile.dependencies()` to do a direct dict lookup
instead of an `entries.get(...)` guard. That assumption turns out to be
wrong: even fresh resolves with the latest Coursier (verified against
v2.1.25-M19) still emit transitive deps without a coord entry — e.g.
`("org.apache.curator", "apache-curator", None)` when resolving
`org.apache.hive:hive-exec:1.1.0`, or `("org.apache.arrow",
"arrow-memory", None)` in the user-reported repro on `hive-exec:3.1.3`.
Both have `classifier=None`, so the `if d.classifier != "pom"` filter
doesn't catch them, and the unguarded lookup raises `KeyError`.

This restores the pre-#22906 `entries.get(...)` skip and adds the
bug-report repro as a regression test.

- `src/python/pants/jvm/resolve/coursier_fetch.py`: restore the
`entries.get(...)` guard in `dependencies()`; update the comment to
reflect that #2884-style missing entries are still observable on current
Coursier.
- `src/python/pants/jvm/resolve/coursier_fetch_test.py`: add
`test_dependencies_skips_transitive_entries_missing_from_lockfile` — the
exact fixture from the bug report. Without the fix it reproduces
`KeyError: ('org.apache.arrow', 'arrow-memory', None)`; with the fix it
passes.

- `CoursierResolvedLockfile.direct_dependencies()` has the same
unguarded pattern but predates #22906; leaving for a follow-up PR.
- The longer-term transitive-closure rewrite drafted in
`plans/20251117_jvm_transitive_dep_bug.md` is also a follow-up.

- [x] New unit test
`test_dependencies_skips_transitive_entries_missing_from_lockfile`
passes with the fix and fails (with the original `KeyError`) without it.
- [x] `./pants test src/python/pants/jvm/resolve/coursier_fetch_test.py
src/python/pants/jvm/resolve/coursier_fetch_integration_test.py` — all
pass.
- [x] `./pants fmt lint check` clean on both changed files.
@WorkerPants WorkerPants added this to the 2.31.x milestone May 19, 2026
@WorkerPants WorkerPants added category:bugfix Bug fixes for released features release-notes:not-required [CI] PR doesn't require mention in release notes labels May 19, 2026
@WorkerPants WorkerPants requested review from grihabor and tdyas May 19, 2026 04:20
@tdyas tdyas merged commit 187cc9e into 2.31.x May 20, 2026
25 checks passed
@tdyas tdyas deleted the cherry-pick-23341-to-2.31.x branch May 20, 2026 01:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

category:bugfix Bug fixes for released features release-notes:not-required [CI] PR doesn't require mention in release notes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants