Created attachment 64429 [details] Automate EXTRA_DIST consistency checking Version: 3.7 SVN (using Devel) OS: Linux Valgrind's regression tests only work reliably in a tar ball generated with "make dist" if the regression test output files all have been specified in EXTRA_DIST in Makefile.am in the same directory. It is easy though to forget mentioning such a file. So it would be good to automate consistency checking of EXTRA_DIST with the actual regression test input files. Reproducible: Didn't try Actual Results: cachegrind/tests/x86/Makefile.am:1: error: insn_sse2.stdout.exp is missing in EXTRA_DIST callgrind/tests/Makefile.am:1: error: simwork-both.stderr.exp is missing in EXTRA_DIST callgrind/tests/Makefile.am:1: error: simwork-both.stdout.exp is missing in EXTRA_DIST callgrind/tests/Makefile.am:1: error: simwork-both.vgtest is missing in EXTRA_DIST callgrind/tests/Makefile.am:1: error: simwork-branch.stderr.exp is missing in EXTRA_DIST callgrind/tests/Makefile.am:1: error: simwork-branch.stdout.exp is missing in EXTRA_DIST callgrind/tests/Makefile.am:1: error: simwork-branch.vgtest is missing in EXTRA_DIST callgrind/tests/Makefile.am:1: error: simwork-cache.stderr.exp is missing in EXTRA_DIST callgrind/tests/Makefile.am:1: error: simwork-cache.stdout.exp is missing in EXTRA_DIST callgrind/tests/Makefile.am:1: error: simwork-cache.vgtest is missing in EXTRA_DIST helgrind/tests/Makefile.am:1: error: tc22_exit_w_lock.stderr.exp-kfail-x86 is missing in EXTRA_DIST memcheck/tests/linux/Makefile.am:1: error: capget.vgtest is missing in EXTRA_DIST memcheck/tests/linux/Makefile.am:1: error: syscalls-2007.vgtest is missing in EXTRA_DIST memcheck/tests/linux/Makefile.am:1: error: syslog-syscall.vgtest is missing in EXTRA_DIST memcheck/tests/linux/Makefile.am:1: error: timerfd-syscall.vgtest is missing in EXTRA_DIST none/tests/Makefile.am:1: error: empty-exe.stderr.exp is missing in EXTRA_DIST none/tests/Makefile.am:1: error: empty-exe.vgtest is missing in EXTRA_DIST Expected Results: .
Note: the output format has been chosen such that it matches that of gcc - most editors should be able to parse that output.
The script looks good to me, and found quite a few missing files. Some minor comments: * Usage of /bin/bash Wouldn't it be better to use /bin/sh ? (except 2 other bash and one zsh, all others scripts are /bin/sh). * I do not understand the reason for the below line: if grep -qw addsuffix $m; then continue; fi It looks like this will e.g. skip processing none/tests/amd64/Makefile.am (and several other Makefile.am containing addsuffix) Unclear why the *.vgtest and *.exp are not checked for these Makefile.am If needed to skip them, would be good to explain why. * would be good to check the *.gdb files (these are gdb input files for gdbserver_tests). * Wouldn't it be better to do this check at the end of regtest rather than at the beginning as these "failures" will not be visible in the summary of the failing tests but the output of the script will very probably not on the screen anymore. You might alsoexit the script with an error status if one or more files are not found.
Created attachment 64467 [details] Automate EXTRA_DIST consistency checking Version two of the attached patch addresses comments (1), (2) and (3). I'm not sure (4) is still relevant for version two of the patch since "make" will now stop if tests/check_tool_makefile finds any inconsistencies and exits with a non-zero exit status.
(In reply to comment #3) > Created an attachment (id=64467) [details] > Automate EXTRA_DIST consistency checking > > Version two of the attached patch addresses comments (1), (2) and (3). I'm not For what concerns comment 2: reason looks clear now (I just had to look :). I understand this addsuffix technique is a shortcut to avoid having the .vgtest, .stdout.exp and .stderr.exp be given explicitely. As for most tests, these are given explicitely, removing the usage of this addsuffix would not be a big work from what I can see. Alternatively, the verification could be done by having a new target in the Makefile.am making a full list of all the files in EXTRA_DIST, and comparing with the files listed with a find -name '*.vgtest' -o -name '*....' This would work whatever the way files are put in EXTRA_DIST. Finally on this subject, I do not understand what is the desired way to "compute" EXTRA_DIST. The comment in none/tests/amd64/Makefile.am about insn_sse3 tells that the files for this test are included in any case, but similar files for e.g. insn_pclmulqdq are not included. So, it is unclear to me if make dist is or not to be influenced by the platform on which the configure is done. > sure (4) is still relevant for version two of the patch since "make" will now > stop if tests/check_tool_makefile finds any inconsistencies and exits with a > non-zero exit status. I would still prefer to have the check at the end of make regtest: regression tests should preferrably not be "blocked/stopped" by one failure (even this kind of failure).
Created attachment 64490 [details] Automate EXTRA_DIST consistency checking Changes in v3 compared to v2: - Moved EXTRA_DIST consistency checking after running regression tests. - Added primitive Makefile parser such that $(addsuffix ...) is understood. - New output: callgrind/tests/Makefile.am:1: error: simwork-both.stderr.exp is missing in EXTRA_DIST callgrind/tests/Makefile.am:1: error: simwork-both.stdout.exp is missing in EXTRA_DIST callgrind/tests/Makefile.am:1: error: simwork-both.vgtest is missing in EXTRA_DIST callgrind/tests/Makefile.am:1: error: simwork-branch.stderr.exp is missing in EXTRA_DIST callgrind/tests/Makefile.am:1: error: simwork-branch.stdout.exp is missing in EXTRA_DIST callgrind/tests/Makefile.am:1: error: simwork-branch.vgtest is missing in EXTRA_DIST callgrind/tests/Makefile.am:1: error: simwork-cache.stderr.exp is missing in EXTRA_DIST callgrind/tests/Makefile.am:1: error: simwork-cache.stdout.exp is missing in EXTRA_DIST callgrind/tests/Makefile.am:1: error: simwork-cache.vgtest is missing in EXTRA_DIST helgrind/tests/Makefile.am:1: error: tc22_exit_w_lock.stderr.exp-kfail-x86 is missing in EXTRA_DIST memcheck/tests/linux/Makefile.am:1: error: capget.vgtest is missing in EXTRA_DIST memcheck/tests/linux/Makefile.am:1: error: syscalls-2007.vgtest is missing in EXTRA_DIST memcheck/tests/linux/Makefile.am:1: error: syslog-syscall.vgtest is missing in EXTRA_DIST memcheck/tests/linux/Makefile.am:1: error: timerfd-syscall.vgtest is missing in EXTRA_DIST memcheck/tests/linux/Makefile.am:1: error: capget is in EXTRA_DIST but doesn't exist memcheck/tests/linux/Makefile.am:1: error: syscalls-2007 is in EXTRA_DIST but doesn't exist memcheck/tests/linux/Makefile.am:1: error: syslog-syscall is in EXTRA_DIST but doesn't exist memcheck/tests/linux/Makefile.am:1: error: timerfd-syscall is in EXTRA_DIST but doesn't exist none/tests/amd64/Makefile.am:1: error: insn_pclmulqdq.stderr.exp is missing in EXTRA_DIST none/tests/amd64/Makefile.am:1: error: insn_pclmulqdq.stdout.exp is missing in EXTRA_DIST none/tests/amd64/Makefile.am:1: error: insn_pclmulqdq.vgtest is missing in EXTRA_DIST none/tests/x86/Makefile.am:1: error: insn_sse.stderr.exp is missing in EXTRA_DIST none/tests/x86/Makefile.am:1: error: insn_sse.stdout.exp is missing in EXTRA_DIST none/tests/x86/Makefile.am:1: error: insn_sse.vgtest is missing in EXTRA_DIST none/tests/x86/Makefile.am:1: error: insn_sse2.stderr.exp is missing in EXTRA_DIST none/tests/x86/Makefile.am:1: error: insn_sse2.stdout.exp is missing in EXTRA_DIST none/tests/x86/Makefile.am:1: error: insn_sse2.vgtest is missing in EXTRA_DIST none/tests/Makefile.am:1: error: empty-exe.stderr.exp is missing in EXTRA_DIST none/tests/Makefile.am:1: error: empty-exe.vgtest is missing in EXTRA_DIST none/tests/s390x/Makefile.am:1: error: cksm.stderr.exp is missing in EXTRA_DIST none/tests/s390x/Makefile.am:1: error: cksm.stdout.exp is missing in EXTRA_DIST none/tests/s390x/Makefile.am:1: error: cksm.vgtest is missing in EXTRA_DIST none/tests/s390x/Makefile.am:1: error: clcl.stderr.exp is missing in EXTRA_DIST none/tests/s390x/Makefile.am:1: error: clcl.stdout.exp is missing in EXTRA_DIST none/tests/s390x/Makefile.am:1: error: clcl.vgtest is missing in EXTRA_DIST none/tests/s390x/Makefile.am:1: error: fgx.stderr.exp is missing in EXTRA_DIST none/tests/s390x/Makefile.am:1: error: fgx.stdout.exp is missing in EXTRA_DIST none/tests/s390x/Makefile.am:1: error: fgx.vgtest is missing in EXTRA_DIST none/tests/s390x/Makefile.am:1: error: mvcl.stderr.exp is missing in EXTRA_DIST none/tests/s390x/Makefile.am:1: error: mvcl.stdout.exp is missing in EXTRA_DIST none/tests/s390x/Makefile.am:1: error: mvcl.vgtest is missing in EXTRA_DIST none/tests/s390x/Makefile.am:1: error: op_exception.stderr.exp is missing in EXTRA_DIST none/tests/s390x/Makefile.am:1: error: op_exception.stdout.exp is missing in EXTRA_DIST none/tests/s390x/Makefile.am:1: error: op_exception.vgtest is missing in EXTRA_DIST none/tests/s390x/Makefile.am:1: error: stck.stderr.exp is missing in EXTRA_DIST none/tests/s390x/Makefile.am:1: error: stck.stdout.exp is missing in EXTRA_DIST none/tests/s390x/Makefile.am:1: error: stck.vgtest is missing in EXTRA_DIST none/tests/s390x/Makefile.am:1: error: stcke.stderr.exp is missing in EXTRA_DIST none/tests/s390x/Makefile.am:1: error: stcke.stdout.exp is missing in EXTRA_DIST none/tests/s390x/Makefile.am:1: error: stcke.vgtest is missing in EXTRA_DIST none/tests/s390x/Makefile.am:1: error: stckf.stderr.exp is missing in EXTRA_DIST none/tests/s390x/Makefile.am:1: error: stckf.stdout.exp is missing in EXTRA_DIST none/tests/s390x/Makefile.am:1: error: stckf.vgtest is missing in EXTRA_DIST none/tests/s390x/Makefile.am:1: error: stfle.stderr.exp is missing in EXTRA_DIST none/tests/s390x/Makefile.am:1: error: stfle.stdout.exp is missing in EXTRA_DIST none/tests/s390x/Makefile.am:1: error: stfle.vgtest is missing in EXTRA_DIST
(In reply to comment #5) > Changes in v3 compared to v2: Nice solution. The big 'one line' awk script can be made more readable by inserting line feeds (this is ok inside single quote) and also | characters at the end of a line means no \ is needed at the end of the line. So, the below is somewhat more readable: parse_makefile() { cat "$1" | awk '/\\$/ { n=$0; sub("\\\\$", "", n); if (line != "") { line = line " " n } else { line=n } } /[^\\]$/ { if (line != "") { print line; line="" }; print}' | awk '{ if (match($0, "^ *([A-Za-z_]*) *= *(.*)$", m)) { var[m[1]] = m[2] }; for (v in var) { gsub("\\$\\(" v "\\)", var[v]) }; while (match($0, "\\$\\( *addsuffix *([^,)]*), *([^)]*)\\)", args)) { split(args[1], suff); split(args[2], name); name_and_suff=""; for (n in name) { for (s in suff) { name_and_suff = name_and_suff " " name[n] suff[s] } }; i=index($0, args[0]); $0 = substr($0, 1, i-1) name_and_suff substr($0, i + length(args[0])) }; print }' | sed 's/\$(noinst_SCRIPTS)//' }
Created attachment 64605 [details] Automate EXTRA_DIST consistency checking Reindented the awk code and fixed a bug in the awk line joining code (end-of-line backslash handling). Output for trunk r12161 after having run ./autogen.sh and ./configure: $ tests/check_tool_makefile *; echo $? 0 Or, in other words: no inconsistencies detected.
(In reply to comment #7) I played with it a bit. (1) We need gawk (mentioned here again only for completeness sake) awk may map to something not powerful enough (2) Handle comments I had changed none/tests/s390x/Makefile.am according to this patch Index: none/tests/s390x/Makefile.am =================================================================== --- none/tests/s390x/Makefile.am (revision 12162) +++ none/tests/s390x/Makefile.am (working copy) @@ -16,12 +16,13 @@ $(addsuffix .stderr.exp,$(INSN_TESTS)) \ $(addsuffix .stdout.exp,$(INSN_TESTS)) \ $(addsuffix .vgtest,$(INSN_TESTS)) \ - ex_sig.stdout.exp ex_sig.stderr.exp ex_sig.vgtest \ ex_clone.stdout.exp ex_clone.stderr.exp ex_clone.vgtest \ op00.stderr.exp1 op00.stderr.exp2 op00.vgtest \ test.h opcodes.h add.h and.h div.h insert.h \ mul.h or.h sub.h test.h xor.h +# ex_sig.stdout.exp ex_sig.stderr.exp ex_sig.vgtest \ + and then invoked like so ./tests/check_tool_makefile none/tests/s390x There were no complaints. The commented out files should be reported missing (3) In memcheck/tests there is a file named filter_memcheck which I forgot to add to dist_noinst_SCRIPTS in Makefile.am there. It would be good to catch that somehow. Or, if that is too complex, perhaps have a command line option which would list all files present in the directory that are not listed in EXTRA_DIST. Well, not all files, we could ignore stuff ending in ~ and the like. What do you think? Florian
Created attachment 64707 [details] Automate EXTRA_DIST consistency checking Ported from GNU awk to POSIX awk and made sure that commented out filenames are handled correctly. Does this help ?
(In reply to comment #9) > Ported from GNU awk to POSIX awk and made sure that commented out filenames are > handled correctly. Does this help ? Yes, it certainly does. Thanks! Are you planning to do something about (3) from comment #7? I think that would be a useful feature. For me anyhow :) One minor thing. The name "check_tool_makefile" is a bit misleading. What you're really doing is to check the makefiles in the regression buckets. So perhaps "check_regtest_makefile" would be a better name.
(In reply to comment #10) > Are you planning to do something about (3) from > comment #7? I think that would be a useful feature. For me anyhow :) The file patterns this script currently looks for are: *.exp* *.gdb *.vgtest. How about adding the pattern "filter*" to this list ? > One minor thing. The name "check_tool_makefile" is a bit misleading. What > you're really doing is to check the makefiles in the regression buckets. So > perhaps "check_regtest_makefile" would be a better name. Actually the script does even more - it currently checks all EXTRA_DIST parameters in all makefiles passed to it. In a clean source tree I get this: $ tests/check_tool_makefile Makefile.am Makefile.am:1: error: valgrind.spec is in EXTRA_DIST but doesn't exist
Created attachment 64738 [details] Automate EXTRA_DIST consistency checking Renamed script from check_tool_makefile into check_makefile_consistency and added consistency checks for the filter* scripts.
Applied v6 as r12214.