Hi all, After reading through all the mailing list feedback carefully and spending time studying the bitbake fetcher code, I think I now have a better picture of what went wrong with my approach. I wanted to summarize my understanding before attempting v6.
The core problem with v5: The bigger issue is that intercepting at the vardepvalue level in base.bbclass was the wrong approach entirely. when I suppress the hash computation for recipes with missing SRCREV I am also accidentally breaking something important bitbake uses that hash to know when to re-run do_fetch. So if a branch or tag changes upstream do_fetch would silently not re-run which is worse than the original problem. Other issues I missed: - Using d.getVar(candidate, False) misses MACHINE/DISTRO overrides. Paul is right that override resolution is the bitbake parser's job, not something OE-core should be reimplementing. - Hardcoding git/gitsm/hg/svn was fragile. bitbake already has supports_srcrev() on each fetcher class for exactly this purpose. - The parse-time warning was wrong. src-uri-bad is purely a QA-time check I should not have added parse-time warnings. - The problem is broader than just missing SRCREV as a SRCREV set to a branch or tag name instead of a commit hash causes the same reproducibility issues. - The tag= skip was wrong since the selftest recipe should use INSANE_SKIP instead of skipping tag= globally. - The CHECKLAYER_REQUIRED_TESTS decision needs proper discussion not a unilateral call in the cover letter. Where I think the fix should go: Richard suggested fixing this in the bitbake fetchers themselves using supports_srcrev(). I am not sure if that means changes to the BitBake repository itself, or just using supports_srcrev() within OE-core. Could someone clarify the right direction? My current thinking for OE-core is a simple QA check in insane.bbclass using oe.qa.handle_error() with supports_srcrev() to identify which URIs need checking, along with fixing the gitunpackoffline selftest with INSANE_SKIP. But I want to make sure this aligns with what the maintainers have in mind before writing more code. Thanks again for the thorough review it was genuinely helpful. Sai Sneha On Tue, 2 Jun 2026 at 1:56 PM, Yoann Congal <[email protected]> wrote: > On Tue Jun 2, 2026 at 8:29 AM CEST, Sai Sneha wrote: > > Add a proper QA check test_missing_srcrev() in do_recipe_qa that > > fires at build time via oe.qa.handle_error(), using the shared > > oe.qa.check_uri_srcrev() helper to avoid code duplication. > > > > Severity is controlled per layer: > > - WARN_QA:append = " missing-srcrev" for all layers (warning by default) > > - ERROR_QA:append:layer-core = " missing-srcrev" for oe-core (strict) > > > > This follows the same pattern as missing-metadata and missing-maintainer > > which are warnings globally but errors for oe-core. > > > > Note on CHECKLAYER_REQUIRED_TESTS: missing-srcrev is intentionally > > not added to CHECKLAYER_REQUIRED_TESTS because that would promote it > > to ERROR_QA for all layers globally during normal builds, contradicting > > the layer-specific enforcement requested in Bug 16051 Comment 3. > > Instead, yocto-check-layer.bbclass enforces it directly during > > checklayer validation runs. > > Neither CHECKLAYER_REQUIRED_TESTS nor yocto-check-layer does not appear > in this patch. Maybe move this note to the next patch? > > > > > 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 v5: > > - No changes to this patch > > > > Changes in v4: > > - Refactored to use shared oe.qa.check_uri_srcrev() helper > > - Eliminates duplicated URI parsing logic > > - Added note explaining why CHECKLAYER_REQUIRED_TESTS is not used > > > > Changes in v3: > > - Added AI-Generated disclosure and Reported-by tag > > > > Changes in v2: > > - Initial public submission > > meta/classes-global/insane.bbclass | 13 +++++++++++++ > > 1 file changed, 13 insertions(+) > > > > diff --git a/meta/classes-global/insane.bbclass > b/meta/classes-global/insane.bbclass > > index 04700be71c..1fe426646d 100644 > > --- a/meta/classes-global/insane.bbclass > > +++ b/meta/classes-global/insane.bbclass > > @@ -49,6 +49,9 @@ ERROR_QA ?= "\ > > ERROR_QA:append = " ${@bb.utils.filter('DISTRO_FEATURES', 'usrmerge', > d)}" > > WARN_QA:append:layer-core = " missing-metadata missing-maintainer" > > > > +WARN_QA:append = " missing-srcrev" > > +ERROR_QA:append:layer-core = " missing-srcrev" > > + > > FAKEROOT_QA = "host-user-contaminated" > > FAKEROOT_QA[doc] = "QA tests which need to run under fakeroot. If any \ > > enabled tests are listed here, the do_package_qa task will run under > fakeroot." > > @@ -1455,6 +1458,15 @@ python do_qa_unpack() { > > python do_recipe_qa() { > > import re > > > > + def test_missing_srcrev(pn, d): > > + import oe.qa > > + src_uri = (d.getVar('SRC_URI', False) or '').split() > > + for uri in src_uri: > > + rev = oe.qa.check_uri_srcrev(pn, uri, d) > > + if rev is None: > > + oe.qa.handle_error("missing-srcrev", > > + "%s: SRCREV not set for SCM URI %s" % (pn, uri), d) > > + > > def test_naming(pn, d): > > if pn.endswith("-native") and not > bb.data.inherits_class("native", d): > > oe.qa.handle_error("recipe-naming", "Recipe %s appears > native but is not, should inherit native" % pn, d) > > @@ -1497,6 +1509,7 @@ python do_recipe_qa() { > > > > pn = d.getVar('PN') > > test_naming(pn, d) > > + test_missing_srcrev(pn, d) > > test_missing_metadata(pn, d) > > test_missing_maintainer(pn, d) > > test_srcuri(pn, d) > > > -- > Yoann Congal > Smile ECS > >
-=-=-=-=-=-=-=-=-=-=-=- Links: You receive all messages sent to this group. View/Reply Online (#238140): https://lists.openembedded.org/g/openembedded-core/message/238140 Mute This Topic: https://lists.openembedded.org/mt/119607376/21656 Group Owner: [email protected] Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [[email protected]] -=-=-=-=-=-=-=-=-=-=-=-
