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

[Ralf Wildenhues]

Hi Jim,

* 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

Cheers,
Ralf