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.
Follow-up from #2987 (review: #2987 (review)), tracking three items deferred out of that PR's scope.
1.
update_packages.shhas no way to detect a wrong/stale directory suffix at bump timefind_package.pynow resolves PyPI names to on-disk spec directory suffixes reliably, and writesthe resolved suffix to
packages-to-update.txt. Butupdate_packages.shconsumes that suffix bybuilding
packages/python-$pkg/python-$pkg.specdirectly, with no re-validation. If the suffixwere ever wrong (a future regression, a manually-edited
packages-to-update.txt, etc.),rpmspecfails,
rpm_versioncomes back empty,rpmdev-vercmpmatches 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.shcall intofind_package.py's resolution (e.g. exposeresolve_package_diras a small CLI entry point) instead of rebuilding the path itself, or atminimum fail loudly when
rpmspec/rpmdev-vercmpdon't behave as expected.2. Test workflow path filters don't cover
packages/**Neither
.github/workflows/test-find-package.ymlnor.github/workflows/find-script-test.ymltriggers on changes under
packages/**.test_integration.pyhardcodes expected on-diskdirectory 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.pyortest/**instead.Fix: add
packages/**to both workflows'pull_request.pathsfilters. Needs a push with theworkflowOAuth scope (the agent session that authored #2987 didn't have it).3.
update-pulp-packages.ymlbypassesfind_package.py's resolution entirelyThis workflow (triggered on pushes to
automation/requirements.txtonrpm/develop, i.e. aftera dependabot merge) diffs the added
+name==versionlines and feeds them straight intobuild_matrix.py, which now sharesparse_package_list()withfind_package.pybut does notgo through
resolve_package_dir()at all.matrix.package_namehere is the raw PyPI name as itappears in the requirements diff, passed directly to
update_packages.sh's naivepackages/python-$pkg/python-$pkg.spectemplate.Any package needing name resolution (separator mismatches, own
python-prefix, etc. — the sameclass 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. havebuild_matrix.pyresolveeach package's directory suffix before emitting the matrix.