Hi all, Thank you for the thorough review. I clearly need to step back and rethink the approach before sending more patches. I will continue the discussion on the bug tracker.
Best regards, Sai Sneha On Wed, 3 Jun 2026 at 1:20 AM, Peter Kjellerstedt < [email protected]> wrote: > I was under the impression that a tag= in the SRC_URI is there to > complement the SRCREV, not replace it. I.e., SRCREV should always exist, > and if a tag= also exists then it is verified that it matches the SRCREV. > > > > //Peter > > > > *From:* [email protected] < > [email protected]> *On Behalf Of *Sai Sneha via > lists.openembedded.org > *Sent:* den 2 juni 2026 08:39 > *To:* Paul Barker <[email protected]> > *Cc:* [email protected]; [email protected]; > [email protected]; [email protected] > *Subject:* Re: [OE-core] [PATCH v4 2/4] base.bbclass: warn when SRCREV is > missing for SCM URIs at parse time > > > > 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 (#238082): https://lists.openembedded.org/g/openembedded-core/message/238082 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]] -=-=-=-=-=-=-=-=-=-=-=-
