Generate the SDK container's repo section, and hash the renderer - #367
Open
jetm wants to merge 3 commits into
Open
Generate the SDK container's repo section, and hash the renderer#367jetm wants to merge 3 commits into
jetm wants to merge 3 commits into
Conversation
Editing repoconf.py does not invalidate any task hash, so a corrected gpgkey path or baseurl can be committed, built, and never reach the SDK: bitbake reports do_compile as sstate-valid, restores the old object, and the container ships the file rendered before the edit. Nothing warns. Two independent mechanisms have to line up before bitbake tracks an addpylib module, and neither was in place. PyLibNode.eval registers a submodule's functions only when the package names it in BBIMPORTS (bb/parse/ast.py), and this package declared none, so repoconf never entered modulecode_deps at all. Registration alone would not have been enough either: the registered key is the fully qualified avocado_sdk_metadata.repoconf.<func>, while called_node_name records the literal spelling at the call site (bb/codeparser.py) and bb/data.py intersects the two as exact sets. An aliased import therefore records a name that can never match its own registration. Declaring BBIMPORTS and dropping the alias closes both. The verbose call sites are the price of the second half; an `as repoconf` import reads better and silently unhashes the module.
The suite under meta-avocado/tests is executed by nothing. The tree's only unittest invocation is scoped to meta-avocado-sbom/tests, so these tests have never run outside a developer's shell since they were added. That matters more here than the file count suggests. What a generated .repo section tells dnf to verify is only otherwise observable by building an SDK and reading the file out of the container, and these tests exist as the substitute for that build. A guard nobody runs is indistinguishable from one that always passes: weakening the priority check to a falsy test, for instance, would sort every section written at priority=0 to dnf's default while CI stayed green. Scoped to the lib, the tests and the SDK recipes rather than the whole layer, so an unrelated recipe edit does not queue a run that can only pass. No pip install step, unlike the sbom contract workflow - the suite is standard library only, so there is no dependency whose failure could shrink it rather than fail it.
Enabling metadata verification would have verified everything except the
one repository every SDK container reads. The [avocado-sdk] section was
a checked-in file carrying a hardcoded gpgcheck=0 and no repo_gpgcheck,
so the switch that reaches every generated section could not reach it.
Nothing errors and nothing warns; the build simply looks verified.
Rendering it through the same function makes one setting govern every
.repo section the tree installs. The rendered block differs from the
deleted file by an explicit repo_gpgcheck=0, which is dnf's own default,
so behaviour is unchanged until the switch is set.
Two smaller decisions the diff would otherwise leave unexplained.
The literals live in render_sdk_repo_section rather than in the recipe
because a recipe is a task the suite cannot import: spelled there, a
typo in the section name or the path would pass every test and 404 on
the container's first transaction. The deleted file was at least data a
reviewer could diff, and that property should not be lost in the move.
priority stays a required argument even though this caller passes None.
Making it default to None would let a per-target section be added
without one, rendering at dnf's default of 99 and sorting below every
sibling ranked 1..N - a wrong package on a device, with no build error
and no failing test. Passing None explicitly says the same thing and
keeps the omission a TypeError.
The {REPO_BASE} sed goes with the file it templated. It has matched
nothing since the template switched to the ${repo_url} placeholder,
which the client substitutes at install time instead. AVOCADO_REPO_BASE
now has no bitbake consumer at all, though kas/base.yml and
build-all-containers.sh still set and print it; that predates this
change and is left alone rather than swept in.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Enabling repository-metadata verification would have verified everything except the one repository every SDK container reads. The
[avocado-sdk]section was a checked-in file with a hardcodedgpgcheck=0and norepo_gpgcheck, so the switch that reaches every generated section could not reach it. Nothing errors and nothing warns; the build just looks verified.Three commits, each standing alone:
1. Hash the repo-config renderer into task signatures. A latent bug this change would otherwise have made worse. Editing
repoconf.pyinvalidated no task hash, so a correctedgpgkeypath could be committed, built, and never reach the SDK: bitbake reportsdo_compileas sstate-valid and ships the previously generated file. Two mechanisms both had to line up and neither did.PyLibNode.evalregisters an addpylib submodule's functions only when the package names it inBBIMPORTS, and this package named none. Registration alone would not have sufficed either: the registered key is the fully qualifiedavocado_sdk_metadata.repoconf.<func>, whilecalled_node_namerecords the literal spelling at the call site and the two are intersected as exact sets, so an aliased import records a name that can never match its own registration.2. Run the renderer tests in CI. The suite under
meta-avocado/testswas executed by nothing; the only unittest invocation in the tree is scoped tometa-avocado-sbom/tests. What a generated.reposection tells dnf to verify is otherwise only observable by building an SDK and reading the file out of the container, and these tests are the substitute for that build.3. Generate the section. The rendered block differs from the deleted file by an explicit
repo_gpgcheck=0, which is dnf's own default, so behaviour is unchanged until the switch is set.Two decisions the diff would otherwise leave unexplained. The literals live in
render_sdk_repo_sectionrather than in the recipe because a recipe is a task the test suite cannot import: spelled there, a typo in the section name or the path would pass every test and 404 on the container's first transaction. Andprioritystays a required argument even though this caller passesNone: defaulting it would let a per-target section be added without one, rendering at dnf's default of 99 and sorting below every sibling ranked 1..N, with no build error and no failing test.The
{REPO_BASE}sed goes with the file it templated. It has matched nothing since the template switched to the${repo_url}placeholder in August 2025, which the client substitutes at install time instead.Testing
22 unit tests, including mutation checks confirming each new guard kills its mutant (restoring the
prioritydefault fails 1; a one-word path typo fails 2). Both recipe python bodies parse.No build has been run.
bitbake avocado-sdk-repos -c compileplus a read of the produced file is the remaining validation.Known limitation, deliberately out of scope
[main] gpgcheck=Truein the same recipe'sdnf.confis the container-wide default for a repo that sets nothing. Every rendered section setsgpgcheckexplicitly and the per-repo value wins, so it decides nothing today; it would decide for a.repofile added to the container by someone else. Named in the docs and the module docstring rather than left silent.Stacking
Based on
sbom-cve/repo-gpgcheck(#365), which adds the renderer and therepo_gpgcheckswitch. Review that one first; this PR's diff is only the three commits above.