Skip to content

Scope repoclosure's persistent-repo exclude to the package's own binary names - #442

Merged
Odilhao merged 1 commit into
theforeman:masterfrom
ogajduse:fix-repoclosure-exclude-stale-siblings
Aug 4, 2026
Merged

Scope repoclosure's persistent-repo exclude to the package's own binary names#442
Odilhao merged 1 commit into
theforeman:masterfrom
ogajduse:fix-repoclosure-exclude-stale-siblings

Conversation

@ogajduse

@ogajduse ogajduse commented Aug 4, 2026

Copy link
Copy Markdown
Member

Summary

--newest --best (#436) resolves Requires against the newest package across all repos
combined, while still checking every repo's own newest package in isolation. A subpackage that
self-pins via Requires: %{name} = %{version}-%{release} (e.g. a -doc/-cockpit sibling)
then false-positives the instant a newer build of the same SRPM lands anywhere else in the
checked set — the persistent staging repo's stale copy has nothing left to satisfy its exact
pin against.

Reproduced against the real theforeman/foreman-packaging#13921 and #13922 CI failures
(rubygem-foreman_remote_execution and rubygem-foreman_leapp bumps), using the live
el9-foreman-plugins-nightly-staging repo and the real repoclosure/yum.conf.

Fix

Exclude the package under test from the persistent staging repos it's checked against, since
the PR's own freshly-built repo (repo0/downloaded_rpms) is authoritative for it. Scoped
with dnf's per-repo --setopt=<repo>.excludepkgs=, never applied to the fresh repo itself.

Exclude list is resolved to the package's exact binary names via rpmspec, never a
name-prefix glob — a glob would also match unrelated packages sharing a name prefix (e.g.
foreman* also matching foreman-proxy, rubygem-hammer_cli* also matching
rubygem-hammer_cli_foreman), silently masking a real cross-package regression on those
packages — exactly the failure mode --best (#436) exists to catch
(theforeman/foreman-packaging#13647/#13661/#13688). Falls back to the exact hostname
(never a glob) if the spec fails to parse; resolves to no exclusion at all for repoclosure-only
hosts with no package directory.

Testing

  • New unit tests for repoclosure.py's extracted build_command().
  • New/updated functional tests covering: single and multiple persistent target repos, the
    downloaded_rpms release flow, repoclosure-only (spec-less) hosts, and a package with an
    intentionally unparseable spec (exercises the exact-hostname fallback branch).
  • Full suite: 135 passed. pylint: 10.00/10.
  • Manually re-validated against real dnf4 (matching the CI environment) with synthetic and
    live-repo reproductions of both the original false positive and the cross-package regression
    --best must keep catching.

🤖 Generated with Claude Code

…ry names

--newest --best (obal#436) resolves Requires against the newest package
across all repos while still checking every repo's own newest package in
isolation. A subpackage that self-pins via
"Requires: %{name} = %{version}-%{release}" (e.g. a "-doc"/"-cockpit"
sibling) then false-positives the instant a newer build of the same SRPM
lands anywhere else in the checked set, since the persistent staging
repo's stale copy has nothing left to satisfy its exact pin against.

Exclude the package under test from the persistent staging repos it's
checked against, since the PR's own freshly-built repo is authoritative
for it. Scoped with dnf's per-repo --setopt=<repo>.excludepkgs=, and
resolved to the package's exact binary names via rpmspec (never a glob)
so bumping e.g. "foreman" or "rubygem-hammer_cli" can't also exclude an
unrelated package that happens to share a name prefix and silently mask
a real cross-package regression - the exact failure mode theforeman#436 exists to
catch (theforeman/foreman-packaging#13647/#13661/#13688).

Reproduced against the real theforeman/foreman-packaging#13921/#13922
CI failures and the live el9-foreman-plugins-nightly-staging repo.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

@Odilhao Odilhao left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved, tested locally and it worked against the PRs that are broken atm on foreman-packaging

@Odilhao
Odilhao merged commit a341733 into theforeman:master Aug 4, 2026
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants