[PATCH 1/2] ptest-cargo: move run-ptest rc variable initialisation

[PATCH 1/2] ptest-cargo: move run-ptest rc variable initialisation

[Yoann Congal]

From: Yoann Congal <yoann.congal@smile.fr>

ptest-cargo run-ptest can be generated in two fashions: generated from
scratch or appended to an exiting run-ptest file. The rc variable used
to track tests failure was only initialized in "generated from scratch"
case. Which lead to errors in the "appended" case.

Move the rc variable initialisation to the common code of both case to
fix this problem.

Signed-off-by: Yoann Congal <yoann.congal@smile.fr>
Cc: Gyorgy Sarvari <skandigraun@gmail.com>
---
 meta/classes-recipe/ptest-cargo.bbclass | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/meta/classes-recipe/ptest-cargo.bbclass b/meta/classes-recipe/ptest-cargo.bbclass
index ece25ff1eb..e35de1042e 100644
--- a/meta/classes-recipe/ptest-cargo.bbclass
+++ b/meta/classes-recipe/ptest-cargo.bbclass
@@ -103,10 +103,10 @@ python do_install_ptest_cargo() {
     with open(ptest_script, "a") as f:
         if not script_exists:
             f.write("#!/bin/sh\n")
-            f.write("rc=0\n")                
         else:
             f.write(f"\necho \"\"\n")
             f.write(f"echo \"## starting to run rust tests ##\"\n")               
+        f.write("rc=0\n")
         for test_path in test_paths:
             script = textwrap.dedent(f"""\
                 if ! {test_path} {rust_test_args}

[PATCH 2/2] rpm-sequoia: remove rc initialisation from run-ptest

[Yoann Congal]

From: Yoann Congal <yoann.congal@smile.fr>

With the previous patch "ptest-cargo: move run-ptest rc variable
initialisation", rc is initialised by the ptest-cargo class.
Remove the (now) redundant initialisation and the related comment.

Signed-off-by: Yoann Congal <yoann.congal@smile.fr>
---
 meta/recipes-devtools/rpm-sequoia/files/run-ptest | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/meta/recipes-devtools/rpm-sequoia/files/run-ptest b/meta/recipes-devtools/rpm-sequoia/files/run-ptest
index 83e59b8031..36adfba7b4 100644
--- a/meta/recipes-devtools/rpm-sequoia/files/run-ptest
+++ b/meta/recipes-devtools/rpm-sequoia/files/run-ptest
@@ -7,9 +7,4 @@
 # The test that ptest-cargo class generates requires the
 # TEST_DONT_BUILD_LIB environment variable to be set before
 # it would be executed, and this is what this script does.
-# The "rc" variable is used by the script generated by
-# ptest-cargo, and is expected to exist, in case the
-# run-test script exists, and not created by that class.
-
-rc=0
 export TEST_DONT_BUILD_LIB=1

Re: [PATCH 1/2] ptest-cargo: move run-ptest rc variable initialisation

[Gyorgy Sarvari]

On 8/28/25 14:05, Yoann Congal wrote:
> From: Yoann Congal <yoann.congal@smile.fr>
>
> ptest-cargo run-ptest can be generated in two fashions: generated from
> scratch or appended to an exiting run-ptest file. The rc variable used
> to track tests failure was only initialized in "generated from scratch"
> case. Which lead to errors in the "appended" case.
>
> Move the rc variable initialisation to the common code of both case to
> fix this problem.
>
> Signed-off-by: Yoann Congal <yoann.congal@smile.fr>
> Cc: Gyorgy Sarvari <skandigraun@gmail.com>
> ---
>  meta/classes-recipe/ptest-cargo.bbclass | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/meta/classes-recipe/ptest-cargo.bbclass b/meta/classes-recipe/ptest-cargo.bbclass
> index ece25ff1eb..e35de1042e 100644
> --- a/meta/classes-recipe/ptest-cargo.bbclass
> +++ b/meta/classes-recipe/ptest-cargo.bbclass
> @@ -103,10 +103,10 @@ python do_install_ptest_cargo() {
>      with open(ptest_script, "a") as f:
>          if not script_exists:
>              f.write("#!/bin/sh\n")
> -            f.write("rc=0\n")                
>          else:
>              f.write(f"\necho \"\"\n")
>              f.write(f"echo \"## starting to run rust tests ##\"\n")               
> +        f.write("rc=0\n")
>          for test_path in test_paths:
>              script = textwrap.dedent(f"""\
>                  if ! {test_path} {rust_test_args}

I'm not sure if this is correct. This value is what's returned at the
end[1], but here it is set to 0 unconditionally, even though it might
have a previous value.

As an example, I have almost sent an updated librsvg ptest patch[2] -
while writing v2, I noticed that it has not only C, but also Rust tests,
so I started to add them too. The resulting run-ptest script first
starts with the C test that I execute, and partial result is stored in
the same variable. And then at the end ptest-cargo appends its own
tests, and continues to use the same rc variable.

One thing I gotta admit is that this rc variable is kinda hidden in this
class.

[1]:
https://git.yoctoproject.org/poky/tree/meta/classes-recipe/ptest-cargo.bbclass#n122
[2]: when I get around it again, I still believe that it will happen...

Re: [PATCH 1/2] ptest-cargo: move run-ptest rc variable initialisation

[Yoann Congal]

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

Le jeu. 28 août 2025 à 14:45, Gyorgy Sarvari <skandigraun@gmail.com> a
écrit :

> On 8/28/25 14:05, Yoann Congal wrote:
> > From: Yoann Congal <yoann.congal@smile.fr>
> >
> > ptest-cargo run-ptest can be generated in two fashions: generated from
> > scratch or appended to an exiting run-ptest file. The rc variable used
> > to track tests failure was only initialized in "generated from scratch"
> > case. Which lead to errors in the "appended" case.
> >
> > Move the rc variable initialisation to the common code of both case to
> > fix this problem.
> >
> > Signed-off-by: Yoann Congal <yoann.congal@smile.fr>
> > Cc: Gyorgy Sarvari <skandigraun@gmail.com>
> > ---
> >  meta/classes-recipe/ptest-cargo.bbclass | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/meta/classes-recipe/ptest-cargo.bbclass
> b/meta/classes-recipe/ptest-cargo.bbclass
> > index ece25ff1eb..e35de1042e 100644
> > --- a/meta/classes-recipe/ptest-cargo.bbclass
> > +++ b/meta/classes-recipe/ptest-cargo.bbclass
> > @@ -103,10 +103,10 @@ python do_install_ptest_cargo() {
> >      with open(ptest_script, "a") as f:
> >          if not script_exists:
> >              f.write("#!/bin/sh\n")
> > -            f.write("rc=0\n")
> >          else:
> >              f.write(f"\necho \"\"\n")
> >              f.write(f"echo \"## starting to run rust tests ##\"\n")
>
> > +        f.write("rc=0\n")
> >          for test_path in test_paths:
> >              script = textwrap.dedent(f"""\
> >                  if ! {test_path} {rust_test_args}
>
> I'm not sure if this is correct. This value is what's returned at the
> end[1], but here it is set to 0 unconditionally, even though it might
> have a previous value.
>
> As an example, I have almost sent an updated librsvg ptest patch[2] -
> while writing v2, I noticed that it has not only C, but also Rust tests,
> so I started to add them too. The resulting run-ptest script first
> starts with the C test that I execute, and partial result is stored in
> the same variable. And then at the end ptest-cargo appends its own
> tests, and continues to use the same rc variable.
>

You're right!

How about we initialize rc to 0 in ptest-cargo ONLY IF it is not already
defined at that point?
That would allow custom run-ptest scripts to setup their rc variable but it
would also allow scripts that don't want to bother with it (like
rpm-sequoia) to completely ignore it.


> One thing I gotta admit is that this rc variable is kinda hidden in this
> class.
>
> [1]:
>
> https://git.yoctoproject.org/poky/tree/meta/classes-recipe/ptest-cargo.bbclass#n122
> [2]: when I get around it again, I still believe that it will happen...
>


-- 
Yoann Congal
Smile ECS

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

Re: [PATCH 1/2] ptest-cargo: move run-ptest rc variable initialisation

[Gyorgy Sarvari]

On 8/28/25 16:01, Yoann Congal wrote:
>
>
> Le jeu. 28 août 2025 à 14:45, Gyorgy Sarvari <skandigraun@gmail.com> a
> écrit :
>
>     On 8/28/25 14:05, Yoann Congal wrote:
>     > From: Yoann Congal <yoann.congal@smile.fr>
>     >
>     > ptest-cargo run-ptest can be generated in two fashions:
>     generated from
>     > scratch or appended to an exiting run-ptest file. The rc
>     variable used
>     > to track tests failure was only initialized in "generated from
>     scratch"
>     > case. Which lead to errors in the "appended" case.
>     >
>     > Move the rc variable initialisation to the common code of both
>     case to
>     > fix this problem.
>     >
>     > Signed-off-by: Yoann Congal <yoann.congal@smile.fr>
>     > Cc: Gyorgy Sarvari <skandigraun@gmail.com>
>     > ---
>     >  meta/classes-recipe/ptest-cargo.bbclass | 2 +-
>     >  1 file changed, 1 insertion(+), 1 deletion(-)
>     >
>     > diff --git a/meta/classes-recipe/ptest-cargo.bbclass
>     b/meta/classes-recipe/ptest-cargo.bbclass
>     > index ece25ff1eb..e35de1042e 100644
>     > --- a/meta/classes-recipe/ptest-cargo.bbclass
>     > +++ b/meta/classes-recipe/ptest-cargo.bbclass
>     > @@ -103,10 +103,10 @@ python do_install_ptest_cargo() {
>     >      with open(ptest_script, "a") as f:
>     >          if not script_exists:
>     >              f.write("#!/bin/sh\n")
>     > -            f.write("rc=0\n")               
>     >          else:
>     >              f.write(f"\necho \"\"\n")
>     >              f.write(f"echo \"## starting to run rust tests
>     ##\"\n")               
>     > +        f.write("rc=0\n")
>     >          for test_path in test_paths:
>     >              script = textwrap.dedent(f"""\
>     >                  if ! {test_path} {rust_test_args}
>
>     I'm not sure if this is correct. This value is what's returned at the
>     end[1], but here it is set to 0 unconditionally, even though it might
>     have a previous value.
>
>     As an example, I have almost sent an updated librsvg ptest patch[2] -
>     while writing v2, I noticed that it has not only C, but also Rust
>     tests,
>     so I started to add them too. The resulting run-ptest script first
>     starts with the C test that I execute, and partial result is stored in
>     the same variable. And then at the end ptest-cargo appends its own
>     tests, and continues to use the same rc variable.
>
>
> You're right!
>
> How about we initialize rc to 0 in ptest-cargo ONLY IF it is not
> already defined at that point?
> That would allow custom run-ptest scripts to setup their rc variable
> but it would also allow scripts that don't want to bother with it
> (like rpm-sequoia) to completely ignore it.

This doesn't sound bad, personally I don't have anything against it - as
long as the original value is also kept, I think that would do.

Thinking a bit about it, it could be also removed from the rpm-sequoia
script in any case - by the end of the script it is either defined by an
error, or the last "exit $rc" will just become "exit" which implicitly
turns into "exit $?", which is also not that bad. (Though not everyone
likes such variables, and personally I'm not extremely attached either)

>  
>
>     One thing I gotta admit is that this rc variable is kinda hidden
>     in this
>     class.
>
>     [1]:
>     https://git.yoctoproject.org/poky/tree/meta/classes-recipe/ptest-cargo.bbclass#n122
>     [2]: when I get around it again, I still believe that it will
>     happen...
>
>
>
> -- 
> Yoann Congal
> Smile ECS