bug-gnulib@gnu.org mirror (unofficial)
 help / color / mirror / Atom feed
* [PATCH v3 0/2] Fix syntax-check on macOS/FreeBSD
@ 2018-12-13 15:34 Roman Bolshakov
  2018-12-13 15:34 ` [PATCH v3 1/2] maint.mk: Split long argument lists Roman Bolshakov
                   ` (2 more replies)
  0 siblings, 3 replies; 9+ messages in thread
From: Roman Bolshakov @ 2018-12-13 15:34 UTC (permalink / raw)
  To: bug-gnulib; +Cc: Roman Bolshakov

Hello,

There was an issue with syntax-check on FreeBSD reported a few years
ago:
https://www.redhat.com/archives/libvir-list/2015-August/msg00758.html
http://lists.gnu.org/archive/html/bug-gnulib/2015-08/msg00019.html

The patch series attempts to resolve the issue on gnulib side. With
related changes on libvirt side I can run make syntax-check on macOS.

* modules/maintainer-makefile (configure.ac): Look for the best
  available ggrep or grep with AC_PROG_GREP.
* top/maint.mk: Split long argument lists with xargs. Replace grep
  invocations with $(GREP).

v2: http://lists.gnu.org/archive/html/bug-gnulib/2018-12/msg00026.html

Changes since v2:
- removed /dev/null argument for grep invocations with "-l" flag
- reformatted all affected multi-line commands to be more consistent
  with the style of the code around with regards to &&, || and |
- removed dependency on mkdir in sc_prohibit_always-defined_macros
- rewrote sc_po_check to avoid running perl with $(generated_files)
  multiple times
- rewrote sc_cross_check_PATH_usage_in_tests to simplify indentation

v1: http://lists.gnu.org/archive/html/bug-gnulib/2018-11/msg00069.html

Changes since v1:
- removed VC_ARG_MAX
- removed "-n" flag in xargs invocations
- added /dev/null as an extra file for uniform grep output regardless of
  number of files. The file isn't added if grep invocation has "-L" flag
- added dependency on AC_PROG_GREP to modules/maintainer-makefile

--
Best regards,
Roman

Roman Bolshakov (2):
  maint.mk: Split long argument lists
  maint.mk: Replace grep with $(GREP)

 modules/maintainer-makefile |   1 +
 top/maint.mk                | 207 +++++++++++++++++++++---------------
 2 files changed, 123 insertions(+), 85 deletions(-)

-- 
2.17.2 (Apple Git-113)



^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH v3 1/2] maint.mk: Split long argument lists
  2018-12-13 15:34 [PATCH v3 0/2] Fix syntax-check on macOS/FreeBSD Roman Bolshakov
@ 2018-12-13 15:34 ` Roman Bolshakov
  2019-01-02 19:31   ` Eric Blake
  2018-12-13 15:34 ` [PATCH v3 2/2] maint.mk: Replace grep with $(GREP) Roman Bolshakov
  2018-12-20 13:06 ` [PATCH v3 0/2] Fix syntax-check on macOS/FreeBSD Roman Bolshakov
  2 siblings, 1 reply; 9+ messages in thread
From: Roman Bolshakov @ 2018-12-13 15:34 UTC (permalink / raw)
  To: bug-gnulib; +Cc: Roman Bolshakov

$(VC_LIST_EXCEPT) is usually expanded into arguments for a command.
When a project contains too many, some operating systems can't pass all
the arguments because they hit the limit of arguments. FreeBSD and macOS
are known to have the limit of 256k of arguments.

More on the issue:
http://lists.gnu.org/archive/html/bug-gnulib/2015-08/msg00019.html
https://www.redhat.com/archives/libvir-list/2015-August/msg00758.html

xargs without flags can be used to limit number of arguments. The
default number of arguments (max-args for "-n" flag) is
platform-specific. If argument length exceeds default value for "-s"
flag (max-chars), xargs will feed less arguments than max-args.

Signed-off-by: Roman Bolshakov <r.bolshakov@yadro.com>
---
 top/maint.mk | 135 ++++++++++++++++++++++++++++++++-------------------
 1 file changed, 86 insertions(+), 49 deletions(-)

diff --git a/top/maint.mk b/top/maint.mk
index 4889ebacc..1152ad698 100644
--- a/top/maint.mk
+++ b/top/maint.mk
@@ -302,32 +302,46 @@ define _sc_search_regexp
    fi;									\
 									\
    : Filter by content;							\
-   test -n "$$files" && test -n "$$containing"				\
-     && { files=$$(grep -l "$$containing" $$files); } || :;		\
-   test -n "$$files" && test -n "$$non_containing"			\
-     && { files=$$(grep -vl "$$non_containing" $$files); } || :;	\
+   test -n "$$files"							\
+     && test -n "$$containing"						\
+     && { files=$$(echo "$$files" | xargs grep -l "$$containing"); }	\
+     || :;								\
+   test -n "$$files"							\
+     && test -n "$$non_containing"					\
+     && { files=$$(echo "$$files" | xargs grep -vl "$$non_containing"); } \
+     || :;								\
 									\
    : Check for the construct;						\
    if test -n "$$files"; then						\
      if test -n "$$prohibit"; then					\
-       grep $$with_grep_options $(_ignore_case) -nE "$$prohibit" $$files \
+       echo "$$files" /dev/null						\
+         | xargs							\
+             grep $$with_grep_options $(_ignore_case) -nE "$$prohibit"	\
          | grep -vE "$${exclude:-^$$}"					\
-         && { msg="$$halt" $(_sc_say_and_exit) } || :;			\
+         && { msg="$$halt" $(_sc_say_and_exit) }			\
+         || :;								\
      else								\
-       grep $$with_grep_options $(_ignore_case) -LE "$$require" $$files \
-           | grep .							\
-         && { msg="$$halt" $(_sc_say_and_exit) } || :;			\
+       echo "$$files"							\
+         | xargs							\
+             grep $$with_grep_options $(_ignore_case) -LE "$$require"	\
+         | grep .							\
+         && { msg="$$halt" $(_sc_say_and_exit) }			\
+         || :;								\
      fi									\
    else :;								\
    fi || :;
 endef
 
 sc_avoid_if_before_free:
-	@$(srcdir)/$(_build-aux)/useless-if-before-free			\
-		$(useless_free_options)					\
-	    $$($(VC_LIST_EXCEPT) | grep -v useless-if-before-free) &&	\
-	  { echo '$(ME): found useless "if" before "free" above' 1>&2;	\
-	    exit 1; } || :
+	@$(VC_LIST_EXCEPT)						\
+	  | grep -v useless-if-before-free				\
+	  | xargs							\
+	      $(srcdir)/$(_build-aux)/useless-if-before-free		\
+	      $(useless_free_options)					\
+	  && { printf '$(ME): found useless "if"'			\
+		      ' before "free" above\n' 1>&2;			\
+	       exit 1; }						\
+	  || :
 
 sc_cast_of_argument_to_free:
 	@prohibit='\<free *\( *\(' halt="don't cast free argument"	\
@@ -399,25 +413,31 @@ sc_error_exit_success:
 # "FATAL:" should be fully upper-cased in error messages
 # "WARNING:" should be fully upper-cased, or fully lower-cased
 sc_error_message_warn_fatal:
-	@grep -nEA2 '[^rp]error *\(' $$($(VC_LIST_EXCEPT))		\
-	    | grep -E '"Warning|"Fatal|"fatal' &&			\
-	  { echo '$(ME): use FATAL, WARNING or warning'	1>&2;		\
-	    exit 1; } || :
+	@$(VC_LIST_EXCEPT)						\
+	  | xargs grep -nEA2 '[^rp]error *\(' /dev/null			\
+	  | grep -E '"Warning|"Fatal|"fatal'				\
+	  && { echo '$(ME): use FATAL, WARNING or warning' 1>&2;	\
+	       exit 1; }						\
+	  || :
 
 # Error messages should not start with a capital letter
 sc_error_message_uppercase:
-	@grep -nEA2 '[^rp]error *\(' $$($(VC_LIST_EXCEPT))		\
-	    | grep -E '"[A-Z]'						\
-	    | grep -vE '"FATAL|"WARNING|"Java|"C#|PRIuMAX' &&		\
-	  { echo '$(ME): found capitalized error message' 1>&2;		\
-	    exit 1; } || :
+	@$(VC_LIST_EXCEPT)						\
+	  | xargs grep -nEA2 '[^rp]error *\(' /dev/null			\
+	  | grep -E '"[A-Z]'						\
+	  | grep -vE '"FATAL|"WARNING|"Java|"C#|PRIuMAX'		\
+	  && { echo '$(ME): found capitalized error message' 1>&2;	\
+	       exit 1; }						\
+	  || :
 
 # Error messages should not end with a period
 sc_error_message_period:
-	@grep -nEA2 '[^rp]error *\(' $$($(VC_LIST_EXCEPT))		\
-	    | grep -E '[^."]\."' &&					\
-	  { echo '$(ME): found error message ending in period' 1>&2;	\
-	    exit 1; } || :
+	@$(VC_LIST_EXCEPT)						\
+	  | xargs grep -nEA2 '[^rp]error *\(' /dev/null			\
+	  | grep -E '[^."]\."'						\
+	  && { echo '$(ME): found error message ending in period' 1>&2;	\
+	       exit 1; }						\
+	  || :
 
 sc_file_system:
 	@prohibit=file''system						\
@@ -845,9 +865,14 @@ sc_prohibit_always-defined_macros:
 	  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 -E -f - $$($(VC_LIST_EXCEPT))		\
-	    && { echo '$(ME): define the above via some gnulib .h file'	\
-		  1>&2;  exit 1; } || :;				\
+	  regex=`$(def_sym_regex)`; export regex;			\
+	  $(VC_LIST_EXCEPT)						\
+	    | xargs sh -c 'echo $$regex | grep -E -f - "$$@"'		\
+	   	       dummy /dev/null					\
+	    && { printf '$(ME): define the above'			\
+			' via some gnulib .h file\n' 1>&2;		\
+	         exit 1; }						\
+	    || :;							\
 	fi
 # ==================================================================
 
@@ -927,9 +952,11 @@ require_exactly_one_NL_at_EOF_ =					\
     }									\
   END { exit defined $$fail }
 sc_prohibit_empty_lines_at_EOF:
-	@perl -le '$(require_exactly_one_NL_at_EOF_)' $$($(VC_LIST_EXCEPT)) \
-	  || { echo '$(ME): empty line(s) or no newline at EOF'		\
-		1>&2; exit 1; } || :
+	@$(VC_LIST_EXCEPT)						\
+	  | xargs perl -le '$(require_exactly_one_NL_at_EOF_)'		\
+	  || { echo '$(ME): empty line(s) or no newline at EOF' 1>&2;	\
+	       exit 1; }						\
+	  || :
 
 # Make sure we don't use st_blocks.  Use ST_NBLOCKS instead.
 # This is a bit of a kludge, since it prevents use of the string
@@ -972,9 +999,12 @@ prohibit_doubled_word_ =						\
 ignore_doubled_word_match_RE_ ?= ^$$
 
 sc_prohibit_doubled_word:
-	@perl -n -0777 $(prohibit_doubled_word_) $$($(VC_LIST_EXCEPT))	\
+	@$(VC_LIST_EXCEPT)						\
+	  | xargs perl -n -0777 $(prohibit_doubled_word_)		\
 	  | grep -vE '$(ignore_doubled_word_match_RE_)'			\
-	  | grep . && { echo '$(ME): doubled words' 1>&2; exit 1; } || :
+	  | grep .							\
+	  && { echo '$(ME): doubled words' 1>&2; exit 1; }		\
+	  || :
 
 # A regular expression matching undesirable combinations of words like
 # "can not"; this matches them even when the two words appear on different
@@ -998,10 +1028,12 @@ prohibit_undesirable_word_seq_ =					\
 ignore_undesirable_word_sequence_RE_ ?= ^$$
 
 sc_prohibit_undesirable_word_seq:
-	@perl -n -0777 $(prohibit_undesirable_word_seq_)		\
-	     $$($(VC_LIST_EXCEPT))					\
-	  | grep -vE '$(ignore_undesirable_word_sequence_RE_)' | grep .	\
-	  && { echo '$(ME): undesirable word sequence' >&2; exit 1; } || :
+	@$(VC_LIST_EXCEPT)						\
+	  | xargs perl -n -0777 $(prohibit_undesirable_word_seq_)	\
+	  | grep -vE '$(ignore_undesirable_word_sequence_RE_)'		\
+	  | grep .							\
+	  && { echo '$(ME): undesirable word sequence' >&2; exit 1; }   \
+	  || :
 
 # Except for shell files and for loops, double semicolon is probably a mistake
 sc_prohibit_double_semicolon:
@@ -1033,7 +1065,8 @@ sc_prohibit_test_double_equal:
 # definition of LDADD from the appropriate Makefile.am and exits 0
 # when it contains "ICONV".
 sc_proper_name_utf8_requires_ICONV:
-	@progs=$$(grep -l 'proper_name_utf8 ''("' $$($(VC_LIST_EXCEPT)));\
+	@progs=$$($(VC_LIST_EXCEPT)					\
+		    | xargs grep -l 'proper_name_utf8 ''("');		\
 	if test "x$$progs" != x; then					\
 	  fail=0;							\
 	  for p in $$progs; do						\
@@ -1155,10 +1188,11 @@ sc_po_check:
 	@if test -f $(po_file); then					\
 	  grep -E -v '^(#|$$)' $(po_file)				\
 	    | grep -v '^src/false\.c$$' | sort > $@-1;			\
-	  files=$$(perl $(perl_translatable_files_list_)		\
-	    $$($(VC_LIST_EXCEPT)) $(generated_files));			\
-	  grep -E -l '$(_gl_translatable_string_re)' $$files		\
-	    | $(SED) 's|^$(_dot_escaped_srcdir)/||' | sort -u > $@-2;	\
+	  { $(VC_LIST_EXCEPT); echo $(generated_files); }		\
+	    | xargs perl $(perl_translatable_files_list_)		\
+	    | xargs grep -E -l '$(_gl_translatable_string_re)'		\
+	    | $(SED) 's|^$(_dot_escaped_srcdir)/||'			\
+	    | sort -u > $@-2;						\
 	  diff -u -L $(po_file) -L $(po_file) $@-1 $@-2			\
 	    || { printf '$(ME): '$(fix_po_file_diag) 1>&2; exit 1; };	\
 	  rm -f $@-1 $@-2;						\
@@ -1229,11 +1263,14 @@ sc_cross_check_PATH_usage_in_tests:
 	    || { echo "$@: $(_hv_file) lacks conforming use of init.sh" 1>&2; \
 		 exit 1; };						\
 	  good=$$(grep -E '$(_hv_regex_strong)' $(_hv_file));		\
-	  grep -LFx "$$good"						\
-		$$(grep -lE '$(_hv_regex_weak)' $$($(VC_LIST_EXCEPT)))	\
-	      | grep . &&						\
-	    { echo "$(ME): the above files use path_prepend_ inconsistently" \
-		1>&2; exit 1; } || :;					\
+	  $(VC_LIST_EXCEPT)						\
+	    | xargs grep -lE '$(_hv_regex_weak)'			\
+	    | xargs grep -LFx "$$good"					\
+	    | grep .							\
+	    && { printf "$(ME): the above files use"			\
+			" path_prepend_ inconsistently\n" 1>&2;		\
+		 exit 1; }						\
+	    || :;							\
 	fi
 
 # BRE regex of file contents to identify a test script.
-- 
2.17.2 (Apple Git-113)



^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH v3 2/2] maint.mk: Replace grep with $(GREP)
  2018-12-13 15:34 [PATCH v3 0/2] Fix syntax-check on macOS/FreeBSD Roman Bolshakov
  2018-12-13 15:34 ` [PATCH v3 1/2] maint.mk: Split long argument lists Roman Bolshakov
@ 2018-12-13 15:34 ` Roman Bolshakov
  2018-12-20 13:06 ` [PATCH v3 0/2] Fix syntax-check on macOS/FreeBSD Roman Bolshakov
  2 siblings, 0 replies; 9+ messages in thread
From: Roman Bolshakov @ 2018-12-13 15:34 UTC (permalink / raw)
  To: bug-gnulib; +Cc: Roman Bolshakov

A project that uses maint.mk can specify regular expressions that are
not supported in system grep. Autoconf can discover an alias for GNU
grep and set it in GREP but it takes no effect for maint.mk

The patch provides an ability to use GNU grep if it was discovered by
autoconf and by calling GNU grep we don't get the messages in syntax-check:
  prohibit_diagnostic_without_format
  grep: empty (sub)expression
  grep: empty (sub)expression
  grep: empty (sub)expression
  grep: empty (sub)expression
  grep: empty (sub)expression
  grep: empty (sub)expression

Signed-off-by: Roman Bolshakov <r.bolshakov@yadro.com>
---
 modules/maintainer-makefile |   1 +
 top/maint.mk                | 116 ++++++++++++++++++------------------
 2 files changed, 59 insertions(+), 58 deletions(-)

diff --git a/modules/maintainer-makefile b/modules/maintainer-makefile
index 39b51583c..13b8c546a 100644
--- a/modules/maintainer-makefile
+++ b/modules/maintainer-makefile
@@ -14,6 +14,7 @@ configure.ac:
 AC_CONFIG_COMMANDS_PRE([m4_ifdef([AH_HEADER],
   [AC_SUBST([CONFIG_INCLUDE], m4_defn([AH_HEADER]))])])
 AC_REQUIRE([AC_PROG_SED])
+AC_REQUIRE([AC_PROG_GREP])
 
 Makefile.am:
 EXTRA_DIST += $(top_srcdir)/maint.mk
diff --git a/top/maint.mk b/top/maint.mk
index 1152ad698..c981567ba 100644
--- a/top/maint.mk
+++ b/top/maint.mk
@@ -46,7 +46,7 @@ member-check =								\
 # Do not save the original name or timestamp in the .tar.gz file.
 # Use --rsyncable if available.
 gzip_rsyncable := \
-  $(shell gzip --help 2>/dev/null|grep rsyncable >/dev/null \
+  $(shell gzip --help 2>/dev/null|$(GREP) rsyncable >/dev/null \
     && printf %s --rsyncable)
 GZIP_ENV = '--no-name --best $(gzip_rsyncable)'
 
@@ -83,9 +83,9 @@ _sc_excl = \
   $(or $(exclude_file_name_regexp--$@),^$$)
 VC_LIST_EXCEPT = \
   $(VC_LIST) | $(SED) 's|^$(_dot_escaped_srcdir)/||' \
-	| if test -f $(srcdir)/.x-$@; then grep -vEf $(srcdir)/.x-$@; \
-	  else grep -Ev -e "$${VC_LIST_EXCEPT_DEFAULT-ChangeLog}"; fi \
-	| grep -Ev -e '($(VC_LIST_ALWAYS_EXCLUDE_REGEX)|$(_sc_excl))' \
+	| if test -f $(srcdir)/.x-$@; then $(GREP) -vEf $(srcdir)/.x-$@; \
+	  else $(GREP) -Ev -e "$${VC_LIST_EXCEPT_DEFAULT-ChangeLog}"; fi \
+	| $(GREP) -Ev -e '($(VC_LIST_ALWAYS_EXCLUDE_REGEX)|$(_sc_excl))' \
 	$(_prepend_srcdir_prefix)
 
 ifeq ($(origin prev_version_file), undefined)
@@ -292,23 +292,23 @@ define _sc_search_regexp
 									\
    : Filter by file name;						\
    if test -n "$$in_files"; then					\
-     files=$$(find $(srcdir) | grep -E "$$in_files"			\
-              | grep -Ev '$(_sc_excl)');				\
+     files=$$(find $(srcdir) | $(GREP) -E "$$in_files"			\
+              | $(GREP) -Ev '$(_sc_excl)');				\
    else									\
      files=$$($(VC_LIST_EXCEPT));					\
      if test -n "$$in_vc_files"; then					\
-       files=$$(echo "$$files" | grep -E "$$in_vc_files");		\
+       files=$$(echo "$$files" | $(GREP) -E "$$in_vc_files");		\
      fi;								\
    fi;									\
 									\
    : Filter by content;							\
    test -n "$$files"							\
      && test -n "$$containing"						\
-     && { files=$$(echo "$$files" | xargs grep -l "$$containing"); }	\
+     && { files=$$(echo "$$files" | xargs $(GREP) -l "$$containing"); }	\
      || :;								\
    test -n "$$files"							\
      && test -n "$$non_containing"					\
-     && { files=$$(echo "$$files" | xargs grep -vl "$$non_containing"); } \
+     && { files=$$(echo "$$files" | xargs $(GREP) -vl "$$non_containing"); } \
      || :;								\
 									\
    : Check for the construct;						\
@@ -316,15 +316,15 @@ define _sc_search_regexp
      if test -n "$$prohibit"; then					\
        echo "$$files" /dev/null						\
          | xargs							\
-             grep $$with_grep_options $(_ignore_case) -nE "$$prohibit"	\
-         | grep -vE "$${exclude:-^$$}"					\
+             $(GREP) $$with_grep_options $(_ignore_case) -nE "$$prohibit" \
+         | $(GREP) -vE "$${exclude:-^$$}"				\
          && { msg="$$halt" $(_sc_say_and_exit) }			\
          || :;								\
      else								\
        echo "$$files"							\
          | xargs							\
-             grep $$with_grep_options $(_ignore_case) -LE "$$require"	\
-         | grep .							\
+             $(GREP) $$with_grep_options $(_ignore_case) -LE "$$require" \
+         | $(GREP) .							\
          && { msg="$$halt" $(_sc_say_and_exit) }			\
          || :;								\
      fi									\
@@ -334,7 +334,7 @@ endef
 
 sc_avoid_if_before_free:
 	@$(VC_LIST_EXCEPT)						\
-	  | grep -v useless-if-before-free				\
+	  | $(GREP) -v useless-if-before-free				\
 	  | xargs							\
 	      $(srcdir)/$(_build-aux)/useless-if-before-free		\
 	      $(useless_free_options)					\
@@ -414,8 +414,8 @@ sc_error_exit_success:
 # "WARNING:" should be fully upper-cased, or fully lower-cased
 sc_error_message_warn_fatal:
 	@$(VC_LIST_EXCEPT)						\
-	  | xargs grep -nEA2 '[^rp]error *\(' /dev/null			\
-	  | grep -E '"Warning|"Fatal|"fatal'				\
+	  | xargs $(GREP) -nEA2 '[^rp]error *\(' /dev/null		\
+	  | $(GREP) -E '"Warning|"Fatal|"fatal'				\
 	  && { echo '$(ME): use FATAL, WARNING or warning' 1>&2;	\
 	       exit 1; }						\
 	  || :
@@ -423,9 +423,9 @@ sc_error_message_warn_fatal:
 # Error messages should not start with a capital letter
 sc_error_message_uppercase:
 	@$(VC_LIST_EXCEPT)						\
-	  | xargs grep -nEA2 '[^rp]error *\(' /dev/null			\
-	  | grep -E '"[A-Z]'						\
-	  | grep -vE '"FATAL|"WARNING|"Java|"C#|PRIuMAX'		\
+	  | xargs $(GREP) -nEA2 '[^rp]error *\(' /dev/null		\
+	  | $(GREP) -E '"[A-Z]'						\
+	  | $(GREP) -vE '"FATAL|"WARNING|"Java|"C#|PRIuMAX'		\
 	  && { echo '$(ME): found capitalized error message' 1>&2;	\
 	       exit 1; }						\
 	  || :
@@ -433,8 +433,8 @@ sc_error_message_uppercase:
 # Error messages should not end with a period
 sc_error_message_period:
 	@$(VC_LIST_EXCEPT)						\
-	  | xargs grep -nEA2 '[^rp]error *\(' /dev/null			\
-	  | grep -E '[^."]\."'						\
+	  | xargs $(GREP) -nEA2 '[^rp]error *\(' /dev/null		\
+	  | $(GREP) -E '[^."]\."'					\
 	  && { echo '$(ME): found error message ending in period' 1>&2;	\
 	       exit 1; }						\
 	  || :
@@ -479,8 +479,8 @@ perl_config_h_first_ =							\
 # You must include <config.h> before including any other header file.
 # This can possibly be via a package-specific header, if given by cfg.mk.
 sc_require_config_h_first:
-	@if $(VC_LIST_EXCEPT) | grep '\.c$$' > /dev/null; then		\
-	  files=$$($(VC_LIST_EXCEPT) | grep '\.c$$') &&			\
+	@if $(VC_LIST_EXCEPT) | $(GREP) '\.c$$' > /dev/null; then	\
+	  files=$$($(VC_LIST_EXCEPT) | $(GREP) '\.c$$') &&		\
 	  perl -n $(perl_config_h_first_) $$files ||			\
 	    { echo '$(ME): the above files include some other header'	\
 		'before <config.h>' 1>&2; exit 1; } || :;		\
@@ -498,10 +498,10 @@ sc_prohibit_HAVE_MBRTOWC:
 define _sc_header_without_use
   dummy=; : so we do not need a semicolon before each use;		\
   h_esc=`echo '[<"]'"$$h"'[">]'|$(SED) 's/\./\\\\./g'`;			\
-  if $(VC_LIST_EXCEPT) | grep '\.c$$' > /dev/null; then			\
-    files=$$(grep -l '^# *include '"$$h_esc"				\
-	     $$($(VC_LIST_EXCEPT) | grep '\.c$$')) &&			\
-    grep -LE "$$re" $$files | grep . &&					\
+  if $(VC_LIST_EXCEPT) | $(GREP) '\.c$$' > /dev/null; then		\
+    files=$$($(GREP) -l '^# *include '"$$h_esc"				\
+	     $$($(VC_LIST_EXCEPT) | $(GREP) '\.c$$')) &&		\
+    $(GREP) -LE "$$re" $$files | $(GREP) . &&				\
       { echo "$(ME): the above files include $$h but don't use it"	\
 	1>&2; exit 1; } || :;						\
   else :;								\
@@ -755,9 +755,9 @@ Exit_base := $(notdir $(Exit_witness_file))
 sc_require_test_exit_idiom:
 	@if test -f $(srcdir)/$(Exit_witness_file); then		\
 	  die=0;							\
-	  for i in $$(grep -l -F 'srcdir/$(Exit_base)'			\
+	  for i in $$($(GREP) -l -F 'srcdir/$(Exit_base)'		\
 		$$($(VC_LIST) tests)); do				\
-	    tail -n1 $$i | grep '^Exit .' > /dev/null			\
+	    tail -n1 $$i | $(GREP) '^Exit .' > /dev/null		\
 	      && : || { die=1; echo $$i; }				\
 	  done;								\
 	  test $$die = 1 &&						\
@@ -862,12 +862,12 @@ endef
 # Don't define macros that we already get from gnulib header files.
 sc_prohibit_always-defined_macros:
 	@if test -d $(gnulib_dir); then					\
-	  case $$(echo all: | grep -l -f - Makefile) in Makefile);; *)	\
+	  case $$(echo all: | $(GREP) -l -f - Makefile) in Makefile);; *) \
 	    echo '$(ME): skipping $@: you lack GNU grep' 1>&2; exit 0;;	\
 	  esac;								\
 	  regex=`$(def_sym_regex)`; export regex;			\
 	  $(VC_LIST_EXCEPT)						\
-	    | xargs sh -c 'echo $$regex | grep -E -f - "$$@"'		\
+	    | xargs sh -c 'echo $$regex | $(GREP) -E -f - "$$@"'	\
 	   	       dummy /dev/null					\
 	    && { printf '$(ME): define the above'			\
 			' via some gnulib .h file\n' 1>&2;		\
@@ -878,7 +878,7 @@ sc_prohibit_always-defined_macros:
 
 # Prohibit checked in backup files.
 sc_prohibit_backup_files:
-	@$(VC_LIST) | grep '~$$' &&				\
+	@$(VC_LIST) | $(GREP) '~$$' &&					\
 	  { echo '$(ME): found version controlled backup file' 1>&2;	\
 	    exit 1; } || :
 
@@ -1001,8 +1001,8 @@ ignore_doubled_word_match_RE_ ?= ^$$
 sc_prohibit_doubled_word:
 	@$(VC_LIST_EXCEPT)						\
 	  | xargs perl -n -0777 $(prohibit_doubled_word_)		\
-	  | grep -vE '$(ignore_doubled_word_match_RE_)'			\
-	  | grep .							\
+	  | $(GREP) -vE '$(ignore_doubled_word_match_RE_)'		\
+	  | $(GREP) .							\
 	  && { echo '$(ME): doubled words' 1>&2; exit 1; }		\
 	  || :
 
@@ -1030,8 +1030,8 @@ ignore_undesirable_word_sequence_RE_ ?= ^$$
 sc_prohibit_undesirable_word_seq:
 	@$(VC_LIST_EXCEPT)						\
 	  | xargs perl -n -0777 $(prohibit_undesirable_word_seq_)	\
-	  | grep -vE '$(ignore_undesirable_word_sequence_RE_)'		\
-	  | grep .							\
+	  | $(GREP) -vE '$(ignore_undesirable_word_sequence_RE_)'	\
+	  | $(GREP) .							\
 	  && { echo '$(ME): undesirable word sequence' >&2; exit 1; }   \
 	  || :
 
@@ -1066,7 +1066,7 @@ sc_prohibit_test_double_equal:
 # when it contains "ICONV".
 sc_proper_name_utf8_requires_ICONV:
 	@progs=$$($(VC_LIST_EXCEPT)					\
-		    | xargs grep -l 'proper_name_utf8 ''("');		\
+		    | xargs $(GREP) -l 'proper_name_utf8 ''("');	\
 	if test "x$$progs" != x; then					\
 	  fail=0;							\
 	  for p in $$progs; do						\
@@ -1075,7 +1075,7 @@ sc_proper_name_utf8_requires_ICONV:
 	      -ne 'exit !(/^LDADD =(.+?[^\\]\n)/ms && $$1 =~ /ICONV/)'	\
 	      $$dir/Makefile.am && continue;				\
 	    base=$$(basename "$$p" .c);					\
-	    grep "$${base}_LDADD.*ICONV)" $$dir/Makefile.am > /dev/null	\
+	    $(GREP) "$${base}_LDADD.*ICONV)" $$dir/Makefile.am > /dev/null	\
 	      || { fail=1; echo 1>&2 "$(ME): $$p uses proper_name_utf8"; }; \
 	  done;								\
 	  test $$fail = 1 &&						\
@@ -1136,12 +1136,12 @@ sc_makefile_at_at_check:
           -e ' && !/(\w+)\s+=.*\@\1\@$$/'				\
           -e ''$(_makefile_at_at_check_exceptions)			\
 	  -e 'and (print "$$ARGV:$$.: $$_"), $$m=1; END {exit !$$m}'	\
-	    $$($(VC_LIST_EXCEPT) | grep -E '(^|/)(Makefile\.am|[^/]+\.mk)$$') \
+	    $$($(VC_LIST_EXCEPT) | $(GREP) -E '(^|/)(Makefile\.am|[^/]+\.mk)$$') \
 	  && { echo '$(ME): use $$(...), not @...@' 1>&2; exit 1; } || :
 
 news-check: NEWS
 	$(AM_V_GEN)if $(SED) -n $(news-check-lines-spec)p $<		\
-	    | grep -E $(news-check-regexp) >/dev/null; then		\
+	    | $(GREP) -E $(news-check-regexp) >/dev/null; then		\
 	  :;								\
 	else								\
 	  echo 'NEWS: $$(news-check-regexp) failed to match' 1>&2;	\
@@ -1186,11 +1186,11 @@ generated_files ?= $(srcdir)/lib/*.[ch]
 _gl_translatable_string_re ?= \b(N?_|gettext *)\([^)"]*("|$$)
 sc_po_check:
 	@if test -f $(po_file); then					\
-	  grep -E -v '^(#|$$)' $(po_file)				\
-	    | grep -v '^src/false\.c$$' | sort > $@-1;			\
+	  $(GREP) -E -v '^(#|$$)' $(po_file)				\
+	    | $(GREP) -v '^src/false\.c$$' | sort > $@-1;		\
 	  { $(VC_LIST_EXCEPT); echo $(generated_files); }		\
 	    | xargs perl $(perl_translatable_files_list_)		\
-	    | xargs grep -E -l '$(_gl_translatable_string_re)'		\
+	    | xargs $(GREP) -E -l '$(_gl_translatable_string_re)'	\
 	    | $(SED) 's|^$(_dot_escaped_srcdir)/||'			\
 	    | sort -u > $@-2;						\
 	  diff -u -L $(po_file) -L $(po_file) $@-1 $@-2			\
@@ -1256,17 +1256,17 @@ _hv_regex_weak ?= ^ *\. .*/init\.sh"
 _hv_regex_strong ?= ^ *\. "\$${srcdir=\.}/init\.sh"
 sc_cross_check_PATH_usage_in_tests:
 	@if test -f $(_hv_file); then					\
-	  grep -l 'VERSION mismatch' $(_hv_file) >/dev/null		\
+	  $(GREP) -l 'VERSION mismatch' $(_hv_file) >/dev/null		\
 	    || { echo "$@: skipped: no such file: $(_hv_file)" 1>&2;	\
 		 exit 0; };						\
-	  grep -lE '$(_hv_regex_strong)' $(_hv_file) >/dev/null		\
+	  $(GREP) -lE '$(_hv_regex_strong)' $(_hv_file) >/dev/null	\
 	    || { echo "$@: $(_hv_file) lacks conforming use of init.sh" 1>&2; \
 		 exit 1; };						\
-	  good=$$(grep -E '$(_hv_regex_strong)' $(_hv_file));		\
+	  good=$$($(GREP) -E '$(_hv_regex_strong)' $(_hv_file));	\
 	  $(VC_LIST_EXCEPT)						\
-	    | xargs grep -lE '$(_hv_regex_weak)'			\
-	    | xargs grep -LFx "$$good"					\
-	    | grep .							\
+	    | xargs $(GREP) -lE '$(_hv_regex_weak)'			\
+	    | xargs $(GREP) -LFx "$$good"				\
+	    | $(GREP) .							\
 	    && { printf "$(ME): the above files use"			\
 			" path_prepend_ inconsistently\n" 1>&2;		\
 		 exit 1; }						\
@@ -1463,7 +1463,7 @@ check: $(gl_public_submodule_commit)
 ALL_RECURSIVE_TARGETS += alpha beta stable
 alpha beta stable: $(local-check) writable-files $(submodule-checks)
 	$(AM_V_GEN)test $@ = stable					\
-	  && { echo $(VERSION) | grep -E '^[0-9]+(\.[0-9]+)+$$'		\
+	  && { echo $(VERSION) | $(GREP) -E '^[0-9]+(\.[0-9]+)+$$'	\
 	       || { echo "invalid version string: $(VERSION)" 1>&2; exit 1;};}\
 	  || :
 	$(AM_V_at)$(MAKE) vc-diff-check
@@ -1561,7 +1561,7 @@ refresh-gnulib-patches:
 	       -e 'END{defined $$d and print $$d}' bootstrap.conf);	\
 	  test -n "$$t" && gl=$$t;					\
 	fi;								\
-	for diff in $$(cd $$gl; git ls-files | grep '\.diff$$'); do	\
+	for diff in $$(cd $$gl; git ls-files | $(GREP) '\.diff$$'); do	\
 	  b=$$(printf %s "$$diff"|$(SED) 's/\.diff$$//');		\
 	  VERSION_CONTROL=none						\
 	    patch "$(gnulib_dir)/$$b" "$$gl/$$diff" || exit 1;		\
@@ -1604,7 +1604,7 @@ update-copyright-env ?=
 # in the file .x-update-copyright.
 .PHONY: update-copyright
 update-copyright:
-	$(AM_V_GEN)grep -l -w Copyright                                  \
+	$(AM_V_GEN)$(GREP) -l -w Copyright                               \
 	  $$(export VC_LIST_EXCEPT_DEFAULT=COPYING && $(VC_LIST_EXCEPT)) \
 	  | $(update-copyright-env) xargs $(srcdir)/$(_build-aux)/$@
 
@@ -1618,9 +1618,9 @@ _gl_TS_dir ?= src
 ALL_RECURSIVE_TARGETS += sc_tight_scope
 sc_tight_scope: tight-scope.mk
 	@fail=0;							\
-	if ! grep '^ *export _gl_TS_headers *=' $(srcdir)/cfg.mk	\
+	if ! $(GREP) '^ *export _gl_TS_headers *=' $(srcdir)/cfg.mk	\
 		> /dev/null						\
-	   && ! grep -w noinst_HEADERS $(srcdir)/$(_gl_TS_dir)/Makefile.am \
+	   && ! $(GREP) -w noinst_HEADERS $(srcdir)/$(_gl_TS_dir)/Makefile.am \
 		> /dev/null 2>&1; then					\
 	    echo '$(ME): skipping $@';					\
 	else								\
@@ -1692,12 +1692,12 @@ _gl_tight_scope: $(bin_PROGRAMS)
 	hdr=`for f in $(_gl_TS_headers); do				\
 	       test -f $$f && d= || d=$(srcdir)/; echo $$d$$f; done`;	\
 	( printf '%s\n' '__.*' $(_gl_TS_unmarked_extern_functions);	\
-	  grep -h -A1 '^extern .*[^;]$$' $$src				\
-	    | grep -vE '^(extern |--|#)' | $(SED) 's/ .*//; /^$$/d';	\
+	  $(GREP) -h -A1 '^extern .*[^;]$$' $$src			\
+	    | $(GREP) -vE '^(extern |--|#)' | $(SED) 's/ .*//; /^$$/d';	\
 	  perl -lne							\
 	     '$(_gl_TS_function_match) and print $$1' $$hdr;		\
 	) | sort -u | $(SED) "$$sed_wrap" > $$t;			\
-	nm -g $(_gl_TS_obj_files)|$(SED) -n 's/.* T //p'|grep -Ev -f $$t \
+	nm -g $(_gl_TS_obj_files)|$(SED) -n 's/.* T //p'|$(GREP) -Ev -f $$t \
 	  && { echo the above functions should have static scope >&2;	\
 	       exit 1; } || : ;						\
 	( printf '%s\n' '__.*' main $(_gl_TS_unmarked_extern_vars);	\
@@ -1705,7 +1705,7 @@ _gl_tight_scope: $(bin_PROGRAMS)
 		$$hdr $(_gl_TS_other_headers)				\
 	) | sort -u | $(SED) "$$sed_wrap" > $$t;			\
 	nm -g $(_gl_TS_obj_files) | $(SED) -n 's/.* [BCDGRS] //p'	\
-            | sort -u | grep -Ev -f $$t					\
+            | sort -u | $(GREP) -Ev -f $$t				\
 	  && { echo the above variables should have static scope >&2;	\
 	       exit 1; } || :
 # TS-end
-- 
2.17.2 (Apple Git-113)



^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH v3 0/2] Fix syntax-check on macOS/FreeBSD
  2018-12-13 15:34 [PATCH v3 0/2] Fix syntax-check on macOS/FreeBSD Roman Bolshakov
  2018-12-13 15:34 ` [PATCH v3 1/2] maint.mk: Split long argument lists Roman Bolshakov
  2018-12-13 15:34 ` [PATCH v3 2/2] maint.mk: Replace grep with $(GREP) Roman Bolshakov
@ 2018-12-20 13:06 ` Roman Bolshakov
  2019-01-02 20:14   ` Eric Blake
  2 siblings, 1 reply; 9+ messages in thread
From: Roman Bolshakov @ 2018-12-20 13:06 UTC (permalink / raw)
  To: Bruno Haible, Eric Blake; +Cc: bug-gnulib

On Thu, Dec 13, 2018 at 06:34:51PM +0300, Roman Bolshakov wrote:
> Hello,
> 
> There was an issue with syntax-check on FreeBSD reported a few years
> ago:
> https://www.redhat.com/archives/libvir-list/2015-August/msg00758.html
> http://lists.gnu.org/archive/html/bug-gnulib/2015-08/msg00019.html
> 
> The patch series attempts to resolve the issue on gnulib side. With
> related changes on libvirt side I can run make syntax-check on macOS.
> 

Hi Bruno and Eric,

what do you think of the patchset?

Thanks,
Roman


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v3 1/2] maint.mk: Split long argument lists
  2018-12-13 15:34 ` [PATCH v3 1/2] maint.mk: Split long argument lists Roman Bolshakov
@ 2019-01-02 19:31   ` Eric Blake
  0 siblings, 0 replies; 9+ messages in thread
From: Eric Blake @ 2019-01-02 19:31 UTC (permalink / raw)
  To: Roman Bolshakov, bug-gnulib


[-- Attachment #1.1: Type: text/plain, Size: 5876 bytes --]

On 12/13/18 9:34 AM, Roman Bolshakov wrote:
> $(VC_LIST_EXCEPT) is usually expanded into arguments for a command.
> When a project contains too many, some operating systems can't pass all
> the arguments because they hit the limit of arguments. FreeBSD and macOS
> are known to have the limit of 256k of arguments.
> 
> More on the issue:
> http://lists.gnu.org/archive/html/bug-gnulib/2015-08/msg00019.html
> https://www.redhat.com/archives/libvir-list/2015-August/msg00758.html
> 
> xargs without flags can be used to limit number of arguments.

This part is fine.

> The
> default number of arguments (max-args for "-n" flag) is
> platform-specific. If argument length exceeds default value for "-s"
> flag (max-chars), xargs will feed less arguments than max-args.

This was relevant to earlier versions, but is now just fluff; I'd drop
it.  What's more, you didn't mention that we are relying on 'echo
$(long_list) | xargs' working because the shell's built-in echo does not
use exec and can therefore process a larger list than what exec
enforces.  I don't mind tweaking that on commit.

> 
> Signed-off-by: Roman Bolshakov <r.bolshakov@yadro.com>
> ---
>  top/maint.mk | 135 ++++++++++++++++++++++++++++++++-------------------
>  1 file changed, 86 insertions(+), 49 deletions(-)
> 
> diff --git a/top/maint.mk b/top/maint.mk
> index 4889ebacc..1152ad698 100644
> --- a/top/maint.mk
> +++ b/top/maint.mk
> @@ -302,32 +302,46 @@ define _sc_search_regexp
>     fi;									\
>  									\
>     : Filter by content;							\
> -   test -n "$$files" && test -n "$$containing"				\
> -     && { files=$$(grep -l "$$containing" $$files); } || :;		\
> -   test -n "$$files" && test -n "$$non_containing"			\
> -     && { files=$$(grep -vl "$$non_containing" $$files); } || :;	\
> +   test -n "$$files"							\
> +     && test -n "$$containing"						\

It feels like extra churn to have split this line with no change to its
content, but I also don't mind keeping it split.

> +     && { files=$$(echo "$$files" | xargs grep -l "$$containing"); }	\
> +     || :;								\
> +   test -n "$$files"							\
> +     && test -n "$$non_containing"					\
> +     && { files=$$(echo "$$files" | xargs grep -vl "$$non_containing"); } \
> +     || :;								\
>  									\
>     : Check for the construct;						\
>     if test -n "$$files"; then						\
>       if test -n "$$prohibit"; then					\
> -       grep $$with_grep_options $(_ignore_case) -nE "$$prohibit" $$files \
> +       echo "$$files" /dev/null						\
> +         | xargs							\
> +             grep $$with_grep_options $(_ignore_case) -nE "$$prohibit"	\

Not quite canonical. That can cause xargs to invoke:
grep a b c
grep /dev/null

instead of the more typical:

grep /dev/null a b
grep /dev/null c

Remember, 'grep one' output differs from 'grep one two' - so the GOAL is
to ensure that grep always has at least two file, by always including
/dev/null in the list of arguments to EVERY grep invocation, rather than
only to the LAST grep invocation.

But, that said, your formulation "works" in the sense that it is
unlikely that any one single filename would push beyond exec limits, and
therefore xargs will either make a single call (no difference); or split
things so that the first call has more than one file (no /dev/null, but
since there are multiple files things work) and the second call has just
/dev/null (which will have no output, so it won't matter that grep was
called with one file instead of multiple); or split things so that the
first has more than one file, and the second has more than one file
(including /dev/null).

I'm still inclined to touch this up to use the canonical formulation:

'echo $(long_list) | xargs grep /dev/null'

rather than your formulation:

'echo $(long_list) /dev/null | xargs grep'

just so future readers don't have to revisit why yours happens to work.

>           | grep -vE "$${exclude:-^$$}"					\
> -         && { msg="$$halt" $(_sc_say_and_exit) } || :;			\
> +         && { msg="$$halt" $(_sc_say_and_exit) }			\
> +         || :;								\

Again, this change is just churn.  I don't know that I would have done
it, but I'm also not in the mood to undo your effort.

> @@ -399,25 +413,31 @@ sc_error_exit_success:
>  # "FATAL:" should be fully upper-cased in error messages
>  # "WARNING:" should be fully upper-cased, or fully lower-cased
>  sc_error_message_warn_fatal:
> -	@grep -nEA2 '[^rp]error *\(' $$($(VC_LIST_EXCEPT))		\
> -	    | grep -E '"Warning|"Fatal|"fatal' &&			\
> -	  { echo '$(ME): use FATAL, WARNING or warning'	1>&2;		\
> -	    exit 1; } || :
> +	@$(VC_LIST_EXCEPT)						\
> +	  | xargs grep -nEA2 '[^rp]error *\(' /dev/null			\

For comparison to above, this conversion was canonical.

> @@ -845,9 +865,14 @@ sc_prohibit_always-defined_macros:
>  	  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 -E -f - $$($(VC_LIST_EXCEPT))		\
> -	    && { echo '$(ME): define the above via some gnulib .h file'	\
> -		  1>&2;  exit 1; } || :;				\
> +	  regex=`$(def_sym_regex)`; export regex;			\

Using `` is odd, especially since...


> @@ -1033,7 +1065,8 @@ sc_prohibit_test_double_equal:
>  # definition of LDADD from the appropriate Makefile.am and exits 0
>  # when it contains "ICONV".
>  sc_proper_name_utf8_requires_ICONV:
> -	@progs=$$(grep -l 'proper_name_utf8 ''("' $$($(VC_LIST_EXCEPT)));\
> +	@progs=$$($(VC_LIST_EXCEPT)					\

...we are using $$() elsewhere.

I don't mind making those changes, if it will let me push today.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v3 0/2] Fix syntax-check on macOS/FreeBSD
  2018-12-20 13:06 ` [PATCH v3 0/2] Fix syntax-check on macOS/FreeBSD Roman Bolshakov
@ 2019-01-02 20:14   ` Eric Blake
  2019-01-03 14:44     ` Eric Blake
  2019-01-06 21:36     ` Bruno Haible
  0 siblings, 2 replies; 9+ messages in thread
From: Eric Blake @ 2019-01-02 20:14 UTC (permalink / raw)
  To: Roman Bolshakov, Bruno Haible; +Cc: bug-gnulib


[-- Attachment #1.1: Type: text/plain, Size: 1096 bytes --]

On 12/20/18 7:06 AM, Roman Bolshakov wrote:
> On Thu, Dec 13, 2018 at 06:34:51PM +0300, Roman Bolshakov wrote:
>> Hello,
>>
>> There was an issue with syntax-check on FreeBSD reported a few years
>> ago:
>> https://www.redhat.com/archives/libvir-list/2015-August/msg00758.html
>> http://lists.gnu.org/archive/html/bug-gnulib/2015-08/msg00019.html
>>
>> The patch series attempts to resolve the issue on gnulib side. With
>> related changes on libvirt side I can run make syntax-check on macOS.
>>
> 
> Hi Bruno and Eric,
> 
> what do you think of the patchset?

Your change is larger than 10 lines, but is mechanical enough that I
felt that it qualified under the tiny change rules without you having
copyright assignment on file.  But if you want to make further gnulib
contributions, completing the copyright assignment is probably worth doing.

Sorry for the holiday delay. I've now applied the patches, with tweaks
as discussed on 1/2.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v3 0/2] Fix syntax-check on macOS/FreeBSD
  2019-01-02 20:14   ` Eric Blake
@ 2019-01-03 14:44     ` Eric Blake
  2019-01-03 15:28       ` Eric Blake
  2019-01-06 21:36     ` Bruno Haible
  1 sibling, 1 reply; 9+ messages in thread
From: Eric Blake @ 2019-01-03 14:44 UTC (permalink / raw)
  To: Roman Bolshakov, Bruno Haible; +Cc: bug-gnulib


[-- Attachment #1.1: Type: text/plain, Size: 1277 bytes --]

On 1/2/19 2:14 PM, Eric Blake wrote:
> On 12/20/18 7:06 AM, Roman Bolshakov wrote:
>> On Thu, Dec 13, 2018 at 06:34:51PM +0300, Roman Bolshakov wrote:
>>> Hello,
>>>
>>> There was an issue with syntax-check on FreeBSD reported a few years
>>> ago:
>>> https://www.redhat.com/archives/libvir-list/2015-August/msg00758.html
>>> http://lists.gnu.org/archive/html/bug-gnulib/2015-08/msg00019.html
>>>
>>> The patch series attempts to resolve the issue on gnulib side. With
>>> related changes on libvirt side I can run make syntax-check on macOS.
>>>
>>
>> Hi Bruno and Eric,
>>
>> what do you think of the patchset?
> 
> Your change is larger than 10 lines, but is mechanical enough that I
> felt that it qualified under the tiny change rules without you having
> copyright assignment on file.  But if you want to make further gnulib
> contributions, completing the copyright assignment is probably worth doing.
> 
> Sorry for the holiday delay. I've now applied the patches, with tweaks
> as discussed on 1/2.

And I botched something in _sc_search_regexp. Will push the followup
patch as soon as I diagnose the issue.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v3 0/2] Fix syntax-check on macOS/FreeBSD
  2019-01-03 14:44     ` Eric Blake
@ 2019-01-03 15:28       ` Eric Blake
  0 siblings, 0 replies; 9+ messages in thread
From: Eric Blake @ 2019-01-03 15:28 UTC (permalink / raw)
  To: Roman Bolshakov, Bruno Haible; +Cc: bug-gnulib


[-- Attachment #1.1: Type: text/plain, Size: 732 bytes --]

On 1/3/19 8:44 AM, Eric Blake wrote:
> And I botched something in _sc_search_regexp. Will push the followup
> patch as soon as I diagnose the issue.
> 

D'oh, I killed an important backslash:


+++ b/top/maint.mk
@@ -314,7 +314,7 @@ define _sc_search_regexp
    : Check for the construct;						\
    if test -n "$$files"; then						\
      if test -n "$$prohibit"; then					\
-       echo "$$files"
+       echo "$$files"							\
          | xargs $(GREP) $$with_grep_options $(_ignore_case) -nE	\
 		"$$prohibit" /dev/null					\
          | $(GREP) -vE "$${exclude:-^$$}"				\



-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v3 0/2] Fix syntax-check on macOS/FreeBSD
  2019-01-02 20:14   ` Eric Blake
  2019-01-03 14:44     ` Eric Blake
@ 2019-01-06 21:36     ` Bruno Haible
  1 sibling, 0 replies; 9+ messages in thread
From: Bruno Haible @ 2019-01-06 21:36 UTC (permalink / raw)
  To: Eric Blake; +Cc: Roman Bolshakov, bug-gnulib

Eric Blake wrote:
> I've now applied the patches, with tweaks as discussed on 1/2.

In GNU gettext, I now see this message when using the top-level GNUmakefile:

/bin/bash: rsyncable: command not found

The reason is that GNU gettext imports the relevant files directly (via
'gnulib-tool --copy-file'), ignoring the module description. This is a
reasonable thing to do, since GNU gettext does not ship GNUmakefile nor
maint.mk in the tarballs; it uses them only in the git checkouts.

This patch fixes it for me.


2019-01-06  Bruno Haible  <bruno@clisp.org>

	maintainer-makefile: Make the configure.ac section optional.
	* top/maint.mk (GREP, SED): Define if not defined.

diff --git a/top/maint.mk b/top/maint.mk
index 4b57410..4e37efe 100644
--- a/top/maint.mk
+++ b/top/maint.mk
@@ -21,6 +21,12 @@
 # ME := $(word $(words $(MAKEFILE_LIST)),$(MAKEFILE_LIST))
 ME := maint.mk
 
+# These variables ought to be defined through the configure.ac section
+# of the module description. But some packages import this file directly,
+# ignoring the module description.
+GREP ?= grep
+SED ?= sed
+
 # Helper variables.
 _empty =
 _sp = $(_empty) $(_empty)



^ permalink raw reply related	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2019-01-06 21:36 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-12-13 15:34 [PATCH v3 0/2] Fix syntax-check on macOS/FreeBSD Roman Bolshakov
2018-12-13 15:34 ` [PATCH v3 1/2] maint.mk: Split long argument lists Roman Bolshakov
2019-01-02 19:31   ` Eric Blake
2018-12-13 15:34 ` [PATCH v3 2/2] maint.mk: Replace grep with $(GREP) Roman Bolshakov
2018-12-20 13:06 ` [PATCH v3 0/2] Fix syntax-check on macOS/FreeBSD Roman Bolshakov
2019-01-02 20:14   ` Eric Blake
2019-01-03 14:44     ` Eric Blake
2019-01-03 15:28       ` Eric Blake
2019-01-06 21:36     ` Bruno Haible

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).