diff mbox series

[2/2] fetch: bb.fatal when trying to checksum non-existing files.

Message ID 20220708205407.1680137-2-ptsneves@gmail.com
State New
Headers show
Series [1/2] fetch2: local files only in DL_DIR becomes fatal error | expand

Commit Message

Paulo Neves July 8, 2022, 8:54 p.m. UTC
If the local fetcher was not able to find the file anywhere but it
was included in the SRC_URI for checksumming just make it a fatal
error.
---
 lib/bb/fetch2/__init__.py | 2 +-
 lib/bb/tests/fetch.py     | 5 +++++
 2 files changed, 6 insertions(+), 1 deletion(-)

Comments

Alexandre Belloni July 13, 2022, 9:48 a.m. UTC | #1
Hello,

I believe this is the cause of the following failure:

2022-07-13 08:32:51,511 - oe-selftest - INFO - bbtests.BitbakeTests.test_invalid_recipe_src_uri (subunit.RemotedTestCase)
2022-07-13 08:32:51,512 - oe-selftest - INFO -  ... FAIL
2022-07-13 08:32:51,512 - oe-selftest - INFO - 3: 18/58 119/493 (3.15s) (bbtests.BitbakeTests.test_invalid_recipe_src_uri)
2022-07-13 08:32:51,512 - oe-selftest - INFO - testtools.testresult.real._StringException: Traceback (most recent call last):
  File "/home/pokybuild/yocto-worker/oe-selftest-centos/build/meta/lib/oeqa/selftest/cases/bbtests.py", line 148, in test_invalid_recipe_src_uri
    bitbake('-ccleanall man-db')
  File "/home/pokybuild/yocto-worker/oe-selftest-centos/build/meta/lib/oeqa/utils/commands.py", line 229, in bitbake
    return runCmd(cmd, ignore_status, timeout, output_log=output_log, **options)
  File "/home/pokybuild/yocto-worker/oe-selftest-centos/build/meta/lib/oeqa/utils/commands.py", line 207, in runCmd
    raise AssertionError("Command '%s' returned non-zero exit status %d:\n%s" % (command, result.status, exc_output))
AssertionError: Command 'bitbake  -ccleanall man-db' returned non-zero exit status 1:
/usr/lib64/python3.6/importlib/_bootstrap.py:219: ImportWarning: can't resolve package from __spec__ or __package__, falling back on __name__ and __path__
  return f(*args, **kwds)
NOTE: Reconnecting to bitbake server...
Loading cache...done.
Loaded 1718 entries from dependency cache.
Parsing recipes...ERROR: /home/pokybuild/yocto-worker/oe-selftest-centos/build/meta/recipes-extended/man-db/man-db_2.10.2.bb: Unable to get checksum for man-db SRC_URI entry invalid: file could not be found
ERROR: Parsing halted due to errors, see error messages above

I guess you have to update
bbtests.BitbakeTests.test_invalid_recipe_src_uri too

On 08/07/2022 22:54:07+0200, Paulo Neves wrote:
> If the local fetcher was not able to find the file anywhere but it
> was included in the SRC_URI for checksumming just make it a fatal
> error.
> ---
>  lib/bb/fetch2/__init__.py | 2 +-
>  lib/bb/tests/fetch.py     | 5 +++++
>  2 files changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/lib/bb/fetch2/__init__.py b/lib/bb/fetch2/__init__.py
> index 5f05278a..8184b55e 100644
> --- a/lib/bb/fetch2/__init__.py
> +++ b/lib/bb/fetch2/__init__.py
> @@ -1237,7 +1237,7 @@ def get_checksum_file_list(d):
>                              " This means there is no way to get the file except for an orphaned copy"
>                              " in DL_DIR.") % (d.getVar('PN'), os.path.basename(f)))
>                      else:
> -                        bb.warn("Unable to get checksum for %s SRC_URI entry %s: file could not be found" % (d.getVar('PN'), os.path.basename(f)))
> +                        bb.fatal("Unable to get checksum for %s SRC_URI entry %s: file could not be found" % (d.getVar('PN'), os.path.basename(f)))
>                  filelist.append(f + ":" + str(os.path.exists(f)))
>  
>      return " ".join(filelist)
> diff --git a/lib/bb/tests/fetch.py b/lib/bb/tests/fetch.py
> index 3ebd9fd7..5b577b06 100644
> --- a/lib/bb/tests/fetch.py
> +++ b/lib/bb/tests/fetch.py
> @@ -700,6 +700,11 @@ class FetcherLocalTest(FetcherTest):
>          with self.assertRaises(bb.BBHandledException):
>              bb.fetch.get_checksum_file_list(self.d)
>  
> +    def test_local_checksum_fails_no_file(self):
> +        self.d.setVar("SRC_URI", "file://404")
> +        with self.assertRaises(bb.BBHandledException):
> +            bb.fetch.get_checksum_file_list(self.d)
> +
>      def test_local(self):
>          tree = self.fetchUnpack(['file://a', 'file://dir/c'])
>          self.assertEqual(tree, ['a', 'dir/c'])
> -- 
> 2.25.1
> 

> 
> -=-=-=-=-=-=-=-=-=-=-=-
> Links: You receive all messages sent to this group.
> View/Reply Online (#13813): https://lists.openembedded.org/g/bitbake-devel/message/13813
> Mute This Topic: https://lists.openembedded.org/mt/92260795/3617179
> Group Owner: bitbake-devel+owner@lists.openembedded.org
> Unsubscribe: https://lists.openembedded.org/g/bitbake-devel/unsub [alexandre.belloni@bootlin.com]
> -=-=-=-=-=-=-=-=-=-=-=-
>
Paulo Neves July 13, 2022, 10:10 a.m. UTC | #2
Hello,

Well this illustrates the kind of issue I wanted to catch, but this 
test's condition will now not be reachable at all.
Previously, the checksum failed with only a warning, while the failure 
occurred in the download stage of the local fetcher.
Now as the checksum fails and it is immediately fatal there is no way to 
reach a download state at all.

Should this test be removed, then? In my patch i added the appropriate 
test and it is in the bitbake self test which does not depend on poky or 
openembedded core. Let me know what you think.

Paulo Neves


On 7/13/22 11:48, Alexandre Belloni wrote:
> Hello,
>
> I believe this is the cause of the following failure:
>
> 2022-07-13 08:32:51,511 - oe-selftest - INFO - bbtests.BitbakeTests.test_invalid_recipe_src_uri (subunit.RemotedTestCase)
> 2022-07-13 08:32:51,512 - oe-selftest - INFO -  ... FAIL
> 2022-07-13 08:32:51,512 - oe-selftest - INFO - 3: 18/58 119/493 (3.15s) (bbtests.BitbakeTests.test_invalid_recipe_src_uri)
> 2022-07-13 08:32:51,512 - oe-selftest - INFO - testtools.testresult.real._StringException: Traceback (most recent call last):
>    File "/home/pokybuild/yocto-worker/oe-selftest-centos/build/meta/lib/oeqa/selftest/cases/bbtests.py", line 148, in test_invalid_recipe_src_uri
>      bitbake('-ccleanall man-db')
>    File "/home/pokybuild/yocto-worker/oe-selftest-centos/build/meta/lib/oeqa/utils/commands.py", line 229, in bitbake
>      return runCmd(cmd, ignore_status, timeout, output_log=output_log, **options)
>    File "/home/pokybuild/yocto-worker/oe-selftest-centos/build/meta/lib/oeqa/utils/commands.py", line 207, in runCmd
>      raise AssertionError("Command '%s' returned non-zero exit status %d:\n%s" % (command, result.status, exc_output))
> AssertionError: Command 'bitbake  -ccleanall man-db' returned non-zero exit status 1:
> /usr/lib64/python3.6/importlib/_bootstrap.py:219: ImportWarning: can't resolve package from __spec__ or __package__, falling back on __name__ and __path__
>    return f(*args, **kwds)
> NOTE: Reconnecting to bitbake server...
> Loading cache...done.
> Loaded 1718 entries from dependency cache.
> Parsing recipes...ERROR: /home/pokybuild/yocto-worker/oe-selftest-centos/build/meta/recipes-extended/man-db/man-db_2.10.2.bb: Unable to get checksum for man-db SRC_URI entry invalid: file could not be found
> ERROR: Parsing halted due to errors, see error messages above
>
> I guess you have to update
> bbtests.BitbakeTests.test_invalid_recipe_src_uri too
>
> On 08/07/2022 22:54:07+0200, Paulo Neves wrote:
>> If the local fetcher was not able to find the file anywhere but it
>> was included in the SRC_URI for checksumming just make it a fatal
>> error.
>> ---
>>   lib/bb/fetch2/__init__.py | 2 +-
>>   lib/bb/tests/fetch.py     | 5 +++++
>>   2 files changed, 6 insertions(+), 1 deletion(-)
>>
>> diff --git a/lib/bb/fetch2/__init__.py b/lib/bb/fetch2/__init__.py
>> index 5f05278a..8184b55e 100644
>> --- a/lib/bb/fetch2/__init__.py
>> +++ b/lib/bb/fetch2/__init__.py
>> @@ -1237,7 +1237,7 @@ def get_checksum_file_list(d):
>>                               " This means there is no way to get the file except for an orphaned copy"
>>                               " in DL_DIR.") % (d.getVar('PN'), os.path.basename(f)))
>>                       else:
>> -                        bb.warn("Unable to get checksum for %s SRC_URI entry %s: file could not be found" % (d.getVar('PN'), os.path.basename(f)))
>> +                        bb.fatal("Unable to get checksum for %s SRC_URI entry %s: file could not be found" % (d.getVar('PN'), os.path.basename(f)))
>>                   filelist.append(f + ":" + str(os.path.exists(f)))
>>   
>>       return " ".join(filelist)
>> diff --git a/lib/bb/tests/fetch.py b/lib/bb/tests/fetch.py
>> index 3ebd9fd7..5b577b06 100644
>> --- a/lib/bb/tests/fetch.py
>> +++ b/lib/bb/tests/fetch.py
>> @@ -700,6 +700,11 @@ class FetcherLocalTest(FetcherTest):
>>           with self.assertRaises(bb.BBHandledException):
>>               bb.fetch.get_checksum_file_list(self.d)
>>   
>> +    def test_local_checksum_fails_no_file(self):
>> +        self.d.setVar("SRC_URI", "file://404")
>> +        with self.assertRaises(bb.BBHandledException):
>> +            bb.fetch.get_checksum_file_list(self.d)
>> +
>>       def test_local(self):
>>           tree = self.fetchUnpack(['file://a', 'file://dir/c'])
>>           self.assertEqual(tree, ['a', 'dir/c'])
>> -- 
>> 2.25.1
>>
>> -=-=-=-=-=-=-=-=-=-=-=-
>> Links: You receive all messages sent to this group.
>> View/Reply Online (#13813): https://lists.openembedded.org/g/bitbake-devel/message/13813
>> Mute This Topic: https://lists.openembedded.org/mt/92260795/3617179
>> Group Owner: bitbake-devel+owner@lists.openembedded.org
>> Unsubscribe: https://lists.openembedded.org/g/bitbake-devel/unsub [alexandre.belloni@bootlin.com]
>> -=-=-=-=-=-=-=-=-=-=-=-
>>
>
Richard Purdie July 13, 2022, 12:28 p.m. UTC | #3
On Wed, 2022-07-13 at 12:10 +0200, Paulo Neves wrote:
> Hello,
> 
> Well this illustrates the kind of issue I wanted to catch, but this 
> test's condition will now not be reachable at all.
> Previously, the checksum failed with only a warning, while the failure 
> occurred in the download stage of the local fetcher.
> Now as the checksum fails and it is immediately fatal there is no way to 
> reach a download state at all.
> 
> Should this test be removed, then? In my patch i added the appropriate 
> test and it is in the bitbake self test which does not depend on poky or 
> openembedded core. Let me know what you think.

The test makes sense, we probably just need to remove the        
bitbake('-ccleanall man-db') lines since the issue changed from runtime
to parsing.

Cheers,

Richard
Patrick Williams July 26, 2022, 4:09 a.m. UTC | #4
On Fri, Jul 08, 2022 at 10:54:07PM +0200, Paulo Neves wrote:
> If the local fetcher was not able to find the file anywhere but it
> was included in the SRC_URI for checksumming just make it a fatal
> error.
> ---
>  lib/bb/fetch2/__init__.py | 2 +-
>  lib/bb/tests/fetch.py     | 5 +++++
>  2 files changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/lib/bb/fetch2/__init__.py b/lib/bb/fetch2/__init__.py
> index 5f05278a..8184b55e 100644
> --- a/lib/bb/fetch2/__init__.py
> +++ b/lib/bb/fetch2/__init__.py
> @@ -1237,7 +1237,7 @@ def get_checksum_file_list(d):
>                              " This means there is no way to get the file except for an orphaned copy"
>                              " in DL_DIR.") % (d.getVar('PN'), os.path.basename(f)))
>                      else:
> -                        bb.warn("Unable to get checksum for %s SRC_URI entry %s: file could not be found" % (d.getVar('PN'), os.path.basename(f)))
> +                        bb.fatal("Unable to get checksum for %s SRC_URI entry %s: file could not be found" % (d.getVar('PN'), os.path.basename(f)))

Now that we've picked up this change, it seems to have caused a bunch of
irritating hard failures where we use to just get irritating warnings.
Our recipes probably aren't 100% ideal, but we had a number of recipes
which are not pulled into all our machine configs, and could end up with
broken SRC_URIs on machine configs they are not intended to be used on.

There are some more complex examples, but one easy example is a recipe
which had `file://${MACHINE}/eeprom.h` in its SRC_URI[1].  Any machine
which didn't provide this file, even if it never intended to use the
recipe, now fails when we picked up this change.

I know we've been ignoring the warning(s) for a while on these kinds of
failures, so it is our own fault, but it is kind of annoying the new
behavior.  We now have to make sure every recipe not only parses validly
on all machine configs but it also has to have fully populated SRC_URIs
even when the recipe is never used on that machine config.

1. https://github.com/facebook/openbmc/blob/f0d9511ad2fbd563a6b793093cdac557c5ef2a89/meta-facebook/recipes-utils/mac-util/mac-util_0.1.bb#L12
Alexander Kanavin July 26, 2022, 5:35 a.m. UTC | #5
On Tue, 26 Jul 2022 at 06:09, Patrick Williams <patrick@stwcx.xyz> wrote:
> There are some more complex examples, but one easy example is a recipe
> which had `file://${MACHINE}/eeprom.h` in its SRC_URI[1].  Any machine
> which didn't provide this file, even if it never intended to use the
> recipe, now fails when we picked up this change.

This is not how machine-specific versions of the same file are
supposed to be listed. Just use `file://eeprom.h`, and bitbake will
look for it in subdirectories that include $MACHINE, and if not found
there, a default version will be picked (which you can make bogus, so
builds will fail at recipe build stage instead of parsing stage).

Example:
https://git.yoctoproject.org/poky/tree/meta/recipes-graphics/xorg-xserver/xserver-xf86-config?h=master-next

> I know we've been ignoring the warning(s) for a while on these kinds of
> failures, so it is our own fault, but it is kind of annoying the new
> behavior.  We now have to make sure every recipe not only parses validly
> on all machine configs but it also has to have fully populated SRC_URIs
> even when the recipe is never used on that machine config.
>
> 1. https://github.com/facebook/openbmc/blob/f0d9511ad2fbd563a6b793093cdac557c5ef2a89/meta-facebook/recipes-utils/mac-util/mac-util_0.1.bb#L12

Look, this is bleeding edge. And bleeding edge sometimes bleeds and
breaks your builds. You signed up for it; if you don't like it there,
use stable branches, or use controlled transitions to milestone tags.

Alex
Richard Purdie July 26, 2022, 6:39 a.m. UTC | #6
On Mon, 2022-07-25 at 23:09 -0500, Patrick Williams wrote:
> On Fri, Jul 08, 2022 at 10:54:07PM +0200, Paulo Neves wrote:
> > If the local fetcher was not able to find the file anywhere but it
> > was included in the SRC_URI for checksumming just make it a fatal
> > error.
> > ---
> >  lib/bb/fetch2/__init__.py | 2 +-
> >  lib/bb/tests/fetch.py     | 5 +++++
> >  2 files changed, 6 insertions(+), 1 deletion(-)
> > 
> > diff --git a/lib/bb/fetch2/__init__.py b/lib/bb/fetch2/__init__.py
> > index 5f05278a..8184b55e 100644
> > --- a/lib/bb/fetch2/__init__.py
> > +++ b/lib/bb/fetch2/__init__.py
> > @@ -1237,7 +1237,7 @@ def get_checksum_file_list(d):
> >                              " This means there is no way to get the file except for an orphaned copy"
> >                              " in DL_DIR.") % (d.getVar('PN'), os.path.basename(f)))
> >                      else:
> > -                        bb.warn("Unable to get checksum for %s SRC_URI entry %s: file could not be found" % (d.getVar('PN'), os.path.basename(f)))
> > +                        bb.fatal("Unable to get checksum for %s SRC_URI entry %s: file could not be found" % (d.getVar('PN'), os.path.basename(f)))
> 
> Now that we've picked up this change, it seems to have caused a bunch of
> irritating hard failures where we use to just get irritating warnings.
> Our recipes probably aren't 100% ideal, but we had a number of recipes
> which are not pulled into all our machine configs, and could end up with
> broken SRC_URIs on machine configs they are not intended to be used on.
> 
> There are some more complex examples, but one easy example is a recipe
> which had `file://${MACHINE}/eeprom.h` in its SRC_URI[1].  Any machine
> which didn't provide this file, even if it never intended to use the
> recipe, now fails when we picked up this change.
> 
> I know we've been ignoring the warning(s) for a while on these kinds of
> failures, so it is our own fault, but it is kind of annoying the new
> behavior.  We now have to make sure every recipe not only parses validly
> on all machine configs but it also has to have fully populated SRC_URIs
> even when the recipe is never used on that machine config.
> 
> 1. https://github.com/facebook/openbmc/blob/f0d9511ad2fbd563a6b793093cdac557c5ef2a89/meta-facebook/recipes-utils/mac-util/mac-util_0.1.bb#L12

I'm just going from memory but it might help to set COMPATIBLE_MACHINE
in the recipe so that it isn't fully parsed for the machines it isn't
intended for.

Cheers,

Richard
Paulo Neves July 26, 2022, 7:01 a.m. UTC | #7
That is what I do. If the recipe is machine specific it should be marked as
such.

On Tue, 26 Jul 2022 at 08:39, Richard Purdie <
richard.purdie@linuxfoundation.org> wrote:

> On Mon, 2022-07-25 at 23:09 -0500, Patrick Williams wrote:
> > On Fri, Jul 08, 2022 at 10:54:07PM +0200, Paulo Neves wrote:
> > > If the local fetcher was not able to find the file anywhere but it
> > > was included in the SRC_URI for checksumming just make it a fatal
> > > error.
> > > ---
> > >  lib/bb/fetch2/__init__.py | 2 +-
> > >  lib/bb/tests/fetch.py     | 5 +++++
> > >  2 files changed, 6 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/lib/bb/fetch2/__init__.py b/lib/bb/fetch2/__init__.py
> > > index 5f05278a..8184b55e 100644
> > > --- a/lib/bb/fetch2/__init__.py
> > > +++ b/lib/bb/fetch2/__init__.py
> > > @@ -1237,7 +1237,7 @@ def get_checksum_file_list(d):
> > >                              " This means there is no way to get the
> file except for an orphaned copy"
> > >                              " in DL_DIR.") % (d.getVar('PN'),
> os.path.basename(f)))
> > >                      else:
> > > -                        bb.warn("Unable to get checksum for %s
> SRC_URI entry %s: file could not be found" % (d.getVar('PN'),
> os.path.basename(f)))
> > > +                        bb.fatal("Unable to get checksum for %s
> SRC_URI entry %s: file could not be found" % (d.getVar('PN'),
> os.path.basename(f)))
> >
> > Now that we've picked up this change, it seems to have caused a bunch of
> > irritating hard failures where we use to just get irritating warnings.
> > Our recipes probably aren't 100% ideal, but we had a number of recipes
> > which are not pulled into all our machine configs, and could end up with
> > broken SRC_URIs on machine configs they are not intended to be used on.
> >
> > There are some more complex examples, but one easy example is a recipe
> > which had `file://${MACHINE}/eeprom.h` in its SRC_URI[1].  Any machine
> > which didn't provide this file, even if it never intended to use the
> > recipe, now fails when we picked up this change.
> >
> > I know we've been ignoring the warning(s) for a while on these kinds of
> > failures, so it is our own fault, but it is kind of annoying the new
> > behavior.  We now have to make sure every recipe not only parses validly
> > on all machine configs but it also has to have fully populated SRC_URIs
> > even when the recipe is never used on that machine config.
> >
> > 1.
> https://github.com/facebook/openbmc/blob/f0d9511ad2fbd563a6b793093cdac557c5ef2a89/meta-facebook/recipes-utils/mac-util/mac-util_0.1.bb#L12
>
> I'm just going from memory but it might help to set COMPATIBLE_MACHINE
> in the recipe so that it isn't fully parsed for the machines it isn't
> intended for.
>
> Cheers,
>
> Richard
>
>
>
Patrick Williams July 26, 2022, 3:57 p.m. UTC | #8
On Tue, Jul 26, 2022 at 07:35:21AM +0200, Alexander Kanavin wrote:
> On Tue, 26 Jul 2022 at 06:09, Patrick Williams <patrick@stwcx.xyz> wrote:
> > There are some more complex examples, but one easy example is a recipe
> > which had `file://${MACHINE}/eeprom.h` in its SRC_URI[1].  Any machine
> > which didn't provide this file, even if it never intended to use the
> > recipe, now fails when we picked up this change.
> 
> This is not how machine-specific versions of the same file are
> supposed to be listed. Just use `file://eeprom.h`, and bitbake will
> look for it in subdirectories that include $MACHINE, and if not found
> there, a default version will be picked (which you can make bogus, so
> builds will fail at recipe build stage instead of parsing stage).
> 
> Example:
> https://git.yoctoproject.org/poky/tree/meta/recipes-graphics/xorg-xserver/xserver-xf86-config?h=master-next

I agree.  The person who originally wrote the recipe didn't know about
the automatic addition of overrides with the FILESEXTRAPATHS.  As I
wrote this was just one easy example.  I fixed this one to both use the
override implicitly (as you also suggested) and put a default 'eeprom.h'
with a `#error`[1].

A seriously more complex example we have is that we have our own u-boot
recipes and used the same `u-boot-foo.inc` file names as are present in
`meta/`.  It happens that the recipes in `meta/recipes-bsp/u-boot` end
up, even though we'd never use them, using our `u-boot-foo.inc` files
and ending up with totally bogus SRC_URIs in the parsing phase (and
thus failing).  Essentially, whenever there is a new version of u-boot
in `meta` we're going to end up having to make some bogus empty
files[2].

> 
> > I know we've been ignoring the warning(s) for a while on these kinds of
> > failures, so it is our own fault, but it is kind of annoying the new
> > behavior.  We now have to make sure every recipe not only parses validly
> > on all machine configs but it also has to have fully populated SRC_URIs
> > even when the recipe is never used on that machine config.
> >
> > 1. https://github.com/facebook/openbmc/blob/f0d9511ad2fbd563a6b793093cdac557c5ef2a89/meta-facebook/recipes-utils/mac-util/mac-util_0.1.bb#L12
> 
> Look, this is bleeding edge. And bleeding edge sometimes bleeds and
> breaks your builds. You signed up for it; if you don't like it there,
> use stable branches, or use controlled transitions to milestone tags.

I agree we signed up for the bleeding edge, partially to be more aware
of changes like this.  Hopefully this wasn't worded too poorly, but I am
not complaining that "things are currently broken for me".  I was trying
to raise awareness that this change isn't as pleasant to downstream layer
maintainers as it might seem on the surface.  If there is an improvement
or revert, great; if there isn't, we'll deal with it.

1. https://github.com/facebook/openbmc/commit/c4e6ffacb73a567c83458f6a1b7e5bb0a032b770
2. https://github.com/facebook/openbmc/tree/c4e6ffacb73a567c83458f6a1b7e5bb0a032b770/meta-aspeed/recipes-bsp/u-boot/files/openbmc-fb
Patrick Williams July 26, 2022, 4:01 p.m. UTC | #9
On Tue, Jul 26, 2022 at 09:01:31AM +0200, Paulo Neves wrote:

> > I'm just going from memory but it might help to set COMPATIBLE_MACHINE
> > in the recipe so that it isn't fully parsed for the machines it isn't
> > intended for.
> That is what I do. If the recipe is machine specific it should be marked as
> such.
> 

Thanks for the info.  At first blush it doesn't sound pleasant to
maintain a list like this for each recipe (since we have over 30
different machines), but we might be able to take a tactical approach on
specific problematic recipes and/or leverage some overrides to better
classify the machine feature that causes the recipe to be necessary.
Alexander Kanavin July 27, 2022, noon UTC | #10
On Tue, 26 Jul 2022 at 17:57, Patrick Williams <patrick@stwcx.xyz> wrote:
> A seriously more complex example we have is that we have our own u-boot
> recipes and used the same `u-boot-foo.inc` file names as are present in
> `meta/`.  It happens that the recipes in `meta/recipes-bsp/u-boot` end
> up, even though we'd never use them, using our `u-boot-foo.inc` files
> and ending up with totally bogus SRC_URIs in the parsing phase (and
> thus failing).  Essentially, whenever there is a new version of u-boot
> in `meta` we're going to end up having to make some bogus empty
> files[2].

Reusing the same filenames to me seems like asking for trouble. It's
really better to name your own u-boot items in a way that is
guaranteed not to get in the way with what items in meta/ look for.
u-boot-openbmc-* or similar would do.

Alex
Quentin Schulz July 27, 2022, 2:50 p.m. UTC | #11
Hi Patrick,

On 7/26/22 17:57, Patrick Williams wrote:
> On Tue, Jul 26, 2022 at 07:35:21AM +0200, Alexander Kanavin wrote:
>> On Tue, 26 Jul 2022 at 06:09, Patrick Williams <patrick@stwcx.xyz> wrote:
>>> There are some more complex examples, but one easy example is a recipe
>>> which had `file://${MACHINE}/eeprom.h` in its SRC_URI[1].  Any machine
>>> which didn't provide this file, even if it never intended to use the
>>> recipe, now fails when we picked up this change.
>>
>> This is not how machine-specific versions of the same file are
>> supposed to be listed. Just use `file://eeprom.h`, and bitbake will
>> look for it in subdirectories that include $MACHINE, and if not found
>> there, a default version will be picked (which you can make bogus, so
>> builds will fail at recipe build stage instead of parsing stage).
>>
>> Example:
>> https://git.yoctoproject.org/poky/tree/meta/recipes-graphics/xorg-xserver/xserver-xf86-config?h=master-next
> 
> I agree.  The person who originally wrote the recipe didn't know about
> the automatic addition of overrides with the FILESEXTRAPATHS.  As I
> wrote this was just one easy example.  I fixed this one to both use the
> override implicitly (as you also suggested) and put a default 'eeprom.h'
> with a `#error`[1].
> 
> A seriously more complex example we have is that we have our own u-boot
> recipes and used the same `u-boot-foo.inc` file names as are present in
> `meta/`.  It happens that the recipes in `meta/recipes-bsp/u-boot` end
> up, even though we'd never use them, using our `u-boot-foo.inc` files
> and ending up with totally bogus SRC_URIs in the parsing phase (and
> thus failing).  Essentially, whenever there is a new version of u-boot
> in `meta` we're going to end up having to make some bogus empty
> files[2].
> 

Alexander already suggested to use a slightly different recipe name (and 
then you can pick which recipe you want from a configuration file with 
PREFERRED_PROVIDER_u-boot = "" IIRC), but I'm actually surprised by the 
behavior you mention seeing.

The original u-boot recipe uses "require" with just the filename, c.f. 
https://cgit.openembedded.org/openembedded-core/tree/meta/recipes-bsp/u-boot/u-boot_2022.07.bb?h=master, 
etc... there's not a single recipes-bsp/u-boot returned by git grep in 
openembedded-core... My understanding of the require mechanism so far is 
that if you pass it a filename, it'll only look into the current 
directory (where the recipe is located) and if it has a path, it'll look 
for that path in all layers (well, BBPATH, which in the majority of 
cases should be LAYERDIR, the root of the layer). Since there is no path 
in require for the u-boot recipes in openmebedded-core, I don't 
understand what you're saying happens happens. c.f. 
https://docs.yoctoproject.org/bitbake/bitbake-user-manual/bitbake-user-manual-metadata.html#require-directive 
for the doc on require directive.

This highlights one of the following:
  - a rogue bbappend or some weird configuration in one the layers in use,
  - a bug/corner-case in require directive,

Maybe there's something to fix or document, hence why I'm writing this 
mail, it'd be good to actually be able to reproduce this and understand 
why it happens.

Cheers,
Quentin
Patrick Williams July 27, 2022, 7:16 p.m. UTC | #12
On Wed, Jul 27, 2022 at 04:50:37PM +0200, Quentin Schulz wrote:
> On 7/26/22 17:57, Patrick Williams wrote:
> > On Tue, Jul 26, 2022 at 07:35:21AM +0200, Alexander Kanavin wrote:
> >> On Tue, 26 Jul 2022 at 06:09, Patrick Williams <patrick@stwcx.xyz> wrote:

> The original u-boot recipe uses "require" with just the filename, c.f. 
> https://cgit.openembedded.org/openembedded-core/tree/meta/recipes-bsp/u-boot/u-boot_2022.07.bb?h=master, 
> etc... there's not a single recipes-bsp/u-boot returned by git grep in 
> openembedded-core... My understanding of the require mechanism so far is 
> that if you pass it a filename, it'll only look into the current 
> directory (where the recipe is located) and if it has a path, it'll look 
> for that path in all layers (well, BBPATH, which in the majority of 
> cases should be LAYERDIR, the root of the layer). Since there is no path 
> in require for the u-boot recipes in openmebedded-core, I don't 
> understand what you're saying happens happens. c.f. 
> https://docs.yoctoproject.org/bitbake/bitbake-user-manual/bitbake-user-manual-metadata.html#require-directive 
> for the doc on require directive.
> 
> This highlights one of the following:
>   - a rogue bbappend or some weird configuration in one the layers in use,

I dug into it a little more.  It is a rogue bbappend in the u-boot case
and not an issue with the require.

We have an internal tree that contains all the u-boot code along side
the recipes, along with a u-boot%.bbappend to replace the external
SRC_URI with an internal ("file://"-based) one.  This gets applied to
the `meta/recipes-bsp` versions even though we don't have copies of
their code.
diff mbox series

Patch

diff --git a/lib/bb/fetch2/__init__.py b/lib/bb/fetch2/__init__.py
index 5f05278a..8184b55e 100644
--- a/lib/bb/fetch2/__init__.py
+++ b/lib/bb/fetch2/__init__.py
@@ -1237,7 +1237,7 @@  def get_checksum_file_list(d):
                             " This means there is no way to get the file except for an orphaned copy"
                             " in DL_DIR.") % (d.getVar('PN'), os.path.basename(f)))
                     else:
-                        bb.warn("Unable to get checksum for %s SRC_URI entry %s: file could not be found" % (d.getVar('PN'), os.path.basename(f)))
+                        bb.fatal("Unable to get checksum for %s SRC_URI entry %s: file could not be found" % (d.getVar('PN'), os.path.basename(f)))
                 filelist.append(f + ":" + str(os.path.exists(f)))
 
     return " ".join(filelist)
diff --git a/lib/bb/tests/fetch.py b/lib/bb/tests/fetch.py
index 3ebd9fd7..5b577b06 100644
--- a/lib/bb/tests/fetch.py
+++ b/lib/bb/tests/fetch.py
@@ -700,6 +700,11 @@  class FetcherLocalTest(FetcherTest):
         with self.assertRaises(bb.BBHandledException):
             bb.fetch.get_checksum_file_list(self.d)
 
+    def test_local_checksum_fails_no_file(self):
+        self.d.setVar("SRC_URI", "file://404")
+        with self.assertRaises(bb.BBHandledException):
+            bb.fetch.get_checksum_file_list(self.d)
+
     def test_local(self):
         tree = self.fetchUnpack(['file://a', 'file://dir/c'])
         self.assertEqual(tree, ['a', 'dir/c'])