bug#31744: 26.1; Improvements to make tags and make -C test

[Noam Postavsky <npostavs@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 2484 bytes --]

Eli Zaretskii <eliz@gnu.org> writes:

>> Oh, hmm.  The original reason, is that when looking at a source file in
>> Emacs, and then hitting M-. I get a prompt to visit the TAGS table,
>> which starts from the source directory.  Then I have to go looking for
>> the TAGS file in the corresponding build directory.
>
> I have grown a habit a long time ago to "M-x visit-tags-table" before
> I issue the first "M-." command.
>
> The default prompt is fine, because what else would you expect Emacs
> to do in that case?

Yeah, there's no way for Emacs to know where the build directory is.
Maybe I'll add symlinks locally to work around this.

>> especially since every build directory will have identical TAGS
>> files anyway
>
> That's what happens currently, but it isn't carved in stone.  We
> could, for example, have TAGS reflect only the files that are compiled
> in on the current platform; other projects (like GDB, for example) do
> just that.  Then each build will have a different TAGS file.

I hope not, that sounds inconvenient to me.

> No, I think the principle is that the source tree holds everything
> that comes with a release tarball.

Okay, I dropped this change, and updated the FIXME comment to mention
this instead.


Going back to a question from earlier in the thread:

>> +ifeq ($(TEST_INTERACTIVE), yes)
>> +	HOME=/nonexistent $(emacs) \
>> +	  -l ert ${ert_opts} \
>> +	  $(patsubst %,-l %,$(if $(findstring $(TEST_LOAD_EL),yes),$ELFILES,$(ELFILES:.el=)))  \
>> +	  $(TEST_RUN_ERT)
>> +else
>>  	-@${MAKE} -k  ${LOGFILES}
>> -	@$(emacs) -l ert -f ert-summarize-tests-batch-and-exit ${LOGFILES}
>> +	@$(emacs) --batch -l ert -f ert-summarize-tests-batch-and-exit ${LOGFILES}
>> +endif

> Not sure I understand the HOME trick: why not use -Q?

$(emacs) already includes -Q (or rather, the equivalent long options
--no-init-file --no-site-file --no-site-lisp).  The HOME trick was added
in [d17aa3e535] and [412c38aa28].  I can't find a specific discussion,
but http://lists.gnu.org/archive/html/emacs-devel/2017-05/msg00641.html
seems related.  I've added a comment to explain this better.

[412c38aa28]: 2017-05-30 08:39:39 -0700
  Stop make check interacting with HOME
  https://git.savannah.gnu.org/cgit/emacs.git/commit/?id=412c38aa28dd7e8363b481a09d1df62c40f9a5b7
[d17aa3e535]: 2017-05-30 12:50:54 -0400
  Reduce scope of recent test/Makefile HOME change
  https://git.savannah.gnu.org/cgit/emacs.git/commit/?id=d17aa3e535bba5e93ff188d5460c91001074255e


[-- Attachment #2: patch --]
[-- Type: text/plain, Size: 2398 bytes --]

From 9078572053aed5e7cd9aa26d9dcff9409b7148e4 Mon Sep 17 00:00:00 2001
From: Noam Postavsky <npostavs@gmail.com>
Date: Thu, 7 Dec 2017 04:31:47 -0500
Subject: [PATCH v2 1/3] Make 'tags' targets respect --with-silent-rules
 (Bug#31744)

* lwlib/Makefile.in (TAGS):
* lisp/Makefile.in (TAGS):
* src/Makefile.in (TAGS): Use AM_V_GEN and AM_V_at.
* src/Makefile.in: Note that TAGS are generated in build dir.
---
 lisp/Makefile.in  | 6 +++---
 lwlib/Makefile.in | 2 +-
 src/Makefile.in   | 5 +++--
 3 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/lisp/Makefile.in b/lisp/Makefile.in
index d4709bd79d..05fca9579f 100644
--- a/lisp/Makefile.in
+++ b/lisp/Makefile.in
@@ -259,9 +259,9 @@ ${ETAGS}:
 ## compile-main.  But maybe this is not even necessary any more now
 ## that this uses relative filenames.
 TAGS: ${ETAGS} ${tagsfiles}
-	rm -f $@
-	touch $@
-	ls ${tagsfiles} | xargs $(XARGS_LIMIT) "${ETAGS}" -a -o $@
+	$(AM_V_at)rm -f $@
+	$(AM_V_at)touch $@
+	$(AM_V_GEN)ls ${tagsfiles} | xargs $(XARGS_LIMIT) "${ETAGS}" -a -o $@
 
 
 # The src/Makefile.in has its own set of dependencies and when they decide
diff --git a/lwlib/Makefile.in b/lwlib/Makefile.in
index 32d7a91f9b..6bd2608381 100644
--- a/lwlib/Makefile.in
+++ b/lwlib/Makefile.in
@@ -131,6 +131,6 @@ FORCE:
 .PHONY: tags FORCE
 tags: TAGS
 TAGS: ${ETAGS} $(ctagsfiles)
-	${ETAGS} $(ctagsfiles)
+	$(AM_V_GEN)${ETAGS} $(ctagsfiles)
 
 ### Makefile.in ends here
diff --git a/src/Makefile.in b/src/Makefile.in
index 15ca1667d6..6ed8f3cc91 100644
--- a/src/Makefile.in
+++ b/src/Makefile.in
@@ -674,13 +674,14 @@ ${ETAGS}:
 ctagsfiles1 = $(wildcard ${srcdir}/*.[hc])
 ctagsfiles2 = $(wildcard ${srcdir}/*.m)
 
-## FIXME? In out-of-tree builds, should TAGS be generated in srcdir?
+## In out-of-tree builds, TAGS are generated in the build dir, like
+## other non-bootstrap build products (see Bug#31744).
 
 ## This does not need to depend on ../lisp and ../lwlib TAGS files,
 ## because etags "--include" only includes a pointer to the file,
 ## rather than the file contents.
 TAGS: ${ETAGS} $(ctagsfiles1) $(ctagsfiles2)
-	${ETAGS} --include=../lisp/TAGS --include=$(lwlibdir)/TAGS \
+	$(AM_V_GEN)${ETAGS} --include=../lisp/TAGS --include=$(lwlibdir)/TAGS \
 	  --regex='{c}/[ 	]*DEFVAR_[A-Z_ 	(]+"\([^"]+\)"/\1/' \
 	  --regex='{c}/[ 	]*DEFVAR_[A-Z_ 	(]+"[^"]+",[ 	]\([A-Za-z0-9_]+\)/\1/' \
 	  $(ctagsfiles1) \
-- 
2.11.0


[-- Attachment #3: patch --]
[-- Type: text/plain, Size: 4057 bytes --]

From 2c2dfd66718d464288c91824ecff8564ee6f3d91 Mon Sep 17 00:00:00 2001
From: Noam Postavsky <npostavs@gmail.com>
Date: Sat, 16 Dec 2017 20:06:11 -0500
Subject: [PATCH v2 2/3] ; test/Makefile.in: Add TEST_INTERACTIVE option
 (Bug#31744).

* test/README: Note the new option.
---
 test/Makefile.in | 33 +++++++++++++++++++++++++++------
 test/README      |  6 ++++++
 2 files changed, 33 insertions(+), 6 deletions(-)

diff --git a/test/Makefile.in b/test/Makefile.in
index e6b3f77523..451513a747 100644
--- a/test/Makefile.in
+++ b/test/Makefile.in
@@ -81,7 +81,7 @@ EMACS_EXTRAOPT=
 # Command line flags for Emacs.
 # Apparently MSYS bash would convert "-L :" to "-L ;" anyway,
 # but we might as well be explicit.
-EMACSOPT = -batch --no-site-file --no-site-lisp -L "$(SEPCHAR)$(srcdir)" $(EMACS_EXTRAOPT)
+EMACSOPT = --no-init-file --no-site-file --no-site-lisp -L "$(SEPCHAR)$(srcdir)" $(EMACS_EXTRAOPT)
 
 # Prevent any settings in the user environment causing problems.
 unexport EMACSDATA EMACSDOC EMACSPATH GREP_OPTIONS
@@ -94,6 +94,15 @@ GDB =
 # supported everywhere.
 TEST_LOCALE = C
 
+# Set this to 'yes' to run the tests in an interactive instance.
+TEST_INTERACTIVE ?= no
+
+ifeq ($(TEST_INTERACTIVE),yes)
+TEST_RUN_ERT = --eval "(ert ${SELECTOR_ACTUAL})"
+else
+TEST_RUN_ERT = --batch --eval "(ert-run-tests-batch-and-exit ${SELECTOR_ACTUAL})" ${WRITE_LOG}
+endif
+
 # Whether to run tests from .el files in preference to .elc, we do
 # this by default since it gives nicer stacktraces.
 TEST_LOAD_EL ?= yes
@@ -120,6 +129,11 @@ emacs =
  EMACS_TEST_DIRECTORY=$(abspath $(srcdir)) \
  $(GDB) "$(EMACS)" $(MODULES_EMACSOPT) $(EMACSOPT)
 
+# Set HOME to a nonexistent directory to prevent tests from accessing
+# it accidentally (e.g., popping up a gnupg dialog if ~/.authinfo.gpg
+# exists, or writing to ~/.bzr.log when running bzr commands).
+TEST_HOME = /nonexistent
+
 test_module_dir := $(srcdir)/data/emacs-module
 
 .PHONY: all check
@@ -128,7 +142,7 @@ all:
 
 SELECTOR_DEFAULT = (quote (not (or (tag :expensive-test) (tag :unstable))))
 SELECTOR_EXPENSIVE = (quote (not (tag :unstable)))
-SELECTOR_ALL = nil
+SELECTOR_ALL = t
 ifdef SELECTOR
 SELECTOR_ACTUAL=$(SELECTOR)
 else ifndef MAKECMDGOALS
@@ -145,7 +159,7 @@ SELECTOR_ACTUAL=
 
 ## Byte-compile all test files to test for errors.
 %.elc: %.el
-	$(AM_V_ELC)$(emacs) -f batch-byte-compile $<
+	$(AM_V_ELC)$(emacs) --batch -f batch-byte-compile $<
 
 ## Save logs, and show logs for failed tests.
 WRITE_LOG = > $@ 2>&1 || { STAT=$$?; cat $@; exit $$STAT; }
@@ -158,9 +172,9 @@ testloadfile =
 
 %.log: %.elc
 	$(AM_V_at)${MKDIR_P} $(dir $@)
-	$(AM_V_GEN)HOME=/nonexistent $(emacs) \
+	$(AM_V_GEN)HOME=$(TEST_HOME) $(emacs) \
 	  -l ert ${ert_opts} -l $(testloadfile) \
-	  --eval "(ert-run-tests-batch-and-exit ${SELECTOR_ACTUAL})" ${WRITE_LOG}
+	  $(TEST_RUN_ERT)
 
 ifeq (@HAVE_MODULES@, yes)
 maybe_exclude_module_tests :=
@@ -260,8 +274,15 @@ .PHONY:
 ## We can't put LOGFILES as prerequisites, because that would stop the
 ## summarizing step from running when there is an error.
 check-doit:
+ifeq ($(TEST_INTERACTIVE), yes)
+	HOME=$(TEST_HOME) $(emacs) \
+	  -l ert ${ert_opts} \
+	  $(patsubst %,-l %,$(if $(findstring $(TEST_LOAD_EL),yes),$ELFILES,$(ELFILES:.el=)))  \
+	  $(TEST_RUN_ERT)
+else
 	-@${MAKE} -k  ${LOGFILES}
-	@$(emacs) -l ert -f ert-summarize-tests-batch-and-exit ${LOGFILES}
+	@$(emacs) --batch -l ert -f ert-summarize-tests-batch-and-exit ${LOGFILES}
+endif
 
 .PHONY: mostlyclean clean bootstrap-clean distclean maintainer-clean
 
diff --git a/test/README b/test/README
index 1cd9db3bb8..c1dde2e0d0 100644
--- a/test/README
+++ b/test/README
@@ -50,6 +50,12 @@ nicer backtraces.  To run the compiled version of a test use
 
     make TEST_LOAD_EL=no ...
 
+The tests are run in batch mode by default; sometimes it's useful to
+get precisely the same environment but run in interactive mode for
+debugging.  To do that, use
+
+    make TEST_INTERACTIVE=yes ...
+
 \f
 (Also, see etc/compilation.txt for compilation mode font lock tests.)
 
-- 
2.11.0


[-- Attachment #4: patch --]
[-- Type: text/plain, Size: 2642 bytes --]

From a86a31659937556059f34beb75df1cbff73c5464 Mon Sep 17 00:00:00 2001
From: Noam Postavsky <npostavs@gmail.com>
Date: Wed, 6 Jun 2018 21:25:52 -0400
Subject: [PATCH v2 3/3] ; Reduce quoting for SELECTOR in 'make -C test'
 (Bug#31744)

Before:

    make -C test SELECTOR='\"foo\"'
    make -C test SELECTOR='(quote (tag :some-tag))'

After:

    make -C test SELECTOR='"foo"'
    make -C test SELECTOR='(tag :some-tag)'

* test/Makefile.in: Use single quotes around the command line call to
ert, this means the user doesn't have to backslash escape double
quotes when writing lisp strings for the selector.  Also wrap the
SELECTOR value in (quote ...) so the user won't have to type it
in (and not get tempted to use the '... reader syntax form which would
now fail to work due to using single quotes around the whole shell
arg).
* test/README: Update instructions accordingly.
---
 test/Makefile.in | 8 ++++----
 test/README      | 5 ++++-
 2 files changed, 8 insertions(+), 5 deletions(-)

diff --git a/test/Makefile.in b/test/Makefile.in
index 451513a747..597ef91311 100644
--- a/test/Makefile.in
+++ b/test/Makefile.in
@@ -98,9 +98,9 @@ TEST_LOCALE =
 TEST_INTERACTIVE ?= no
 
 ifeq ($(TEST_INTERACTIVE),yes)
-TEST_RUN_ERT = --eval "(ert ${SELECTOR_ACTUAL})"
+TEST_RUN_ERT = --eval '(ert (quote ${SELECTOR_ACTUAL}))'
 else
-TEST_RUN_ERT = --batch --eval "(ert-run-tests-batch-and-exit ${SELECTOR_ACTUAL})" ${WRITE_LOG}
+TEST_RUN_ERT = --batch --eval '(ert-run-tests-batch-and-exit (quote ${SELECTOR_ACTUAL}))' ${WRITE_LOG}
 endif
 
 # Whether to run tests from .el files in preference to .elc, we do
@@ -140,8 +140,8 @@ .PHONY:
 
 all: check
 
-SELECTOR_DEFAULT = (quote (not (or (tag :expensive-test) (tag :unstable))))
-SELECTOR_EXPENSIVE = (quote (not (tag :unstable)))
+SELECTOR_DEFAULT = (not (or (tag :expensive-test) (tag :unstable)))
+SELECTOR_EXPENSIVE = (not (tag :unstable))
 SELECTOR_ALL = t
 ifdef SELECTOR
 SELECTOR_ACTUAL=$(SELECTOR)
diff --git a/test/README b/test/README
index c1dde2e0d0..e473248c9e 100644
--- a/test/README
+++ b/test/README
@@ -42,7 +42,10 @@ except the tests tagged as expensive.
 
 If your test file contains the tests "test-foo", "test2-foo" and
 "test-foo-remote", and you want to run only the former two tests, you
-could use a selector regexp: "make <filename> SELECTOR='\"foo$$\"'".
+could use a selector regexp (note that the "$" needs to be doubled to
+protect against "make" variable expansion):
+
+    make <filename> SELECTOR='"foo$$"'
 
 Note that although the test files are always compiled (unless they set
 no-byte-compile), the source files will be run by default, to give
-- 
2.11.0