[patch+7.8] PR testsuite/16602 runtest deletes files in-src-tree
Commit Message
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
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 <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.
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
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
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
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
@@ -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
@@ -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}
+}
@@ -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}
+}
@@ -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}
+}
@@ -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}
+}
@@ -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}
+}
@@ -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}
+}
@@ -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}
+}
@@ -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}
+}
@@ -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}
+}
@@ -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
@@ -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}
+}
@@ -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}
+}
@@ -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}
+}
@@ -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}
+}
@@ -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}
+}
@@ -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}
+}
@@ -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}
+}