Re: [PATCH] maint.mk: prohibit definition of symbols defined by gnulib

[Jim Meyering]

Ralf Wildenhues wrote:
> * Jim Meyering wrote on Sun, May 09, 2010 at 07:28:17PM CEST:
>> It is unlike most other sc_ rules in that it creates a temporary
>> file, .re-defmac.  However, it does take care to remove it upon
>> receipt of a catchable signal.
>
> This looks a bit odd to me:
>
>> +.re-defmac:
>> +    @gen_h=$(gl_generated_headers_);                                \
>> +    (cd $(gnulib_dir)/lib;                                          \
>> +      for f in *.in.h $(gl_other_headers_); do                      \
>> +        perl -lne '$(gl_extract_significant_defines_)' $$f;         \
>> +      done;                                                         \
>> +    ) | sort -u                                                     \
>> +      | grep -Ev '^ATTRIBUTE_NORETURN'                              \
>> +      | sed 's/^/^ *# *define /;s/$$/\\>/'                          \
>> +      > address@hidden
>> +    @mv address@hidden $@
>> +
>> +define gl_trap_
>> +  Exit () { set +e; (exit $$1); exit $$1; };                                
>> \
>> +  for sig in 1 2 3 13 15; do                                                
>> \
>> +    eval "trap 'Exit $$(expr $$sig + 128)' $$sig";                  \
>> +  done
>> +endef
>> +
>> +# Don't define macros that we already get from gnulib header files.
>> +sc_prohibit_always-defined_macros: .re-defmac
>> +    @if test -d $(gnulib_dir); then                                 \
>> +      trap 'rc=$$?; rm -f .re-defmac; exit $$rc' 0;                 \
>
> There is a race in that the file won't be removed if the signal arrives
> after .re-defmac has been updated but before this trap is installed.
> Why not instead add it to CLEANFILES (or another fitting variable), or,
> since you're in GNU make land anyway, use .INTERMEDIATE?  (Not sure if
> you might need to mark it phony, or provide proper prerequisites for it
> in that case.)
>
> Also, how about using a file name space specific for the sc_* rules,
> e.g., .sc_re-defmac, so that you don't happen to remove a user file with
> that name?
>
> I'd add .re-defmac-t to CLEANFILES, too.
>
>> +      $(gl_trap_);                                                  \
>> +      grep -f .re-defmac $$($(VC_LIST_EXCEPT))                      \
>> +        && { echo '$(ME): define the above via some gnulib .h file' \
>> +              1>&2;  exit 1; } || :;                                \
>> +    fi

Hi Ralf,
Thanks for the good feedback.
I've found a way to eliminate the temporary file altogether.

>From 204e7feaf617b2e14fdf1226dfaf858e984c1737 Mon Sep 17 00:00:00 2001
From: Jim Meyering <address@hidden>
Date: Mon, 10 May 2010 09:29:09 +0200
Subject: [PATCH] maint.mk: avoid using a temporary file in the 
always-defined-macros check

* top/maint.mk (.re-defmac): Remove rule.
(gl_trap_): Remove definition.
(sc_prohibit_always-defined_macros): Rewrite not to create and
depend on a temporary file.  Instead, depend on GNU grep's ability
to read a list of regular expressions from stdin when given "-f -".
---
 ChangeLog    |    9 +++++++++
 top/maint.mk |   23 ++++++++---------------
 2 files changed, 17 insertions(+), 15 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index f2efae5..5510f68 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,12 @@
+2010-05-10  Jim Meyering  <address@hidden>
+
+       maint.mk: avoid using a temporary file in the always-defined-macros 
check
+       * top/maint.mk (.re-defmac): Remove rule.
+       (gl_trap_): Remove definition.
+       (sc_prohibit_always-defined_macros): Rewrite not to create and
+       depend on a temporary file.  Instead, depend on GNU grep's ability
+       to read a list of regular expressions from stdin when given "-f -".
+
 2010-05-09  Bruno Haible  <address@hidden>

        Update to GNU gettext 0.18.
diff --git a/top/maint.mk b/top/maint.mk
index 2e11ac1..867e4be 100644
--- a/top/maint.mk
+++ b/top/maint.mk
@@ -679,31 +679,24 @@ gl_extract_significant_defines_ = \

 # Create a list of regular expressions matching the names
 # of macros that are guaranteed to be defined by parts of gnulib.
-.re-defmac:
-       @gen_h=$(gl_generated_headers_);                                \
+define def_sym_regex
+       gen_h=$(gl_generated_headers_);                                 \
        (cd $(gnulib_dir)/lib;                                          \
          for f in *.in.h $(gl_other_headers_); do                      \
            perl -lne '$(gl_extract_significant_defines_)' $$f;         \
          done;                                                         \
        ) | sort -u                                                     \
          | grep -Ev '^ATTRIBUTE_NORETURN'                              \
-         | sed 's/^/^ *# *define /;s/$$/\\>/'                          \
-         > address@hidden
-       @mv address@hidden $@
-
-define gl_trap_
-  Exit () { set +e; (exit $$1); exit $$1; };                           \
-  for sig in 1 2 3 13 15; do                                           \
-    eval "trap 'Exit $$(expr $$sig + 128)' $$sig";                     \
-  done
+         | sed 's/^/^ *# *define /;s/$$/\\>/'
 endef

 # Don't define macros that we already get from gnulib header files.
-sc_prohibit_always-defined_macros: .re-defmac
+sc_prohibit_always-defined_macros:
        @if test -d $(gnulib_dir); then                                 \
-         trap 'rc=$$?; rm -f .re-defmac; exit $$rc' 0;                 \
-         $(gl_trap_);                                                  \
-         grep -f .re-defmac $$($(VC_LIST_EXCEPT))                      \
+         case $$(echo all: | grep -l -f - Makefile) in Makefile);; *)  \
+           echo '$(ME): skipping $@: you lack GNU grep' 1>&2; exit 0;; \
+         esac;                                                         \
+         $(def_sym_regex) | grep -f - $$($(VC_LIST_EXCEPT))            \
            && { echo '$(ME): define the above via some gnulib .h file' \
                  1>&2;  exit 1; } || :;                                \
        fi
--
1.7.1.189.g07419