Re: [PATCH] maint.mk: syntax-check: prohibit HAVE_<header>_H that are always true

[Jim Meyering]

Pádraig Brady wrote:
> On 27/04/10 13:15, Jim Meyering wrote:
>> I've just pushed the patch below.
>> It exposed a few unnecessary #if tests in coreutils:
>
> Very nice check.

Thanks.

>> Hence the compromise of searching for all of them at once,
>> with the downside that occasionally you'll be warned about
>> a line like this one (from the patch above)
>>
>>   # if HAVE_NETINET_IN_H && HAVE_NFS_NFS_CLNT_H && HAVE_NFS_VFS_H
>>
>> and it'll be up to you to determine which it's talking about.
>
> That should be good enough.

I think so, too.
Typically it will happen once per project (when enabling the check),
so it's not worth more effort.

However, I've just noticed an infelicity (a bug ;-)
I tried it in libvirt, which stores gnulib in .gnulib/,
and was surprised to see this in the output of *every* "make" command:

    ls: cannot access *.in.h: No such file or directory

That's because libvirt does not yet define $(gnulib_dir)
(easy to fix, via cfg.mk), and because my new rule used
some_var := $(shell ...),
every single make command would run that code.
Here's a fix.

>From 411e141164861435385bb9bbb10012da28a077c0 Mon Sep 17 00:00:00 2001
From: Jim Meyering <address@hidden>
Date: Tue, 27 Apr 2010 16:32:40 +0200
Subject: [PATCH] maint.mk: avoid side-effect in latest syntax-check

* top/maint.mk (sc_prohibit_always_true_header_tests): Rework not
to run commands via $(shell...), and hence to incur cost only when
the new rule is actually run.
---
 ChangeLog    |    5 +++++
 top/maint.mk |   15 ++++++++-------
 2 files changed, 13 insertions(+), 7 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 64a9a3e..4d6e473 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,10 @@
 2010-04-27  Jim Meyering  <address@hidden>

+       maint.mk: avoid side-effect in latest syntax-check
+       * top/maint.mk (sc_prohibit_always_true_header_tests): Rework not
+       to run commands via $(shell...), and hence to incur cost only when
+       the new rule is actually run.
+
        maint.mk: syntax-check: prohibit HAVE_<header>_H that are always true
        Derive the list of guaranteed header names from gnulib/lib/*.in.h,
        and use that to create a regexp used to detect all #if HAVE_..._H uses.
diff --git a/top/maint.mk b/top/maint.mk
index 159fe83..8d9a522 100644
--- a/top/maint.mk
+++ b/top/maint.mk
@@ -649,20 +649,21 @@ sc_useless_cpp_parens:
 # using the appropriate gnulib module.  CAUTION: for each "unnecessary"
 # #if HAVE_HEADER_H that you remove, be sure that your project explicitly
 # requires the gnulib module that guarantees the usability of that header.
-gl_assured_headers_ := \
-  $(shell cd $(gnulib_dir)/lib && ls -1 *.in.h|sed 's/\.in\.h$$/ \\/')
+gl_assured_headers_ = \
+  cd $(gnulib_dir)/lib && echo *.in.h|sed 's/\.in\.h//'

 # Convert the list of names to upper case, and replace each space with "|".
 az_ = abcdefghijklmnopqrstuvwxyz
 AZ_ = ABCDEFGHIJKLMNOPQRSTUVWXYZ
-gl_header_upper_case_or_ := \
-  $(shell echo $(gl_assured_headers_)                                  \
+gl_header_upper_case_or_ =                                             \
+  $$($(gl_assured_headers_)                                            \
     | tr $(az_)/.- $(AZ_)___                                           \
     | tr -s ' ' '|'                                                    \
-   )
-gl_have_header_regex_ = HAVE_($(gl_header_upper_case_or_))_H
+    )
 sc_prohibit_always_true_header_tests:
-       @prohibit='\<$(gl_have_header_regex_)\>'                        \
+       @or=$(gl_header_upper_case_or_);                                \
+       re="HAVE_($$or)_H";                                             \
+       prohibit='\<'"$$re"'\>'                                         \
        halt='do not test the above HAVE_<header>_H symbol(s);\n'\
 '  with the corresponding gnulib module, they are always true'         \
          $(_sc_search_regexp)
--
1.7.1.328.g9993c