Skip to content

Harden package name/version resolution around find_package.py's remaining gaps #2988

Description

@Odilhao

Follow-up from #2987 (review: #2987 (review)), tracking three items deferred out of that PR's scope.

1. update_packages.sh has no way to detect a wrong/stale directory suffix at bump time

find_package.py now resolves PyPI names to on-disk spec directory suffixes reliably, and writes
the resolved suffix to packages-to-update.txt. But update_packages.sh consumes that suffix by
building packages/python-$pkg/python-$pkg.spec directly, with no re-validation. If the suffix
were ever wrong (a future regression, a manually-edited packages-to-update.txt, etc.), rpmspec
fails, rpm_version comes back empty, rpmdev-vercmp matches none of its branches, and the script
(running under set +e) still exits 0 — the bump job goes green having bumped nothing.

Possible fix: have update_packages.sh call into find_package.py's resolution (e.g. expose
resolve_package_dir as a small CLI entry point) instead of rebuilding the path itself, or at
minimum fail loudly when rpmspec/rpmdev-vercmp don't behave as expected.

2. Test workflow path filters don't cover packages/**

Neither .github/workflows/test-find-package.yml nor .github/workflows/find-script-test.yml
triggers on changes under packages/**. test_integration.py hardcodes expected on-disk
directory suffixes for ~40 packages; a rename-only PR under packages/ won't run these tests,
so a break introduced there surfaces on the next unrelated PR that happens to touch
find_package.py or test/** instead.

Fix: add packages/** to both workflows' pull_request.paths filters. Needs a push with the
workflow OAuth scope (the agent session that authored #2987 didn't have it).

3. update-pulp-packages.yml bypasses find_package.py's resolution entirely

This workflow (triggered on pushes to automation/requirements.txt on rpm/develop, i.e. after
a dependabot merge) diffs the added +name==version lines and feeds them straight into
build_matrix.py, which now shares parse_package_list() with find_package.py but does not
go through resolve_package_dir() at all. matrix.package_name here is the raw PyPI name as it
appears in the requirements diff, passed directly to update_packages.sh's naive
packages/python-$pkg/python-$pkg.spec template.

Any package needing name resolution (separator mismatches, own python- prefix, etc. — the same
class of bug #2987 fixed for the scheduled scan) would silently fail here too, under a different
trigger than the one #2987's tests cover.

Fix: route this path through resolve_package_dir() as well, e.g. have build_matrix.py resolve
each package's directory suffix before emitting the matrix.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Fields

    Red Hat Jira

    None yet

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions