Hi all,
Following up on the suggestions to fix this in the BitBake
fetchers using supports_srcrev(), I have tried implementing that
approach locally. Here is what I did and how it works.
The fix has two parts:
Part 1 — BitBake fix (prevent parse crash):
The crash happens in setup_revisions() when srcrev_internal_helper()
raises a FetchError for missing SRCREV. This propagates up through
expandWithRefs() as an ExpansionError which halts parsing for all
recipes. The fix catches FetchError in setup_revisions() and sets
a missing_srcrev flag on the FetchData object:
def setup_revisions(self, d):
try:
self.revision = srcrev_internal_helper(self, d, self.name)
except FetchError:
self.revision = None
self.missing_srcrev = True
git.py and hg.py also needed a guard to prevent calling
latest_revision() when missing_srcrev is set, since that would
trigger a live network fetch under BB_NO_NETWORK=1.
Part 2 — OE-core QA check (insane.bbclass):
A simple test_missing_srcrev() in do_recipe_qa uses supports_srcrev()
to identify which URIs need checking:
def test_missing_srcrev(pn, d):
import bb.fetch2
try:
fetcher = bb.fetch2.Fetch(
(d.getVar('SRC_URI') or '').split(), d)
for url in fetcher.ud:
ud = fetcher.ud[url]
if ud.method.supports_srcrev() and \
getattr(ud, 'missing_srcrev', False):
oe.qa.handle_error("missing-srcrev",
"%s: SRCREV not set for SCM URI %s" % (pn, url), d)
except bb.fetch2.FetchError:
pass
Test results:
$ bitbake -p 2>&1 | grep -E "(WARNING|ERROR|SRCREV)"
WARNING: You are running bitbake under WSLv2...
Summary: There was 1 WARNING message.
(no parse crash, all recipes parse cleanly)
$ bitbake -c recipe_qa -f test-fetcher 2>&1 | grep -E
"(WARNING|ERROR|missing-srcrev)"
WARNING: test-fetcher-1.0-r0 do_recipe_qa: QA Issue: test-fetcher:
SRCREV not set for SCM URI git://github.com/example/project.git
[missing-srcrev]
$ bitbake -c recipe_qa test-srcrev 2>&1 | grep -E
"(WARNING|ERROR|missing-srcrev)"
ERROR: test-srcrev-1.0-r0 do_recipe_qa: QA Issue: test-srcrev:
SRCREV not set for SCM URI git://github.com/example/project.git
[missing-srcrev]
Selftest results:
test_git_unpack_nonetwork PASSED
test_git_unpack_nonetwork_fail PASSED
test_git_patchtool PASSED
test_git_patchtool2 PASSED
test_just_parse PASSED
test_warnings_errors PASSED
test_invalid_recipe_src_uri PASSED
Changes are minimal:
BitBake: 3 files, 8 insertions, 3 deletions
OE-core: 1 file, 16 insertions, 1 deletion
Does this approach align with what was suggested? I am happy to
refine it further based on the feedback.
Best regards,
Sai Sneha
On Fri, 5 Jun 2026 at 11:45 AM, Sai Sneha via lists.openembedded.org
<[email protected]> wrote:
> 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 (#238282):
https://lists.openembedded.org/g/openembedded-core/message/238282
Mute This Topic: https://lists.openembedded.org/mt/119719972/21656
Group Owner: [email protected]
Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub
[[email protected]]
-=-=-=-=-=-=-=-=-=-=-=-
