| Message ID | 20260519100904.235971-1-saisneha196@gmail.com |
|---|---|
| Headers | show |
| Series | Add missing SRCREV check for SCM URIs | expand |
On Tue, 2026-05-19 at 15:39 +0530, Sai Sneha via lists.openembedded.org wrote: > This patch series adds a check for git:// (and other SCM) URIs in > SRC_URI that are missing a corresponding SRCREV, addressing > Bugzilla #16051 [1] and building on Corentin Guillevic's RFC series [2]. > > == Problem == > > A recipe with a git:// URI and no SRCREV causes two problems: > > 1. Under BB_NO_NETWORK=1, parsing fails with a FetchError that halts > parsing of ALL recipes, not just the offending one. This breaks CI > pipelines that use "bitbake --parse-only" as a sanity check, which > was the original motivation reported by Ross Burton [3]. > > 2. Without BB_NO_NETWORK, BitBake silently queries the remote > repository on every parse, breaking reproducibility. > > == Why the trivial fix doesn't work == > > The obvious fix of checking SRCREV in insane.bbclass do_recipe_qa > was attempted by Corentin Guillevic [2] but fails because > fetcher_hashes_dummyfunc[vardepvalue] expands SRCREV at parse time > BEFORE QA checks run. This triggers get_autorev() which causes: > > "AUTOREV/SRCPV set too late for the fetcher to work properly" > > This was confirmed by Mathieu Dubois-Briand's autobuilder testing [2]. > Corentin's series needed 4 patches including devtool/recipetool > workarounds to paper over the expansion crash. > > == This series' approach == > > We intercept at the vardepvalue expansion point itself in base.bbclass: > > fetcher_hashes_dummyfunc[vardepvalue] = \ > "${@bb.fetch.get_hashvalue(d) if check_srcrev_set(d) else ''}" > > check_srcrev_set() uses d.getVar(candidate, False) — the False flag > prevents expansion entirely, so get_autorev() is never triggered and > no network access occurs. This solves the root cause in one patch > without needing devtool/recipetool changes. > > == Design decisions == > > 1. Why not use srcrev_internal_helper()? > > srcrev_internal_helper() expands SRCREV internally, which triggers > get_autorev() and live git ls-remote calls — exactly what we are > trying to prevent. We intentionally use d.getVar(candidate, False) > instead. Our fallback chain mirrors BitBake's resolution order > exactly, as confirmed by BitBake's own error messages: > SRCREV_default:pn-X, SRCREV_default, SRCREV:pn-X, SRCREV > > 2. Why duplicate warnings at parse time AND QA time? > > Parse-time warnings (base.bbclass) are necessary for CI pipelines > running "bitbake --parse-only" with BB_NO_NETWORK=1 — they would > miss the issue entirely if only QA-time warnings existed. > QA-time warnings (insane.bbclass) are necessary for proper > WARN_QA/ERROR_QA layer control and QA framework integration. > This pattern is already established — src-uri-bad fires at both > parse and QA time. > > 3. Why hardcode git/gitsm/hg/svn schemes? > > These are the fetchers that require SRCREV. This is a known > limitation — if new SCM fetchers are added in future, this list > will need updating. Noted as a follow-on improvement. > > == Enforcement levels == > > Normal builds, all layers → WARN_QA (warning, build continues) > oe-core layer → ERROR_QA (error, strict enforcement) > yocto-check-layer runs → ERROR (layer validation fails) > > This matches the graduated enforcement requested in Bug 16051 > Comments 2 and 3. > > == Testing == > > Tested on openembedded-core with: > > - Recipe with missing SRCREV → WARNING/ERROR at parse time ✓ > - Recipe with SRCREV = "${AUTOREV}" → silent passthrough ✓ > - Recipe with rev= in URI → no false positive ✓ > - Recipe with multiple SCM URIs → all missing SRCREVs reported ✓ > - oe-core recipe → ERROR instead of WARNING ✓ > - yocto-check-layer on layer with missing SRCREV → FAIL ✓ > - 952 other recipes parse cleanly → 0 errors ✓ > > [1] https://bugzilla.yoctoproject.org/show_bug.cgi?id=16051 > [2] https://lore.kernel.org/openembedded-core/20260304104342.869457-1-corentin.guillevic@smile.fr/ > [3] https://lists.openembedded.org/g/openembedded-devel/topic/115911777 > > Sai Sneha (3): > base.bbclass: warn when SRCREV is missing for SCM URIs at parse time > insane.bbclass: add missing-srcrev QA check for SCM URIs > yocto-check-layer.bbclass: enforce missing SRCREV check during layer validation > > meta/classes-global/base.bbclass | 40 +++++++++++++++++++ > meta/classes-global/insane.bbclass | 32 +++++++++++++++ > meta/classes-global/yocto-check-layer.bbclass| 32 +++++++++++++++ > 3 files changed, 104 insertions(+), 2 deletions(-) Hi Sai, Why do we need to implement the same test three times? Even if the test needs to run from different places, we should be able to factor out common code to a function. And we shouldn't need to re-implement this in yocto-check-layer anyway, Yoann's comment in bugzilla said to add the QA check to CHECKLAYER_REQUIRED_TESTS [1]. Patches 3 & 4 of Corentin's series [2] are not workarounds, they are ensuring that recipes modified by recipetool or devtool only use ${AUTOREV} if it is appropriate. [1]: https://bugzilla.yoctoproject.org/show_bug.cgi?id=16051#c2 [2]: https://lore.kernel.org/openembedded-core/20260304104342.869457-1-corentin.guillevic@smile.fr/T/ Best regards,
This patch series adds a check for git:// (and other SCM) URIs in SRC_URI that are missing a corresponding SRCREV, addressing Bugzilla #16051 [1] and building on Corentin Guillevic's RFC series [2]. == Problem == A recipe with a git:// URI and no SRCREV causes two problems: 1. Under BB_NO_NETWORK=1, parsing fails with a FetchError that halts parsing of ALL recipes, not just the offending one. This breaks CI pipelines that use "bitbake --parse-only" as a sanity check, which was the original motivation reported by Ross Burton [3]. 2. Without BB_NO_NETWORK, BitBake silently queries the remote repository on every parse, breaking reproducibility. == Why the trivial fix doesn't work == The obvious fix of checking SRCREV in insane.bbclass do_recipe_qa was attempted by Corentin Guillevic [2] but fails because fetcher_hashes_dummyfunc[vardepvalue] expands SRCREV at parse time BEFORE QA checks run. This triggers get_autorev() which causes: "AUTOREV/SRCPV set too late for the fetcher to work properly" This was confirmed by Mathieu Dubois-Briand's autobuilder testing [2]. Corentin's series needed 4 patches including devtool/recipetool workarounds to paper over the expansion crash. == This series' approach == We intercept at the vardepvalue expansion point itself in base.bbclass: fetcher_hashes_dummyfunc[vardepvalue] = \ "${@bb.fetch.get_hashvalue(d) if check_srcrev_set(d) else ''}" check_srcrev_set() uses d.getVar(candidate, False) — the False flag prevents expansion entirely, so get_autorev() is never triggered and no network access occurs. This solves the root cause in one patch without needing devtool/recipetool changes. == Design decisions == 1. Why not use srcrev_internal_helper()? srcrev_internal_helper() expands SRCREV internally, which triggers get_autorev() and live git ls-remote calls — exactly what we are trying to prevent. We intentionally use d.getVar(candidate, False) instead. Our fallback chain mirrors BitBake's resolution order exactly, as confirmed by BitBake's own error messages: SRCREV_default:pn-X, SRCREV_default, SRCREV:pn-X, SRCREV 2. Why duplicate warnings at parse time AND QA time? Parse-time warnings (base.bbclass) are necessary for CI pipelines running "bitbake --parse-only" with BB_NO_NETWORK=1 — they would miss the issue entirely if only QA-time warnings existed. QA-time warnings (insane.bbclass) are necessary for proper WARN_QA/ERROR_QA layer control and QA framework integration. This pattern is already established — src-uri-bad fires at both parse and QA time. 3. Why hardcode git/gitsm/hg/svn schemes? These are the fetchers that require SRCREV. This is a known limitation — if new SCM fetchers are added in future, this list will need updating. Noted as a follow-on improvement. == Enforcement levels == Normal builds, all layers → WARN_QA (warning, build continues) oe-core layer → ERROR_QA (error, strict enforcement) yocto-check-layer runs → ERROR (layer validation fails) This matches the graduated enforcement requested in Bug 16051 Comments 2 and 3. == Testing == Tested on openembedded-core with: - Recipe with missing SRCREV → WARNING/ERROR at parse time ✓ - Recipe with SRCREV = "${AUTOREV}" → silent passthrough ✓ - Recipe with rev= in URI → no false positive ✓ - Recipe with multiple SCM URIs → all missing SRCREVs reported ✓ - oe-core recipe → ERROR instead of WARNING ✓ - yocto-check-layer on layer with missing SRCREV → FAIL ✓ - 952 other recipes parse cleanly → 0 errors ✓ [1] https://bugzilla.yoctoproject.org/show_bug.cgi?id=16051 [2] https://lore.kernel.org/openembedded-core/20260304104342.869457-1-corentin.guillevic@smile.fr/ [3] https://lists.openembedded.org/g/openembedded-devel/topic/115911777 Sai Sneha (3): base.bbclass: warn when SRCREV is missing for SCM URIs at parse time insane.bbclass: add missing-srcrev QA check for SCM URIs yocto-check-layer.bbclass: enforce missing SRCREV check during layer validation meta/classes-global/base.bbclass | 40 +++++++++++++++++++ meta/classes-global/insane.bbclass | 32 +++++++++++++++ meta/classes-global/yocto-check-layer.bbclass| 32 +++++++++++++++ 3 files changed, 104 insertions(+), 2 deletions(-)