[PATCH 0/3] Use frozen flag for cargo build

[PATCH 0/3] Use frozen flag for cargo build

[frederic.martinsons]

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

This patch series force the usage of --frozen instead of --offline during cargo build.
This has the advantage to be sure that Cargo.lock file will not be modified.
Moreover, raise a clear error when Cargo.lock is not present.
For this to work, we must modify ourself the Cargo.lock before building
in case there are git repo that have been patched.

The last commit is to handle the specific case of rust-hello-world recipe.
If this recipe is no more necessary, we can drop this last patch.

The following changes since commit 9b5b850d6a6982bb8ff14dcfbb6769b293638293:

  libarchive: ignore CVE-2023-30571 (2023-07-30 12:00:13 +0100)

are available in the Git repository at:

  https://gitlab.com/fmartinsons/openembedded-core 15104-check-cargo-lock

Frederic Martinsons (3):
  cargo.bbclass: Use --frozen flag for cargo operations
  cargo_common.bbclass: Handle Cargo.lock modifications for git
    dependencies
  cargo.bbclass: Offer a way to use --offline instead of --frozen

 meta/classes-recipe/cargo.bbclass             |  9 +++-
 meta/classes-recipe/cargo_common.bbclass      | 43 +++++++++++++++++++
 .../rust-example/rust-hello-world_git.bb      |  2 +
 3 files changed, 53 insertions(+), 1 deletion(-)

-- 
2.34.1

[PATCH 1/3] cargo.bbclass: Use --frozen flag for cargo operations

[frederic.martinsons]

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

It supersed the --offline flag and guarantee that Cargo.lock
file will not be modified during the build.

Signed-off-by: Frederic Martinsons <frederic.martinsons@gmail.com>
---
 meta/classes-recipe/cargo.bbclass | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/meta/classes-recipe/cargo.bbclass b/meta/classes-recipe/cargo.bbclass
index 3ef0bbbb44..8c0b92df8d 100644
--- a/meta/classes-recipe/cargo.bbclass
+++ b/meta/classes-recipe/cargo.bbclass
@@ -39,7 +39,12 @@ MANIFEST_PATH ??= "${S}/${CARGO_SRC_DIR}/Cargo.toml"
 
 RUSTFLAGS ??= ""
 BUILD_MODE = "${@['--release', ''][d.getVar('DEBUG_BUILD') == '1']}"
-CARGO_BUILD_FLAGS = "-v --offline --target ${RUST_HOST_SYS} ${BUILD_MODE} --manifest-path=${MANIFEST_PATH}"
+# --frozen flag will prevent network access (which is required since only
+# the do_fetch step is authorized to access network)
+# and will require an up to date Cargo.lock file.
+# This force the package being built to already ship a Cargo.lock, in the end
+# this is what we want, at least, for reproducibility of the build.
+CARGO_BUILD_FLAGS = "-v --frozen --target ${RUST_HOST_SYS} ${BUILD_MODE} --manifest-path=${MANIFEST_PATH}"
 
 # This is based on the content of CARGO_BUILD_FLAGS and generally will need to
 # change if CARGO_BUILD_FLAGS changes.
-- 
2.34.1

[PATCH 2/3] cargo_common.bbclass: Handle Cargo.lock modifications for git dependencies

[frederic.martinsons]

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

Now we use --frozen, Cargo.lock cannot be modified by cargo build.
These patched git dependencies requires that the git url is removed
from Cargo.lock.

Fixes #15104

Signed-off-by: Frederic Martinsons <frederic.martinsons@gmail.com>
---
 meta/classes-recipe/cargo_common.bbclass | 40 ++++++++++++++++++++++++
 1 file changed, 40 insertions(+)

diff --git a/meta/classes-recipe/cargo_common.bbclass b/meta/classes-recipe/cargo_common.bbclass
index db54826ddb..01afb74640 100644
--- a/meta/classes-recipe/cargo_common.bbclass
+++ b/meta/classes-recipe/cargo_common.bbclass
@@ -117,6 +117,8 @@ cargo_common_do_configure () {
 }
 
 python cargo_common_do_patch_paths() {
+    import shutil
+
     cargo_config = os.path.join(d.getVar("CARGO_HOME"), "config")
     if not os.path.exists(cargo_config):
         return
@@ -146,6 +148,44 @@ python cargo_common_do_patch_paths() {
             print('\n[patch."%s"]' % k, file=config)
             for name in v:
                 print(name, file=config)
+
+    if not patches:
+        return
+
+    # Cargo.lock file is needed for to be sure that artifacts
+    # downloaded by the fetch steps are those expected by the
+    # project and that the possible patches are correctly applied.
+    # Moreover since we do not want any modification
+    # of this file (for reproducibility purpose), we prevent it by
+    # using --frozen flag (in CARGO_BUILD_FLAGS) and raise a clear error
+    # here is better than letting cargo tell (in case the file is missing)
+    # "Cargo.lock should be modified but --frozen was given"
+
+    manifest_path = d.getVar("MANIFEST_PATH", True)
+    lockfile = os.path.join(os.path.dirname(manifest_path), "Cargo.lock")
+    if not os.path.exists(lockfile):
+        bb.fatal(f"{lockfile} file doesn't exist")
+
+    # There are patched files and so Cargo.lock should be modified but we use
+    # --frozen so let's handle that modifications here.
+    #
+    # Note that a "better" (more elegant ?) would have been to use cargo update for
+    # patched packages:
+    #  cargo update --offline -p package_1 -p package_2
+    # But this is not possible since it requires that cargo local git db
+    # to be populated and this is not the case as we fetch git repo ourself.
+
+    newlines = []
+    with open(lockfile, "r") as f:
+        for line in f.readlines():
+            if not line.startswith("source = \"git"):
+                newlines.append(line)
+
+    lockfile_mod = lockfile + ".new"
+    with open(lockfile_mod, "w") as f:
+        f.writelines(newlines)
+
+    shutil.move(lockfile_mod, lockfile)
 }
 do_configure[postfuncs] += "cargo_common_do_patch_paths"
 
-- 
2.34.1

[PATCH 3/3] cargo.bbclass: Offer a way to use --offline instead of --frozen

[frederic.martinsons]

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

And use that for rust-hello-world recipe that did not ship
a Cargo.lock file.

Signed-off-by: Frederic Martinsons <frederic.martinsons@gmail.com>
---
 meta/classes-recipe/cargo.bbclass                          | 4 +++-
 meta/classes-recipe/cargo_common.bbclass                   | 3 +++
 meta/recipes-extended/rust-example/rust-hello-world_git.bb | 2 ++
 3 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/meta/classes-recipe/cargo.bbclass b/meta/classes-recipe/cargo.bbclass
index 8c0b92df8d..d8ab94f2b4 100644
--- a/meta/classes-recipe/cargo.bbclass
+++ b/meta/classes-recipe/cargo.bbclass
@@ -39,12 +39,14 @@ MANIFEST_PATH ??= "${S}/${CARGO_SRC_DIR}/Cargo.toml"
 
 RUSTFLAGS ??= ""
 BUILD_MODE = "${@['--release', ''][d.getVar('DEBUG_BUILD') == '1']}"
+
 # --frozen flag will prevent network access (which is required since only
 # the do_fetch step is authorized to access network)
 # and will require an up to date Cargo.lock file.
 # This force the package being built to already ship a Cargo.lock, in the end
 # this is what we want, at least, for reproducibility of the build.
-CARGO_BUILD_FLAGS = "-v --frozen --target ${RUST_HOST_SYS} ${BUILD_MODE} --manifest-path=${MANIFEST_PATH}"
+CARGO_EXTRA_FLAGS =  "${@['--frozen', '--offline'][d.getVar('CARGO_NO_FROZEN') == '1']}"
+CARGO_BUILD_FLAGS = "-v ${CARGO_EXTRA_FLAGS} --target ${RUST_HOST_SYS} ${BUILD_MODE} --manifest-path=${MANIFEST_PATH}"
 
 # This is based on the content of CARGO_BUILD_FLAGS and generally will need to
 # change if CARGO_BUILD_FLAGS changes.
diff --git a/meta/classes-recipe/cargo_common.bbclass b/meta/classes-recipe/cargo_common.bbclass
index 01afb74640..d17501182f 100644
--- a/meta/classes-recipe/cargo_common.bbclass
+++ b/meta/classes-recipe/cargo_common.bbclass
@@ -152,6 +152,9 @@ python cargo_common_do_patch_paths() {
     if not patches:
         return
 
+    if d.getVar("CARGO_NO_FROZEN") == 1:
+        return
+
     # Cargo.lock file is needed for to be sure that artifacts
     # downloaded by the fetch steps are those expected by the
     # project and that the possible patches are correctly applied.
diff --git a/meta/recipes-extended/rust-example/rust-hello-world_git.bb b/meta/recipes-extended/rust-example/rust-hello-world_git.bb
index 1d91109b51..cad184837f 100644
--- a/meta/recipes-extended/rust-example/rust-hello-world_git.bb
+++ b/meta/recipes-extended/rust-example/rust-hello-world_git.bb
@@ -14,6 +14,8 @@ SUMMARY = "Hello World by Cargo for Rust"
 HOMEPAGE = "https://github.com/meta-rust/rust-hello-world"
 LICENSE = "MIT | Apache-2.0"
 
+CARGO_NO_FROZEN = "1"
+
 S = "${WORKDIR}/git"
 
 BBCLASSEXTEND = "native"
-- 
2.34.1

Re: [OE-core] [PATCH 0/3] Use frozen flag for cargo build

[Alexander Kanavin]

I think you need to better describe the benefits. What are the
improvements? Can we just stay with --offline?

There's lots of intricate code added around cargo.lock handling, with
special casing to revert to previous behavior via new variable, but
I'm struggling to understand: to what end?

Alex





On Mon, 31 Jul 2023 at 11:44, Frederic Martinsons
<frederic.martinsons@gmail.com> wrote:
>
> From: Frederic Martinsons <frederic.martinsons@gmail.com>
>
> This patch series force the usage of --frozen instead of --offline during cargo build.
> This has the advantage to be sure that Cargo.lock file will not be modified.
> Moreover, raise a clear error when Cargo.lock is not present.
> For this to work, we must modify ourself the Cargo.lock before building
> in case there are git repo that have been patched.
>
> The last commit is to handle the specific case of rust-hello-world recipe.
> If this recipe is no more necessary, we can drop this last patch.
>
> The following changes since commit 9b5b850d6a6982bb8ff14dcfbb6769b293638293:
>
>   libarchive: ignore CVE-2023-30571 (2023-07-30 12:00:13 +0100)
>
> are available in the Git repository at:
>
>   https://gitlab.com/fmartinsons/openembedded-core 15104-check-cargo-lock
>
> Frederic Martinsons (3):
>   cargo.bbclass: Use --frozen flag for cargo operations
>   cargo_common.bbclass: Handle Cargo.lock modifications for git
>     dependencies
>   cargo.bbclass: Offer a way to use --offline instead of --frozen
>
>  meta/classes-recipe/cargo.bbclass             |  9 +++-
>  meta/classes-recipe/cargo_common.bbclass      | 43 +++++++++++++++++++
>  .../rust-example/rust-hello-world_git.bb      |  2 +
>  3 files changed, 53 insertions(+), 1 deletion(-)
>
> --
> 2.34.1
>
>
> -=-=-=-=-=-=-=-=-=-=-=-
> Links: You receive all messages sent to this group.
> View/Reply Online (#185128): https://lists.openembedded.org/g/openembedded-core/message/185128
> Mute This Topic: https://lists.openembedded.org/mt/100458213/1686489
> Group Owner: openembedded-core+owner@lists.openembedded.org
> Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [alex.kanavin@gmail.com]
> -=-=-=-=-=-=-=-=-=-=-=-
>

Re: [OE-core] [PATCH 2/3] cargo_common.bbclass: Handle Cargo.lock modifications for git dependencies

[Richard Purdie]

On Mon, 2023-07-31 at 11:44 +0200, Frederic Martinsons wrote:
> From: Frederic Martinsons <frederic.martinsons@gmail.com>
> 
> Now we use --frozen, Cargo.lock cannot be modified by cargo build.
> These patched git dependencies requires that the git url is removed
> from Cargo.lock.
> 
> Fixes #15104
> 
> Signed-off-by: Frederic Martinsons <frederic.martinsons@gmail.com>
> ---
>  meta/classes-recipe/cargo_common.bbclass | 40 ++++++++++++++++++++++++
>  1 file changed, 40 insertions(+)
> 
> diff --git a/meta/classes-recipe/cargo_common.bbclass b/meta/classes-recipe/cargo_common.bbclass
> index db54826ddb..01afb74640 100644
> --- a/meta/classes-recipe/cargo_common.bbclass
> +++ b/meta/classes-recipe/cargo_common.bbclass
> @@ -117,6 +117,8 @@ cargo_common_do_configure () {
>  }
>  
>  python cargo_common_do_patch_paths() {
> +    import shutil
> +
>      cargo_config = os.path.join(d.getVar("CARGO_HOME"), "config")
>      if not os.path.exists(cargo_config):
>          return
> @@ -146,6 +148,44 @@ python cargo_common_do_patch_paths() {
>              print('\n[patch."%s"]' % k, file=config)
>              for name in v:
>                  print(name, file=config)
> +
> +    if not patches:
> +        return
> +
> +    # Cargo.lock file is needed for to be sure that artifacts
> +    # downloaded by the fetch steps are those expected by the
> +    # project and that the possible patches are correctly applied.
> +    # Moreover since we do not want any modification
> +    # of this file (for reproducibility purpose), we prevent it by
> +    # using --frozen flag (in CARGO_BUILD_FLAGS) and raise a clear error
> +    # here is better than letting cargo tell (in case the file is missing)
> +    # "Cargo.lock should be modified but --frozen was given"
> +
> +    manifest_path = d.getVar("MANIFEST_PATH", True)
> +    lockfile = os.path.join(os.path.dirname(manifest_path), "Cargo.lock")
> +    if not os.path.exists(lockfile):
> +        bb.fatal(f"{lockfile} file doesn't exist")
> +
> +    # There are patched files and so Cargo.lock should be modified but we use
> +    # --frozen so let's handle that modifications here.
> +    #
> +    # Note that a "better" (more elegant ?) would have been to use cargo update for
> +    # patched packages:
> +    #  cargo update --offline -p package_1 -p package_2
> +    # But this is not possible since it requires that cargo local git db
> +    # to be populated and this is not the case as we fetch git repo ourself.
> +
> +    newlines = []
> +    with open(lockfile, "r") as f:
> +        for line in f.readlines():
> +            if not line.startswith("source = \"git"):
> +                newlines.append(line)
> +
> +    lockfile_mod = lockfile + ".new"
> +    with open(lockfile_mod, "w") as f:
> +        f.writelines(newlines)
> +
> +    shutil.move(lockfile_mod, lockfile)
>  }
>  do_configure[postfuncs] += "cargo_common_do_patch_paths"

This patch isn't "wrong" but there are some tips we've picked up over
the years of doing this which might help things in the future.

Some questions:

What happens if we run the cargo_common_do_patch_paths function
multiple times?
What happens if the user hits Ctrl+C at an inopportune moment?

In this case the function is mostly ok but it might easily change in
the future to something that isn't.

The best practise we've come to is the pattern of firstly copy X to
X.orig if X.orig doesn't exist. This ensures you always start from a
pristine backup. You can then directly open and write to X reading from
X.orig.

This ensures that both of the questions I ask above become non-issues.

I'm know I'm being a little picky by mentioning this but I do want to
raise awareness of the better pattern in general and make sure that
examples in the class code are good as people will copy them!

Cheers,

Richard

Re: [OE-core] [PATCH 3/3] cargo.bbclass: Offer a way to use --offline instead of --frozen

[Richard Purdie]

On Mon, 2023-07-31 at 11:44 +0200, Frederic Martinsons wrote:
> From: Frederic Martinsons <frederic.martinsons@gmail.com>
> 
> And use that for rust-hello-world recipe that did not ship
> a Cargo.lock file.
> 
> Signed-off-by: Frederic Martinsons <frederic.martinsons@gmail.com>
> ---
>  meta/classes-recipe/cargo.bbclass                          | 4 +++-
>  meta/classes-recipe/cargo_common.bbclass                   | 3 +++
>  meta/recipes-extended/rust-example/rust-hello-world_git.bb | 2 ++
>  3 files changed, 8 insertions(+), 1 deletion(-)

Could you send a patch to the manual please adding the new variable
CARGO_NO_FROZEN?

I'm torn on this one as we could have simpler code which did:

CARGO_FREEZE_OPTION = "--frozen"

and then have the hello-world recipe:

CARGO_FREEZE_OPTION = "--offline"

but I'm torn on whether that is better or not...

Cheers,

Richard

Re: [OE-core] [PATCH 3/3] cargo.bbclass: Offer a way to use --offline instead of --frozen

[Frédéric Martinsons]

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

Thanks Richard, but I'd prefer to drop this patch if possible.
I think the rust-hello-world is no more useful and that we can replace it
by a more "real" example like zvariant
from meta-selftest.

Let's see what others think about this

On Tue, 1 Aug 2023 at 10:46, Richard Purdie <
richard.purdie@linuxfoundation.org> wrote:

> On Mon, 2023-07-31 at 11:44 +0200, Frederic Martinsons wrote:
> > From: Frederic Martinsons <frederic.martinsons@gmail.com>
> >
> > And use that for rust-hello-world recipe that did not ship
> > a Cargo.lock file.
> >
> > Signed-off-by: Frederic Martinsons <frederic.martinsons@gmail.com>
> > ---
> >  meta/classes-recipe/cargo.bbclass                          | 4 +++-
> >  meta/classes-recipe/cargo_common.bbclass                   | 3 +++
> >  meta/recipes-extended/rust-example/rust-hello-world_git.bb | 2 ++
> >  3 files changed, 8 insertions(+), 1 deletion(-)
>
> Could you send a patch to the manual please adding the new variable
> CARGO_NO_FROZEN?
>
> I'm torn on this one as we could have simpler code which did:
>
> CARGO_FREEZE_OPTION = "--frozen"
>
> and then have the hello-world recipe:
>
> CARGO_FREEZE_OPTION = "--offline"
>
> but I'm torn on whether that is better or not...
>
> Cheers,
>
> Richard
>

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

Re: [OE-core] [PATCH 2/3] cargo_common.bbclass: Handle Cargo.lock modifications for git dependencies

[Frédéric Martinsons]

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

Thank you for the tips, will work on a v2 implementing that you suggested

On Tue, 1 Aug 2023 at 10:44, Richard Purdie <
richard.purdie@linuxfoundation.org> wrote:

> On Mon, 2023-07-31 at 11:44 +0200, Frederic Martinsons wrote:
> > From: Frederic Martinsons <frederic.martinsons@gmail.com>
> >
> > Now we use --frozen, Cargo.lock cannot be modified by cargo build.
> > These patched git dependencies requires that the git url is removed
> > from Cargo.lock.
> >
> > Fixes #15104
> >
> > Signed-off-by: Frederic Martinsons <frederic.martinsons@gmail.com>
> > ---
> >  meta/classes-recipe/cargo_common.bbclass | 40 ++++++++++++++++++++++++
> >  1 file changed, 40 insertions(+)
> >
> > diff --git a/meta/classes-recipe/cargo_common.bbclass
> b/meta/classes-recipe/cargo_common.bbclass
> > index db54826ddb..01afb74640 100644
> > --- a/meta/classes-recipe/cargo_common.bbclass
> > +++ b/meta/classes-recipe/cargo_common.bbclass
> > @@ -117,6 +117,8 @@ cargo_common_do_configure () {
> >  }
> >
> >  python cargo_common_do_patch_paths() {
> > +    import shutil
> > +
> >      cargo_config = os.path.join(d.getVar("CARGO_HOME"), "config")
> >      if not os.path.exists(cargo_config):
> >          return
> > @@ -146,6 +148,44 @@ python cargo_common_do_patch_paths() {
> >              print('\n[patch."%s"]' % k, file=config)
> >              for name in v:
> >                  print(name, file=config)
> > +
> > +    if not patches:
> > +        return
> > +
> > +    # Cargo.lock file is needed for to be sure that artifacts
> > +    # downloaded by the fetch steps are those expected by the
> > +    # project and that the possible patches are correctly applied.
> > +    # Moreover since we do not want any modification
> > +    # of this file (for reproducibility purpose), we prevent it by
> > +    # using --frozen flag (in CARGO_BUILD_FLAGS) and raise a clear error
> > +    # here is better than letting cargo tell (in case the file is
> missing)
> > +    # "Cargo.lock should be modified but --frozen was given"
> > +
> > +    manifest_path = d.getVar("MANIFEST_PATH", True)
> > +    lockfile = os.path.join(os.path.dirname(manifest_path),
> "Cargo.lock")
> > +    if not os.path.exists(lockfile):
> > +        bb.fatal(f"{lockfile} file doesn't exist")
> > +
> > +    # There are patched files and so Cargo.lock should be modified but
> we use
> > +    # --frozen so let's handle that modifications here.
> > +    #
> > +    # Note that a "better" (more elegant ?) would have been to use
> cargo update for
> > +    # patched packages:
> > +    #  cargo update --offline -p package_1 -p package_2
> > +    # But this is not possible since it requires that cargo local git db
> > +    # to be populated and this is not the case as we fetch git repo
> ourself.
> > +
> > +    newlines = []
> > +    with open(lockfile, "r") as f:
> > +        for line in f.readlines():
> > +            if not line.startswith("source = \"git"):
> > +                newlines.append(line)
> > +
> > +    lockfile_mod = lockfile + ".new"
> > +    with open(lockfile_mod, "w") as f:
> > +        f.writelines(newlines)
> > +
> > +    shutil.move(lockfile_mod, lockfile)
> >  }
> >  do_configure[postfuncs] += "cargo_common_do_patch_paths"
>
> This patch isn't "wrong" but there are some tips we've picked up over
> the years of doing this which might help things in the future.
>
> Some questions:
>
> What happens if we run the cargo_common_do_patch_paths function
> multiple times?
> What happens if the user hits Ctrl+C at an inopportune moment?
>
> In this case the function is mostly ok but it might easily change in
> the future to something that isn't.
>
> The best practise we've come to is the pattern of firstly copy X to
> X.orig if X.orig doesn't exist. This ensures you always start from a
> pristine backup. You can then directly open and write to X reading from
> X.orig.
>
> This ensures that both of the questions I ask above become non-issues.
>
> I'm know I'm being a little picky by mentioning this but I do want to
> raise awareness of the better pattern in general and make sure that
> examples in the class code are good as people will copy them!
>
> Cheers,
>
> Richard
>
>
>
>
>
>
>
>
>
>

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

Re: [OE-core] [PATCH 3/3] cargo.bbclass: Offer a way to use --offline instead of --frozen

[Richard Purdie]

On Tue, 2023-08-01 at 11:00 +0200, Frédéric Martinsons wrote:
> Thanks Richard, but I'd prefer to drop this patch if possible.
> I think the rust-hello-world is no more useful and that we can
> replace it by a more "real" example like zvariant
> from meta-selftest.
> 
> Let's see what others think about this

After some thought, I'm in favour. Lets drop it.

Cheers,

Richard

Re: [OE-core] [PATCH 0/3] Use frozen flag for cargo build

[Frédéric Martinsons]

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

Hello Alexander,

I tried to explain in the code and commit comments, you may also want to
read the ticket: https://bugzilla.yoctoproject.org/show_bug.cgi?id=15104
and I try to explain and ask for help in a dedicated topic on this very
list: see https://lists.openembedded.org/g/openembedded-core/topic/100254129

But to summarize, Cargo.lock is required for various purposes and it
includes the patching path process to work.
I think having no Cargo.lock for a rust recipe built under yocto is not
desirable.

For the  --frozen flag, it put harder constraints than --offline (--offline
prevent network access but doesn't prevent modification
of Cargo.lock)

The special case to revert the behavior introduced was made to comply with
rust-hello-world existence. I would like
to suppress this recipe instead of having this patch but I don't know if it
is desirable and I'd like to think
for other points of views.


On Mon, 31 Jul 2023 at 19:48, Alexander Kanavin <alex.kanavin@gmail.com>
wrote:

> I think you need to better describe the benefits. What are the
> improvements? Can we just stay with --offline?
>
> There's lots of intricate code added around cargo.lock handling, with
> special casing to revert to previous behavior via new variable, but
> I'm struggling to understand: to what end?
>
> Alex
>
>
>
>
>
> On Mon, 31 Jul 2023 at 11:44, Frederic Martinsons
> <frederic.martinsons@gmail.com> wrote:
> >
> > From: Frederic Martinsons <frederic.martinsons@gmail.com>
> >
> > This patch series force the usage of --frozen instead of --offline
> during cargo build.
> > This has the advantage to be sure that Cargo.lock file will not be
> modified.
> > Moreover, raise a clear error when Cargo.lock is not present.
> > For this to work, we must modify ourself the Cargo.lock before building
> > in case there are git repo that have been patched.
> >
> > The last commit is to handle the specific case of rust-hello-world
> recipe.
> > If this recipe is no more necessary, we can drop this last patch.
> >
> > The following changes since commit
> 9b5b850d6a6982bb8ff14dcfbb6769b293638293:
> >
> >   libarchive: ignore CVE-2023-30571 (2023-07-30 12:00:13 +0100)
> >
> > are available in the Git repository at:
> >
> >   https://gitlab.com/fmartinsons/openembedded-core
> 15104-check-cargo-lock
> >
> > Frederic Martinsons (3):
> >   cargo.bbclass: Use --frozen flag for cargo operations
> >   cargo_common.bbclass: Handle Cargo.lock modifications for git
> >     dependencies
> >   cargo.bbclass: Offer a way to use --offline instead of --frozen
> >
> >  meta/classes-recipe/cargo.bbclass             |  9 +++-
> >  meta/classes-recipe/cargo_common.bbclass      | 43 +++++++++++++++++++
> >  .../rust-example/rust-hello-world_git.bb      |  2 +
> >  3 files changed, 53 insertions(+), 1 deletion(-)
> >
> > --
> > 2.34.1
> >
> >
> > -=-=-=-=-=-=-=-=-=-=-=-
> > Links: You receive all messages sent to this group.
> > View/Reply Online (#185128):
> https://lists.openembedded.org/g/openembedded-core/message/185128
> > Mute This Topic: https://lists.openembedded.org/mt/100458213/1686489
> > Group Owner: openembedded-core+owner@lists.openembedded.org
> > Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [
> alex.kanavin@gmail.com]
> > -=-=-=-=-=-=-=-=-=-=-=-
> >
>

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

Re: [OE-core] [PATCH 3/3] cargo.bbclass: Offer a way to use --offline instead of --frozen

[Frédéric Martinsons]

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

Ok but this raises other questions then, this recipe is part of
packagegroup-core-tools-tesapps
<https://git.openembedded.org/openembedded-core/tree/meta/recipes-core/packagegroups/packagegroup-core-tools-testapps.bb#n28>
and is part of a test case
<https://git.openembedded.org/openembedded-core/tree/meta/lib/oeqa/runtime/cases/rust.py#n52>
So what to do with those ? Suppress them too or adapt them with another
recipe (zvariant ?)

On Tue, 1 Aug 2023 at 11:07, Richard Purdie <
richard.purdie@linuxfoundation.org> wrote:

> On Tue, 2023-08-01 at 11:00 +0200, Frédéric Martinsons wrote:
> > Thanks Richard, but I'd prefer to drop this patch if possible.
> > I think the rust-hello-world is no more useful and that we can
> > replace it by a more "real" example like zvariant
> > from meta-selftest.
> >
> > Let's see what others think about this
>
> After some thought, I'm in favour. Lets drop it.
>
> Cheers,
>
> Richard
>

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

Re: [OE-core] [PATCH 3/3] cargo.bbclass: Offer a way to use --offline instead of --frozen

[Richard Purdie]

On Tue, 2023-08-01 at 11:20 +0200, Frédéric Martinsons wrote:
> Ok but this raises other questions then, this recipe is part of
> packagegroup-core-tools-tesapps

Just drop that.

> and is part of a test case So what to do with those ? Suppress them
> too or adapt them with another recipe (zvariant ?)

I think the other test cases supersede this one so that can be dropped
too?

Cheers,

Richard

Re: [OE-core] [PATCH 3/3] cargo.bbclass: Offer a way to use --offline instead of --frozen

[Alexander Kanavin]

I have no special feelings about rust-hello-world; if all places where
it's currently used can be reworked to use real world components,
that's totally fine.

Alex

On Tue, 1 Aug 2023 at 11:38, Richard Purdie
<richard.purdie@linuxfoundation.org> wrote:
>
> On Tue, 2023-08-01 at 11:20 +0200, Frédéric Martinsons wrote:
> > Ok but this raises other questions then, this recipe is part of
> > packagegroup-core-tools-tesapps
>
> Just drop that.
>
> > and is part of a test case So what to do with those ? Suppress them
> > too or adapt them with another recipe (zvariant ?)
>
> I think the other test cases supersede this one so that can be dropped
> too?
>
> Cheers,
>
> Richard
>
> -=-=-=-=-=-=-=-=-=-=-=-
> Links: You receive all messages sent to this group.
> View/Reply Online (#185222): https://lists.openembedded.org/g/openembedded-core/message/185222
> Mute This Topic: https://lists.openembedded.org/mt/100458217/1686489
> Group Owner: openembedded-core+owner@lists.openembedded.org
> Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [alex.kanavin@gmail.com]
> -=-=-=-=-=-=-=-=-=-=-=-
>

Re: [OE-core] [PATCH 0/3] Use frozen flag for cargo build

[Alexander Kanavin]

Thanks for explaining, I suppose you can simply rework the patchset to
remove rust-hello-world, and rework all places where it's used (you
can grep the poky source tree :)

Alex

On Tue, 1 Aug 2023 at 11:09, Frédéric Martinsons
<frederic.martinsons@gmail.com> wrote:
>
> Hello Alexander,
>
> I tried to explain in the code and commit comments, you may also want to read the ticket: https://bugzilla.yoctoproject.org/show_bug.cgi?id=15104
> and I try to explain and ask for help in a dedicated topic on this very list: see https://lists.openembedded.org/g/openembedded-core/topic/100254129
>
> But to summarize, Cargo.lock is required for various purposes and it includes the patching path process to work.
> I think having no Cargo.lock for a rust recipe built under yocto is not desirable.
>
> For the  --frozen flag, it put harder constraints than --offline (--offline prevent network access but doesn't prevent modification
> of Cargo.lock)
>
> The special case to revert the behavior introduced was made to comply with rust-hello-world existence. I would like
> to suppress this recipe instead of having this patch but I don't know if it is desirable and I'd like to think
> for other points of views.
>
>
> On Mon, 31 Jul 2023 at 19:48, Alexander Kanavin <alex.kanavin@gmail.com> wrote:
>>
>> I think you need to better describe the benefits. What are the
>> improvements? Can we just stay with --offline?
>>
>> There's lots of intricate code added around cargo.lock handling, with
>> special casing to revert to previous behavior via new variable, but
>> I'm struggling to understand: to what end?
>>
>> Alex
>>
>>
>>
>>
>>
>> On Mon, 31 Jul 2023 at 11:44, Frederic Martinsons
>> <frederic.martinsons@gmail.com> wrote:
>> >
>> > From: Frederic Martinsons <frederic.martinsons@gmail.com>
>> >
>> > This patch series force the usage of --frozen instead of --offline during cargo build.
>> > This has the advantage to be sure that Cargo.lock file will not be modified.
>> > Moreover, raise a clear error when Cargo.lock is not present.
>> > For this to work, we must modify ourself the Cargo.lock before building
>> > in case there are git repo that have been patched.
>> >
>> > The last commit is to handle the specific case of rust-hello-world recipe.
>> > If this recipe is no more necessary, we can drop this last patch.
>> >
>> > The following changes since commit 9b5b850d6a6982bb8ff14dcfbb6769b293638293:
>> >
>> >   libarchive: ignore CVE-2023-30571 (2023-07-30 12:00:13 +0100)
>> >
>> > are available in the Git repository at:
>> >
>> >   https://gitlab.com/fmartinsons/openembedded-core 15104-check-cargo-lock
>> >
>> > Frederic Martinsons (3):
>> >   cargo.bbclass: Use --frozen flag for cargo operations
>> >   cargo_common.bbclass: Handle Cargo.lock modifications for git
>> >     dependencies
>> >   cargo.bbclass: Offer a way to use --offline instead of --frozen
>> >
>> >  meta/classes-recipe/cargo.bbclass             |  9 +++-
>> >  meta/classes-recipe/cargo_common.bbclass      | 43 +++++++++++++++++++
>> >  .../rust-example/rust-hello-world_git.bb      |  2 +
>> >  3 files changed, 53 insertions(+), 1 deletion(-)
>> >
>> > --
>> > 2.34.1
>> >
>> >
>> > -=-=-=-=-=-=-=-=-=-=-=-
>> > Links: You receive all messages sent to this group.
>> > View/Reply Online (#185128): https://lists.openembedded.org/g/openembedded-core/message/185128
>> > Mute This Topic: https://lists.openembedded.org/mt/100458213/1686489
>> > Group Owner: openembedded-core+owner@lists.openembedded.org
>> > Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [alex.kanavin@gmail.com]
>> > -=-=-=-=-=-=-=-=-=-=-=-
>> >

Re: [OE-core] [PATCH 3/3] cargo.bbclass: Offer a way to use --offline instead of --frozen

[Frédéric Martinsons]

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

zariant rust recipe is tested for devtool cases there
<https://git.openembedded.org/openembedded-core/tree/meta/lib/oeqa/selftest/cases/devtool.py#n891>

The test case mentioned earlier seems to verify that we can run rust test
code, do we want to keep testing
that rust binary can run ? If so, how to keep testing this while removing
rust-hello-world recipes ?

maybe we can add a simple "println!("Hello world") " in this file
<https://git.openembedded.org/openembedded-core/tree/meta/lib/oeqa/files/test.rs>
,
compile it like it is already done and run it
to check its output ?

What do you think?


On Tue, 1 Aug 2023 at 11:41, Alexander Kanavin <alex.kanavin@gmail.com>
wrote:

> I have no special feelings about rust-hello-world; if all places where
> it's currently used can be reworked to use real world components,
> that's totally fine.
>
> Alex
>
> On Tue, 1 Aug 2023 at 11:38, Richard Purdie
> <richard.purdie@linuxfoundation.org> wrote:
> >
> > On Tue, 2023-08-01 at 11:20 +0200, Frédéric Martinsons wrote:
> > > Ok but this raises other questions then, this recipe is part of
> > > packagegroup-core-tools-tesapps
> >
> > Just drop that.
> >
> > > and is part of a test case So what to do with those ? Suppress them
> > > too or adapt them with another recipe (zvariant ?)
> >
> > I think the other test cases supersede this one so that can be dropped
> > too?
> >
> > Cheers,
> >
> > Richard
> >
> > -=-=-=-=-=-=-=-=-=-=-=-
> > Links: You receive all messages sent to this group.
> > View/Reply Online (#185222):
> https://lists.openembedded.org/g/openembedded-core/message/185222
> > Mute This Topic: https://lists.openembedded.org/mt/100458217/1686489
> > Group Owner: openembedded-core+owner@lists.openembedded.org
> > Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [
> alex.kanavin@gmail.com]
> > -=-=-=-=-=-=-=-=-=-=-=-
> >
>

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

Re: [OE-core] [PATCH 3/3] cargo.bbclass: Offer a way to use --offline instead of --frozen

[Richard Purdie]

On Tue, 2023-08-01 at 11:55 +0200, Frédéric Martinsons wrote:
> zariant rust recipe is tested for devtool cases there
> 
> The test case mentioned earlier seems to verify that we can run rust
> test code, do we want to keep testing
> that rust binary can run ? If so, how to keep testing this while
> removing rust-hello-world recipes ?
> 
> maybe we can add a simple "println!("Hello world") " in this file ,
> compile it like it is already done and run it
> to check its output ?
> 
> What do you think?

Doesn't test_rust_compile already also run the rust code it compiled?

Cheers,

Richard

Re: [OE-core] [PATCH 3/3] cargo.bbclass: Offer a way to use --offline instead of --frozen

[Alex Kiernan]

On Tue, Aug 1, 2023 at 11:01 AM Richard Purdie
<richard.purdie@linuxfoundation.org> wrote:
>
> On Tue, 2023-08-01 at 11:55 +0200, Frédéric Martinsons wrote:
> > zariant rust recipe is tested for devtool cases there
> >
> > The test case mentioned earlier seems to verify that we can run rust
> > test code, do we want to keep testing
> > that rust binary can run ? If so, how to keep testing this while
> > removing rust-hello-world recipes ?
> >
> > maybe we can add a simple "println!("Hello world") " in this file ,
> > compile it like it is already done and run it
> > to check its output ?
> >
> > What do you think?
>
> Doesn't test_rust_compile already also run the rust code it compiled?
>

Yes and checks the exit code - really not sure I see any additional
value in turning it into another "Hello World". I'm fairly sure the
cargo test gets you another "Hello World" test (but again only checks
the return value). I think the rust-hello-world test is covered in
other ways now.


-- 
Alex Kiernan

Re: [OE-core] [PATCH 3/3] cargo.bbclass: Offer a way to use --offline instead of --frozen

[Frédéric Martinsons]

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

Ok, thanks you all for the answers. I'll then add a new patch to make this
cleansing and drop the patch I made to use --offline instead of --frozen
(no more needed)

Le mar. 1 août 2023, 12:04, Alex Kiernan <alex.kiernan@gmail.com> a écrit :

> On Tue, Aug 1, 2023 at 11:01 AM Richard Purdie
> <richard.purdie@linuxfoundation.org> wrote:
> >
> > On Tue, 2023-08-01 at 11:55 +0200, Frédéric Martinsons wrote:
> > > zariant rust recipe is tested for devtool cases there
> > >
> > > The test case mentioned earlier seems to verify that we can run rust
> > > test code, do we want to keep testing
> > > that rust binary can run ? If so, how to keep testing this while
> > > removing rust-hello-world recipes ?
> > >
> > > maybe we can add a simple "println!("Hello world") " in this file ,
> > > compile it like it is already done and run it
> > > to check its output ?
> > >
> > > What do you think?
> >
> > Doesn't test_rust_compile already also run the rust code it compiled?
> >
>
> Yes and checks the exit code - really not sure I see any additional
> value in turning it into another "Hello World". I'm fairly sure the
> cargo test gets you another "Hello World" test (but again only checks
> the return value). I think the rust-hello-world test is covered in
> other ways now.
>
>
> --
> Alex Kiernan
>

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

[PATCH 0/3] Use frozen flag for cargo build

[frederic.martinsons]

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

This patch series force the usage of --frozen instead of --offline during cargo build.
This has the advantage to be sure that Cargo.lock file will not be modified.
Moreover, raise a clear error when Cargo.lock is not present.
For this to work, we must modify ourself the Cargo.lock before building
in case there are git repo that have been patched.

The last commit was for removing rust-hello-world, patch to the
documentation project will follow.

The following changes since commit 692e414aed5313ff275b69e93179aa7c559700f3:

  ffmpeg: Fix wrong code found with gas/2.41 (2023-08-01 09:49:14 +0100)

are available in the Git repository at:

  https://gitlab.com/fmartinsons/openembedded-core 15104-check-cargo-lock

Frederic Martinsons (3):
  cargo.bbclass: Use --frozen flag for cargo operations
  cargo_common.bbclass: Handle Cargo.lock modifications for git
    dependencies
  drop rust-hello-world recipe

 meta/classes-recipe/cargo.bbclass             |  7 +++-
 meta/classes-recipe/cargo_common.bbclass      | 41 +++++++++++++++++++
 meta/conf/distro/include/maintainers.inc      |  1 -
 meta/lib/oeqa/runtime/cases/rust.py           | 12 ------
 .../packagegroup-core-tools-testapps.bb       |  3 --
 .../rust-hello-world/0001-enable-LTO.patch    | 24 -----------
 .../rust-example/rust-hello-world_git.bb      | 19 ---------
 7 files changed, 47 insertions(+), 60 deletions(-)
 delete mode 100644 meta/recipes-extended/rust-example/rust-hello-world/0001-enable-LTO.patch
 delete mode 100644 meta/recipes-extended/rust-example/rust-hello-world_git.bb

-- 
2.34.1

Re: [PATCH 0/3] Use frozen flag for cargo build

[Frédéric Martinsons]

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

Le mer. 2 août 2023, 15:16, <frederic.martinsons@gmail.com> a écrit :

> From: Frederic Martinsons <frederic.martinsons@gmail.com>
>
> This patch series force the usage of --frozen instead of --offline during
> cargo build.
> This has the advantage to be sure that Cargo.lock file will not be
> modified.
> Moreover, raise a clear error when Cargo.lock is not present.
> For this to work, we must modify ourself the Cargo.lock before building
> in case there are git repo that have been patched.
>
> The last commit was for removing rust-hello-world, patch to the
> documentation project will follow.
>
> The following changes since commit
> 692e414aed5313ff275b69e93179aa7c559700f3:
>
>   ffmpeg: Fix wrong code found with gas/2.41 (2023-08-01 09:49:14 +0100)
>
> are available in the Git repository at:
>
>   https://gitlab.com/fmartinsons/openembedded-core 15104-check-cargo-lock
>
> Frederic Martinsons (3):
>   cargo.bbclass: Use --frozen flag for cargo operations
>   cargo_common.bbclass: Handle Cargo.lock modifications for git
>     dependencies
>   drop rust-hello-world recipe
>
>  meta/classes-recipe/cargo.bbclass             |  7 +++-
>  meta/classes-recipe/cargo_common.bbclass      | 41 +++++++++++++++++++
>  meta/conf/distro/include/maintainers.inc      |  1 -
>  meta/lib/oeqa/runtime/cases/rust.py           | 12 ------
>  .../packagegroup-core-tools-testapps.bb       |  3 --
>  .../rust-hello-world/0001-enable-LTO.patch    | 24 -----------
>  .../rust-example/rust-hello-world_git.bb      | 19 ---------
>  7 files changed, 47 insertions(+), 60 deletions(-)
>  delete mode 100644
> meta/recipes-extended/rust-example/rust-hello-world/0001-enable-LTO.patch
>  delete mode 100644 meta/recipes-extended/rust-example/
> rust-hello-world_git.bb
>
> --
> 2.34.1
>

Sorry, I missed the v2 part in header. These are the follow up after
remarks made yesterday (about removal of rust-hello-world and robustness
when modifying Cargo.lock file in place)

>

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