* [PATCH 1/2] fetch2: local files only in DL_DIR becomes fatal error @ 2022-07-08 20:54 Paulo Neves 2022-07-08 20:54 ` [PATCH 2/2] fetch: bb.fatal when trying to checksum non-existing files Paulo Neves 2022-07-09 6:52 ` [bitbake-devel] [PATCH 1/2] fetch2: local files only in DL_DIR becomes fatal error Richard Purdie 0 siblings, 2 replies; 17+ messages in thread From: Paulo Neves @ 2022-07-08 20:54 UTC (permalink / raw) To: bitbake-devel; +Cc: Paulo Neves When trying to checksum local files, if a given file is not found anywhere else than the DL_DIR then this means that the the build is inconsistent, and unreproducible. This also means that if the DL_DIR is removed or not available the build does not know how to fetch the file and will fail. With this commit we fail earlier and consistently on this condition. Signed-off-by: Paulo Neves <ptsneves@gmail.com> --- lib/bb/fetch2/__init__.py | 4 +++- lib/bb/tests/fetch.py | 7 +++++++ 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/lib/bb/fetch2/__init__.py b/lib/bb/fetch2/__init__.py index ac557176..5f05278a 100644 --- a/lib/bb/fetch2/__init__.py +++ b/lib/bb/fetch2/__init__.py @@ -1233,7 +1233,9 @@ def get_checksum_file_list(d): if f.startswith(dl_dir): # The local fetcher's behaviour is to return a path under DL_DIR if it couldn't find the file anywhere else if os.path.exists(f): - bb.warn("Getting checksum for %s SRC_URI entry %s: file not found except in DL_DIR" % (d.getVar('PN'), os.path.basename(f))) + bb.fatal(("Getting checksum for %s SRC_URI entry %s: file not found except in DL_DIR." + " 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))) filelist.append(f + ":" + str(os.path.exists(f))) diff --git a/lib/bb/tests/fetch.py b/lib/bb/tests/fetch.py index ee41bff4..3ebd9fd7 100644 --- a/lib/bb/tests/fetch.py +++ b/lib/bb/tests/fetch.py @@ -693,6 +693,13 @@ class FetcherLocalTest(FetcherTest): flst.sort() return flst + def test_local_checksum_fails_if_only_in_dldir(self): + with open(os.path.join(self.dldir, "on_dl_dir"), "wb"): + pass + self.d.setVar("SRC_URI", "file://on_dl_dir") + 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 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 2/2] fetch: bb.fatal when trying to checksum non-existing files. 2022-07-08 20:54 [PATCH 1/2] fetch2: local files only in DL_DIR becomes fatal error Paulo Neves @ 2022-07-08 20:54 ` Paulo Neves 2022-07-13 9:48 ` [bitbake-devel] " Alexandre Belloni 2022-07-26 4:09 ` Patrick Williams 2022-07-09 6:52 ` [bitbake-devel] [PATCH 1/2] fetch2: local files only in DL_DIR becomes fatal error Richard Purdie 1 sibling, 2 replies; 17+ messages in thread From: Paulo Neves @ 2022-07-08 20:54 UTC (permalink / raw) To: bitbake-devel; +Cc: Paulo Neves 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 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [bitbake-devel] [PATCH 2/2] fetch: bb.fatal when trying to checksum non-existing files. 2022-07-08 20:54 ` [PATCH 2/2] fetch: bb.fatal when trying to checksum non-existing files Paulo Neves @ 2022-07-13 9:48 ` Alexandre Belloni 2022-07-13 10:10 ` Paulo Neves 2022-07-26 4:09 ` Patrick Williams 1 sibling, 1 reply; 17+ messages in thread From: Alexandre Belloni @ 2022-07-13 9:48 UTC (permalink / raw) To: Paulo Neves; +Cc: bitbake-devel 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] > -=-=-=-=-=-=-=-=-=-=-=- > -- Alexandre Belloni, co-owner and COO, Bootlin Embedded Linux and Kernel engineering https://bootlin.com ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [bitbake-devel] [PATCH 2/2] fetch: bb.fatal when trying to checksum non-existing files. 2022-07-13 9:48 ` [bitbake-devel] " Alexandre Belloni @ 2022-07-13 10:10 ` Paulo Neves 2022-07-13 12:28 ` Richard Purdie 0 siblings, 1 reply; 17+ messages in thread From: Paulo Neves @ 2022-07-13 10:10 UTC (permalink / raw) To: Alexandre Belloni; +Cc: bitbake-devel 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] >> -=-=-=-=-=-=-=-=-=-=-=- >> > ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [bitbake-devel] [PATCH 2/2] fetch: bb.fatal when trying to checksum non-existing files. 2022-07-13 10:10 ` Paulo Neves @ 2022-07-13 12:28 ` Richard Purdie 0 siblings, 0 replies; 17+ messages in thread From: Richard Purdie @ 2022-07-13 12:28 UTC (permalink / raw) To: Paulo Neves, Alexandre Belloni; +Cc: bitbake-devel 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 ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 2/2] fetch: bb.fatal when trying to checksum non-existing files. 2022-07-08 20:54 ` [PATCH 2/2] fetch: bb.fatal when trying to checksum non-existing files Paulo Neves 2022-07-13 9:48 ` [bitbake-devel] " Alexandre Belloni @ 2022-07-26 4:09 ` Patrick Williams 2022-07-26 5:35 ` [bitbake-devel] " Alexander Kanavin 2022-07-26 6:39 ` Richard Purdie 1 sibling, 2 replies; 17+ messages in thread From: Patrick Williams @ 2022-07-26 4:09 UTC (permalink / raw) To: Paulo Neves; +Cc: bitbake-devel, Richard Purdie [-- Attachment #1: Type: text/plain, Size: 2262 bytes --] 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 -- Patrick Williams [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [bitbake-devel] [PATCH 2/2] fetch: bb.fatal when trying to checksum non-existing files. 2022-07-26 4:09 ` Patrick Williams @ 2022-07-26 5:35 ` Alexander Kanavin 2022-07-26 15:57 ` Patrick Williams 2022-07-26 6:39 ` Richard Purdie 1 sibling, 1 reply; 17+ messages in thread From: Alexander Kanavin @ 2022-07-26 5:35 UTC (permalink / raw) To: Patrick Williams; +Cc: Paulo Neves, bitbake-devel, Richard Purdie 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 ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [bitbake-devel] [PATCH 2/2] fetch: bb.fatal when trying to checksum non-existing files. 2022-07-26 5:35 ` [bitbake-devel] " Alexander Kanavin @ 2022-07-26 15:57 ` Patrick Williams 2022-07-27 12:00 ` Alexander Kanavin 2022-07-27 14:50 ` Quentin Schulz 0 siblings, 2 replies; 17+ messages in thread From: Patrick Williams @ 2022-07-26 15:57 UTC (permalink / raw) To: Alexander Kanavin; +Cc: Paulo Neves, bitbake-devel, Richard Purdie [-- Attachment #1: Type: text/plain, Size: 3149 bytes --] 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 [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [bitbake-devel] [PATCH 2/2] fetch: bb.fatal when trying to checksum non-existing files. 2022-07-26 15:57 ` Patrick Williams @ 2022-07-27 12:00 ` Alexander Kanavin 2022-07-27 14:50 ` Quentin Schulz 1 sibling, 0 replies; 17+ messages in thread From: Alexander Kanavin @ 2022-07-27 12:00 UTC (permalink / raw) To: Patrick Williams; +Cc: Paulo Neves, bitbake-devel, Richard Purdie 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 ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [bitbake-devel] [PATCH 2/2] fetch: bb.fatal when trying to checksum non-existing files. 2022-07-26 15:57 ` Patrick Williams 2022-07-27 12:00 ` Alexander Kanavin @ 2022-07-27 14:50 ` Quentin Schulz 2022-07-27 19:16 ` Patrick Williams 1 sibling, 1 reply; 17+ messages in thread From: Quentin Schulz @ 2022-07-27 14:50 UTC (permalink / raw) To: Patrick Williams, Alexander Kanavin Cc: Paulo Neves, bitbake-devel, Richard Purdie 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 ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [bitbake-devel] [PATCH 2/2] fetch: bb.fatal when trying to checksum non-existing files. 2022-07-27 14:50 ` Quentin Schulz @ 2022-07-27 19:16 ` Patrick Williams 0 siblings, 0 replies; 17+ messages in thread From: Patrick Williams @ 2022-07-27 19:16 UTC (permalink / raw) To: Quentin Schulz Cc: Alexander Kanavin, Paulo Neves, bitbake-devel, Richard Purdie [-- Attachment #1: Type: text/plain, Size: 1723 bytes --] 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. -- Patrick Williams [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 2/2] fetch: bb.fatal when trying to checksum non-existing files. 2022-07-26 4:09 ` Patrick Williams 2022-07-26 5:35 ` [bitbake-devel] " Alexander Kanavin @ 2022-07-26 6:39 ` Richard Purdie 2022-07-26 7:01 ` Paulo Neves 1 sibling, 1 reply; 17+ messages in thread From: Richard Purdie @ 2022-07-26 6:39 UTC (permalink / raw) To: Patrick Williams, Paulo Neves; +Cc: bitbake-devel 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 ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 2/2] fetch: bb.fatal when trying to checksum non-existing files. 2022-07-26 6:39 ` Richard Purdie @ 2022-07-26 7:01 ` Paulo Neves 2022-07-26 16:01 ` Patrick Williams 0 siblings, 1 reply; 17+ messages in thread From: Paulo Neves @ 2022-07-26 7:01 UTC (permalink / raw) To: Richard Purdie; +Cc: Patrick Williams, bitbake-devel [-- Attachment #1: Type: text/plain, Size: 2790 bytes --] 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 > > > [-- Attachment #2: Type: text/html, Size: 3736 bytes --] ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 2/2] fetch: bb.fatal when trying to checksum non-existing files. 2022-07-26 7:01 ` Paulo Neves @ 2022-07-26 16:01 ` Patrick Williams 0 siblings, 0 replies; 17+ messages in thread From: Patrick Williams @ 2022-07-26 16:01 UTC (permalink / raw) To: Paulo Neves; +Cc: Richard Purdie, bitbake-devel [-- Attachment #1: Type: text/plain, Size: 701 bytes --] 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. -- Patrick Williams [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [bitbake-devel] [PATCH 1/2] fetch2: local files only in DL_DIR becomes fatal error 2022-07-08 20:54 [PATCH 1/2] fetch2: local files only in DL_DIR becomes fatal error Paulo Neves 2022-07-08 20:54 ` [PATCH 2/2] fetch: bb.fatal when trying to checksum non-existing files Paulo Neves @ 2022-07-09 6:52 ` Richard Purdie 2022-07-09 7:19 ` Paulo Neves 1 sibling, 1 reply; 17+ messages in thread From: Richard Purdie @ 2022-07-09 6:52 UTC (permalink / raw) To: Paulo Neves, bitbake-devel On Fri, 2022-07-08 at 22:54 +0200, Paulo Neves wrote: > When trying to checksum local files, if a given file is not found > anywhere else than the DL_DIR then this means that the the build is > inconsistent, and unreproducible. > > This also means that if the DL_DIR is removed or not available the > build does not know how to fetch the file and will fail. With this > commit we fail earlier and consistently on this condition. > > Signed-off-by: Paulo Neves <ptsneves@gmail.com> > --- > lib/bb/fetch2/__init__.py | 4 +++- > lib/bb/tests/fetch.py | 7 +++++++ > 2 files changed, 10 insertions(+), 1 deletion(-) > > diff --git a/lib/bb/fetch2/__init__.py b/lib/bb/fetch2/__init__.py > index ac557176..5f05278a 100644 > --- a/lib/bb/fetch2/__init__.py > +++ b/lib/bb/fetch2/__init__.py > @@ -1233,7 +1233,9 @@ def get_checksum_file_list(d): > if f.startswith(dl_dir): > # The local fetcher's behaviour is to return a path under DL_DIR if it couldn't find the file anywhere else > if os.path.exists(f): > - bb.warn("Getting checksum for %s SRC_URI entry %s: file not found except in DL_DIR" % (d.getVar('PN'), os.path.basename(f))) > + bb.fatal(("Getting checksum for %s SRC_URI entry %s: file not found except in DL_DIR." > + " 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))) > filelist.append(f + ":" + str(os.path.exists(f))) Did you manage to trigger that error in a real world use case? I've just been looking at this code and it is horribly old and outdated. I can't help wonder if we shouldn't do something a bit more invasive to clean it up a bit more. I suspect it does some of these things for old/obsolete reasons... I ask since I'm wondering if anyone ever hits these code paths in a valid usecase. Thanks for working on it, we do need to improve some of these things. Cheers, Richard ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [bitbake-devel] [PATCH 1/2] fetch2: local files only in DL_DIR becomes fatal error 2022-07-09 6:52 ` [bitbake-devel] [PATCH 1/2] fetch2: local files only in DL_DIR becomes fatal error Richard Purdie @ 2022-07-09 7:19 ` Paulo Neves [not found] ` <a7dffab1-9b0c-fab8-a538-81c3d0065834@gmail.com> 0 siblings, 1 reply; 17+ messages in thread From: Paulo Neves @ 2022-07-09 7:19 UTC (permalink / raw) To: Richard Purdie; +Cc: bitbake-devel [-- Attachment #1: Type: text/plain, Size: 3181 bytes --] On Sat, Jul 9, 2022, 08:52 Richard Purdie < richard.purdie@linuxfoundation.org> wrote: > On Fri, 2022-07-08 at 22:54 +0200, Paulo Neves wrote: > > When trying to checksum local files, if a given file is not found > > anywhere else than the DL_DIR then this means that the the build is > > inconsistent, and unreproducible. > > > > This also means that if the DL_DIR is removed or not available the > > build does not know how to fetch the file and will fail. With this > > commit we fail earlier and consistently on this condition. > > > > Signed-off-by: Paulo Neves <ptsneves@gmail.com> > > --- > > lib/bb/fetch2/__init__.py | 4 +++- > > lib/bb/tests/fetch.py | 7 +++++++ > > 2 files changed, 10 insertions(+), 1 deletion(-) > > > > diff --git a/lib/bb/fetch2/__init__.py b/lib/bb/fetch2/__init__.py > > index ac557176..5f05278a 100644 > > --- a/lib/bb/fetch2/__init__.py > > +++ b/lib/bb/fetch2/__init__.py > > @@ -1233,7 +1233,9 @@ def get_checksum_file_list(d): > > if f.startswith(dl_dir): > > # The local fetcher's behaviour is to return a path > under DL_DIR if it couldn't find the file anywhere else > > if os.path.exists(f): > > - bb.warn("Getting checksum for %s SRC_URI entry > %s: file not found except in DL_DIR" % (d.getVar('PN'), > os.path.basename(f))) > > + bb.fatal(("Getting checksum for %s SRC_URI > entry %s: file not found except in DL_DIR." > > + " 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))) > > filelist.append(f + ":" + str(os.path.exists(f))) > > Did you manage to trigger that error in a real world use case? > > I've just been looking at this code and it is horribly old and > outdated. I can't help wonder if we shouldn't do something a bit more > invasive to clean it up a bit more. I suspect it does some of these > things for old/obsolete reasons... > > I ask since I'm wondering if anyone ever hits these code paths in a > valid usecase. > > Thanks for working on it, we do need to improve some of these things. > > Cheers, > > Richard > Hey Richard, I think these codepaths are hit on mostly on bugs(bad uri parsing) or bad files(bad permissions). The use case i added on the tests is also clearly bad, but I have seen builds going on for very long in the state of warning, which becomes fatal with the patch, because the dl_dir was stable and used by all users. So expect this patch to increase the reports of latent issues people just ignored. Yesterday I also hit this warning organically exactly on the situation of the test: a directory mentioned on the SRC_URI which did not exist in any FILESPATHS but somehow existed on the dl_dir. So it is real. I will investigate the whole default to dl_dir if not found and remove it if I cannot find a good reason. Paulo Neves > [-- Attachment #2: Type: text/html, Size: 4447 bytes --] ^ permalink raw reply [flat|nested] 17+ messages in thread
[parent not found: <a7dffab1-9b0c-fab8-a538-81c3d0065834@gmail.com>]
* Re: [bitbake-devel] [PATCH 1/2] fetch2: local files only in DL_DIR becomes fatal error [not found] ` <a7dffab1-9b0c-fab8-a538-81c3d0065834@gmail.com> @ 2022-07-09 13:20 ` Richard Purdie 0 siblings, 0 replies; 17+ messages in thread From: Richard Purdie @ 2022-07-09 13:20 UTC (permalink / raw) To: Paulo Neves; +Cc: bitbake-devel On Sat, 2022-07-09 at 13:12 +0200, Paulo Neves wrote: > > > On 7/9/22 09:19, Paulo Neves wrote: > > > > > > > > > On Sat, Jul 9, 2022, 08:52 Richard Purdie > > <richard.purdie@linuxfoundation.org> wrote: > > > > > On Fri, 2022-07-08 at 22:54 +0200, Paulo Neves wrote: > > > > When trying to checksum local files, if a given file is not > > > found > > > > anywhere else than the DL_DIR then this means that the the > > > build is > > > > inconsistent, and unreproducible. > > > > > > > > This also means that if the DL_DIR is removed or not available > > > the > > > > build does not know how to fetch the file and will fail. With > > > this > > > > commit we fail earlier and consistently on this condition. > > > > > > > > Signed-off-by: Paulo Neves <ptsneves@gmail.com> > > > > --- > > > > lib/bb/fetch2/__init__.py | 4 +++- > > > > lib/bb/tests/fetch.py | 7 +++++++ > > > > 2 files changed, 10 insertions(+), 1 deletion(-) > > > > > > > > diff --git a/lib/bb/fetch2/__init__.py > > > b/lib/bb/fetch2/__init__.py > > > > index ac557176..5f05278a 100644 > > > > --- a/lib/bb/fetch2/__init__.py > > > > +++ b/lib/bb/fetch2/__init__.py > > > > @@ -1233,7 +1233,9 @@ def get_checksum_file_list(d): > > > > if f.startswith(dl_dir): > > > > # The local fetcher's behaviour is to > > > return a path under DL_DIR if it couldn't find the file anywhere > > > else > > > > if os.path.exists(f): > > > > - bb.warn("Getting checksum for %s > > > SRC_URI entry %s: file not found except in DL_DIR" % > > > (d.getVar('PN'), os.path.basename(f))) > > > > + bb.fatal(("Getting checksum for %s > > > SRC_URI entry %s: file not found except in DL_DIR." > > > > + " 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))) > > > > filelist.append(f + ":" + > > > str(os.path.exists(f))) > > > > > > Did you manage to trigger that error in a real world use case? > > > > > > I've just been looking at this code and it is horribly old and > > > outdated. I can't help wonder if we shouldn't do something a bit > > > more > > > invasive to clean it up a bit more. I suspect it does some of > > > these > > > things for old/obsolete reasons... > > > > > > I ask since I'm wondering if anyone ever hits these code paths > > > in a > > > valid usecase. > > > > > > Thanks for working on it, we do need to improve some of these > > > things. > > > > > > Cheers, > > > > > > Richard > > > > > > > Hey Richard, > > > > I think these codepaths are hit on mostly on bugs(bad uri parsing) > > or bad files(bad permissions). > > > > The use case i added on the tests is also clearly bad, but I have > > seen builds going on for very long in the state of warning, which > > becomes fatal with the patch, because the dl_dir was stable and > > used by all users. So expect this patch to increase the reports of > > latent issues people just ignored. > > > > Yesterday I also hit this warning organically exactly on the > > situation of the test: a directory mentioned on the SRC_URI which > > did not exist in any FILESPATHS but somehow existed on the dl_dir. > > So it is real. > > > > I will investigate the whole default to dl_dir if not found and > > remove it if I cannot find a good reason. > > > > Paulo Neves > > I did some archeology trying to find why DL_DIR has anything to do > with the local fetcher and the reason probably comes from this[1] and > this[2]. The commit is about sstate cache and i think it considers > DL_DIR as valid mirror directory in agreement with > lib/bb/fetch2/README[2]. > > The problem is that [3] from the same README "spec", is either > contradictory with with [2], senseless, or accurate depending on the > usage of the local file:// fetcher. For a SRC_URI file, what it is > senseless to search DL_DIR if we have the FILEPATHS? If it is not at > hand and only in the DL_DIR then it is irreproducible and contradicts > [3] (the case this patch argues for). sstate.bbclass sets DL_DIR to SSTATE_DIR in the context of sstate downloads. It also puts SSTATE_DIR in FILESPATH which I suspect it did not back in 2011. I therefore suspect we now have a better fix for the issue as FILESPATH is set and the change from 2011 is now obsolete for sstate. Or I hope so anyway! Cheers, Richard ^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2022-07-27 19:16 UTC | newest]
Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-07-08 20:54 [PATCH 1/2] fetch2: local files only in DL_DIR becomes fatal error Paulo Neves
2022-07-08 20:54 ` [PATCH 2/2] fetch: bb.fatal when trying to checksum non-existing files Paulo Neves
2022-07-13 9:48 ` [bitbake-devel] " Alexandre Belloni
2022-07-13 10:10 ` Paulo Neves
2022-07-13 12:28 ` Richard Purdie
2022-07-26 4:09 ` Patrick Williams
2022-07-26 5:35 ` [bitbake-devel] " Alexander Kanavin
2022-07-26 15:57 ` Patrick Williams
2022-07-27 12:00 ` Alexander Kanavin
2022-07-27 14:50 ` Quentin Schulz
2022-07-27 19:16 ` Patrick Williams
2022-07-26 6:39 ` Richard Purdie
2022-07-26 7:01 ` Paulo Neves
2022-07-26 16:01 ` Patrick Williams
2022-07-09 6:52 ` [bitbake-devel] [PATCH 1/2] fetch2: local files only in DL_DIR becomes fatal error Richard Purdie
2022-07-09 7:19 ` Paulo Neves
[not found] ` <a7dffab1-9b0c-fab8-a538-81c3d0065834@gmail.com>
2022-07-09 13:20 ` Richard Purdie
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.