Hi Paul, One more thing I wanted to address, your concern about AI-driven changes in this sensitive area.
I understand the concern and I think it is valid. The tag= bug that was caught validates your point, AI-assisted code can miss edge cases. That is precisely why I disclosed the AI involvement honestly, and why your review was valuable in catching it. To be clear about my role: I used Claude as a development aid, not as a replacement for my own judgment. Every design decision, test, and fix was made by me personally. I reproduced the selftest failure locally, identified the root cause, and applied the correct fix. I have now run the full bbtests.BitbakeTests suite (32/33 passing). If there are specific additional scenarios you would like me to verify, I am happy to do so. Best regards, Sai Sneha On Tue, 2 Jun 2026 at 12:09 PM, Sai Sneha via lists.openembedded.org <[email protected]> wrote: > Hi Paul, > > You were right to raise the tag= concern — it turned out to be exactly the > cause of the selftest failure reported by Mathieu. Recipes using tag= in > SRC_URI intentionally leave SRCREV empty, and my check was incorrectly > treating this as missing. > > The fix adds tag= to the list of URI parameters that bypass the SRCREV > check, alongside the existing rev= handling: > > if params.get('rev', '') or params.get('tag', ''): > return '' > > I have verified the fix by running the full bbtests.BitbakeTests selftest > suite — 32/33 passed. The one failure (test_image_manifest) requires > building a full image and fails due to network constraints in my > environment, unrelated to this patch. > > I have sent v5 with this fix. > > Best regards, > Sai Sneha > On Thu, 28 May 2026 at 8:39 PM, Paul Barker <[email protected]> wrote: > >> On Wed, 2026-05-27 at 16:47 +0530, Sai Sneha via lists.openembedded.org >> wrote: >> > A recipe with a git:// (or other SCM) URI in SRC_URI must have a >> > corresponding SRCREV set. Without it, BitBake performs a live query >> > on the remote repository at every parse, breaking reproducibility >> > and causing parse failures under BB_NO_NETWORK=1. >> > >> > The trivial fix of checking SRCREV in insane.bbclass do_recipe_qa >> > fails because fetcher_hashes_dummyfunc[vardepvalue] expands SRCREV >> > at parse time before QA checks run, causing a FetchError that halts >> > parsing entirely. This was identified in Corentin Guillevic's RFC >> > series and confirmed by Mathieu Dubois-Briand's autobuilder testing. >> > >> > The fix intercepts at the vardepvalue expansion point by introducing >> > check_srcrev_set() which uses the shared oe.qa.check_uri_srcrev() >> > helper. The function is called conditionally: >> > >> > fetcher_hashes_dummyfunc[vardepvalue] = >> > "${@bb.fetch.get_hashvalue(d) if check_srcrev_set(d) else ''}" >> > >> > Note on design choice: we intentionally use d.getVar(candidate, False) >> > in oe.qa.check_uri_srcrev() rather than srcrev_internal_helper() >> > because the latter expands SRCREV, triggering get_autorev() and live >> > network fetches -- the exact problem this patch fixes. >> > >> > Note: parse-time warnings intentionally complement the QA-time warnings >> > added in insane.bbclass. This is necessary because CI pipelines running >> > 'bitbake --parse-only' with BB_NO_NETWORK=1 (the original motivation >> > from Ross Burton) would miss the issue entirely if only QA-time >> > warnings existed. This pattern is already established in the codebase >> > (e.g. src-uri-bad fires at both parse and QA time). >> > >> > Reported-by: Yoann Congal <[email protected]> >> > Fixes: https://bugzilla.yoctoproject.org/show_bug.cgi?id=16051 >> > AI-Generated: Developed with assistance from Anthropic Claude >> > Signed-off-by: Sai Sneha <[email protected]> >> > --- >> > >> > Changes in v4: >> > - Refactored to use shared oe.qa.check_uri_srcrev() helper >> > - Eliminates duplicated URI parsing logic >> > - Added docstring explaining True/False return values >> > >> > Changes in v3: >> > - Added AI-Generated disclosure and Reported-by tag >> > - Added design decision notes to commit message >> > >> > Changes in v2: >> > - Initial public submission >> > meta/classes-global/base.bbclass | 30 ++++++++++++++++++++++++++++-- >> > 1 file changed, 28 insertions(+), 2 deletions(-) >> > >> > diff --git a/meta/classes-global/base.bbclass >> b/meta/classes-global/base.bbclass >> > index 62f2814bb7..7de896ca7d 100644 >> > --- a/meta/classes-global/base.bbclass >> > +++ b/meta/classes-global/base.bbclass >> > @@ -164,14 +164,40 @@ def setup_hosttools_dir(dest, toolsvar, d, >> fatal=True): >> > >> > if notfound and fatal: >> > bb.fatal("The following required tools (as specified by >> HOSTTOOLS) appear to be unavailable in PATH, please install them in order >> to proceed:\n %s" % " ".join(notfound)) >> > + >> > +def check_srcrev_set(d): >> > + """ >> > + Inspect SRC_URI for SCM URIs missing a valid SRCREV. >> > + >> > + Returns True if all SCM URIs have a valid SRCREV or AUTOREV set, >> > + allowing get_hashvalue(d) to proceed normally. >> > + >> > + Returns False if any SCM URI is missing SRCREV, suppressing >> > + get_hashvalue(d) to prevent a FetchError parse crash. >> > + """ >> > + import oe.qa >> > + src_uri = (d.getVar('SRC_URI', False) or '').split() >> > + pn = d.getVar('PN') >> > + for uri in src_uri: >> > + rev = oe.qa.check_uri_srcrev(pn, uri, d) >> > + if rev is None: >> > + # SRCREV missing — warn and suppress hash expansion >> > + if bb.utils.contains('ERROR_QA', 'missing-srcrev', True, >> False, d): >> > + bb.error("%s: SRCREV not set for %s" % (pn, uri)) >> > + elif bb.utils.contains('WARN_QA', 'missing-srcrev', True, >> False, d): >> > + bb.warn("%s: SRCREV not set for %s" % (pn, uri)) >> > + return False >> > + if rev and '${AUTOREV}' in rev: >> > + return True >> > + return True >> > + >> > >> > # We can't use vardepvalue against do_fetch directly since that would >> overwrite >> > # the other task dependencies so we use an indirect function. >> > python fetcher_hashes_dummyfunc() { >> > return >> > } >> > -fetcher_hashes_dummyfunc[vardepvalue] = "${@bb.fetch.get_hashvalue(d)}" >> > - >> > +fetcher_hashes_dummyfunc[vardepvalue] = "${@bb.fetch.get_hashvalue(d) >> if check_srcrev_set(d) else ''}" >> >> How confident are you that this is a safe change to make? >> >> This is an area where I would not make AI-driven changes myself, as a >> change here could cause quite subtle issues. We may not have sufficient >> test coverage to catch such issues. >> >> Specifically, what happens if a SRC_URI has a 'tag=...' property, SRCREV >> is not set, and the tag is changed upstream? >> >> Best regards, >> >> -- >> Paul Barker >> >> > > >
-=-=-=-=-=-=-=-=-=-=-=- Links: You receive all messages sent to this group. View/Reply Online (#238008): https://lists.openembedded.org/g/openembedded-core/message/238008 Mute This Topic: https://lists.openembedded.org/mt/119510786/21656 Group Owner: [email protected] Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [[email protected]] -=-=-=-=-=-=-=-=-=-=-=-
