Bug 16051 - Add a check for git:// SRC_URI without a SRCREV
Summary: Add a check for git:// SRC_URI without a SRCREV
Status: IN PROGRESS DESIGN
Alias: None
Product: Build Testing
Classification: QA/Testing
Component: sanity (show other bugs)
Version: unspecified
Hardware: Other Multiple
: Medium+ enhancement
Target Milestone: 6.1 M1
Assignee: Sai Sneha
QA Contact:
URL:
Whiteboard:
Depends on:
Blocks:
 
Reported: 2025-10-31 15:02 UTC by Yoann Congal
Modified: 2026-07-21 09:22 UTC (History)
2 users (show)

See Also:
OS type for building Yocto: ---
Type of Regression: ---
Verified:
Documentation change: Yes (doc changes required)


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Yoann Congal 2025-10-31 15:02:56 UTC
A git:// SRC_URI without a SRCREV triggers a parse error under BB_NO_NETWORK, break the mirroring tests and hit upstream servers on each build. While this might be acceptable for extern layers, we try to avoid that for YP layers (oe-core and meta-openembedded).

A recipe QA check will help avoid that.

Ref: https://lists.openembedded.org/g/openembedded-devel/topic/115911777#msg121235
Comment 1 Yoann Congal 2025-10-31 15:50:44 UTC
Note: it would be nice to add this check to the Yocto check-layer test set.
Comment 2 Yoann Congal 2025-10-31 18:20:33 UTC
Idea: add a QA check outside of checklayer then add it to CHECKLAYER_REQUIRED_TESTS
This way users can still benefit from the QA check without running checklayer.
Comment 3 Yoann Congal 2025-11-06 15:44:25 UTC
Idea: Use "layer-specific" QA checks to enforce it on oe-core (and most likely meta-openembedded) but not by default.
Comment 4 Yoann Congal 2026-03-06 17:02:13 UTC
The trivial implementation of this enhancement seem to have triggered underlying problems: Corentin has posted a RFC series about this:
[RFC PATCH 0/4] Clean AUTOREV usage, add revision check for every SCM URI
https://lore.kernel.org/openembedded-core/20260304104342.869457-1-corentin.guillevic@smile.fr/T/#u
Comment 5 Yoann Congal 2026-04-03 09:25:41 UTC
(Unassigning myself since a do not realistically have the time to work on this)
Comment 6 Sai Sneha 2026-05-12 05:36:29 UTC
Hi, I would like to work on this issue as part of my Yocto contribution work. I have been reviewing the discussion around the missing SRCREV QA check, AUTOREV handling, and the related RFC patch series from Corentin Guillevic. I am currently reproducing the issue locally and analyzing the interactions between SRCREV expansion, SCM URIs, and fetcher behavior. If there are no objections, I would like to work on proposing a fix for this bug.
Comment 7 Yoann Congal 2026-05-12 07:10:18 UTC
Hi Sai,

Thank you for working on this :)

I'm assigning the ticket to you to make it official.
Comment 8 Sai Sneha 2026-06-04 06:51:42 UTC
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 much clearer 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
Comment 9 Randy MacLeod 2026-06-04 14:25:54 UTC
Sai, Please also post your last comment/sumary to the email thread 
if you haven't do already.
Comment 10 Sai Sneha 2026-06-05 06:13:00 UTC
Ok Sure, Randy
Comment 11 Sai Sneha 2026-06-09 09:07:51 UTC
Following up on suggestions to fix this in the BitBake 
fetchers using supports_srcrev() and I have tried ipmlementing 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 feedback.
Comment 12 Yoann Congal 2026-06-15 13:21:46 UTC
Link to the work in progress patch:
[PATCH v5 0/4] Add missing SRCREV check for SCM URIs - Sai Sneha
https://lore.kernel.org/all/20260602062940.3107241-1-saisneha196@gmail.com/
Comment 13 Sai Sneha 2026-07-21 09:22:08 UTC
Hi all,

I will not be able to continue working on this bug and am 
unassigning myself so someone else can pick it up.

Thank you all for the guidance.

Sai Sneha