| Message ID | 20230825171625.1213968-1-pkj@axis.com |
|---|---|
| State | New |
| Headers | show |
| Series | base.bbclass: Do not fail during parsing if ${SRCREV} does not exist | expand |
On Fri, 2023-08-25 at 19:16 +0200, Peter Kjellerstedt wrote: > After commit a8e7b0f932 (base/package: Move source revision information > from PV to PKGV) was integrated, having a recipe with a SRCREV that > currently cannot be fetched would lead to an exception during parsing. > Catch that exception and instead raise bb.parse.SkipRecipe. That way > the parsing continues as it should. Instead you now get a meaningful > error if you try build a recipe with a SRCREV that cannot be fetched, > e.g.: > > ERROR: Nothing PROVIDES 'psplash' > psplash was skipped: Fetcher failure: Unable to resolve 'unknown-ref' > in upstream git repository in git ls-remote output for > git.yoctoproject.org/psplash Something doesn't sound quite right in this description. You've said "a SRCREV that currently cannot be fetched" but "unknown-ref" will never be fetchable and is completely invalid as a revision. I'd guess bitbake is assuming it is a tag and trying to resolve it. I really don't like complicating the core code when it doesn't make sense to, particularly when it relates to cornercases most people don't hit. I think SkipRecipe is also the incorrect thing to do here, particularly when no reason is being passed back to bitbake to give back to the user. I suspect if you set the SRCREV to something that looks like a revision, like "badbadbadbadbadbadbadbadbadbadbadbadbadb" it will avoid the problems. Alternatively, just add the SkipRecipe locally along with your unknown-ref. Cheers, Richard
FWIW: I have seen this in some meta-evil-bsp and the unfortunate part is that only the first parsing fatal error is shown at time, so to find all recipes which now need SkipRecipe, because your bsp vendor is evil, takes possibly many parsing cycles. But in the end nobody should work with evil vendors, so maybe we should take the sticks to them not to core. On Wed, Aug 30, 2023 at 11:04 AM Richard Purdie < richard.purdie@linuxfoundation.org> wrote: > On Fri, 2023-08-25 at 19:16 +0200, Peter Kjellerstedt wrote: > > After commit a8e7b0f932 (base/package: Move source revision information > > from PV to PKGV) was integrated, having a recipe with a SRCREV that > > currently cannot be fetched would lead to an exception during parsing. > > Catch that exception and instead raise bb.parse.SkipRecipe. That way > > the parsing continues as it should. Instead you now get a meaningful > > error if you try build a recipe with a SRCREV that cannot be fetched, > > e.g.: > > > > ERROR: Nothing PROVIDES 'psplash' > > psplash was skipped: Fetcher failure: Unable to resolve 'unknown-ref' > > in upstream git repository in git ls-remote output for > > git.yoctoproject.org/psplash > > Something doesn't sound quite right in this description. You've said "a > SRCREV that currently cannot be fetched" but "unknown-ref" will never > be fetchable and is completely invalid as a revision. I'd guess bitbake > is assuming it is a tag and trying to resolve it. > > I really don't like complicating the core code when it doesn't make > sense to, particularly when it relates to cornercases most people don't > hit. I think SkipRecipe is also the incorrect thing to do here, > particularly when no reason is being passed back to bitbake to give > back to the user. > > I suspect if you set the SRCREV to something that looks like a > revision, like "badbadbadbadbadbadbadbadbadbadbadbadbadb" it will avoid > the problems. Alternatively, just add the SkipRecipe locally along with > your unknown-ref. > > Cheers, > > Richard > > > -=-=-=-=-=-=-=-=-=-=-=- > Links: You receive all messages sent to this group. > View/Reply Online (#186874): > https://lists.openembedded.org/g/openembedded-core/message/186874 > Mute This Topic: https://lists.openembedded.org/mt/100960059/3617156 > Group Owner: openembedded-core+owner@lists.openembedded.org > Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [ > Martin.Jansa@gmail.com] > -=-=-=-=-=-=-=-=-=-=-=- > >
> -----Original Message----- > From: Richard Purdie <richard.purdie@linuxfoundation.org> > Sent: den 30 augusti 2023 11:04 > To: Peter Kjellerstedt <peter.kjellerstedt@axis.com>; openembedded-core@lists.openembedded.org > Subject: Re: [OE-core] [PATCH] base.bbclass: Do not fail during parsing if ${SRCREV} does not exist > > On Fri, 2023-08-25 at 19:16 +0200, Peter Kjellerstedt wrote: > > After commit a8e7b0f932 (base/package: Move source revision information > > from PV to PKGV) was integrated, having a recipe with a SRCREV that > > currently cannot be fetched would lead to an exception during parsing. > > Catch that exception and instead raise bb.parse.SkipRecipe. That way > > the parsing continues as it should. Instead you now get a meaningful > > error if you try build a recipe with a SRCREV that cannot be fetched, > > e.g.: > > > > ERROR: Nothing PROVIDES 'psplash' > > psplash was skipped: Fetcher failure: Unable to resolve 'unknown-ref' > > in upstream git repository in git ls-remote output for > > git.yoctoproject.org/psplash > > Something doesn't sound quite right in this description. You've said "a > SRCREV that currently cannot be fetched" but "unknown-ref" will never > be fetchable and is completely invalid as a revision. I'd guess bitbake > is assuming it is a tag and trying to resolve it. You are correct in that the problem only occurs when non-SHA1 references are used in SRCREV and that should probably be mentioned in the commit message. > I really don't like complicating the core code when it doesn't make > sense to, particularly when it relates to cornercases most people don't > hit. It is true that the use of non-SHA1 references in SRCREV is not recommended, but the bitbake code does have explicit support for it. And while we are working on changing our recipes to not use tags in SRCREV for released code, it is unfortunately not trivial to do so as we have other tooling that currently relies on being able to find the tag names in SRCREV. :( However, not doing anything to improve the OE code is not an option I would like to see, because the following is the output I get with the current code (modified to remove the actual names): WARNING: .../meta-axis/recipes-foo/bar/recipe1_git.bb: Exception during build_dependencies for fetcher_hashes_dummyfunc WARNING: .../meta-axis/recipes-foo/bar/recipe1_git.bb: Error during finalise of .../meta-axis/recipes-foo/bar/recipe1_git.bb WARNING: .../meta-axis/recipes-foo/bar/recipe2_git.bb: Exception during build_dependencies for fetcher_hashes_dummyfunc WARNING: .../meta-axis/recipes-foo/bar/recipe2_git.bb: Error during finalise of .../meta-axis/recipes-foo/bar/recipe2_git.bb WARNING: .../meta-axis/recipes-foo/bar/recipe3_git.bb: Exception during build_dependencies for fetcher_hashes_dummyfunc WARNING: .../meta-axis/recipes-foo/bar/recipe3_git.bb: Error during finalise of .../meta-axis/recipes-foo/bar/recipe3_git.bb WARNING: .../meta-axis/recipes-foo/bar/recipe4_git.bb: Exception during build_dependencies for fetcher_hashes_dummyfunc WARNING: .../meta-axis/recipes-foo/bar/recipe4_git.bb: Error during finalise of .../meta-axis/recipes-foo/bar/recipe4_git.bb WARNING: .../meta-axis/recipes-foo/bar/recipe5_git.bb: Exception during build_dependencies for fetcher_hashes_dummyfunc WARNING: .../meta-axis/recipes-foo/bar/recipe5_git.bb: Error during finalise of .../meta-axis/recipes-foo/bar/recipe5_git.bb WARNING: .../meta-axis/recipes-foo/bar/recipe6_git.bb: Exception during build_dependencies for fetcher_hashes_dummyfunc WARNING: .../meta-axis/recipes-foo/bar/recipe6_git.bb: Error during finalise of .../meta-axis/recipes-foo/bar/recipe6_git.bb WARNING: .../meta-axis/recipes-foo/bar/recipe7_git.bb: Exception during build_dependencies for fetcher_hashes_dummyfunc WARNING: .../meta-axis/recipes-foo/bar/recipe7_git.bb: Error during finalise of .../meta-axis/recipes-foo/bar/recipe7_git.bb WARNING: .../meta-axis/recipes-foo/bar/recipe8_git.bb: Exception during build_dependencies for fetcher_hashes_dummyfunc WARNING: .../meta-axis/recipes-foo/bar/recipe8_git.bb: Error during finalise of .../meta-axis/recipes-foo/bar/recipe8_git.bb WARNING: .../meta-axis/recipes-foo/bar/recipe9_git.bb: Exception during build_dependencies for fetcher_hashes_dummyfunc WARNING: .../meta-axis/recipes-foo/bar/recipe9_git.bb: Error during finalise of .../meta-axis/recipes-foo/bar/recipe9_git.bb WARNING: .../meta-axis/recipes-foo/bar/recipe10_git.bb: Exception during build_dependencies for fetcher_hashes_dummyfunc WARNING: .../meta-axis/recipes-foo/bar/recipe10_git.bb: Error during finalise of .../meta-axis/recipes-foo/bar/recipe10_git.bb WARNING: .../meta-axis/recipes-foo/bar/recipe11_git.bb: Exception during build_dependencies for fetcher_hashes_dummyfunc WARNING: .../meta-axis/recipes-foo/bar/recipe11_git.bb: Error during finalise of .../meta-axis/recipes-foo/bar/recipe11_git.bb WARNING: .../meta-axis/recipes-foo/bar/recipe12_git.bb: Exception during build_dependencies for fetcher_hashes_dummyfunc WARNING: .../meta-axis/recipes-foo/bar/recipe12_git.bb: Error during finalise of .../meta-axis/recipes-foo/bar/recipe12_git.bb WARNING: .../meta-axis/recipes-foo/bar/recipe13_git.bb: Exception during build_dependencies for fetcher_hashes_dummyfunc WARNING: .../meta-axis/recipes-foo/bar/recipe13_git.bb: Error during finalise of .../meta-axis/recipes-foo/bar/recipe13_git.bb WARNING: .../meta-axis/recipes-foo/bar/recipe14_git.bb: Exception during build_dependencies for fetcher_hashes_dummyfunc WARNING: .../meta-axis/recipes-foo/bar/recipe14_git.bb: Error during finalise of .../meta-axis/recipes-foo/bar/recipe14_git.bb ERROR: ExpansionError during parsing .../meta-axis/recipes-foo/bar/recipe2_git.bb Traceback (most recent call last): File "/home/pkj/dists/cfp-master/poky/bitbake/lib/bb/fetch2/__init__.py", line 1718, in Fetch.__init__(urls=['git://gitserver.axis.com:29418/recipe2.git;protocol=ssh;nobranch=1'], d=<bb.data_smart.DataSmart object at 0x7f55f95cd2a0>, cache=True, localonly=False, connection_cache=None): try: > self.ud[url] = FetchData(url, d, localonly) except NonLocalMethod: File "/home/pkj/dists/cfp-master/poky/bitbake/lib/bb/fetch2/__init__.py", line 1347, in FetchData.__init__(url='git://gitserver.axis.com:29418/recipe2.git;protocol=ssh;nobranch=1', d=<bb.data_smart.DataSmart object at 0x7f55f95cd2a0>, localonly=False): if hasattr(self.method, "urldata_init"): > self.method.urldata_init(self, d) File "/home/pkj/dists/cfp-master/poky/bitbake/lib/bb/fetch2/git.py", line 271, in Git.urldata_init(ud=<bb.fetch2.FetchData object at 0x7f5606ba3880>, d=<bb.data_smart.DataSmart object at 0x7f55f95cd2a0>): ud.unresolvedrev[name] = ud.revisions[name] > ud.revisions[name] = self.latest_revision(ud, d, name) File "/home/pkj/dists/cfp-master/poky/bitbake/lib/bb/fetch2/__init__.py", line 1662, in Git.latest_revision(ud=<bb.fetch2.FetchData object at 0x7f5606ba3880>, d=<bb.data_smart.DataSmart object at 0x7f55f95cd2a0>, name='default'): except KeyError: > revs[key] = rev = self._latest_revision(ud, d, name) return rev File "/home/pkj/dists/cfp-master/poky/bitbake/lib/bb/fetch2/git.py", line 799, in Git._latest_revision(ud=<bb.fetch2.FetchData object at 0x7f5606ba3880>, d=<bb.data_smart.DataSmart object at 0x7f55f95cd2a0>, name='default'): return sha1 > raise bb.fetch2.FetchError("Unable to resolve '%s' in upstream git repository in git ls-remote output for %s" % \ (ud.unresolvedrev[name], ud.host+ud.path)) bb.data_smart.ExpansionError: Failure expanding variable fetcher_hashes_dummyfunc[vardepvalue], expression was ${@bb.fetch.get_hashvalue(d)} which triggered exception FetchError: Fetcher failure: Unable to resolve 'v1.2.3' in upstream git repository in git ls-remote output for gitserver.axis.com:29418/recipe2.git The variable dependency chain for the failure is: fetcher_hashes_dummyfunc[vardepvalue] That is not very user friendly, especially as the user running into this may not even be interested in building any of the failing recipes. And since this happens during parsing, it also prevents the user from, e.g., running `bitbake -e` on the failing recipes. > I think SkipRecipe is also the incorrect thing to do here, > particularly when no reason is being passed back to bitbake to give > back to the user. After having given this some more thought and testing, I believe it actually is as simple as to just ignore the FetchError instead with: def fetcher_get_hashvalue(d): # Catch and ignore any fetcher errors here to avoid errors during recipe # parsing due to references (e.g., tags) that cannot be retrieved. Any # error will be reported if the fetch task for the recipe is run. try: return bb.fetch.get_hashvalue(d) except bb.fetch2.FetchError: return "" With that applied, I get the exact same behavior as we have in Mickledore, i.e., no errors during recipe parsing, and the following error if I actually try to build one of the recipes that cannot be fetched (with an unknown reference): ERROR: psplash-0.1+git-r0 do_fetch: Bitbake Fetcher Error: FetchError("Unable to resolve 'unknown-ref' in upstream git repository in git ls-remote output for git.yoctoproject.org/psplash", None) or with an unknown SHA-1: ERROR: psplash-0.1+git-r0 do_fetch: Fetcher failure: Unable to find revision badbadbadbadbadbadbadbadbadbadbadbadbadb in branch master even from upstream ERROR: psplash-0.1+git-r0 do_fetch: Bitbake Fetcher Error: FetchError('Unable to fetch URL from any source.', 'git://git.yoctoproject.org/psplash;branch=master;protocol=https') An alternative to adding the fetcher_get_hashvalue() function to base.bbclass would be to add a safe version of bb.fetch.get_hashvalue() that does not raise FetchError and thus can be used directly in the fetcher_hashes_dummyfunc[vardepvalue], e.g.: def get_safe_hashvalue(d, method_name='sortable_revision'): try: return get_hashvalue(d, method_name=method_name) except FetchError: return "" Another alternative would be to be able to pass an option to get_hashvalue() to ignore errors, e.g.: def get_hashvalue(d, method_name='sortable_revision', ignore_errors=False): try: pkgv, revs = _get_srcrev(d, method_name=method_name) return " ".join(revs) except FetchError as e: if ignore_errors: return "" raise e I guess that the most Pythonic is to have the function in base.bbclass, but from a Bitbake perspective it may be cleaner to keep the error handling in the fetcher. My personal choice would be the last alternative. > I suspect if you set the SRCREV to something that looks like a > revision, like "badbadbadbadbadbadbadbadbadbadbadbadbadb" it will avoid > the problems. Alternatively, just add the SkipRecipe locally along with > your unknown-ref. > > Cheers, > > Richard I hope this can persuade you to accept an updated patch with either of the suggested alternatives above as I believe this is an improvement to the code and not just complicating it. //Peter
diff --git a/meta/classes-global/base.bbclass b/meta/classes-global/base.bbclass index 7c774d250f..fb55a6eee9 100644 --- a/meta/classes-global/base.bbclass +++ b/meta/classes-global/base.bbclass @@ -131,7 +131,13 @@ def setup_hosttools_dir(dest, toolsvar, d, fatal=True): python fetcher_hashes_dummyfunc() { return } -fetcher_hashes_dummyfunc[vardepvalue] = "${@bb.fetch.get_hashvalue(d)}" +fetcher_hashes_dummyfunc[vardepvalue] = "${@validate_hashvalue(d)}" + +def validate_hashvalue(d): + try: + bb.fetch.get_hashvalue(d) + except bb.fetch2.FetchError as e: + raise bb.parse.SkipRecipe(e) addtask fetch do_fetch[dirs] = "${DL_DIR}"
After commit a8e7b0f932 (base/package: Move source revision information from PV to PKGV) was integrated, having a recipe with a SRCREV that currently cannot be fetched would lead to an exception during parsing. Catch that exception and instead raise bb.parse.SkipRecipe. That way the parsing continues as it should. Instead you now get a meaningful error if you try build a recipe with a SRCREV that cannot be fetched, e.g.: ERROR: Nothing PROVIDES 'psplash' psplash was skipped: Fetcher failure: Unable to resolve 'unknown-ref' in upstream git repository in git ls-remote output for git.yoctoproject.org/psplash Signed-off-by: Peter Kjellerstedt <peter.kjellerstedt@axis.com> --- meta/classes-global/base.bbclass | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-)