diff mbox series

[master,mickledore,2/2] fetch2/crate: Correct unpack for a crate that matches the recipe name

Message ID 20230429012329.1799116-2-pkj@axis.com
State New
Headers show
Series [master,mickledore,1/2] fetch2/crate: Simplify extraction of crate names and versions from URIs | expand

Commit Message

Peter Kjellerstedt April 29, 2023, 1:23 a.m. UTC
The crate fetcher handles a crate with a name that matches the recipe's
name specially by placing the unpacked code in the current directory
(which typically is ${S}) rather than together with the sources for the
other crates. This broke when the URI names for all crates were changed
recently to include the version in the name.

Correct the crate fetcher to test against ${BP} instead of ${BPN}.
Also add a test case to the selftests to avoid this breaking again.

Signed-off-by: Peter Kjellerstedt <peter.kjellerstedt@axis.com>
---
 bitbake/lib/bb/fetch2/crate.py |  4 ++--
 bitbake/lib/bb/tests/fetch.py  | 25 +++++++++++++++++++++++++
 2 files changed, 27 insertions(+), 2 deletions(-)

Comments

Frédéric Martinsons April 29, 2023, 6:48 a.m. UTC | #1
Hello Peter,

Do you think it will solve the issue I mentioned there
<https://bugzilla.yoctoproject.org/show_bug.cgi?id=15012#c4> ?

On Sat, 29 Apr 2023 at 03:23, Peter Kjellerstedt <
peter.kjellerstedt@axis.com> wrote:

> The crate fetcher handles a crate with a name that matches the recipe's
> name specially by placing the unpacked code in the current directory
> (which typically is ${S}) rather than together with the sources for the
> other crates. This broke when the URI names for all crates were changed
> recently to include the version in the name.
>
> Correct the crate fetcher to test against ${BP} instead of ${BPN}.
> Also add a test case to the selftests to avoid this breaking again.
>
> Signed-off-by: Peter Kjellerstedt <peter.kjellerstedt@axis.com>
> ---
>  bitbake/lib/bb/fetch2/crate.py |  4 ++--
>  bitbake/lib/bb/tests/fetch.py  | 25 +++++++++++++++++++++++++
>  2 files changed, 27 insertions(+), 2 deletions(-)
>
> diff --git a/bitbake/lib/bb/fetch2/crate.py
> b/bitbake/lib/bb/fetch2/crate.py
> index 2b8b6bc7a1..3310ed0050 100644
> --- a/bitbake/lib/bb/fetch2/crate.py
> +++ b/bitbake/lib/bb/fetch2/crate.py
> @@ -98,8 +98,8 @@ class Crate(Wget):
>          save_cwd = os.getcwd()
>          os.chdir(rootdir)
>
> -        pn = d.getVar('BPN')
> -        if pn == ud.parm.get('name'):
> +        bp = d.getVar('BP')
> +        if bp == ud.parm.get('name'):
>              cmd = "tar -xz --no-same-owner -f %s" % thefile
>          else:
>              cargo_bitbake = self._cargo_bitbake_path(rootdir)
> diff --git a/bitbake/lib/bb/tests/fetch.py b/bitbake/lib/bb/tests/fetch.py
> index 6ef0836f2b..23bfa788db 100644
> --- a/bitbake/lib/bb/tests/fetch.py
> +++ b/bitbake/lib/bb/tests/fetch.py
> @@ -2391,6 +2391,31 @@ class CrateTest(FetcherTest):
>          self.assertTrue(os.path.exists(self.tempdir +
> "/cargo_home/bitbake/glob-0.2.11/.cargo-checksum.json"))
>          self.assertTrue(os.path.exists(self.tempdir +
> "/cargo_home/bitbake/glob-0.2.11/src/lib.rs"))
>
> +    @skipIfNoNetwork()
> +    def test_crate_url_matching_recipe(self):
> +
> +        self.d.setVar('BP', 'glob-0.2.11')
> +
> +        uri = "crate://crates.io/glob/0.2.11"
> +        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"], "glob-0.2.11")
> +        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)), ['download',
> 'glob-0.2.11', 'unpacked'])
> +        self.assertEqual(sorted(os.listdir(self.tempdir + "/download")),
> ['glob-0.2.11.crate', 'glob-0.2.11.crate.done'])
> +        self.assertTrue(os.path.exists(self.tempdir + "/glob-0.2.11/src/
> lib.rs"))
> +
>      @skipIfNoNetwork()
>      def test_crate_url_params(self):
>
>
> -=-=-=-=-=-=-=-=-=-=-=-
> Links: You receive all messages sent to this group.
> View/Reply Online (#14758):
> https://lists.openembedded.org/g/bitbake-devel/message/14758
> Mute This Topic: https://lists.openembedded.org/mt/98571257/6213388
> Group Owner: bitbake-devel+owner@lists.openembedded.org
> Unsubscribe: https://lists.openembedded.org/g/bitbake-devel/unsub [
> frederic.martinsons@gmail.com]
> -=-=-=-=-=-=-=-=-=-=-=-
>
>
Peter Kjellerstedt April 29, 2023, 8:22 p.m. UTC | #2
Yes. While the original issue was written about missing checksuming, which was fixed by requiring checksums for the crate URIs in the SRC_URI, the additional concern you raised is fixed by my patch. It restores the support for using a crate as the main component for the recipe. This was in the bbclass all along, it just broke when the names were changed to include the crate versions.

You can actually workaround the problem by setting S = "${CARGO_VENDORING_DIRECTORY}/${BP}", but I believe it is better to restore the intended functionality of the bbclass. It also makes things much more intuitive when using devtool modify for such a recipe since the source is where you expect it to be.

@Alexandre Belloni: Since I assume it will be you that pulls this into testing, feel free to add the following to the commit message:

[Yocto #15012]

//Peter

From: Frédéric Martinsons <frederic.martinsons@gmail.com>
Sent: den 29 april 2023 08:48
To: Peter Kjellerstedt <peter.kjellerstedt@axis.com>
Cc: bitbake-devel@lists.openembedded.org
Subject: Re: [bitbake-devel] [master][mickledore][PATCH 2/2] fetch2/crate: Correct unpack for a crate that matches the recipe name

Hello Peter,

Do you think it will solve the issue I mentioned there<https://bugzilla.yoctoproject.org/show_bug.cgi?id=15012#c4> ?

On Sat, 29 Apr 2023 at 03:23, Peter Kjellerstedt <peter.kjellerstedt@axis.com<mailto:peter.kjellerstedt@axis.com>> wrote:
The crate fetcher handles a crate with a name that matches the recipe's
name specially by placing the unpacked code in the current directory
(which typically is ${S}) rather than together with the sources for the
other crates. This broke when the URI names for all crates were changed
recently to include the version in the name.

Correct the crate fetcher to test against ${BP} instead of ${BPN}.
Also add a test case to the selftests to avoid this breaking again.

Signed-off-by: Peter Kjellerstedt <peter.kjellerstedt@axis.com<mailto:peter.kjellerstedt@axis.com>>
---
 bitbake/lib/bb/fetch2/crate.py |  4 ++--
 bitbake/lib/bb/tests/fetch.py  | 25 +++++++++++++++++++++++++
 2 files changed, 27 insertions(+), 2 deletions(-)

diff --git a/bitbake/lib/bb/fetch2/crate.py b/bitbake/lib/bb/fetch2/crate.py
index 2b8b6bc7a1..3310ed0050 100644
--- a/bitbake/lib/bb/fetch2/crate.py
+++ b/bitbake/lib/bb/fetch2/crate.py
@@ -98,8 +98,8 @@ class Crate(Wget):
         save_cwd = os.getcwd()
         os.chdir(rootdir)

-        pn = d.getVar('BPN')
-        if pn == ud.parm.get('name'):
+        bp = d.getVar('BP')
+        if bp == ud.parm.get('name'):
             cmd = "tar -xz --no-same-owner -f %s" % thefile
         else:
             cargo_bitbake = self._cargo_bitbake_path(rootdir)
diff --git a/bitbake/lib/bb/tests/fetch.py b/bitbake/lib/bb/tests/fetch.py
index 6ef0836f2b..23bfa788db 100644
--- a/bitbake/lib/bb/tests/fetch.py
+++ b/bitbake/lib/bb/tests/fetch.py
@@ -2391,6 +2391,31 @@ class CrateTest(FetcherTest):
         self.assertTrue(os.path.exists(self.tempdir + "/cargo_home/bitbake/glob-0.2.11/.cargo-checksum.json"))
         self.assertTrue(os.path.exists(self.tempdir + "/cargo_home/bitbake/glob-0.2.11/src/lib.rs<http://lib.rs>"))

+    @skipIfNoNetwork()
+    def test_crate_url_matching_recipe(self):
+
+        self.d.setVar('BP', 'glob-0.2.11')
+
+        uri = "crate://crates.io/glob/0.2.11<http://crates.io/glob/0.2.11>"
+        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"], "glob-0.2.11")
+        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)), ['download', 'glob-0.2.11', 'unpacked'])
+        self.assertEqual(sorted(os.listdir(self.tempdir + "/download")), ['glob-0.2.11.crate', 'glob-0.2.11.crate.done'])
+        self.assertTrue(os.path.exists(self.tempdir + "/glob-0.2.11/src/lib.rs<http://lib.rs>"))
+
     @skipIfNoNetwork()
     def test_crate_url_params(self):


-=-=-=-=-=-=-=-=-=-=-=-
Links: You receive all messages sent to this group.
View/Reply Online (#14758): https://lists.openembedded.org/g/bitbake-devel/message/14758
Mute This Topic: https://lists.openembedded.org/mt/98571257/6213388
Group Owner: bitbake-devel+owner@lists.openembedded.org<mailto:bitbake-devel%2Bowner@lists.openembedded.org>
Unsubscribe: https://lists.openembedded.org/g/bitbake-devel/unsub [frederic.martinsons@gmail.com<mailto:frederic.martinsons@gmail.com>]
-=-=-=-=-=-=-=-=-=-=-=-
Frédéric Martinsons April 30, 2023, 10 a.m. UTC | #3
Great, thanks Peter for the confirmation!

Le sam. 29 avr. 2023, 22:22, Peter Kjellerstedt <peter.kjellerstedt@axis.com>
a écrit :

> Yes. While the original issue was written about missing checksuming, which
> was fixed by requiring checksums for the crate URIs in the SRC_URI, the
> additional concern you raised is fixed by my patch. It restores the support
> for using a crate as the main component for the recipe. This was in the
> bbclass all along, it just broke when the names were changed to include the
> crate versions.
>
>
>
> You can actually workaround the problem by setting S =
> "${CARGO_VENDORING_DIRECTORY}/${BP}", but I believe it is better to
> restore the intended functionality of the bbclass. It also makes things
> much more intuitive when using devtool modify for such a recipe since the
> source is where you expect it to be.
>
>
>
> @Alexandre Belloni: Since I assume it will be you that pulls this into
> testing, feel free to add the following to the commit message:
>
>
>
> [Yocto #15012]
>
>
>
> //Peter
>
>
>
> *From:* Frédéric Martinsons <frederic.martinsons@gmail.com>
> *Sent:* den 29 april 2023 08:48
> *To:* Peter Kjellerstedt <peter.kjellerstedt@axis.com>
> *Cc:* bitbake-devel@lists.openembedded.org
> *Subject:* Re: [bitbake-devel] [master][mickledore][PATCH 2/2]
> fetch2/crate: Correct unpack for a crate that matches the recipe name
>
>
>
> Hello Peter,
>
>
>
> Do you think it will solve the issue I mentioned there
> <https://bugzilla.yoctoproject.org/show_bug.cgi?id=15012#c4> ?
>
>
>
> On Sat, 29 Apr 2023 at 03:23, Peter Kjellerstedt <
> peter.kjellerstedt@axis.com> wrote:
>
> The crate fetcher handles a crate with a name that matches the recipe's
> name specially by placing the unpacked code in the current directory
> (which typically is ${S}) rather than together with the sources for the
> other crates. This broke when the URI names for all crates were changed
> recently to include the version in the name.
>
> Correct the crate fetcher to test against ${BP} instead of ${BPN}.
> Also add a test case to the selftests to avoid this breaking again.
>
> Signed-off-by: Peter Kjellerstedt <peter.kjellerstedt@axis.com>
> ---
>  bitbake/lib/bb/fetch2/crate.py |  4 ++--
>  bitbake/lib/bb/tests/fetch.py  | 25 +++++++++++++++++++++++++
>  2 files changed, 27 insertions(+), 2 deletions(-)
>
> diff --git a/bitbake/lib/bb/fetch2/crate.py
> b/bitbake/lib/bb/fetch2/crate.py
> index 2b8b6bc7a1..3310ed0050 100644
> --- a/bitbake/lib/bb/fetch2/crate.py
> +++ b/bitbake/lib/bb/fetch2/crate.py
> @@ -98,8 +98,8 @@ class Crate(Wget):
>          save_cwd = os.getcwd()
>          os.chdir(rootdir)
>
> -        pn = d.getVar('BPN')
> -        if pn == ud.parm.get('name'):
> +        bp = d.getVar('BP')
> +        if bp == ud.parm.get('name'):
>              cmd = "tar -xz --no-same-owner -f %s" % thefile
>          else:
>              cargo_bitbake = self._cargo_bitbake_path(rootdir)
> diff --git a/bitbake/lib/bb/tests/fetch.py b/bitbake/lib/bb/tests/fetch.py
> index 6ef0836f2b..23bfa788db 100644
> --- a/bitbake/lib/bb/tests/fetch.py
> +++ b/bitbake/lib/bb/tests/fetch.py
> @@ -2391,6 +2391,31 @@ class CrateTest(FetcherTest):
>          self.assertTrue(os.path.exists(self.tempdir +
> "/cargo_home/bitbake/glob-0.2.11/.cargo-checksum.json"))
>          self.assertTrue(os.path.exists(self.tempdir +
> "/cargo_home/bitbake/glob-0.2.11/src/lib.rs"))
>
> +    @skipIfNoNetwork()
> +    def test_crate_url_matching_recipe(self):
> +
> +        self.d.setVar('BP', 'glob-0.2.11')
> +
> +        uri = "crate://crates.io/glob/0.2.11"
> +        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"], "glob-0.2.11")
> +        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)), ['download',
> 'glob-0.2.11', 'unpacked'])
> +        self.assertEqual(sorted(os.listdir(self.tempdir + "/download")),
> ['glob-0.2.11.crate', 'glob-0.2.11.crate.done'])
> +        self.assertTrue(os.path.exists(self.tempdir + "/glob-0.2.11/src/
> lib.rs"))
> +
>      @skipIfNoNetwork()
>      def test_crate_url_params(self):
>
>
> -=-=-=-=-=-=-=-=-=-=-=-
> Links: You receive all messages sent to this group.
> View/Reply Online (#14758):
> https://lists.openembedded.org/g/bitbake-devel/message/14758
> Mute This Topic: https://lists.openembedded.org/mt/98571257/6213388
> Group Owner: bitbake-devel+owner@lists.openembedded.org
> Unsubscribe: https://lists.openembedded.org/g/bitbake-devel/unsub [
> frederic.martinsons@gmail.com]
> -=-=-=-=-=-=-=-=-=-=-=-
>
>
diff mbox series

Patch

diff --git a/bitbake/lib/bb/fetch2/crate.py b/bitbake/lib/bb/fetch2/crate.py
index 2b8b6bc7a1..3310ed0050 100644
--- a/bitbake/lib/bb/fetch2/crate.py
+++ b/bitbake/lib/bb/fetch2/crate.py
@@ -98,8 +98,8 @@  class Crate(Wget):
         save_cwd = os.getcwd()
         os.chdir(rootdir)
 
-        pn = d.getVar('BPN')
-        if pn == ud.parm.get('name'):
+        bp = d.getVar('BP')
+        if bp == ud.parm.get('name'):
             cmd = "tar -xz --no-same-owner -f %s" % thefile
         else:
             cargo_bitbake = self._cargo_bitbake_path(rootdir)
diff --git a/bitbake/lib/bb/tests/fetch.py b/bitbake/lib/bb/tests/fetch.py
index 6ef0836f2b..23bfa788db 100644
--- a/bitbake/lib/bb/tests/fetch.py
+++ b/bitbake/lib/bb/tests/fetch.py
@@ -2391,6 +2391,31 @@  class CrateTest(FetcherTest):
         self.assertTrue(os.path.exists(self.tempdir + "/cargo_home/bitbake/glob-0.2.11/.cargo-checksum.json"))
         self.assertTrue(os.path.exists(self.tempdir + "/cargo_home/bitbake/glob-0.2.11/src/lib.rs"))
 
+    @skipIfNoNetwork()
+    def test_crate_url_matching_recipe(self):
+
+        self.d.setVar('BP', 'glob-0.2.11')
+
+        uri = "crate://crates.io/glob/0.2.11"
+        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"], "glob-0.2.11")
+        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)), ['download', 'glob-0.2.11', 'unpacked'])
+        self.assertEqual(sorted(os.listdir(self.tempdir + "/download")), ['glob-0.2.11.crate', 'glob-0.2.11.crate.done'])
+        self.assertTrue(os.path.exists(self.tempdir + "/glob-0.2.11/src/lib.rs"))
+
     @skipIfNoNetwork()
     def test_crate_url_params(self):