diff mbox series

fetch2: Add checksum capability for crate fetcher

Message ID 20230315131513.50635-1-frederic.martinsons@gmail.com
State Accepted, archived
Commit 4920686c13dd66f9bfa4f7dd38d6e955f153eeec
Headers show
Series fetch2: Add checksum capability for crate fetcher | expand

Commit Message

Frédéric Martinsons March 15, 2023, 1:15 p.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

Alex Kiernan March 15, 2023, 3:03 p.m. UTC | #1
On Wed, Mar 15, 2023 at 1:15 PM Frederic Martinsons
<frederic.martinsons@gmail.com> wrote:
>
> 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>

I really don't think this is the right direction, at least not by
default. If a crate is your primary build artefact then this is
useful:

https://bugzilla.yoctoproject.org/show_bug.cgi?id=15012

But doing this for all the dependency crates we're just repeating
what's in the Cargo.lock which should always be part of the primary
artefact.

> ---
>  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 (#14563): https://lists.openembedded.org/g/bitbake-devel/message/14563
> Mute This Topic: https://lists.openembedded.org/mt/97626961/3618097
> Group Owner: bitbake-devel+owner@lists.openembedded.org
> Unsubscribe: https://lists.openembedded.org/g/bitbake-devel/unsub [alex.kiernan@gmail.com]
> -=-=-=-=-=-=-=-=-=-=-=-
>
Frédéric Martinsons March 15, 2023, 3:25 p.m. UTC | #2
On Wed, Mar 15, 2023 at 04:03 PM, Alex Kiernan wrote:

>
> On Wed, Mar 15, 2023 at 1:15 PM Frederic Martinsons
> <frederic.martinsons@gmail.com> wrote:
> >
> > 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>
> 
> I really don't think this is the right direction, at least not by
> default. If a crate is your primary build artefact then this is
> useful:
> 
> https://bugzilla.yoctoproject.org/show_bug.cgi?id=15012
> 
> But doing this for all the dependency crates we're just repeating
> what's in the Cargo.lock which should always be part of the primary
> artefact.
> 

I see what you mean, but will not be useful for library crate which doesn't have
a Cargo.lock ? As you said in another thread, this use case should not be very 
current though (except maybe to run test on it ...).

Another thing I see is that the checkum verification by cargo happen late (build step)
comparing to other fetcher which do this during the fetch.

Anyway, it's your call, I don't mind not having this merged... the dev I lead today
allow me to better understand bitbake fetcher and unit tests.
Alexander Kanavin March 15, 2023, 3:50 p.m. UTC | #3
On Wed, 15 Mar 2023 at 16:03, Alex Kiernan <alex.kiernan@gmail.com> wrote:
> I really don't think this is the right direction, at least not by
> default. If a crate is your primary build artefact then this is
> useful:
>
> https://bugzilla.yoctoproject.org/show_bug.cgi?id=15012
>
> But doing this for all the dependency crates we're just repeating
> what's in the Cargo.lock which should always be part of the primary
> artefact.

Maybe cargo will verify (some) items against Cargo.lock. Maybe it
won't. Upstream cargo isn't something we develop, and it may add and
remove various exceptions and special cases.

Anyway, we cannot actually trust cargo (unlike, say, git), and we need
to do our own checks, and keep the checksums in files external to the
source tree. This will free us from worrying about whether cargo
protects us from supply chain attacks properly or not.

Alex
Martin Jansa March 15, 2023, 5:07 p.m. UTC | #4
On Wed, Mar 15, 2023 at 4:50 PM Alexander Kanavin <alex.kanavin@gmail.com>
wrote:

> On Wed, 15 Mar 2023 at 16:03, Alex Kiernan <alex.kiernan@gmail.com> wrote:
> > I really don't think this is the right direction, at least not by
> > default. If a crate is your primary build artefact then this is
> > useful:
> >
> > https://bugzilla.yoctoproject.org/show_bug.cgi?id=15012
> >
> > But doing this for all the dependency crates we're just repeating
> > what's in the Cargo.lock which should always be part of the primary
> > artefact.
>
> Maybe cargo will verify (some) items against Cargo.lock. Maybe it
> won't. Upstream cargo isn't something we develop, and it may add and
> remove various exceptions and special cases.
>
> Anyway, we cannot actually trust cargo (unlike, say, git), and we need
> to do our own checks, and keep the checksums in files external to the
> source tree. This will free us from worrying about whether cargo
> protects us from supply chain attacks properly or not.
>

Also if bitbake fetcher detects unexpected checksum it can easily rename
the file in DL_DIR and try fetching it again, because that's what fetcher
does for other downloads and fetcher "owns" DL_DIR.

If cargo complains about unexpected checksum, than cargo fails and user
will have to make sure that wrongly fetched crate is removed from DL_DIR.

Regards,
Alex Kiernan March 15, 2023, 5:43 p.m. UTC | #5
On Wed, Mar 15, 2023 at 5:08 PM Martin Jansa <martin.jansa@gmail.com> wrote:
>
> On Wed, Mar 15, 2023 at 4:50 PM Alexander Kanavin <alex.kanavin@gmail.com> wrote:
>>
>> On Wed, 15 Mar 2023 at 16:03, Alex Kiernan <alex.kiernan@gmail.com> wrote:
>> > I really don't think this is the right direction, at least not by
>> > default. If a crate is your primary build artefact then this is
>> > useful:
>> >
>> > https://bugzilla.yoctoproject.org/show_bug.cgi?id=15012
>> >
>> > But doing this for all the dependency crates we're just repeating
>> > what's in the Cargo.lock which should always be part of the primary
>> > artefact.
>>
>> Maybe cargo will verify (some) items against Cargo.lock. Maybe it
>> won't. Upstream cargo isn't something we develop, and it may add and
>> remove various exceptions and special cases.
>>
>> Anyway, we cannot actually trust cargo (unlike, say, git), and we need
>> to do our own checks, and keep the checksums in files external to the
>> source tree. This will free us from worrying about whether cargo
>> protects us from supply chain attacks properly or not.
>
>
> Also if bitbake fetcher detects unexpected checksum it can easily rename the file in DL_DIR and try fetching it again, because that's what fetcher does for other downloads and fetcher "owns" DL_DIR.
>
> If cargo complains about unexpected checksum, than cargo fails and user will have to make sure that wrongly fetched crate is removed from DL_DIR.
>
> Regards,

Okay, you're convincing me :) I've definitely tripped over that!

I guess we need to update the cargo crate updater class and librsvg too.
Frédéric Martinsons March 15, 2023, 5:54 p.m. UTC | #6
Just to tell that my concern was about making this checksum verification
not mandatory, if the recipe didn't specify any checksum for crates, my
modification will let it trough.

If we want to make it mandatory, we just have to make the method
"recommends_checksum" in crate.py return True instead of False.

Alex, when you talk about updating librsvg and cargo crate updater, do you
mind elaborate?

If there is some things to rework/add in my patches I'll be glad to do so
(if you tell me what is expected)

Le mer. 15 mars 2023, 18:43, Alex Kiernan <alex.kiernan@gmail.com> a écrit :

> On Wed, Mar 15, 2023 at 5:08 PM Martin Jansa <martin.jansa@gmail.com>
> wrote:
> >
> > On Wed, Mar 15, 2023 at 4:50 PM Alexander Kanavin <
> alex.kanavin@gmail.com> wrote:
> >>
> >> On Wed, 15 Mar 2023 at 16:03, Alex Kiernan <alex.kiernan@gmail.com>
> wrote:
> >> > I really don't think this is the right direction, at least not by
> >> > default. If a crate is your primary build artefact then this is
> >> > useful:
> >> >
> >> > https://bugzilla.yoctoproject.org/show_bug.cgi?id=15012
> >> >
> >> > But doing this for all the dependency crates we're just repeating
> >> > what's in the Cargo.lock which should always be part of the primary
> >> > artefact.
> >>
> >> Maybe cargo will verify (some) items against Cargo.lock. Maybe it
> >> won't. Upstream cargo isn't something we develop, and it may add and
> >> remove various exceptions and special cases.
> >>
> >> Anyway, we cannot actually trust cargo (unlike, say, git), and we need
> >> to do our own checks, and keep the checksums in files external to the
> >> source tree. This will free us from worrying about whether cargo
> >> protects us from supply chain attacks properly or not.
> >
> >
> > Also if bitbake fetcher detects unexpected checksum it can easily rename
> the file in DL_DIR and try fetching it again, because that's what fetcher
> does for other downloads and fetcher "owns" DL_DIR.
> >
> > If cargo complains about unexpected checksum, than cargo fails and user
> will have to make sure that wrongly fetched crate is removed from DL_DIR.
> >
> > Regards,
>
> Okay, you're convincing me :) I've definitely tripped over that!
>
> I guess we need to update the cargo crate updater class and librsvg too.
>
> --
> Alex Kiernan
>
Alexander Kanavin March 15, 2023, 6:08 p.m. UTC | #7
On Wed, 15 Mar 2023 at 18:54, Frédéric Martinsons
<frederic.martinsons@gmail.com> wrote:
> Alex, when you talk about updating librsvg and cargo crate updater, do you mind elaborate?

classes-recipe/cargo-update-recipe-crates.bbclass should be updated to
write out not just the crate list into SRC_URI, but also their
checksums (this is not hard because it works from Cargo.lock, and that
already provides the checksums).

Then the class should be used to update all recipes (e.g.
python3-bcrypt, python3-cryptography) that use the crate:// fetcher.

Alex
Frédéric Martinsons March 15, 2023, 6:26 p.m. UTC | #8
Alright, I will add another patch for that in the coming days.

But I still need some answers to go further:
    - Should checksum verification for crate fetcher be mandatory ?
    - What about librsvg ? It doesn't include any crate:// but needs to
inherit cargo_common, what should I do for this recipe ?


Le mer. 15 mars 2023, 19:09, Alexander Kanavin <alex.kanavin@gmail.com> a
écrit :

> On Wed, 15 Mar 2023 at 18:54, Frédéric Martinsons
> <frederic.martinsons@gmail.com> wrote:
> > Alex, when you talk about updating librsvg and cargo crate updater, do
> you mind elaborate?
>
> classes-recipe/cargo-update-recipe-crates.bbclass should be updated to
> write out not just the crate list into SRC_URI, but also their
> checksums (this is not hard because it works from Cargo.lock, and that
> already provides the checksums).
>
> Then the class should be used to update all recipes (e.g.
> python3-bcrypt, python3-cryptography) that use the crate:// fetcher.
>
> Alex
>
Alex Kiernan March 15, 2023, 6:47 p.m. UTC | #9
On Wed, Mar 15, 2023 at 6:26 PM Frédéric Martinsons
<frederic.martinsons@gmail.com> wrote:
>
> Alright, I will add another patch for that in the coming days.
>
> But I still need some answers to go further:
>     - Should checksum verification for crate fetcher be mandatory ?
>     - What about librsvg ? It doesn't include any crate:// but needs to inherit cargo_common, what should I do for this recipe ?
>

Sorry that was from the top of my head as a rust recipe - it doesn't
obviously need changes as it's not a cargo build.

>
> Le mer. 15 mars 2023, 19:09, Alexander Kanavin <alex.kanavin@gmail.com> a écrit :
>>
>> On Wed, 15 Mar 2023 at 18:54, Frédéric Martinsons
>> <frederic.martinsons@gmail.com> wrote:
>> > Alex, when you talk about updating librsvg and cargo crate updater, do you mind elaborate?
>>
>> classes-recipe/cargo-update-recipe-crates.bbclass should be updated to
>> write out not just the crate list into SRC_URI, but also their
>> checksums (this is not hard because it works from Cargo.lock, and that
>> already provides the checksums).
>>
>> Then the class should be used to update all recipes (e.g.
>> python3-bcrypt, python3-cryptography) that use the crate:// fetcher.
>>

These ones are the ones which will need work!
Alexander Kanavin March 15, 2023, 6:47 p.m. UTC | #10
On Wed, 15 Mar 2023 at 19:26, Frédéric Martinsons
<frederic.martinsons@gmail.com> wrote:
> But I still need some answers to go further:
>     - Should checksum verification for crate fetcher be mandatory ?

Yes.

>     - What about librsvg ? It doesn't include any crate:// but needs to inherit cargo_common, what should I do for this recipe ?

I think this was mentioned by mistake. You might want to check if
meta-oe recipes would need fixing though.

Alex
Frédéric Martinsons March 15, 2023, 7 p.m. UTC | #11
Ok great, I have everything I need go further on this. Thank you all

Le mer. 15 mars 2023, 19:47, Alexander Kanavin <alex.kanavin@gmail.com> a
écrit :

> On Wed, 15 Mar 2023 at 19:26, Frédéric Martinsons
> <frederic.martinsons@gmail.com> wrote:
> > But I still need some answers to go further:
> >     - Should checksum verification for crate fetcher be mandatory ?
>
> Yes.
>
> >     - What about librsvg ? It doesn't include any crate:// but needs to
> inherit cargo_common, what should I do for this recipe ?
>
> I think this was mentioned by mistake. You might want to check if
> meta-oe recipes would need fixing though.
>
> Alex
>
Frédéric Martinsons March 16, 2023, 8:20 a.m. UTC | #12
Hello Alexander,

I almost finished the patch and I checked in meta-oe for recipes to be
updated,
I see two : python3-pyruvate and uutils-coreutils

But those recipes don't inherit cargo-update-crates,  python3-pyruvate
inherit
python_setuptools3_rust and seems to have been written manually.
And uutils-coreutils has been generated by cargo bitbake.

Should I modify them to use the update class or should I put the checksum
"by hand"?
Moreover, since this dev will be cross dependent (impact on meta-oe,
oe-core and bitbake)
how do we proceed for the different patches to be merged at the same time?

Le mer. 15 mars 2023, 20:00, Frederic Martinsons via lists.openembedded.org
<frederic.martinsons=gmail.com@lists.openembedded.org> a écrit :

> Ok great, I have everything I need go further on this. Thank you all
>
> Le mer. 15 mars 2023, 19:47, Alexander Kanavin <alex.kanavin@gmail.com> a
> écrit :
>
>> On Wed, 15 Mar 2023 at 19:26, Frédéric Martinsons
>> <frederic.martinsons@gmail.com> wrote:
>> > But I still need some answers to go further:
>> >     - Should checksum verification for crate fetcher be mandatory ?
>>
>> Yes.
>>
>> >     - What about librsvg ? It doesn't include any crate:// but needs to
>> inherit cargo_common, what should I do for this recipe ?
>>
>> I think this was mentioned by mistake. You might want to check if
>> meta-oe recipes would need fixing though.
>>
>> Alex
>>
>
> -=-=-=-=-=-=-=-=-=-=-=-
> Links: You receive all messages sent to this group.
> View/Reply Online (#14575):
> https://lists.openembedded.org/g/bitbake-devel/message/14575
> Mute This Topic: https://lists.openembedded.org/mt/97626961/6213388
> Group Owner: bitbake-devel+owner@lists.openembedded.org
> Unsubscribe: https://lists.openembedded.org/g/bitbake-devel/unsub [
> frederic.martinsons@gmail.com]
> -=-=-=-=-=-=-=-=-=-=-=-
>
>
Alexander Kanavin March 16, 2023, 8:23 a.m. UTC | #13
On Thu, 16 Mar 2023 at 09:20, Frédéric Martinsons
<frederic.martinsons@gmail.com> wrote:
> I almost finished the patch and I checked in meta-oe for recipes to be updated,
> I see two : python3-pyruvate and uutils-coreutils
>
> But those recipes don't inherit cargo-update-crates,  python3-pyruvate inherit
> python_setuptools3_rust and seems to have been written manually.
> And uutils-coreutils has been generated by cargo bitbake.
> Should I modify them to use the update class or should I put the checksum "by hand"?

You should convert them to use the official class. Maintaining long
lists of checksums by hand does not scale.

> Moreover, since this dev will be cross dependent (impact on meta-oe, oe-core and bitbake)
> how do we proceed for the different patches to be merged at the same time?

You should submit all needed patches at the same time to respective
mailing lists. Cross-reference them in commit messages (e.g. mention
where the other patches are), and integrators will be able to minimize
the window of breakage.

Alex
Frédéric Martinsons March 16, 2023, 10:06 a.m. UTC | #14
On Thu, 16 Mar 2023 at 09:23, Alexander Kanavin <alex.kanavin@gmail.com>
wrote:
>
> On Thu, 16 Mar 2023 at 09:20, Frédéric Martinsons
> <frederic.martinsons@gmail.com> wrote:
> > I almost finished the patch and I checked in meta-oe for recipes to be
updated,
> > I see two : python3-pyruvate and uutils-coreutils
> >
> > But those recipes don't inherit cargo-update-crates,  python3-pyruvate
inherit
> > python_setuptools3_rust and seems to have been written manually.
> > And uutils-coreutils has been generated by cargo bitbake.
> > Should I modify them to use the update class or should I put the
checksum "by hand"?
>
> You should convert them to use the official class. Maintaining long
> lists of checksums by hand does not scale.
>

I managed easily to update uutils-coreutils
<https://github.com/openembedded/meta-openembedded/tree/master/meta-oe/recipes-core/uutils-coreutils>
(I
removed its README.txt by the way) but
I cannot do it for python3-pyruvate
<https://github.com/openembedded/meta-openembedded/blob/master/meta-python/recipes-devtools/python/python3-pyruvate_1.1.2.bb>
because
I cannot find a Cargo.lock file, the git
repository had once though (see the tag under gitlab
<https://gitlab.com/tschorr/pyruvate/-/tree/1.1.2>) so it seems that it is
related
to the fetch by pypi which doesn't embed the Cargo.lock (or a bug in the
packaging
of the project). I don't know what to do with this one.

> > Moreover, since this dev will be cross dependent (impact on meta-oe,
oe-core and bitbake)
> > how do we proceed for the different patches to be merged at the same
time?
>
> You should submit all needed patches at the same time to respective
> mailing lists. Cross-reference them in commit messages (e.g. mention
> where the other patches are), and integrators will be able to minimize
> the window of breakage.
>
> Alex

Understood
Alexander Kanavin March 16, 2023, 10:42 a.m. UTC | #15
On Thu, 16 Mar 2023 at 11:07, Frédéric Martinsons
<frederic.martinsons@gmail.com> wrote:
> I managed easily to update uutils-coreutils (I removed its README.txt by the way) but
> I cannot do it for python3-pyruvate because I cannot find a Cargo.lock file, the git
> repository had once though (see the tag under gitlab) so it seems that it is related
> to the fetch by pypi which doesn't embed the Cargo.lock (or a bug in the packaging
> of the project). I don't know what to do with this one.

It does seem like a bug, how can it build without Cargo.lock?
You can inject the Cargo.lock from the git repo into the tree, and
generate checksums with that.

Alex
Frédéric Martinsons March 16, 2023, 10:52 a.m. UTC | #16
A rust/cargo project can perfectly be built without Cargo.lock since cargo
will generate
it on the fly.

I'll provide the Cargo.lock from git along with the recipe then.



On Thu, 16 Mar 2023 at 11:42, Alexander Kanavin <alex.kanavin@gmail.com>
wrote:

> On Thu, 16 Mar 2023 at 11:07, Frédéric Martinsons
> <frederic.martinsons@gmail.com> wrote:
> > I managed easily to update uutils-coreutils (I removed its README.txt by
> the way) but
> > I cannot do it for python3-pyruvate because I cannot find a Cargo.lock
> file, the git
> > repository had once though (see the tag under gitlab) so it seems that
> it is related
> > to the fetch by pypi which doesn't embed the Cargo.lock (or a bug in the
> packaging
> > of the project). I don't know what to do with this one.
>
> It does seem like a bug, how can it build without Cargo.lock?
> You can inject the Cargo.lock from the git repo into the tree, and
> generate checksums with that.
>
> Alex
>
Alex Kiernan March 16, 2023, 12:24 p.m. UTC | #17
On Thu, Mar 16, 2023 at 10:52 AM Frédéric Martinsons
<frederic.martinsons@gmail.com> wrote:
>
> A rust/cargo project can perfectly be built without Cargo.lock since cargo will generate
> it on the fly.
>

But what you build is undefined as you'll get the latest non-yanked
crate that satisfies the dependency solver.

> I'll provide the Cargo.lock from git along with the recipe then.
>
>
>
> On Thu, 16 Mar 2023 at 11:42, Alexander Kanavin <alex.kanavin@gmail.com> wrote:
>>
>> On Thu, 16 Mar 2023 at 11:07, Frédéric Martinsons
>> <frederic.martinsons@gmail.com> wrote:
>> > I managed easily to update uutils-coreutils (I removed its README.txt by the way) but
>> > I cannot do it for python3-pyruvate because I cannot find a Cargo.lock file, the git
>> > repository had once though (see the tag under gitlab) so it seems that it is related
>> > to the fetch by pypi which doesn't embed the Cargo.lock (or a bug in the packaging
>> > of the project). I don't know what to do with this one.
>>
>> It does seem like a bug, how can it build without Cargo.lock?
>> You can inject the Cargo.lock from the git repo into the tree, and
>> generate checksums with that.
>>
>> Alex
Frédéric Martinsons March 16, 2023, 12:31 p.m. UTC | #18
True.

Le jeu. 16 mars 2023, 13:24, Alex Kiernan <alex.kiernan@gmail.com> a écrit :

> On Thu, Mar 16, 2023 at 10:52 AM Frédéric Martinsons
> <frederic.martinsons@gmail.com> wrote:
> >
> > A rust/cargo project can perfectly be built without Cargo.lock since
> cargo will generate
> > it on the fly.
> >
>
> But what you build is undefined as you'll get the latest non-yanked
> crate that satisfies the dependency solver.
>
> > I'll provide the Cargo.lock from git along with the recipe then.
> >
> >
> >
> > On Thu, 16 Mar 2023 at 11:42, Alexander Kanavin <alex.kanavin@gmail.com>
> wrote:
> >>
> >> On Thu, 16 Mar 2023 at 11:07, Frédéric Martinsons
> >> <frederic.martinsons@gmail.com> wrote:
> >> > I managed easily to update uutils-coreutils (I removed its README.txt
> by the way) but
> >> > I cannot do it for python3-pyruvate because I cannot find a
> Cargo.lock file, the git
> >> > repository had once though (see the tag under gitlab) so it seems
> that it is related
> >> > to the fetch by pypi which doesn't embed the Cargo.lock (or a bug in
> the packaging
> >> > of the project). I don't know what to do with this one.
> >>
> >> It does seem like a bug, how can it build without Cargo.lock?
> >> You can inject the Cargo.lock from the git repo into the tree, and
> >> generate checksums with that.
> >>
> >> Alex
>
>
>
> --
> Alex Kiernan
>
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