[RFC] Remove files in make clean

Message ID 1416224764-5607-1-git-send-email-yao@codesourcery.com
State New, archived
Headers

Commit Message

Yao Qi Nov. 17, 2014, 11:46 a.m. UTC
  Hi,
I am looking at how 'make clean' in testsuite/ directory remove files
generated during testing.  We manually maintain a list of
executables and shared libraries in Makefile.in, and they are removed
in 'make clean'.  However, the list will be stale as the code evolves.
I am wondering a better approach than the current one.  I find
that name of most the executables is the same as the name of .exp or
.c file, so I modify clean target in testsuite/Makefile.in to delete
executables according to the name of .exp and .c file, in a
speculative way.  As a result, the manually maintained list in
testsuite/gdb.XXX/Makefile.in is shorter, only executables, which are
not covered by the speculation, should be listed.  So this approach
is a hybrid approach, that is most of the executables can be removed,
without any special care and special care is needed for the rest of them.

This is a RFC, because I'd like ask people's opinion on this.  If it
is good, I'll update other gdb.XXX/Makefile.in.

gdb/testsuite:

2014-11-17  Yao Qi  <yao@codesourcery.com>

	* Makefile.in (clean):  Remove files according to .exp and .c
	file name in srcdir.
	* gdb.base/Makefile.in (EXECUTABLES): Remove some entries.
---
 gdb/testsuite/Makefile.in          |  2 ++
 gdb/testsuite/gdb.base/Makefile.in | 61 +++++++++++++-------------------------
 2 files changed, 22 insertions(+), 41 deletions(-)
  

Comments

Joel Brobecker Nov. 23, 2014, 10:17 a.m. UTC | #1
> I am looking at how 'make clean' in testsuite/ directory remove files
> generated during testing.  We manually maintain a list of
> executables and shared libraries in Makefile.in, and they are removed
> in 'make clean'.  However, the list will be stale as the code evolves.
> I am wondering a better approach than the current one.  I find
> that name of most the executables is the same as the name of .exp or
> .c file, so I modify clean target in testsuite/Makefile.in to delete
> executables according to the name of .exp and .c file, in a
> speculative way.  As a result, the manually maintained list in
> testsuite/gdb.XXX/Makefile.in is shorter, only executables, which are
> not covered by the speculation, should be listed.  So this approach
> is a hybrid approach, that is most of the executables can be removed,
> without any special care and special care is needed for the rest of them.
> 
> This is a RFC, because I'd like ask people's opinion on this.  If it
> is good, I'll update other gdb.XXX/Makefile.in.
> 
> gdb/testsuite:
> 
> 2014-11-17  Yao Qi  <yao@codesourcery.com>
> 
> 	* Makefile.in (clean):  Remove files according to .exp and .c
> 	file name in srcdir.
> 	* gdb.base/Makefile.in (EXECUTABLES): Remove some entries.

FWIW, I think we should give it a try, and see how well this works
in practice. I agree with Yao that this is an issue (one that I have
given up on, especially since I moved to out-of-tree builds).
  
Doug Evans Nov. 23, 2014, 7:33 p.m. UTC | #2
On Mon, Nov 17, 2014 at 3:46 AM, Yao Qi <yao@codesourcery.com> wrote:
> Hi,
> I am looking at how 'make clean' in testsuite/ directory remove files
> generated during testing.  We manually maintain a list of
> executables and shared libraries in Makefile.in, and they are removed
> in 'make clean'.  However, the list will be stale as the code evolves.
> I am wondering a better approach than the current one.  I find
> that name of most the executables is the same as the name of .exp or
> .c file, so I modify clean target in testsuite/Makefile.in to delete
> executables according to the name of .exp and .c file, in a
> speculative way.  As a result, the manually maintained list in
> testsuite/gdb.XXX/Makefile.in is shorter, only executables, which are
> not covered by the speculation, should be listed.  So this approach
> is a hybrid approach, that is most of the executables can be removed,
> without any special care and special care is needed for the rest of them.
>
> This is a RFC, because I'd like ask people's opinion on this.  If it
> is good, I'll update other gdb.XXX/Makefile.in.
>
> gdb/testsuite:
>
> 2014-11-17  Yao Qi  <yao@codesourcery.com>
>
>         * Makefile.in (clean):  Remove files according to .exp and .c
>         file name in srcdir.
>         * gdb.base/Makefile.in (EXECUTABLES): Remove some entries.

Removing files according to .exp/.c is still fragile.
Still, it feels like a step in the right direction.

When we run in parallel mode all the output goes in the "outputs" subdir.
This leads to the thought that
IWBN if all one had to do is an rm -rf.
IOW, the only "make clean" rule would be in gdb/testsuite/Makefile.in
and it would just rm -rf the outputs subdirectory (however it is spelled -
I *like* having parallel runs stored separately from non-parallel ones).
But that breaks common usage that I do of running one test and then
expecting to find the binary in testsuite/gdb.foo.

OTOH, given the simplicity and completeness of the rm -rf approach,
I'd be ok with adjusting my muscle-memory (typing habits) and
going with just always putting test output in a subdir.
With a well chosen name it would tab-complete well,
and require little extra typing.

Do we need all the testsuite/*/Makefile files for anything else?
Would going the "always put test output in a subdir" route
allow us to remove all of them?  That's appealing.
  

Patch

diff --git a/gdb/testsuite/Makefile.in b/gdb/testsuite/Makefile.in
index 1c923cd..bf213eb 100644
--- a/gdb/testsuite/Makefile.in
+++ b/gdb/testsuite/Makefile.in
@@ -242,6 +242,8 @@  clean mostlyclean:
 	-rm -f *.dwo *.dwp
 	-rm -rf outputs temp cache
 	-rm -f read1.so expect-read1
+	-find ${srcdir} -name '*.exp' | sed "s/^.*testsuite\/\(.*\)\.exp/\1/" | xargs rm -rf
+	-find ${srcdir} -name '*.c' | sed "s/^.*testsuite\/\(.*\)\.c/\1/" | xargs rm -rf
 	if [ x"${ALL_SUBDIRS}" != x ] ; then \
 	    for dir in ${ALL_SUBDIRS}; \
 	    do \
diff --git a/gdb/testsuite/gdb.base/Makefile.in b/gdb/testsuite/gdb.base/Makefile.in
index dda3169..8378994 100644
--- a/gdb/testsuite/gdb.base/Makefile.in
+++ b/gdb/testsuite/gdb.base/Makefile.in
@@ -1,49 +1,28 @@ 
 VPATH = @srcdir@
 srcdir = @srcdir@
 
-EXECUTABLES = a2-run advance all-types annota1 annota1-watch_thread_num \
-	annota3 anon args arrayidx async attach attach-pie-misread \
-	attach2 auxv bang\! bfp-test bigcore bitfields bitfields2 \
-	break break-always break-entry break-interp-test breako2 \
-	breakpoint-shadow break-on-linker-gcd-function \
-	call-ar-st call-rt-st call-sc-t* call-signals \
-	call-strs callexit callfuncs callfwmall charset checkpoint \
-	chng-syms code_elim1 code_elim2 commands compiler completion complex \
-	condbreak consecutive constvars coremaker cursal cvexpr \
-	dbx-test del disasm-end-cu display dprintf-pending dump dup-sect \
+EXECUTABLES = annota1-watch_thread_num \
+	anon \
+	bang\! \
+	break-interp-test breako2 \
+	call-sc-t* \
+	callfwmall \
+	compiler \
+	dbx-test \
 	dup-sect.debug \
-	dup-sect.stripped ending-run execd-prog expand-psymtabs exprs \
-	fileio find finish fixsection float foll-exec foll-fork foll-vfork \
-	frame-args freebpcmd fullname funcargs gcore \
+	dup-sect.stripped \
 	gcore-buffer-overflow-012* \
-	gdb1090 gdb11530 gdb11531 gdb1250 gdb1555-main gdb1821 gdbvars \
-	hashline1 hashline2 hashline3 hbreak hook-stop-continue \
-	hook-stop-frame huge included infnan info-target int-type \
-	interrupt jit-main jump label langs lineinc list longjmp long_long \
-	macscp mips_pro miscexprs moribund-step multi-forks nodebug \
-	nofield nostdlib opaque overlays pc-fp pending permission \
-	pie-execl1 pie-execl2 pointers pointers2 pr11022 prelinkt \
-	prelinkt.debug prelinkt.stripped printcmds prologue psymtab \
-	ptr-typedef ptype randomize recurse relational relativedebug \
-	reread reread1 restore return return-nodebug-* return2 run \
-	savedregs scope section_command sep sepdebug sepdebug.stripped \
-	sepdebug2 sepdebug2.debug sepdebug2.stripped sepsymtab \
-	sepsymtab.debug sepsymtab.stripped setshow setvar shmain shreloc \
-	sigall sigaltstack sigbpt sigchld siginfo siginfo-addr \
-	siginfo-infcall siginfo-obj signals signull sigrepeat sigstep \
-	sizeof skip skip-solib solib solib-corrupted solib-display-main \
-	solib-nodir solib-overlap-main-0x40000000 solib-symbol-main solib-weak \
-	solib-weak-lib2 solib_sl so-impl-ld so-indr-cl \
-	stack-checking start step-break step-bt step-line step-resume-infcall \
-	step-test store structs-t* structs2 structs3 \
-	symbol-without-target_section term tui-layout twice-tmp \
-	type-opaque-main ui-redirect unload until unwindonsignal \
-	valgrind-db-attach value-double-free varargs vforked-prog \
-	volatile watch-cond watch-cond-infcall watch-non-mem watch-read \
-	watch-vfork watch_thread_num watchpoint watchpoint-cond-gone \
-	watchpoint-hw watchpoint-hw-hit-once watchpoint-solib watchpoints \
-	wchar whatis whatis-exp catch-syscall \
-	pr10179 gnu_vector
+	permission \
+	pie-execl1 pie-execl2 pointers2 prelinkt \
+	prelinkt.debug prelinkt.stripped \
+	reread1 return-nodebug-* \
+	section_command sepdebug.stripped \
+	sepdebug2 sepdebug2.debug sepdebug2.stripped \
+	sepsymtab.debug sepsymtab.stripped \
+	solib-overlap-main-0x40000000 \
+	solib-weak-lib2 solib_sl \
+	structs-t* \
+	twice-tmp
 
 MISCELLANEOUS = coremmap.data dprintf-pendshr.sl ../foobar.baz fixsectshr.sl \
 	pendshr.sl shreloc1.sl shreloc2.sl twice-tmp.c \