diff mbox series

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

Message ID 20230317081916.1857201-1-frederic.martinsons@gmail.com
State New
Headers show
Series [bitbake-devel,v2,1/3] fetch2: Add checksum capability for crate fetcher | expand

Commit Message

Frédéric Martinsons March 17, 2023, 8:19 a.m. UTC 
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(-)

Comments

Frédéric Martinsons March 21, 2023, 5:02 p.m. UTC | #1 
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
>
>
Frédéric Martinsons March 21, 2023, 5:26 p.m. UTC | #2 
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
>>
>>
Alexander Kanavin March 21, 2023, 6:34 p.m. UTC | #3 
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]
> -=-=-=-=-=-=-=-=-=-=-=-
>
Frédéric Martinsons March 21, 2023, 7:51 p.m. UTC | #4 
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]
> > -=-=-=-=-=-=-=-=-=-=-=-
> >
>
diff mbox series

Patch

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