Re: [patch+7.8] PR testsuite/16602 runtest deletes files in-src-tree

[Pedro Alves <palves at redhat dot com>]

I miss some explanation in the commit log / email description
of the problem.

IIUC, when testing out of srcdir, the remote_download call on
non-remote hosts copies the file, and then the "remote_file delete"
call deletes the copy.  But when testing in srcdir, I'm
guessing remote_download copies the file over itself
(or does nothing), and then the "remote_file delete" call
deletes the source file, because it's the same path as the
destination of the previous download.

Correct?

On 07/12/2014 10:05 PM, Jan Kratochvil wrote:
> --- a/gdb/testsuite/gdb.dwarf2/dw2-basic.exp
> +++ b/gdb/testsuite/gdb.dwarf2/dw2-basic.exp
> @@ -36,4 +36,6 @@ gdb_test_no_output "set listsize 1"
>  gdb_test "list func_cu1" "4\[ \t\]+File 1 Line 4"
>  gdb_test "ptype func_cu1" "type = int \\(\\)"
>  
> -remote_file host delete ${remote_dwarf_srcfile}
> +if [is_remote host] {
> +    remote_file host delete ${remote_dwarf_srcfile}
> +}

But then this doesn't sound right.  It makes the
remote_download / remote_file calls be unbalanced, and
will miss deleting the file on non-srcdir testing.

Maybe the condition should be different, something around:

 [is_remote host] \
  || [same_file ${remote_dwarf_srcfile} != ${srcdir}/${subdir}/${dwarf_srcfile}]

 ("same_file" just for illustration.  That might well be a
 string compare, dunno.)

And then it looks like there's a pattern here that could
well be abstracted behind a new procedure so we don't repeat
those conditions all over the place, without a comment
explaining the rationale.  A procedure would allows us
having a single place to put such a comment.

And then maybe gdb_remote_download should itself also
skip the download if !remote host and the destination
is the same as the source, to keep calls balanced.

BTW, a grep for "gdb_remote_download.*host" points at some
more places that look like need the same treatment.

gdb.ada/pp-rec-component.exp:30:    [gdb_remote_download host ${srcdir}/${subdir}/${gdb_test_file_name}.py]
...
lib/perftest.exp:25:    set remote_python_file [gdb_remote_download host ${srcdir}/${subdir}/${testfile}.py]

And then its interesting that the scheme tests just don't
ever delete the files:

 gdb.guile/scm-pretty-print.exp:55:    set remote_scheme_file [gdb_remote_download host \
 gdb.guile/scm-pretty-print.exp:123:set remote_scheme_file [gdb_remote_download host \
 gdb.guile/scm-frame-args.exp:33:set remote_guile_file [gdb_remote_download host \
 gdb.guile/scm-section-script.exp:43:set remote_guile_file [gdb_remote_download host \
 gdb.guile/scm-error.exp:33:set remote_guile_file_1 [gdb_remote_download host \
 gdb.guile/scm-error.exp:36:set remote_guile_file_2 [gdb_remote_download host \

Maybe that's fine.  Maybe we should just never delete
these files, even on remote host testing.  Dunno.  Again,
there seems to be a pattern here we should apply the same fix
to everywhere.  Looks like it's "whenever we want to download
a checked-in, non-generated source file".

-- 
Pedro Alves