[bitbake-devel][PATCH v2 1/3] fetch2: Add checksum capability for crate fetcher

[bitbake-devel][PATCH v2 1/3] fetch2: Add checksum capability for crate fetcher

[frederic.martinsons]

From: Frederic Martinsons <frederic.martinsons@gmail.com>

This change brings checksum verification of each crate
in a recipe, e.g

| SRC_URI += " \
|     crate://crates.io/aho-corasick/0.7.20 \
|     crate://crates.io/atomic-waker/1.1.0 \
|     crate://crates.io/cc/1.0.79 \
| "
|
| SRC_URI[aho-corasick.sha256sum] = "cc936419f96fa211c1b9166887b38e5e40b19958e5b895be7c1f93adec7071ac"
| SRC_URI[atomic-waker.sha256sum] = "debc29dde2e69f9e47506b525f639ed42300fc014a3e007832592448fa8e4599"
| SRC_URI[cc.sha256sum] = "50d30906286121d95be3d479533b458f87493b30a4b5f79a607db8f5d11aa91f"

That will require to move the checksum initialization
after the possible call to urldata_init method in order
for the crate fetcher to parse the url.

Another way of doing could have been implementing a decodeurl
method that would have been specific for each fetcher class.

Signed-off-by: Frederic Martinsons <frederic.martinsons@gmail.com>
---
 lib/bb/fetch2/__init__.py | 12 ++++++------
 lib/bb/tests/fetch.py     | 32 ++++++++++++++++++++++++++++++++
 2 files changed, 38 insertions(+), 6 deletions(-)

diff --git a/lib/bb/fetch2/__init__.py b/lib/bb/fetch2/__init__.py
index cf65727a..3ae83fa8 100644
--- a/lib/bb/fetch2/__init__.py
+++ b/lib/bb/fetch2/__init__.py
@@ -1291,18 +1291,13 @@ class FetchData(object):
 
             if checksum_name in self.parm:
                 checksum_expected = self.parm[checksum_name]
-            elif self.type not in ["http", "https", "ftp", "ftps", "sftp", "s3", "az"]:
+            elif self.type not in ["http", "https", "ftp", "ftps", "sftp", "s3", "az", "crate"]:
                 checksum_expected = None
             else:
                 checksum_expected = d.getVarFlag("SRC_URI", checksum_name)
 
             setattr(self, "%s_expected" % checksum_id, checksum_expected)
 
-        for checksum_id in CHECKSUM_LIST:
-            configure_checksum(checksum_id)
-
-        self.ignore_checksums = False
-
         self.names = self.parm.get("name",'default').split(',')
 
         self.method = None
@@ -1324,6 +1319,11 @@ class FetchData(object):
         if hasattr(self.method, "urldata_init"):
             self.method.urldata_init(self, d)
 
+        for checksum_id in CHECKSUM_LIST:
+            configure_checksum(checksum_id)
+
+        self.ignore_checksums = False
+
         if "localpath" in self.parm:
             # if user sets localpath for file, use it instead.
             self.localpath = self.parm["localpath"]
diff --git a/lib/bb/tests/fetch.py b/lib/bb/tests/fetch.py
index 73eefc59..fd089bc8 100644
--- a/lib/bb/tests/fetch.py
+++ b/lib/bb/tests/fetch.py
@@ -2377,6 +2377,13 @@ class CrateTest(FetcherTest):
         d = self.d
 
         fetcher = bb.fetch2.Fetch(uris, self.d)
+        ud = fetcher.ud[fetcher.urls[0]]
+
+        self.assertIn("name", ud.parm)
+        self.assertEqual(ud.parm["name"], "glob")
+        self.assertIn("downloadfilename", ud.parm)
+        self.assertEqual(ud.parm["downloadfilename"], "glob-0.2.11.crate")
+
         fetcher.download()
         fetcher.unpack(self.tempdir)
         self.assertEqual(sorted(os.listdir(self.tempdir)), ['cargo_home', 'download' , 'unpacked'])
@@ -2394,6 +2401,19 @@ class CrateTest(FetcherTest):
         d = self.d
 
         fetcher = bb.fetch2.Fetch(uris, self.d)
+        ud = fetcher.ud[fetcher.urls[0]]
+
+        self.assertIn("name", ud.parm)
+        self.assertEqual(ud.parm["name"], "glob")
+        self.assertIn("downloadfilename", ud.parm)
+        self.assertEqual(ud.parm["downloadfilename"], "glob-0.2.11.crate")
+
+        ud = fetcher.ud[fetcher.urls[1]]
+        self.assertIn("name", ud.parm)
+        self.assertEqual(ud.parm["name"], "time")
+        self.assertIn("downloadfilename", ud.parm)
+        self.assertEqual(ud.parm["downloadfilename"], "time-0.1.35.crate")
+
         fetcher.download()
         fetcher.unpack(self.tempdir)
         self.assertEqual(sorted(os.listdir(self.tempdir)), ['cargo_home', 'download' , 'unpacked'])
@@ -2403,6 +2423,18 @@ class CrateTest(FetcherTest):
         self.assertTrue(os.path.exists(self.tempdir + "/cargo_home/bitbake/time-0.1.35/.cargo-checksum.json"))
         self.assertTrue(os.path.exists(self.tempdir + "/cargo_home/bitbake/time-0.1.35/src/lib.rs"))
 
+    @skipIfNoNetwork()
+    def test_crate_incorrect_cksum(self):
+        uri = "crate://crates.io/aho-corasick/0.7.20"
+        self.d.setVar('SRC_URI', uri)
+        self.d.setVarFlag("SRC_URI", "aho-corasick.sha256sum", hashlib.sha256("Invalid".encode("utf-8")).hexdigest())
+
+        uris = self.d.getVar('SRC_URI').split()
+
+        fetcher = bb.fetch2.Fetch(uris, self.d)
+        with self.assertRaisesRegexp(bb.fetch2.FetchError, "Fetcher failure for URL"):
+            fetcher.download()
+
 class NPMTest(FetcherTest):
     def skipIfNoNpm():
         import shutil
-- 
2.34.1

[bitbake-devel][PATCH v2 2/3] crate.py: make checksum verification mandatory

[frederic.martinsons]

From: Frederic Martinsons <frederic.martinsons@gmail.com>

Signed-off-by: Frederic Martinsons <frederic.martinsons@gmail.com>
---
 lib/bb/fetch2/crate.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/bb/fetch2/crate.py b/lib/bb/fetch2/crate.py
index f091200d..f8367ed3 100644
--- a/lib/bb/fetch2/crate.py
+++ b/lib/bb/fetch2/crate.py
@@ -33,7 +33,7 @@ class Crate(Wget):
         return ud.type in ['crate']
 
     def recommends_checksum(self, urldata):
-        return False
+        return True
 
     def urldata_init(self, ud, d):
         """
-- 
2.34.1

[bitbake-devel][PATCH v2 3/3] crate.py: authorize crate url with parameters

[frederic.martinsons]

From: Frederic Martinsons <frederic.martinsons@gmail.com>

This allow to have classic fetch parameters
(like destsuffix, sha256, name ...) not being
considered by crate fetcher itself (and so mess
up its download)

Moreover, it allow to overload the name of the downloaded
crate (maybe usefull if there is a naming clash between
two crates coming from different repositories)

Signed-off-by: Frederic Martinsons <frederic.martinsons@gmail.com>
---
 lib/bb/fetch2/crate.py |  9 ++++++---
 lib/bb/tests/fetch.py  | 24 ++++++++++++++++++++++++
 2 files changed, 30 insertions(+), 3 deletions(-)

diff --git a/lib/bb/fetch2/crate.py b/lib/bb/fetch2/crate.py
index f8367ed3..590dc9c1 100644
--- a/lib/bb/fetch2/crate.py
+++ b/lib/bb/fetch2/crate.py
@@ -56,8 +56,10 @@ class Crate(Wget):
         if len(parts) < 5:
             raise bb.fetch2.ParameterError("Invalid URL: Must be crate://HOST/NAME/VERSION", ud.url)
 
-        # last field is version
-        version = parts[len(parts) - 1]
+        # version is expected to be the last token
+        # but ignore possible url parameters which will be used
+        # by the top fetcher class
+        version, _, _ = parts[len(parts) -1].partition(";")
         # second to last field is name
         name = parts[len(parts) - 2]
         # host (this is to allow custom crate registries to be specified
@@ -69,7 +71,8 @@ class Crate(Wget):
 
         ud.url = "https://%s/%s/%s/download" % (host, name, version)
         ud.parm['downloadfilename'] = "%s-%s.crate" % (name, version)
-        ud.parm['name'] = name
+        if 'name' not in ud.parm:
+            ud.parm['name'] = name
 
         logger.debug2("Fetching %s to %s" % (ud.url, ud.parm['downloadfilename']))
 
diff --git a/lib/bb/tests/fetch.py b/lib/bb/tests/fetch.py
index fd089bc8..85cf25e7 100644
--- a/lib/bb/tests/fetch.py
+++ b/lib/bb/tests/fetch.py
@@ -2423,6 +2423,30 @@ class CrateTest(FetcherTest):
         self.assertTrue(os.path.exists(self.tempdir + "/cargo_home/bitbake/time-0.1.35/.cargo-checksum.json"))
         self.assertTrue(os.path.exists(self.tempdir + "/cargo_home/bitbake/time-0.1.35/src/lib.rs"))
 
+    @skipIfNoNetwork()
+    def test_crate_url_params(self):
+
+        uri = "crate://crates.io/aho-corasick/0.7.20;name=aho-corasick-renamed"
+        self.d.setVar('SRC_URI', uri)
+
+        uris = self.d.getVar('SRC_URI').split()
+        d = self.d
+
+        fetcher = bb.fetch2.Fetch(uris, self.d)
+        ud = fetcher.ud[fetcher.urls[0]]
+
+        self.assertIn("name", ud.parm)
+        self.assertEqual(ud.parm["name"], "aho-corasick-renamed")
+        self.assertIn("downloadfilename", ud.parm)
+        self.assertEqual(ud.parm["downloadfilename"], "aho-corasick-0.7.20.crate")
+
+        fetcher.download()
+        fetcher.unpack(self.tempdir)
+        self.assertEqual(sorted(os.listdir(self.tempdir)), ['cargo_home', 'download' , 'unpacked'])
+        self.assertEqual(sorted(os.listdir(self.tempdir + "/download")), ['aho-corasick-0.7.20.crate', 'aho-corasick-0.7.20.crate.done'])
+        self.assertTrue(os.path.exists(self.tempdir + "/cargo_home/bitbake/aho-corasick-0.7.20/.cargo-checksum.json"))
+        self.assertTrue(os.path.exists(self.tempdir + "/cargo_home/bitbake/aho-corasick-0.7.20/src/lib.rs"))
+
     @skipIfNoNetwork()
     def test_crate_incorrect_cksum(self):
         uri = "crate://crates.io/aho-corasick/0.7.20"
-- 
2.34.1

Re: [bitbake-devel][PATCH v2 1/3] fetch2: Add checksum capability for crate fetcher

[Frédéric Martinsons]

[-- Attachment #1: Type: text/plain, Size: 5671 bytes --]

Hello,

Should I verify something more for this series?
I understood that it is a useful development to have in bitbake fetcher
(but will break things if series in oe-core and meta-oe are not applied
after this one) and I'm wondering if something block the review / apply ?

Please tell me, I'll do all my best to secure this.

Have a good day.

Le ven. 17 mars 2023, 09:19, <frederic.martinsons@gmail.com> a écrit :

> From: Frederic Martinsons <frederic.martinsons@gmail.com>
>
> This change brings checksum verification of each crate
> in a recipe, e.g
>
> | SRC_URI += " \
> |     crate://crates.io/aho-corasick/0.7.20 \
> |     crate://crates.io/atomic-waker/1.1.0 \
> |     crate://crates.io/cc/1.0.79 \
> | "
> |
> | SRC_URI[aho-corasick.sha256sum] =
> "cc936419f96fa211c1b9166887b38e5e40b19958e5b895be7c1f93adec7071ac"
> | SRC_URI[atomic-waker.sha256sum] =
> "debc29dde2e69f9e47506b525f639ed42300fc014a3e007832592448fa8e4599"
> | SRC_URI[cc.sha256sum] =
> "50d30906286121d95be3d479533b458f87493b30a4b5f79a607db8f5d11aa91f"
>
> That will require to move the checksum initialization
> after the possible call to urldata_init method in order
> for the crate fetcher to parse the url.
>
> Another way of doing could have been implementing a decodeurl
> method that would have been specific for each fetcher class.
>
> Signed-off-by: Frederic Martinsons <frederic.martinsons@gmail.com>
> ---
>  lib/bb/fetch2/__init__.py | 12 ++++++------
>  lib/bb/tests/fetch.py     | 32 ++++++++++++++++++++++++++++++++
>  2 files changed, 38 insertions(+), 6 deletions(-)
>
> diff --git a/lib/bb/fetch2/__init__.py b/lib/bb/fetch2/__init__.py
> index cf65727a..3ae83fa8 100644
> --- a/lib/bb/fetch2/__init__.py
> +++ b/lib/bb/fetch2/__init__.py
> @@ -1291,18 +1291,13 @@ class FetchData(object):
>
>              if checksum_name in self.parm:
>                  checksum_expected = self.parm[checksum_name]
> -            elif self.type not in ["http", "https", "ftp", "ftps",
> "sftp", "s3", "az"]:
> +            elif self.type not in ["http", "https", "ftp", "ftps",
> "sftp", "s3", "az", "crate"]:
>                  checksum_expected = None
>              else:
>                  checksum_expected = d.getVarFlag("SRC_URI", checksum_name)
>
>              setattr(self, "%s_expected" % checksum_id, checksum_expected)
>
> -        for checksum_id in CHECKSUM_LIST:
> -            configure_checksum(checksum_id)
> -
> -        self.ignore_checksums = False
> -
>          self.names = self.parm.get("name",'default').split(',')
>
>          self.method = None
> @@ -1324,6 +1319,11 @@ class FetchData(object):
>          if hasattr(self.method, "urldata_init"):
>              self.method.urldata_init(self, d)
>
> +        for checksum_id in CHECKSUM_LIST:
> +            configure_checksum(checksum_id)
> +
> +        self.ignore_checksums = False
> +
>          if "localpath" in self.parm:
>              # if user sets localpath for file, use it instead.
>              self.localpath = self.parm["localpath"]
> diff --git a/lib/bb/tests/fetch.py b/lib/bb/tests/fetch.py
> index 73eefc59..fd089bc8 100644
> --- a/lib/bb/tests/fetch.py
> +++ b/lib/bb/tests/fetch.py
> @@ -2377,6 +2377,13 @@ class CrateTest(FetcherTest):
>          d = self.d
>
>          fetcher = bb.fetch2.Fetch(uris, self.d)
> +        ud = fetcher.ud[fetcher.urls[0]]
> +
> +        self.assertIn("name", ud.parm)
> +        self.assertEqual(ud.parm["name"], "glob")
> +        self.assertIn("downloadfilename", ud.parm)
> +        self.assertEqual(ud.parm["downloadfilename"], "glob-0.2.11.crate")
> +
>          fetcher.download()
>          fetcher.unpack(self.tempdir)
>          self.assertEqual(sorted(os.listdir(self.tempdir)), ['cargo_home',
> 'download' , 'unpacked'])
> @@ -2394,6 +2401,19 @@ class CrateTest(FetcherTest):
>          d = self.d
>
>          fetcher = bb.fetch2.Fetch(uris, self.d)
> +        ud = fetcher.ud[fetcher.urls[0]]
> +
> +        self.assertIn("name", ud.parm)
> +        self.assertEqual(ud.parm["name"], "glob")
> +        self.assertIn("downloadfilename", ud.parm)
> +        self.assertEqual(ud.parm["downloadfilename"], "glob-0.2.11.crate")
> +
> +        ud = fetcher.ud[fetcher.urls[1]]
> +        self.assertIn("name", ud.parm)
> +        self.assertEqual(ud.parm["name"], "time")
> +        self.assertIn("downloadfilename", ud.parm)
> +        self.assertEqual(ud.parm["downloadfilename"], "time-0.1.35.crate")
> +
>          fetcher.download()
>          fetcher.unpack(self.tempdir)
>          self.assertEqual(sorted(os.listdir(self.tempdir)), ['cargo_home',
> 'download' , 'unpacked'])
> @@ -2403,6 +2423,18 @@ class CrateTest(FetcherTest):
>          self.assertTrue(os.path.exists(self.tempdir +
> "/cargo_home/bitbake/time-0.1.35/.cargo-checksum.json"))
>          self.assertTrue(os.path.exists(self.tempdir +
> "/cargo_home/bitbake/time-0.1.35/src/lib.rs"))
>
> +    @skipIfNoNetwork()
> +    def test_crate_incorrect_cksum(self):
> +        uri = "crate://crates.io/aho-corasick/0.7.20"
> +        self.d.setVar('SRC_URI', uri)
> +        self.d.setVarFlag("SRC_URI", "aho-corasick.sha256sum",
> hashlib.sha256("Invalid".encode("utf-8")).hexdigest())
> +
> +        uris = self.d.getVar('SRC_URI').split()
> +
> +        fetcher = bb.fetch2.Fetch(uris, self.d)
> +        with self.assertRaisesRegexp(bb.fetch2.FetchError, "Fetcher
> failure for URL"):
> +            fetcher.download()
> +
>  class NPMTest(FetcherTest):
>      def skipIfNoNpm():
>          import shutil
> --
> 2.34.1
>
>

[-- Attachment #2: Type: text/html, Size: 7848 bytes --]

Re: [bitbake-devel][PATCH v2 1/3] fetch2: Add checksum capability for crate fetcher

[Frédéric Martinsons]

[-- Attachment #1: Type: text/plain, Size: 6250 bytes --]

If dependencies between bitbake and other layers is an issue, I can
suppress the patch n2 of this
series to disable the verification of the checksum, and wait for all
patches (bitbake, meta-oe and oe-core)
to be applied.
Finally, I would issue a new patch to make the checksum verification
mandatory.

On Tue, 21 Mar 2023 at 18:02, Frédéric Martinsons <
frederic.martinsons@gmail.com> wrote:

> Hello,
>
> Should I verify something more for this series?
> I understood that it is a useful development to have in bitbake fetcher
> (but will break things if series in oe-core and meta-oe are not applied
> after this one) and I'm wondering if something block the review / apply ?
>
> Please tell me, I'll do all my best to secure this.
>
> Have a good day.
>
> Le ven. 17 mars 2023, 09:19, <frederic.martinsons@gmail.com> a écrit :
>
>> From: Frederic Martinsons <frederic.martinsons@gmail.com>
>>
>> This change brings checksum verification of each crate
>> in a recipe, e.g
>>
>> | SRC_URI += " \
>> |     crate://crates.io/aho-corasick/0.7.20 \
>> |     crate://crates.io/atomic-waker/1.1.0 \
>> |     crate://crates.io/cc/1.0.79 \
>> | "
>> |
>> | SRC_URI[aho-corasick.sha256sum] =
>> "cc936419f96fa211c1b9166887b38e5e40b19958e5b895be7c1f93adec7071ac"
>> | SRC_URI[atomic-waker.sha256sum] =
>> "debc29dde2e69f9e47506b525f639ed42300fc014a3e007832592448fa8e4599"
>> | SRC_URI[cc.sha256sum] =
>> "50d30906286121d95be3d479533b458f87493b30a4b5f79a607db8f5d11aa91f"
>>
>> That will require to move the checksum initialization
>> after the possible call to urldata_init method in order
>> for the crate fetcher to parse the url.
>>
>> Another way of doing could have been implementing a decodeurl
>> method that would have been specific for each fetcher class.
>>
>> Signed-off-by: Frederic Martinsons <frederic.martinsons@gmail.com>
>> ---
>>  lib/bb/fetch2/__init__.py | 12 ++++++------
>>  lib/bb/tests/fetch.py     | 32 ++++++++++++++++++++++++++++++++
>>  2 files changed, 38 insertions(+), 6 deletions(-)
>>
>> diff --git a/lib/bb/fetch2/__init__.py b/lib/bb/fetch2/__init__.py
>> index cf65727a..3ae83fa8 100644
>> --- a/lib/bb/fetch2/__init__.py
>> +++ b/lib/bb/fetch2/__init__.py
>> @@ -1291,18 +1291,13 @@ class FetchData(object):
>>
>>              if checksum_name in self.parm:
>>                  checksum_expected = self.parm[checksum_name]
>> -            elif self.type not in ["http", "https", "ftp", "ftps",
>> "sftp", "s3", "az"]:
>> +            elif self.type not in ["http", "https", "ftp", "ftps",
>> "sftp", "s3", "az", "crate"]:
>>                  checksum_expected = None
>>              else:
>>                  checksum_expected = d.getVarFlag("SRC_URI",
>> checksum_name)
>>
>>              setattr(self, "%s_expected" % checksum_id, checksum_expected)
>>
>> -        for checksum_id in CHECKSUM_LIST:
>> -            configure_checksum(checksum_id)
>> -
>> -        self.ignore_checksums = False
>> -
>>          self.names = self.parm.get("name",'default').split(',')
>>
>>          self.method = None
>> @@ -1324,6 +1319,11 @@ class FetchData(object):
>>          if hasattr(self.method, "urldata_init"):
>>              self.method.urldata_init(self, d)
>>
>> +        for checksum_id in CHECKSUM_LIST:
>> +            configure_checksum(checksum_id)
>> +
>> +        self.ignore_checksums = False
>> +
>>          if "localpath" in self.parm:
>>              # if user sets localpath for file, use it instead.
>>              self.localpath = self.parm["localpath"]
>> diff --git a/lib/bb/tests/fetch.py b/lib/bb/tests/fetch.py
>> index 73eefc59..fd089bc8 100644
>> --- a/lib/bb/tests/fetch.py
>> +++ b/lib/bb/tests/fetch.py
>> @@ -2377,6 +2377,13 @@ class CrateTest(FetcherTest):
>>          d = self.d
>>
>>          fetcher = bb.fetch2.Fetch(uris, self.d)
>> +        ud = fetcher.ud[fetcher.urls[0]]
>> +
>> +        self.assertIn("name", ud.parm)
>> +        self.assertEqual(ud.parm["name"], "glob")
>> +        self.assertIn("downloadfilename", ud.parm)
>> +        self.assertEqual(ud.parm["downloadfilename"],
>> "glob-0.2.11.crate")
>> +
>>          fetcher.download()
>>          fetcher.unpack(self.tempdir)
>>          self.assertEqual(sorted(os.listdir(self.tempdir)),
>> ['cargo_home', 'download' , 'unpacked'])
>> @@ -2394,6 +2401,19 @@ class CrateTest(FetcherTest):
>>          d = self.d
>>
>>          fetcher = bb.fetch2.Fetch(uris, self.d)
>> +        ud = fetcher.ud[fetcher.urls[0]]
>> +
>> +        self.assertIn("name", ud.parm)
>> +        self.assertEqual(ud.parm["name"], "glob")
>> +        self.assertIn("downloadfilename", ud.parm)
>> +        self.assertEqual(ud.parm["downloadfilename"],
>> "glob-0.2.11.crate")
>> +
>> +        ud = fetcher.ud[fetcher.urls[1]]
>> +        self.assertIn("name", ud.parm)
>> +        self.assertEqual(ud.parm["name"], "time")
>> +        self.assertIn("downloadfilename", ud.parm)
>> +        self.assertEqual(ud.parm["downloadfilename"],
>> "time-0.1.35.crate")
>> +
>>          fetcher.download()
>>          fetcher.unpack(self.tempdir)
>>          self.assertEqual(sorted(os.listdir(self.tempdir)),
>> ['cargo_home', 'download' , 'unpacked'])
>> @@ -2403,6 +2423,18 @@ class CrateTest(FetcherTest):
>>          self.assertTrue(os.path.exists(self.tempdir +
>> "/cargo_home/bitbake/time-0.1.35/.cargo-checksum.json"))
>>          self.assertTrue(os.path.exists(self.tempdir +
>> "/cargo_home/bitbake/time-0.1.35/src/lib.rs"))
>>
>> +    @skipIfNoNetwork()
>> +    def test_crate_incorrect_cksum(self):
>> +        uri = "crate://crates.io/aho-corasick/0.7.20"
>> +        self.d.setVar('SRC_URI', uri)
>> +        self.d.setVarFlag("SRC_URI", "aho-corasick.sha256sum",
>> hashlib.sha256("Invalid".encode("utf-8")).hexdigest())
>> +
>> +        uris = self.d.getVar('SRC_URI').split()
>> +
>> +        fetcher = bb.fetch2.Fetch(uris, self.d)
>> +        with self.assertRaisesRegexp(bb.fetch2.FetchError, "Fetcher
>> failure for URL"):
>> +            fetcher.download()
>> +
>>  class NPMTest(FetcherTest):
>>      def skipIfNoNpm():
>>          import shutil
>> --
>> 2.34.1
>>
>>

[-- Attachment #2: Type: text/html, Size: 8614 bytes --]

Re: [bitbake-devel][PATCH v2 1/3] fetch2: Add checksum capability for crate fetcher

[Alexander Kanavin]

I believe all the patches are now in abelloni's staging branch:
https://git.yoctoproject.org/poky-contrib/log/?h=abelloni/master-next

If they need a rework, or otherwise aren't suitable, you'll get feedback.

Note that we're in the feature freeze now; while this is a
comparatively low risk change (in my view), it may have to wait until
after mickledore's out.

Alex

On Tue, 21 Mar 2023 at 18:26, Frederic Martinsons
<frederic.martinsons@gmail.com> wrote:
>
> If dependencies between bitbake and other layers is an issue, I can suppress the patch n2 of this
> series to disable the verification of the checksum, and wait for all patches (bitbake, meta-oe and oe-core)
> to be applied.
> Finally, I would issue a new patch to make the checksum verification mandatory.
>
> On Tue, 21 Mar 2023 at 18:02, Frédéric Martinsons <frederic.martinsons@gmail.com> wrote:
>>
>> Hello,
>>
>> Should I verify something more for this series?
>> I understood that it is a useful development to have in bitbake fetcher (but will break things if series in oe-core and meta-oe are not applied after this one) and I'm wondering if something block the review / apply ?
>>
>> Please tell me, I'll do all my best to secure this.
>>
>> Have a good day.
>>
>> Le ven. 17 mars 2023, 09:19, <frederic.martinsons@gmail.com> a écrit :
>>>
>>> From: Frederic Martinsons <frederic.martinsons@gmail.com>
>>>
>>> This change brings checksum verification of each crate
>>> in a recipe, e.g
>>>
>>> | SRC_URI += " \
>>> |     crate://crates.io/aho-corasick/0.7.20 \
>>> |     crate://crates.io/atomic-waker/1.1.0 \
>>> |     crate://crates.io/cc/1.0.79 \
>>> | "
>>> |
>>> | SRC_URI[aho-corasick.sha256sum] = "cc936419f96fa211c1b9166887b38e5e40b19958e5b895be7c1f93adec7071ac"
>>> | SRC_URI[atomic-waker.sha256sum] = "debc29dde2e69f9e47506b525f639ed42300fc014a3e007832592448fa8e4599"
>>> | SRC_URI[cc.sha256sum] = "50d30906286121d95be3d479533b458f87493b30a4b5f79a607db8f5d11aa91f"
>>>
>>> That will require to move the checksum initialization
>>> after the possible call to urldata_init method in order
>>> for the crate fetcher to parse the url.
>>>
>>> Another way of doing could have been implementing a decodeurl
>>> method that would have been specific for each fetcher class.
>>>
>>> Signed-off-by: Frederic Martinsons <frederic.martinsons@gmail.com>
>>> ---
>>>  lib/bb/fetch2/__init__.py | 12 ++++++------
>>>  lib/bb/tests/fetch.py     | 32 ++++++++++++++++++++++++++++++++
>>>  2 files changed, 38 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/lib/bb/fetch2/__init__.py b/lib/bb/fetch2/__init__.py
>>> index cf65727a..3ae83fa8 100644
>>> --- a/lib/bb/fetch2/__init__.py
>>> +++ b/lib/bb/fetch2/__init__.py
>>> @@ -1291,18 +1291,13 @@ class FetchData(object):
>>>
>>>              if checksum_name in self.parm:
>>>                  checksum_expected = self.parm[checksum_name]
>>> -            elif self.type not in ["http", "https", "ftp", "ftps", "sftp", "s3", "az"]:
>>> +            elif self.type not in ["http", "https", "ftp", "ftps", "sftp", "s3", "az", "crate"]:
>>>                  checksum_expected = None
>>>              else:
>>>                  checksum_expected = d.getVarFlag("SRC_URI", checksum_name)
>>>
>>>              setattr(self, "%s_expected" % checksum_id, checksum_expected)
>>>
>>> -        for checksum_id in CHECKSUM_LIST:
>>> -            configure_checksum(checksum_id)
>>> -
>>> -        self.ignore_checksums = False
>>> -
>>>          self.names = self.parm.get("name",'default').split(',')
>>>
>>>          self.method = None
>>> @@ -1324,6 +1319,11 @@ class FetchData(object):
>>>          if hasattr(self.method, "urldata_init"):
>>>              self.method.urldata_init(self, d)
>>>
>>> +        for checksum_id in CHECKSUM_LIST:
>>> +            configure_checksum(checksum_id)
>>> +
>>> +        self.ignore_checksums = False
>>> +
>>>          if "localpath" in self.parm:
>>>              # if user sets localpath for file, use it instead.
>>>              self.localpath = self.parm["localpath"]
>>> diff --git a/lib/bb/tests/fetch.py b/lib/bb/tests/fetch.py
>>> index 73eefc59..fd089bc8 100644
>>> --- a/lib/bb/tests/fetch.py
>>> +++ b/lib/bb/tests/fetch.py
>>> @@ -2377,6 +2377,13 @@ class CrateTest(FetcherTest):
>>>          d = self.d
>>>
>>>          fetcher = bb.fetch2.Fetch(uris, self.d)
>>> +        ud = fetcher.ud[fetcher.urls[0]]
>>> +
>>> +        self.assertIn("name", ud.parm)
>>> +        self.assertEqual(ud.parm["name"], "glob")
>>> +        self.assertIn("downloadfilename", ud.parm)
>>> +        self.assertEqual(ud.parm["downloadfilename"], "glob-0.2.11.crate")
>>> +
>>>          fetcher.download()
>>>          fetcher.unpack(self.tempdir)
>>>          self.assertEqual(sorted(os.listdir(self.tempdir)), ['cargo_home', 'download' , 'unpacked'])
>>> @@ -2394,6 +2401,19 @@ class CrateTest(FetcherTest):
>>>          d = self.d
>>>
>>>          fetcher = bb.fetch2.Fetch(uris, self.d)
>>> +        ud = fetcher.ud[fetcher.urls[0]]
>>> +
>>> +        self.assertIn("name", ud.parm)
>>> +        self.assertEqual(ud.parm["name"], "glob")
>>> +        self.assertIn("downloadfilename", ud.parm)
>>> +        self.assertEqual(ud.parm["downloadfilename"], "glob-0.2.11.crate")
>>> +
>>> +        ud = fetcher.ud[fetcher.urls[1]]
>>> +        self.assertIn("name", ud.parm)
>>> +        self.assertEqual(ud.parm["name"], "time")
>>> +        self.assertIn("downloadfilename", ud.parm)
>>> +        self.assertEqual(ud.parm["downloadfilename"], "time-0.1.35.crate")
>>> +
>>>          fetcher.download()
>>>          fetcher.unpack(self.tempdir)
>>>          self.assertEqual(sorted(os.listdir(self.tempdir)), ['cargo_home', 'download' , 'unpacked'])
>>> @@ -2403,6 +2423,18 @@ class CrateTest(FetcherTest):
>>>          self.assertTrue(os.path.exists(self.tempdir + "/cargo_home/bitbake/time-0.1.35/.cargo-checksum.json"))
>>>          self.assertTrue(os.path.exists(self.tempdir + "/cargo_home/bitbake/time-0.1.35/src/lib.rs"))
>>>
>>> +    @skipIfNoNetwork()
>>> +    def test_crate_incorrect_cksum(self):
>>> +        uri = "crate://crates.io/aho-corasick/0.7.20"
>>> +        self.d.setVar('SRC_URI', uri)
>>> +        self.d.setVarFlag("SRC_URI", "aho-corasick.sha256sum", hashlib.sha256("Invalid".encode("utf-8")).hexdigest())
>>> +
>>> +        uris = self.d.getVar('SRC_URI').split()
>>> +
>>> +        fetcher = bb.fetch2.Fetch(uris, self.d)
>>> +        with self.assertRaisesRegexp(bb.fetch2.FetchError, "Fetcher failure for URL"):
>>> +            fetcher.download()
>>> +
>>>  class NPMTest(FetcherTest):
>>>      def skipIfNoNpm():
>>>          import shutil
>>> --
>>> 2.34.1
>>>
>
> -=-=-=-=-=-=-=-=-=-=-=-
> Links: You receive all messages sent to this group.
> View/Reply Online (#14609): https://lists.openembedded.org/g/bitbake-devel/message/14609
> Mute This Topic: https://lists.openembedded.org/mt/97668729/1686489
> Group Owner: bitbake-devel+owner@lists.openembedded.org
> Unsubscribe: https://lists.openembedded.org/g/bitbake-devel/unsub [alex.kanavin@gmail.com]
> -=-=-=-=-=-=-=-=-=-=-=-
>

Re: [bitbake-devel][PATCH v2 1/3] fetch2: Add checksum capability for crate fetcher

[Frédéric Martinsons]

[-- Attachment #1: Type: text/plain, Size: 8237 bytes --]

I'm asking because I had a feedback from abelloni about the patch series to
update
openembedded-core concerning a build failure (
https://lists.openembedded.org/g/bitbake-devel/topic/97648804#14600)
and I think there was a misunderstanding about the "applicability" of some
of bitbake's patches that
was required to compile. And I don't know if I managed to clarify that (and
if I understood well).

Anyway, I wasn't aware that there is a feature freeze now, I'll wait for
feedback then.

Thanks Alex.

Le mar. 21 mars 2023, 19:35, Alexander Kanavin <alex.kanavin@gmail.com> a
écrit :

> I believe all the patches are now in abelloni's staging branch:
> https://git.yoctoproject.org/poky-contrib/log/?h=abelloni/master-next
>
> If they need a rework, or otherwise aren't suitable, you'll get feedback.
>
> Note that we're in the feature freeze now; while this is a
> comparatively low risk change (in my view), it may have to wait until
> after mickledore's out.
>
> Alex
>
> On Tue, 21 Mar 2023 at 18:26, Frederic Martinsons
> <frederic.martinsons@gmail.com> wrote:
> >
> > If dependencies between bitbake and other layers is an issue, I can
> suppress the patch n2 of this
> > series to disable the verification of the checksum, and wait for all
> patches (bitbake, meta-oe and oe-core)
> > to be applied.
> > Finally, I would issue a new patch to make the checksum verification
> mandatory.
> >
> > On Tue, 21 Mar 2023 at 18:02, Frédéric Martinsons <
> frederic.martinsons@gmail.com> wrote:
> >>
> >> Hello,
> >>
> >> Should I verify something more for this series?
> >> I understood that it is a useful development to have in bitbake fetcher
> (but will break things if series in oe-core and meta-oe are not applied
> after this one) and I'm wondering if something block the review / apply ?
> >>
> >> Please tell me, I'll do all my best to secure this.
> >>
> >> Have a good day.
> >>
> >> Le ven. 17 mars 2023, 09:19, <frederic.martinsons@gmail.com> a écrit :
> >>>
> >>> From: Frederic Martinsons <frederic.martinsons@gmail.com>
> >>>
> >>> This change brings checksum verification of each crate
> >>> in a recipe, e.g
> >>>
> >>> | SRC_URI += " \
> >>> |     crate://crates.io/aho-corasick/0.7.20 \
> >>> |     crate://crates.io/atomic-waker/1.1.0 \
> >>> |     crate://crates.io/cc/1.0.79 \
> >>> | "
> >>> |
> >>> | SRC_URI[aho-corasick.sha256sum] =
> "cc936419f96fa211c1b9166887b38e5e40b19958e5b895be7c1f93adec7071ac"
> >>> | SRC_URI[atomic-waker.sha256sum] =
> "debc29dde2e69f9e47506b525f639ed42300fc014a3e007832592448fa8e4599"
> >>> | SRC_URI[cc.sha256sum] =
> "50d30906286121d95be3d479533b458f87493b30a4b5f79a607db8f5d11aa91f"
> >>>
> >>> That will require to move the checksum initialization
> >>> after the possible call to urldata_init method in order
> >>> for the crate fetcher to parse the url.
> >>>
> >>> Another way of doing could have been implementing a decodeurl
> >>> method that would have been specific for each fetcher class.
> >>>
> >>> Signed-off-by: Frederic Martinsons <frederic.martinsons@gmail.com>
> >>> ---
> >>>  lib/bb/fetch2/__init__.py | 12 ++++++------
> >>>  lib/bb/tests/fetch.py     | 32 ++++++++++++++++++++++++++++++++
> >>>  2 files changed, 38 insertions(+), 6 deletions(-)
> >>>
> >>> diff --git a/lib/bb/fetch2/__init__.py b/lib/bb/fetch2/__init__.py
> >>> index cf65727a..3ae83fa8 100644
> >>> --- a/lib/bb/fetch2/__init__.py
> >>> +++ b/lib/bb/fetch2/__init__.py
> >>> @@ -1291,18 +1291,13 @@ class FetchData(object):
> >>>
> >>>              if checksum_name in self.parm:
> >>>                  checksum_expected = self.parm[checksum_name]
> >>> -            elif self.type not in ["http", "https", "ftp", "ftps",
> "sftp", "s3", "az"]:
> >>> +            elif self.type not in ["http", "https", "ftp", "ftps",
> "sftp", "s3", "az", "crate"]:
> >>>                  checksum_expected = None
> >>>              else:
> >>>                  checksum_expected = d.getVarFlag("SRC_URI",
> checksum_name)
> >>>
> >>>              setattr(self, "%s_expected" % checksum_id,
> checksum_expected)
> >>>
> >>> -        for checksum_id in CHECKSUM_LIST:
> >>> -            configure_checksum(checksum_id)
> >>> -
> >>> -        self.ignore_checksums = False
> >>> -
> >>>          self.names = self.parm.get("name",'default').split(',')
> >>>
> >>>          self.method = None
> >>> @@ -1324,6 +1319,11 @@ class FetchData(object):
> >>>          if hasattr(self.method, "urldata_init"):
> >>>              self.method.urldata_init(self, d)
> >>>
> >>> +        for checksum_id in CHECKSUM_LIST:
> >>> +            configure_checksum(checksum_id)
> >>> +
> >>> +        self.ignore_checksums = False
> >>> +
> >>>          if "localpath" in self.parm:
> >>>              # if user sets localpath for file, use it instead.
> >>>              self.localpath = self.parm["localpath"]
> >>> diff --git a/lib/bb/tests/fetch.py b/lib/bb/tests/fetch.py
> >>> index 73eefc59..fd089bc8 100644
> >>> --- a/lib/bb/tests/fetch.py
> >>> +++ b/lib/bb/tests/fetch.py
> >>> @@ -2377,6 +2377,13 @@ class CrateTest(FetcherTest):
> >>>          d = self.d
> >>>
> >>>          fetcher = bb.fetch2.Fetch(uris, self.d)
> >>> +        ud = fetcher.ud[fetcher.urls[0]]
> >>> +
> >>> +        self.assertIn("name", ud.parm)
> >>> +        self.assertEqual(ud.parm["name"], "glob")
> >>> +        self.assertIn("downloadfilename", ud.parm)
> >>> +        self.assertEqual(ud.parm["downloadfilename"],
> "glob-0.2.11.crate")
> >>> +
> >>>          fetcher.download()
> >>>          fetcher.unpack(self.tempdir)
> >>>          self.assertEqual(sorted(os.listdir(self.tempdir)),
> ['cargo_home', 'download' , 'unpacked'])
> >>> @@ -2394,6 +2401,19 @@ class CrateTest(FetcherTest):
> >>>          d = self.d
> >>>
> >>>          fetcher = bb.fetch2.Fetch(uris, self.d)
> >>> +        ud = fetcher.ud[fetcher.urls[0]]
> >>> +
> >>> +        self.assertIn("name", ud.parm)
> >>> +        self.assertEqual(ud.parm["name"], "glob")
> >>> +        self.assertIn("downloadfilename", ud.parm)
> >>> +        self.assertEqual(ud.parm["downloadfilename"],
> "glob-0.2.11.crate")
> >>> +
> >>> +        ud = fetcher.ud[fetcher.urls[1]]
> >>> +        self.assertIn("name", ud.parm)
> >>> +        self.assertEqual(ud.parm["name"], "time")
> >>> +        self.assertIn("downloadfilename", ud.parm)
> >>> +        self.assertEqual(ud.parm["downloadfilename"],
> "time-0.1.35.crate")
> >>> +
> >>>          fetcher.download()
> >>>          fetcher.unpack(self.tempdir)
> >>>          self.assertEqual(sorted(os.listdir(self.tempdir)),
> ['cargo_home', 'download' , 'unpacked'])
> >>> @@ -2403,6 +2423,18 @@ class CrateTest(FetcherTest):
> >>>          self.assertTrue(os.path.exists(self.tempdir +
> "/cargo_home/bitbake/time-0.1.35/.cargo-checksum.json"))
> >>>          self.assertTrue(os.path.exists(self.tempdir +
> "/cargo_home/bitbake/time-0.1.35/src/lib.rs"))
> >>>
> >>> +    @skipIfNoNetwork()
> >>> +    def test_crate_incorrect_cksum(self):
> >>> +        uri = "crate://crates.io/aho-corasick/0.7.20"
> >>> +        self.d.setVar('SRC_URI', uri)
> >>> +        self.d.setVarFlag("SRC_URI", "aho-corasick.sha256sum",
> hashlib.sha256("Invalid".encode("utf-8")).hexdigest())
> >>> +
> >>> +        uris = self.d.getVar('SRC_URI').split()
> >>> +
> >>> +        fetcher = bb.fetch2.Fetch(uris, self.d)
> >>> +        with self.assertRaisesRegexp(bb.fetch2.FetchError, "Fetcher
> failure for URL"):
> >>> +            fetcher.download()
> >>> +
> >>>  class NPMTest(FetcherTest):
> >>>      def skipIfNoNpm():
> >>>          import shutil
> >>> --
> >>> 2.34.1
> >>>
> >
> > -=-=-=-=-=-=-=-=-=-=-=-
> > Links: You receive all messages sent to this group.
> > View/Reply Online (#14609):
> https://lists.openembedded.org/g/bitbake-devel/message/14609
> > Mute This Topic: https://lists.openembedded.org/mt/97668729/1686489
> > Group Owner: bitbake-devel+owner@lists.openembedded.org
> > Unsubscribe: https://lists.openembedded.org/g/bitbake-devel/unsub [
> alex.kanavin@gmail.com]
> > -=-=-=-=-=-=-=-=-=-=-=-
> >
>

[-- Attachment #2: Type: text/html, Size: 12701 bytes --]

RE: [bitbake-devel][PATCH v2 3/3] crate.py: authorize crate url with parameters

[Peter Kjellerstedt]

Please cherry-pick this to Kirkstone and Langdale so that they can read 
updated recipes that use crate:// URIs with parameters.

I assume it should be ok to cherry-pick part one of this series too, as 
long as you do not cherry-pick part two, as it would be a breaking change.

//Peter

> -----Original Message-----
> From: bitbake-devel@lists.openembedded.org <bitbake-devel@lists.openembedded.org> On Behalf Of Frederic Martinsons
> Sent: den 17 mars 2023 09:19
> To: bitbake-devel@lists.openembedded.org
> Cc: Frederic Martinsons <frederic.martinsons@gmail.com>
> Subject: [bitbake-devel][PATCH v2 3/3] crate.py: authorize crate url with parameters
> 
> From: Frederic Martinsons <frederic.martinsons@gmail.com>
> 
> This allow to have classic fetch parameters
> (like destsuffix, sha256, name ...) not being
> considered by crate fetcher itself (and so mess
> up its download)
> 
> Moreover, it allow to overload the name of the downloaded
> crate (maybe usefull if there is a naming clash between
> two crates coming from different repositories)
> 
> Signed-off-by: Frederic Martinsons <frederic.martinsons@gmail.com>
> ---
>  lib/bb/fetch2/crate.py |  9 ++++++---
>  lib/bb/tests/fetch.py  | 24 ++++++++++++++++++++++++
>  2 files changed, 30 insertions(+), 3 deletions(-)
> 
> diff --git a/lib/bb/fetch2/crate.py b/lib/bb/fetch2/crate.py
> index f8367ed3..590dc9c1 100644
> --- a/lib/bb/fetch2/crate.py
> +++ b/lib/bb/fetch2/crate.py
> @@ -56,8 +56,10 @@ class Crate(Wget):
>          if len(parts) < 5:
>              raise bb.fetch2.ParameterError("Invalid URL: Must be crate://HOST/NAME/VERSION", ud.url)
> 
> -        # last field is version
> -        version = parts[len(parts) - 1]
> +        # version is expected to be the last token
> +        # but ignore possible url parameters which will be used
> +        # by the top fetcher class
> +        version, _, _ = parts[len(parts) -1].partition(";")
>          # second to last field is name
>          name = parts[len(parts) - 2]
>          # host (this is to allow custom crate registries to be specified
> @@ -69,7 +71,8 @@ class Crate(Wget):
> 
>          ud.url = "https://%s/%s/%s/download" % (host, name, version)
>          ud.parm['downloadfilename'] = "%s-%s.crate" % (name, version)
> -        ud.parm['name'] = name
> +        if 'name' not in ud.parm:
> +            ud.parm['name'] = name
> 
>          logger.debug2("Fetching %s to %s" % (ud.url, ud.parm['downloadfilename']))
> 
> diff --git a/lib/bb/tests/fetch.py b/lib/bb/tests/fetch.py
> index fd089bc8..85cf25e7 100644
> --- a/lib/bb/tests/fetch.py
> +++ b/lib/bb/tests/fetch.py
> @@ -2423,6 +2423,30 @@ class CrateTest(FetcherTest):
>          self.assertTrue(os.path.exists(self.tempdir + "/cargo_home/bitbake/time-0.1.35/.cargo-checksum.json"))
>          self.assertTrue(os.path.exists(self.tempdir + "/cargo_home/bitbake/time-0.1.35/src/lib.rs"))
> 
> +    @skipIfNoNetwork()
> +    def test_crate_url_params(self):
> +
> +        uri = "crate://crates.io/aho-corasick/0.7.20;name=aho-corasick-renamed"
> +        self.d.setVar('SRC_URI', uri)
> +
> +        uris = self.d.getVar('SRC_URI').split()
> +        d = self.d
> +
> +        fetcher = bb.fetch2.Fetch(uris, self.d)
> +        ud = fetcher.ud[fetcher.urls[0]]
> +
> +        self.assertIn("name", ud.parm)
> +        self.assertEqual(ud.parm["name"], "aho-corasick-renamed")
> +        self.assertIn("downloadfilename", ud.parm)
> +        self.assertEqual(ud.parm["downloadfilename"], "aho-corasick-0.7.20.crate")
> +
> +        fetcher.download()
> +        fetcher.unpack(self.tempdir)
> +        self.assertEqual(sorted(os.listdir(self.tempdir)), ['cargo_home', 'download' , 'unpacked'])
> +        self.assertEqual(sorted(os.listdir(self.tempdir + "/download")), ['aho-corasick-0.7.20.crate', 'aho-corasick-0.7.20.crate.done'])
> +        self.assertTrue(os.path.exists(self.tempdir + "/cargo_home/bitbake/aho-corasick-0.7.20/.cargo-checksum.json"))
> +        self.assertTrue(os.path.exists(self.tempdir + "/cargo_home/bitbake/aho-corasick-0.7.20/src/lib.rs"))
> +
>      @skipIfNoNetwork()
>      def test_crate_incorrect_cksum(self):
>          uri = "crate://crates.io/aho-corasick/0.7.20"
> --
> 2.34.1

Re: [bitbake-devel][PATCH v2 3/3] crate.py: authorize crate url with parameters

[Richard Purdie]

On Thu, 2023-04-06 at 03:40 +0000, Peter Kjellerstedt wrote:
> Please cherry-pick this to Kirkstone and Langdale so that they can read 
> updated recipes that use crate:// URIs with parameters.
> 
> I assume it should be ok to cherry-pick part one of this series too, as 
> long as you do not cherry-pick part two, as it would be a breaking change.

If we do this does this mean we're committing to forwards
compatibility? I'm not sure I like that precedent...

Cheers,

Richard

RE: [bitbake-devel][PATCH v2 3/3] crate.py: authorize crate url with parameters

[Peter Kjellerstedt]

> -----Original Message-----
> From: Richard Purdie <richard.purdie@linuxfoundation.org>
> Sent: den 6 april 2023 10:43
> To: Peter Kjellerstedt <peter.kjellerstedt@axis.com>; Steve Sakoman
> <steve@sakoman.com>
> Cc: Frederic Martinsons <frederic.martinsons@gmail.com>; bitbake-
> devel@lists.openembedded.org
> Subject: Re: [bitbake-devel][PATCH v2 3/3] crate.py: authorize crate url
> with parameters
> 
> On Thu, 2023-04-06 at 03:40 +0000, Peter Kjellerstedt wrote:
> > Please cherry-pick this to Kirkstone and Langdale so that they can read
> > updated recipes that use crate:// URIs with parameters.
> >
> > I assume it should be ok to cherry-pick part one of this series too, as
> > long as you do not cherry-pick part two, as it would be a breaking
> change.
> 
> If we do this does this mean we're committing to forwards compatibility? 
> I'm not sure I like that precedent...

While not a requirement, I sure hope we can strive for forward compatibility 
where it does not impact the older branches negatively. At least in cases 
where it affects parsability or involves parts that are hard to override 
in a higher layer.

The problem for me in this case is that Mickledore now requires that all 
URIs have a checksum, which basically requires that parameters are used 
for the crate:// URIs since it is quite common that recipes depend on 
multiple versions of the same crate and thus requires the name= parameter. 
At the same time the crate fetcher in Kirkstone and Langdale will fail 
if any parameters are specified for the URIs. This means it is not possible 
to use the same version of the generated crates.inc files, which causes 
a lot of maintenance burden.

It is also very hard to modify the crate fetcher without actually modifying 
OE-Core. At least I find my Python-fu lacking in regards to how I can 
monkeypatch the fetcher to use a modified version of crate.py.

On the other, it is trivial for you to backport this patch to the two 
branches in OE-Core. And as long as the second patch in the series (which 
requires that checksums are used) is not backported, I cannot see that it 
should have any negative impact.

> 
> Cheers,
> 
> Richard

//Peter

Re: [bitbake-devel][PATCH v2 3/3] crate.py: authorize crate url with parameters

[Richard Purdie]

On Thu, 2023-04-06 at 10:56 +0000, Peter Kjellerstedt wrote:
> > -----Original Message-----
> > From: Richard Purdie <richard.purdie@linuxfoundation.org>
> > Sent: den 6 april 2023 10:43
> > To: Peter Kjellerstedt <peter.kjellerstedt@axis.com>; Steve Sakoman
> > <steve@sakoman.com>
> > Cc: Frederic Martinsons <frederic.martinsons@gmail.com>; bitbake-
> > devel@lists.openembedded.org
> > Subject: Re: [bitbake-devel][PATCH v2 3/3] crate.py: authorize crate url
> > with parameters
> > 
> > On Thu, 2023-04-06 at 03:40 +0000, Peter Kjellerstedt wrote:
> > > Please cherry-pick this to Kirkstone and Langdale so that they can read
> > > updated recipes that use crate:// URIs with parameters.
> > > 
> > > I assume it should be ok to cherry-pick part one of this series too, as
> > > long as you do not cherry-pick part two, as it would be a breaking
> > change.
> > 
> > If we do this does this mean we're committing to forwards compatibility? 
> > I'm not sure I like that precedent...
> 
> While not a requirement, I sure hope we can strive for forward compatibility 
> where it does not impact the older branches negatively. At least in cases 
> where it affects parsability or involves parts that are hard to override 
> in a higher layer.

The trouble is I take this patch, then next time something breaks you
tell me "in the past such changes were only made so that this didn't
break things" and quote it as precedent.

I worry a lot about people relying upon us doing this as it isn't a
guarantee we've ever made.

> The problem for me in this case is that Mickledore now requires that all 
> URIs have a checksum, which basically requires that parameters are used 
> for the crate:// URIs since it is quite common that recipes depend on
> multiple versions of the same crate and thus requires the name= parameter. 
> At the same time the crate fetcher in Kirkstone and Langdale will fail 
> if any parameters are specified for the URIs. This means it is not possible 
> to use the same version of the generated crates.inc files, which causes 
> a lot of maintenance burden.

I understand the problem but you're effectively asking we would always
do this going forward too :/.

> It is also very hard to modify the crate fetcher without actually modifying 
> OE-Core. At least I find my Python-fu lacking in regards to how I can
> monkeypatch the fetcher to use a modified version of crate.py.
> 
> On the other, it is trivial for you to backport this patch to the two
> branches in OE-Core. And as long as the second patch in the series (which 
> requires that checksums are used) is not backported, I cannot see that it 
> should have any negative impact.

This change alone is fine. The behaviour where people expect older
layers to always work with newer code changes is what worries me, a
lot. We have done things to make this happen before and I'm not sure it
is good to continue the expectation.

Cheers,

Richard