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

[Gyorgy Sarvari <skandigraun@gmail.com>]

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