Re: [PATCH v2] ld/testsuite: add support for remote testing in ld-cdtest

On Tue, 30 Jun 2026, Clément Chigot wrote:

> > > This converts the existing to code to use "remote_load" allowing
> > > execution on both native and remote targets.
> > >
> > > The "diff" between the output and the expected result has been
> > > transformed to the usual regexp_diff. The previous could have been
> > > transformed into `remote_exec build diff` but tends to be fickled with
> > > new lines.
> >
> > Arf got Maciej's Redhat email rejected. Trying with [email protected].
> 
> Gentle ping to Maciej.

 Hectic time here since I got back online on Jun 22nd, after a holiday.  

 I can see this works correctly here with native `powerpc64le-linux-gnu' 
system as well as remote `mips-linux-gnu' cross target, enabling the tests 
in the latter case.  I have looked through the change and it seems good to 
me except for one nit across two places; see below.

> > > diff --git a/ld/testsuite/ld-cdtest/cdtest.exp b/ld/testsuite/ld-cdtest/cdtest.exp
> > > index 89fdd581abb..81547af3c9e 100644
> > > --- a/ld/testsuite/ld-cdtest/cdtest.exp
> > > +++ b/ld/testsuite/ld-cdtest/cdtest.exp
> > > @@ -51,44 +47,39 @@ set expected_output "$srcdir/$subdir/cdtest.dat"
> > >  if ![ld_link $CC_FOR_TARGET tmpdir/cdtest "$CFLAGS_FOR_TARGET tmpdir/cdtest-foo.o tmpdir/cdtest-bar.o tmpdir/cdtest-main.o"] {
> > >      fail $test1
> > >  } else {
> > > -    send_log   "tmpdir/cdtest >tmpdir/cdtest.out\n"
> > > -    verbose    "tmpdir/cdtest >tmpdir/cdtest.out"
> > > -    catch "exec tmpdir/cdtest >tmpdir/cdtest.out" exec_output
> > > +    set failed 0
> > >
> > > -    if ![string match "" $exec_output] then {
> > > +    send_log "Running: tmpdir/cdtest\n"
> > > +    verbose "Running: tmpdir/cdtest"
> > > +    set state [remote_load target tmpdir/cdtest]
> > > +    set status [lindex $state 0]
> > > +    set exec_output [lindex $state 1]
> > > +
> > > +    if { $status != "pass"  } {
> > >         send_log "$exec_output\n"
> > >         verbose "$exec_output" 1
> > > -
> > > -       fail $test1
> > > +       set failed 1
> > >      } else {
> > > -       send_log   "diff tmpdir/cdtest.out $expected_output\n"
> > > -       verbose    "diff tmpdir/cdtest.out $expected_output"
> > > -       catch "exec diff tmpdir/cdtest.out $expected_output" exec_output
> > > -       set exec_output [prune_warnings $exec_output]
> > > -
> > > -       if ![string match "" $exec_output] then {
> > > -           send_log "$exec_output\n"
> > > -           verbose  "$exec_output" 1
> > > +       set output_filename "tmpdir/cdtest.out"
> > > +       set_file_contents $output_filename $exec_output
> > >
> > > +       send_log [file_contents $output_filename]
> > > +       verbose [file_contents $output_filename] 2
> > > +       if [regexp_diff $output_filename $expected_output] {
> > >             send_log "Checking against Named Return Value optimization\n"
> > >             verbose  "Checking against Named Return Value optimization" 1
> > >
> > >             set expected_output "$srcdir/$subdir/cdtest-nrv.dat"
> > > -
> > > -           send_log   "diff tmpdir/cdtest.out $expected_output\n"
> > > -           verbose    "diff tmpdir/cdtest.out $expected_output"
> > > -           catch "exec diff tmpdir/cdtest.out $expected_output" exec_output
> > > -           set exec_output [prune_warnings $exec_output]
> > > +           if [regexp_diff $output_filename $expected_output] {
> > > +               set failed 1
> > > +           }
> > >         }
> > > +    }
> > >
> > > -       if [string match "" $exec_output] then {
> > > -           pass $test1
> > > -       } else {
> > > -           send_log "$exec_output\n"
> > > -           verbose  "$exec_output" 1
> > > -
> > > -           fail $test1
> > > -       }
> > > +    if { $failed != 0 } {

 I find checking a boolean value against zero superfluous and tougher to 
parse for a human being.  How about just:

    if { $failed } {

instead?

> > >      } else {
> > > -       send_log   "tmpdir/cdtest >tmpdir/cdtest.out\n"
> > > -       verbose    "tmpdir/cdtest >tmpdir/cdtest.out"
> > > -       catch "exec tmpdir/cdtest >tmpdir/cdtest.out" exec_output
> > > +       set failed 0
> > > +
> > > +       send_log "Running: tmpdir/cdtest\n"
> > > +       verbose "Running: tmpdir/cdtest"
> > > +       set state [remote_load target tmpdir/cdtest]
> > > +       set status [lindex $state 0]
> > > +       set exec_output [lindex $state 1]
> > >
> > > -       if ![string match "" $exec_output] then {
> > > +       if { $status != "pass" } {
> > >             send_log "$exec_output\n"
> > > -           verbose  "$exec_output" 1
> > > +           verbose "$exec_output" 1
> > > +           set failed 1
> > > +       } else {
> > > +           set output_filename "tmpdir/cdtest.out"
> > > +           set_file_contents $output_filename $exec_output
> > >
> > > +           send_log [file_contents $output_filename]
> > > +           verbose [file_contents $output_filename] 2
> > > +           if [regexp_diff $output_filename $expected_output] {
> > > +               set failed 1
> > > +           }
> > > +       }
> > > +
> > > +       if { $failed != 0 } {

 Likewise.

 I can't approve, but I can see Alan has got that covered already and I 
suppose won't object my suggestion.  Thank you for your contribution, a 
worthwhile test robustness improvement.

  Maciej