diff mbox

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

Message ID 20140712210532.GA8417@host2.jankratochvil.net
State Under Review
Headers show

Commit Message

Jan Kratochvil July 12, 2014, 9:05 p.m. UTC
Hi,

this affects testsuite run with GDB built in source tree, that is:
	./configure; make; cd gdb/testsuite; runtest gdb.*/*.exp

This is rejected for GCC but so far GDB always permitted this mode of build.

https://sourceware.org/bugzilla/show_bug.cgi?id=16602
------------------------------------------------------------------------------
cd .../gdb/testsuite.unix.-m64; runtest gdb.python/*.exp
 = OK

cd .../gdb/testsuite; runtest gdb.python/*.exp
[...]
Running ./gdb.python/py-prettyprint.exp ...
ERROR: remote_download to host of ./gdb.python/py-prettyprint.py to /home/jkratoch/redhat/gdb-clean/gdb/testsuite/gdb.python/py-prettyprint.py: cp: cannot stat './gdb.python/py-prettyprint.py': No such file or directory
[...]
Running ./gdb.python/py-mi.exp ...
ERROR: remote_download to host of ./gdb.python/py-prettyprint.py to /home/jkratoch/redhat/gdb-clean/gdb/testsuite/gdb.python/py-prettyprint.py: cp: cannot stat './gdb.python/py-prettyprint.py': No such file or directory
[...]
git status
        deleted:    gdb.python/py-frame-args.py
        deleted:    gdb.python/py-framefilter.py
        deleted:    gdb.python/py-pp-integral.py
        deleted:    gdb.python/py-prettyprint.py
        deleted:    gdb.python/py-section-script.py
        deleted:    gdb.python/py-typeprint.py
------------------------------------------------------------------------------

I have tested every changed file in its unpatched version deletes one file.

I have tested runtest gdb.*/*.exp on patched tree no longer deletes anything.

I have not tested testsuite run with remote host.

I have run normal regression test on
{x86_64,x86_64-m32,i686}-fedora20-linux-gnu with and without gdbserver mode.


Thanks,
Jan
gdb/testsuite/
2014-07-12  Jan Kratochvil  <jan.kratochvil@redhat.com>

	PR testsuite/16602
	* gdb.base/step-line.exp: Wrap "remote_file host delete" into
	[is_remote host] conditional.
	* gdb.dwarf2/dw2-anonymous-func.exp: Likewise.
	* gdb.dwarf2/dw2-basic.exp: Likewise.
	* gdb.dwarf2/dw2-compressed.exp: Likewise.
	* gdb.dwarf2/dw2-filename.exp: Likewise.
	* gdb.dwarf2/dw2-intercu.exp: Likewise.
	* gdb.dwarf2/dw2-intermix.exp: Likewise.
	* gdb.dwarf2/dw2-producer.exp: Likewise.
	* gdb.dwarf2/mac-fileno.exp: Likewise.
	* gdb.python/py-frame-args.exp: Likewise.
	* gdb.python/py-framefilter.exp: Likewise.
	* gdb.python/py-mi.exp: Likewise.
	* gdb.python/py-pp-integral.exp: Likewise.
	* gdb.python/py-pp-re-notag.exp: Likewise.
	* gdb.python/py-prettyprint.exp: Likewise.
	* gdb.python/py-section-script.exp: Likewise.
	* gdb.python/py-typeprint.exp: Likewise.
	* gdb.python/py-xmethods.exp: Likewise.

Comments

Pedro Alves July 15, 2014, 9:27 a.m. UTC | #1
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".
Doug Evans July 15, 2014, 3:19 p.m. UTC | #2
Pedro Alves <palves@redhat.com> writes:
> 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".

I can imagine special cases being made to delete large files
(e.g., core files) from the remote host after the test is done.
But I can't think of a reason to not treat all other files (binaries,
shlibs, scripts, whatever) identically.
Thus I'd amend "Looks like it's ..." to be
"Looks like it's any file downloaded to the host."
If that means leaving all such files on the remote host,
fine with me.
Jan Kratochvil July 15, 2014, 7:33 p.m. UTC | #3
On Tue, 15 Jul 2014 11:27:24 +0200, Pedro Alves wrote:
> 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?

Correct.


> and will miss deleting the file on non-srcdir testing.

I agree my patch has this bug.


> Maybe that's fine.  Maybe we should just never delete
> these files, even on remote host testing.  Dunno.

I have no opinion what should happen in all the other cases, there are many
options how the testsuite could behave.  Just currently it is broken for the
IMO most simple srcdir builds.


Jan
Jan Kratochvil Aug. 18, 2014, 6:15 p.m. UTC | #4
On Tue, 15 Jul 2014 21:33:11 +0200, Jan Kratochvil wrote:
> On Tue, 15 Jul 2014 11:27:24 +0200, Pedro Alves wrote:
> > Maybe that's fine.  Maybe we should just never delete
> > these files, even on remote host testing.  Dunno.
> 
> I have no opinion what should happen in all the other cases, there are many
> options how the testsuite could behave.  Just currently it is broken for the
> IMO most simple srcdir builds.

What is the maintainers' decision how the testsuite should behave so that
I can code the appropriate patch?


Thanks,
Jan
Doug Evans Aug. 18, 2014, 7:16 p.m. UTC | #5
On Mon, Aug 18, 2014 at 11:15 AM, Jan Kratochvil
<jan.kratochvil@redhat.com> wrote:
> On Tue, 15 Jul 2014 21:33:11 +0200, Jan Kratochvil wrote:
>> On Tue, 15 Jul 2014 11:27:24 +0200, Pedro Alves wrote:
>> > Maybe that's fine.  Maybe we should just never delete
>> > these files, even on remote host testing.  Dunno.
>>
>> I have no opinion what should happen in all the other cases, there are many
>> options how the testsuite could behave.  Just currently it is broken for the
>> IMO most simple srcdir builds.
>
> What is the maintainers' decision how the testsuite should behave so that
> I can code the appropriate patch?

Hi.

For reference sake,
My current opinion is that there's no point in deleting the files,
even with remote host testing.
And there is value in not deleting them.
ref: https://sourceware.org/ml/gdb-patches/2014-08/msg00282.html
Pedro Alves Aug. 19, 2014, 10:53 p.m. UTC | #6
On 08/18/2014 08:16 PM, Doug Evans wrote:
> On Mon, Aug 18, 2014 at 11:15 AM, Jan Kratochvil
> <jan.kratochvil@redhat.com> wrote:
>> On Tue, 15 Jul 2014 21:33:11 +0200, Jan Kratochvil wrote:
>>> On Tue, 15 Jul 2014 11:27:24 +0200, Pedro Alves wrote:
>>>> Maybe that's fine.  Maybe we should just never delete
>>>> these files, even on remote host testing.  Dunno.
>>>
>>> I have no opinion what should happen in all the other cases, there are many
>>> options how the testsuite could behave.  Just currently it is broken for the
>>> IMO most simple srcdir builds.
>>
>> What is the maintainers' decision how the testsuite should behave so that
>> I can code the appropriate patch?
> 
> Hi.
> 
> For reference sake,
> My current opinion is that there's no point in deleting the files,
> even with remote host testing.
> And there is value in not deleting them.
> ref: https://sourceware.org/ml/gdb-patches/2014-08/msg00282.html

I agree.

Thanks,
Pedro Alves
diff mbox

Patch

diff --git a/gdb/testsuite/gdb.base/step-line.exp b/gdb/testsuite/gdb.base/step-line.exp
index 94799f8..1de77b3 100644
--- a/gdb/testsuite/gdb.base/step-line.exp
+++ b/gdb/testsuite/gdb.base/step-line.exp
@@ -82,6 +82,8 @@  gdb_test "next" \
          ".*RETURN \\(j\\);.*" \
 	 "next over dummy 10"
 
-remote_file host delete ${remote_linefile}
+if [is_remote host] {
+    remote_file host delete ${remote_linefile}
+}
 
 return 0
diff --git a/gdb/testsuite/gdb.dwarf2/dw2-anonymous-func.exp b/gdb/testsuite/gdb.dwarf2/dw2-anonymous-func.exp
index 029104b..3ee795d 100644
--- a/gdb/testsuite/gdb.dwarf2/dw2-anonymous-func.exp
+++ b/gdb/testsuite/gdb.dwarf2/dw2-anonymous-func.exp
@@ -36,4 +36,6 @@  set remote_dwarf_srcfile [gdb_remote_download host \
 # the debugger does not crash doing so.
 gdb_test "list file1.txt:1" "File 1 Line 1.*File 1 Line 8" "list file1.txt"
 
-remote_file host delete ${remote_dwarf_srcfile}
+if [is_remote host] {
+    remote_file host delete ${remote_dwarf_srcfile}
+}
diff --git a/gdb/testsuite/gdb.dwarf2/dw2-basic.exp b/gdb/testsuite/gdb.dwarf2/dw2-basic.exp
index 7823eaa..16a0b6a 100644
--- 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}
+}
diff --git a/gdb/testsuite/gdb.dwarf2/dw2-compressed.exp b/gdb/testsuite/gdb.dwarf2/dw2-compressed.exp
index 76009d2..9160316 100644
--- a/gdb/testsuite/gdb.dwarf2/dw2-compressed.exp
+++ b/gdb/testsuite/gdb.dwarf2/dw2-compressed.exp
@@ -38,4 +38,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}
+}
diff --git a/gdb/testsuite/gdb.dwarf2/dw2-filename.exp b/gdb/testsuite/gdb.dwarf2/dw2-filename.exp
index c9ead97..8eed1e4 100644
--- a/gdb/testsuite/gdb.dwarf2/dw2-filename.exp
+++ b/gdb/testsuite/gdb.dwarf2/dw2-filename.exp
@@ -43,4 +43,6 @@  gdb_test "interpreter-exec mi -file-list-exec-source-files" \
 # And `info sources' should return the fullname incl. the directories.
 gdb_test "info sources" {[/\\]file1\.txt}
 
-remote_file host delete ${remote_dwarf_srcfile}
+if [is_remote host] {
+    remote_file host delete ${remote_dwarf_srcfile}
+}
diff --git a/gdb/testsuite/gdb.dwarf2/dw2-intercu.exp b/gdb/testsuite/gdb.dwarf2/dw2-intercu.exp
index 4a23c96..8367928 100644
--- a/gdb/testsuite/gdb.dwarf2/dw2-intercu.exp
+++ b/gdb/testsuite/gdb.dwarf2/dw2-intercu.exp
@@ -43,4 +43,6 @@  gdb_test "list func_cu1" "4\[ \t\]+File 1 Line 4"
 
 gdb_test "ptype func_cu1" "type = int2 \\(\\)"
 
-remote_file host delete ${remote_dwarf_srcfile}
+if [is_remote host] {
+    remote_file host delete ${remote_dwarf_srcfile}
+}
diff --git a/gdb/testsuite/gdb.dwarf2/dw2-intermix.exp b/gdb/testsuite/gdb.dwarf2/dw2-intermix.exp
index 2f95572..dc91a0e 100644
--- a/gdb/testsuite/gdb.dwarf2/dw2-intermix.exp
+++ b/gdb/testsuite/gdb.dwarf2/dw2-intermix.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}
+}
diff --git a/gdb/testsuite/gdb.dwarf2/dw2-producer.exp b/gdb/testsuite/gdb.dwarf2/dw2-producer.exp
index 6846b17..65ecb9e 100644
--- a/gdb/testsuite/gdb.dwarf2/dw2-producer.exp
+++ b/gdb/testsuite/gdb.dwarf2/dw2-producer.exp
@@ -34,4 +34,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}
+}
diff --git a/gdb/testsuite/gdb.dwarf2/mac-fileno.exp b/gdb/testsuite/gdb.dwarf2/mac-fileno.exp
index e736211..6f57ce5 100644
--- a/gdb/testsuite/gdb.dwarf2/mac-fileno.exp
+++ b/gdb/testsuite/gdb.dwarf2/mac-fileno.exp
@@ -37,4 +37,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}
+}
diff --git a/gdb/testsuite/gdb.python/py-frame-args.exp b/gdb/testsuite/gdb.python/py-frame-args.exp
index 4c50be8..dcaf711 100644
--- a/gdb/testsuite/gdb.python/py-frame-args.exp
+++ b/gdb/testsuite/gdb.python/py-frame-args.exp
@@ -69,4 +69,6 @@  gdb_test "frame" \
     ".*foo \\(x=42, ss={a = {m = 1}, b = {m = 2}}\\).*" \
     "frame raw,all"
 
-remote_file host delete ${remote_python_file}
+if [is_remote host] {
+    remote_file host delete ${remote_python_file}
+}
diff --git a/gdb/testsuite/gdb.python/py-framefilter.exp b/gdb/testsuite/gdb.python/py-framefilter.exp
index 948ae94..33c8742 100644
--- a/gdb/testsuite/gdb.python/py-framefilter.exp
+++ b/gdb/testsuite/gdb.python/py-framefilter.exp
@@ -188,7 +188,9 @@  gdb_test_multiple "bt 1" $test {
     }
 }
 
-remote_file host delete ${remote_python_file}
+if [is_remote host] {
+    remote_file host delete ${remote_python_file}
+}
 
 # Test with no debuginfo
 
diff --git a/gdb/testsuite/gdb.python/py-mi.exp b/gdb/testsuite/gdb.python/py-mi.exp
index 5959175..1c7a86b 100644
--- a/gdb/testsuite/gdb.python/py-mi.exp
+++ b/gdb/testsuite/gdb.python/py-mi.exp
@@ -357,4 +357,6 @@  mi_list_varobj_children fake.private {
 mi_gdb_test "-var-set-visualizer fake.private gdb.default_visualizer" \
     "\\^done" "Install visualizer on a cplus_fake_child"
 
-remote_file host delete ${remote_python_file}
+if [is_remote host] {
+    remote_file host delete ${remote_python_file}
+}
diff --git a/gdb/testsuite/gdb.python/py-pp-integral.exp b/gdb/testsuite/gdb.python/py-pp-integral.exp
index 8149bde..189f79c 100644
--- a/gdb/testsuite/gdb.python/py-pp-integral.exp
+++ b/gdb/testsuite/gdb.python/py-pp-integral.exp
@@ -36,4 +36,6 @@  gdb_test "print *t" " = Thu Nov 14 02:22:23 2013 \\(1384395743\\)"
 
 gdb_test "print /r *t" "= 1384395743"
 
-remote_file host delete ${remote_python_file}
+if [is_remote host] {
+    remote_file host delete ${remote_python_file}
+}
diff --git a/gdb/testsuite/gdb.python/py-pp-re-notag.exp b/gdb/testsuite/gdb.python/py-pp-re-notag.exp
index 8149bde..189f79c 100644
--- a/gdb/testsuite/gdb.python/py-pp-re-notag.exp
+++ b/gdb/testsuite/gdb.python/py-pp-re-notag.exp
@@ -36,4 +36,6 @@  gdb_test "print *t" " = Thu Nov 14 02:22:23 2013 \\(1384395743\\)"
 
 gdb_test "print /r *t" "= 1384395743"
 
-remote_file host delete ${remote_python_file}
+if [is_remote host] {
+    remote_file host delete ${remote_python_file}
+}
diff --git a/gdb/testsuite/gdb.python/py-prettyprint.exp b/gdb/testsuite/gdb.python/py-prettyprint.exp
index 491b52d..0a41864 100644
--- a/gdb/testsuite/gdb.python/py-prettyprint.exp
+++ b/gdb/testsuite/gdb.python/py-prettyprint.exp
@@ -106,7 +106,9 @@  proc run_lang_tests {exefile lang} {
 
     gdb_continue_to_end
 
-    remote_file host delete ${remote_python_file}
+    if [is_remote host] {
+	remote_file host delete ${remote_python_file}
+    }
 }
 
 run_lang_tests "${binfile}" "c"
@@ -153,4 +155,6 @@  gdb_test_no_output "python enable_lookup_function ()"
 gdb_test "print ss" " = a=< a=<1> b=<$hex>> b=< a=<2> b=<$hex>>" \
     "print ss enabled #2"
 
-remote_file host delete ${remote_python_file}
+if [is_remote host] {
+    remote_file host delete ${remote_python_file}
+}
diff --git a/gdb/testsuite/gdb.python/py-section-script.exp b/gdb/testsuite/gdb.python/py-section-script.exp
index 4f465cc..32e43f4 100644
--- a/gdb/testsuite/gdb.python/py-section-script.exp
+++ b/gdb/testsuite/gdb.python/py-section-script.exp
@@ -73,4 +73,6 @@  gdb_test "continue" ".*Breakpoint.*"
 
 gdb_test "print ss" " = a=<1> b=<2>"
 
-remote_file host delete ${remote_python_file}
+if [is_remote host] {
+    remote_file host delete ${remote_python_file}
+}
diff --git a/gdb/testsuite/gdb.python/py-typeprint.exp b/gdb/testsuite/gdb.python/py-typeprint.exp
index 1de80bd..e2f68d7 100644
--- a/gdb/testsuite/gdb.python/py-typeprint.exp
+++ b/gdb/testsuite/gdb.python/py-typeprint.exp
@@ -52,4 +52,6 @@  gdb_test "whatis bs" "string" "whatis with enabled printer"
 
 gdb_test "whatis s" "templ<string>"
 
-remote_file host delete ${remote_python_file}
+if [is_remote host] {
+    remote_file host delete ${remote_python_file}
+}
diff --git a/gdb/testsuite/gdb.python/py-xmethods.exp b/gdb/testsuite/gdb.python/py-xmethods.exp
index 8824127..5501528 100644
--- a/gdb/testsuite/gdb.python/py-xmethods.exp
+++ b/gdb/testsuite/gdb.python/py-xmethods.exp
@@ -150,4 +150,6 @@  gdb_test_no_output "disable xmethod progspace E_methods;method_int" \
 gdb_test "info xmethod progspace E_methods;method_int" ".* \\\[disabled\\\]" \
   "info xmethod xmethods E_methods;method_int"
 
-remote_file host delete ${xmethods_script}
+if [is_remote host] {
+    remote_file host delete ${xmethods_script}
+}