| Message ID | 20260602062940.3107241-1-saisneha196@gmail.com |
|---|---|
| Headers | show |
| Series | Add missing SRCREV check for SCM URIs | expand |
On Tue, 2026-06-02 at 11:59 +0530, Sai Sneha via lists.openembedded.org wrote: > This patch series adds a check for git:// (and other SCM) URIs in > SRC_URI that are missing a corresponding SRCREV, addressing > Bugzilla #16051 [1] and referencing Corentin Guillevic's RFC series [2]. > > == Changes since v4 == > > - Add tag= to skip condition in oe.qa.check_uri_srcrev() alongside > the existing rev= handling. This fixes a selftest breakage > (test_git_unpack_nonetwork) reported by Mathieu Dubois-Briand where > recipes using tag= in SRC_URI intentionally leave SRCREV empty. We cannot just skip when we see a tag=... parameters. The tag would still need to be looked up in the remote repository at parse time. > - Update docstring to mention tag= parameter > - Verified by running full bbtests.BitbakeTests selftest suite: > 32/33 passed, 1 failure (test_image_manifest) is unrelated as it > requires building a full image and fails due to network constraints > > == Note on CHECKLAYER_REQUIRED_TESTS == > > Paul suggested using CHECKLAYER_REQUIRED_TESTS instead of Patch 4. > We tested this but found it promotes missing-srcrev to ERROR_QA for > ALL layers globally during normal builds, because CHECKLAYER_REQUIRED_TESTS > feeds directly into ERROR_QA. This contradicts Bug 16051 Comment 3 > which requests keeping it as a warning for external layers by default. > The direct implementation in yocto-check-layer.bbclass enforces it > strictly only during checklayer validation without affecting normal builds. The correct way to handle this is via a discussion instead of just re-prompting an LLM. The comments in bugzilla are not requirements, and some human discussion is probably needed to arrive at a resolution. > > == Note on candidates list order == > > Martin suggested reversing the candidates list so plain SRCREV is > tested first. We kept most-specific-first order because it mirrors > BitBake internal srcrev_internal_helper() resolution order exactly, > as confirmed by BitBake own error messages: > SRCREV_default:pn-X, SRCREV_default, SRCREV:pn-X, SRCREV > > == Problem == > > A recipe with a git:// URI and no SRCREV causes two problems: > > 1. Under BB_NO_NETWORK=1, parsing fails with a FetchError that halts > parsing of ALL recipes. This breaks CI pipelines that use > bitbake --parse-only as a sanity check. > > 2. Without BB_NO_NETWORK, BitBake silently queries the remote > repository on every parse, breaking reproducibility. These issues also occur when SRCREV is set to a branch or tag instead of an explicit commit hash. > > == Why the trivial fix does not work == > > The obvious fix of checking SRCREV in insane.bbclass do_recipe_qa > was attempted by Corentin Guillevic [2] but fails because > fetcher_hashes_dummyfunc[vardepvalue] expands SRCREV at parse time > BEFORE QA checks run. Corentin patches 1 and 2 add the QA check, > while patches 3 and 4 ensure devtool and recipetool only use > AUTOREV when appropriate. > > == This series approach == > > We intercept at the vardepvalue expansion point in base.bbclass > using a shared helper oe.qa.check_uri_srcrev() that uses > d.getVar(candidate, False) preventing expansion entirely. I recommend being highly skeptical of AI generating code which modifies fairly low-level things like vardeps or vardepvalue flags. They are powerful, but should not be the first thing you reach for when trying to solve a problem unless you understand fully what they are doing. > > == Design decisions == > > 1. Why not use srcrev_internal_helper() > > It expands SRCREV internally, triggering get_autorev() and live > network fetches. Our fallback chain mirrors BitBake resolution > order exactly without expansion. > > 2. Why duplicate warnings at parse time AND QA time > > Parse-time catches CI parse-only pipelines. QA-time gives proper > layer control. This pattern is established - src-uri-bad does same. Where is this pattern established? src-uri-bad is just handled as a regular QA check in do_recipe_qa(). > > 3. Why hardcode git/gitsm/hg/svn schemes > > Known limitation, noted as follow-on improvement. "Known limitation" is not an answer to the question "Why?". > > == Enforcement levels == > > Normal builds, all layers -> WARN_QA > oe-core layer -> ERROR_QA > yocto-check-layer runs -> ERROR > > == Testing == > > - Recipe with missing SRCREV -> WARNING/ERROR at parse time > - Recipe with SRCREV = AUTOREV -> silent passthrough > - Recipe with rev= in URI -> no false positive > - Recipe with multiple SCM URIs -> all missing SRCREVs reported > - oe-core recipe -> ERROR instead of WARNING > - yocto-check-layer on layer with missing SRCREV -> FAIL > - 952 other recipes parse cleanly -> 0 errors > > [1] https://bugzilla.yoctoproject.org/show_bug.cgi?id=16051 > [2] https://lore.kernel.org/openembedded-core/20260304104342.869457-1-corentin.guillevic@smile.fr/ > [3] https://lists.openembedded.org/g/openembedded-devel/topic/115911777 > > Sai Sneha (4): > oe/qa.py: add check_uri_srcrev() helper for SCM URI SRCREV validation > base.bbclass: warn when SRCREV is missing for SCM URIs at parse time > insane.bbclass: add missing-srcrev QA check for SCM URIs > yocto-check-layer.bbclass: enforce missing SRCREV check during layer validation > > meta/classes-global/base.bbclass | 28 ++++++++++ > meta/classes-global/insane.bbclass | 13 +++++ > meta/classes-global/yocto-check-layer.bbclass | 15 ++++++ > meta/lib/oe/qa.py | 30 +++++++++++ > 4 files changed, 86 insertions(+), 2 deletions(-) > > -- > Note: This patch series was developed with assistance from Anthropic > Claude (AI). All code has been reviewed, tested, and verified by the > contributor. The Signed-off-by line represents the contributor own > certification per the Developer Certificate of Origin. Best regards,