Re: [PATCH] maint.mk: add per-line exclusions to prohibitions

[Jim Meyering]

Eric Blake wrote:
 _sc_search_regexp would let you skip entire files, but not
 individual lines, until now.  As proof that adding in a new
 per-line regex exception mechanism is useful, I found three
 syntax checks that were previously open-coded, only because
 they had to exclude individual lines.

 * maint.mk (_sc_search_regexp): Add $exclude parameter.
 (sc_prohibit_strcmp, sc_unmarked_diagnostics)
 (sc_const_long_option): Use it.

Thanks.
That looks fine, but I haven't tested it.
However, while reviewing it, I realized that you'll want to adjust
slightly.  Once I push the following (tomorrow morning), you can
add an export exclude = line of your own.

Or initialize with export exclude = ^$$ and simplify this part of your patch:

- | grep -vE $${exclude-^$$}  \
+ | grep -vE $$exclude\

From 1b98d11e35032bd2ceb1c4a67b66d4655100eeea Mon Sep 17 00:00:00 2001
From: Jim Meyering meyer...@redhat.com
Date: Thu, 1 Mar 2012 23:13:34 +0100
Subject: [PATCH] maint.mk: avoid spurious failure of _sc_search_regexp-using
 tests

* top/maint.mk: Initialize _sc_search_regexp parameters, so that
envvar settings cannot interfere.  Otherwise, setting envvars like
prohibit=foo require=bar, etc. would cause spurious test failures.
---
 ChangeLog|7 +++
 top/maint.mk |   10 ++
 2 files changed, 17 insertions(+)

diff --git a/ChangeLog b/ChangeLog
index ddc7b54..affd664 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,10 @@
+2012-03-01  Jim Meyering  meyer...@redhat.com
+
+   maint.mk: avoid spurious failure of _sc_search_regexp-using tests
+   * top/maint.mk: Initialize _sc_search_regexp parameters, so that
+   envvar settings cannot interfere.  Otherwise, setting envvars like
+   prohibit=foo require=bar, etc. would cause spurious test failures.
+
 2012-02-29  Eric Blake  ebl...@redhat.com

termios: fix pid_t always, not just for tcgetsid
diff --git a/top/maint.mk b/top/maint.mk
index ee721de..288002b 100644
--- a/top/maint.mk
+++ b/top/maint.mk
@@ -212,6 +212,16 @@ syntax-check: $(local-check)
 # when filtering by name via in_files, we explicitly filter out matching
 # names here as well.

+# Initialize each, so that envvar settings cannot interfere.
+export require =
+export prohibit =
+export in_vc_files =
+export in_files =
+export containing =
+export non_containing =
+export halt =
+export with_grep_options =
+
 # By default, _sc_search_regexp does not ignore case.
 export ignore_case =
 _ignore_case = $$(test -n $$ignore_case  printf %s -i || :)
--
1.7.9.2.324.g1221

Re: [PATCH] maint.mk: add per-line exclusions to prohibitions

[Eric Blake]

On 03/01/2012 03:20 PM, Jim Meyering wrote:
 
 Thanks.
 That looks fine, but I haven't tested it.
 However, while reviewing it, I realized that you'll want to adjust
 slightly.  Once I push the following (tomorrow morning), you can
 add an export exclude = line of your own.

Indeed.

 
 Or initialize with export exclude = ^$$ and simplify this part of your 
 patch:
 
 - | grep -vE $${exclude-^$$}
 \
 + | grep -vE $$exclude  \

Won't work, because then $exclude is always defined to a non-empty
value, but we complain if it both $require and $exclude are non-empty
($require and $prohibit are mutually-exclusive, but $exclude only works
with $prohibit).

 
From 1b98d11e35032bd2ceb1c4a67b66d4655100eeea Mon Sep 17 00:00:00 2001
 From: Jim Meyering meyer...@redhat.com
 Date: Thu, 1 Mar 2012 23:13:34 +0100
 Subject: [PATCH] maint.mk: avoid spurious failure of _sc_search_regexp-using
  tests
 
 * top/maint.mk: Initialize _sc_search_regexp parameters, so that
 envvar settings cannot interfere.  Otherwise, setting envvars like
 prohibit=foo require=bar, etc. would cause spurious test failures.
 ---
  ChangeLog|7 +++
  top/maint.mk |   10 ++
  2 files changed, 17 insertions(+)

Actually, I like your patch, so rather than waiting for tomorrow
morning, I went ahead and pushed your patch, along with the
initialization of exclude, after testing things in libvirt.

-- 
Eric Blake   ebl...@redhat.com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature