Re: [PATCH 3/7] tests: migrate tests/sh.test checks to syntax-checks.

2011-11-22 Thread Stefano Lattarini
On Tuesday 22 November 2011, Gary V wrote:
> Hi Stefano,
> 
> On 22 Nov 2011, at 02:52, Stefano Lattarini wrote:
> > Hi Gary.  Just a quick nit (I haven't looked at the whole
> > series, and not even at the whole patch in fact; sorry).
> 
> No apologies necessary, every little helps!  Thank you.
> 
> > On Monday 21 November 2011, Gary V wrote:
> >>for file
> >> do
> >> -  test -f $file || touch $file
> >> +  test -f "$file" || touch $file
> >> 
> > What's the point of quoting file after `test -f' it it remains
> > unquoted after `touch'?
> 
> Even though we know there is no whitespace in $file because of the
> for loop, there still might be other shell meta-characters in there.
> All uses of $file (including a bunch in the following lines) should
> be quoted correctly,
>
OK, good.

> but that is another patch.
> 
That's perfectly fine, but IMHO it should me mentioned in the commit
message.

Regards,
  Stefano



Re: [PATCH 3/7] tests: migrate tests/sh.test checks to syntax-checks.

2011-11-21 Thread Gary V. Vaughan
Hi Stefano,

On 22 Nov 2011, at 02:52, Stefano Lattarini wrote:
> Hi Gary.  Just a quick nit (I haven't looked at the whole
> series, and not even at the whole patch in fact; sorry).

No apologies necessary, every little helps!  Thank you.

> On Monday 21 November 2011, Gary V wrote:
>>for file
>> do
>> -  test -f $file || touch $file
>> +  test -f "$file" || touch $file
>> 
> What's the point of quoting file after `test -f' it it remains
> unquoted after `touch'?

Even though we know there is no whitespace in $file because of the
for loop, there still might be other shell meta-characters in there.
All uses of $file (including a bunch in the following lines) should
be quoted correctly, but that is another patch.

Cheers,
-- 
Gary V. Vaughan (gary AT gnu DOT org)



Re: [PATCH 3/7] tests: migrate tests/sh.test checks to syntax-checks.

2011-11-21 Thread Stefano Lattarini
Hi Gary.  Just a quick nit (I haven't looked at the whole
series, and not even at the whole patch in fact; sorry).

On Monday 21 November 2011, Gary V wrote:
> for file
>  do
> -  test -f $file || touch $file
> +  test -f "$file" || touch $file
>  
What's the point of quoting file after `test -f' it it remains
unquoted after `touch'?

Regards,
  Stefano



[PATCH 3/7] tests: migrate tests/sh.test checks to syntax-checks.

2011-11-21 Thread Gary V. Vaughan
Some modernization of the legacy testsuite.
* tests/sh.test: Remove.
* Makefile.am (COMMON_TESTS): Adjust.
* cfg.mk (sc_libtool_m4_cc_basename, sc_prohibit_bracket_as_test)
(sc_prohibit_nested_quotes, sc_prohibit_set_dummy_without_shift)
(sc_prohibit_set_minus_minus, sc_prohibit_test_binary_operators)
(sc_prohibit_test_dollar, sc_prohibit_test_minus_e)
(sc_prohibit_test_unary_operators, sc_prohibit_test_X)
(sc_prohibit_Xsed_withou_X, sc_require_function_nl_brace):
Functionally identical tests to what used to be performed by
sh.test, only with coverage of all files.
* bootstrap, build-aux/edit-readme-alpha,
build-aux/extract-trace, build-aux/getopt.m4sh,
build-aux/ltmain.m4sh, configure.ac, m4/libtool.m4, m4/ltdl.m4,
tests/bindir.at, tests/configure-iface.at, tests/cwrapper.at,
tests/darwin.at, tests/defs.m4sh, tests/demo-hardcode.test,
tests/dlloader-api.at, tests/exceptions.at,
tests/getopt-m4sh.at, tests/lalib-syntax.at, tests/link-2.test,
tests/link-order2.at, tests/loadlibrary.at,
tests/lt_dladvise.at, tests/lt_dlexit.at, tests/lt_dlopen_a.at,
tests/lt_dlopenext.at, tests/need_lib_prefix.at,
tests/nonrecursive.at, tests/recursive.at, tests/resident.at,
tests/standalone.at, tests/static.at, tests/stresstest.at,
tests/subproject.at, tests/sysroot.at, tests/tagtrace.test,
tests/testsuite.at: Fix violations of the new syntax checks.

Signed-off-by: Gary V. Vaughan 
---
 Makefile.am |3 +-
 bootstrap   |   21 ---
 build-aux/edit-readme-alpha |4 +-
 build-aux/extract-trace |6 +-
 build-aux/getopt.m4sh   |2 +-
 build-aux/ltmain.m4sh   |2 +-
 cfg.mk  |  104 +
 configure.ac|2 +-
 m4/libtool.m4   |   50 
 m4/ltdl.m4  |2 +-
 tests/bindir.at |   10 ++--
 tests/configure-iface.at|8 +-
 tests/cwrapper.at   |2 +-
 tests/darwin.at |2 +-
 tests/defs.m4sh |4 +-
 tests/demo-hardcode.test|6 +-
 tests/dlloader-api.at   |2 +-
 tests/exceptions.at |2 +-
 tests/getopt-m4sh.at|2 +-
 tests/lalib-syntax.at   |2 +-
 tests/link-2.test   |2 +-
 tests/link-order2.at|8 +-
 tests/loadlibrary.at|2 +-
 tests/lt_dladvise.at|4 +-
 tests/lt_dlexit.at  |2 +-
 tests/lt_dlopen_a.at|2 +-
 tests/lt_dlopenext.at   |4 +-
 tests/need_lib_prefix.at|2 +-
 tests/nonrecursive.at   |4 +-
 tests/recursive.at  |4 +-
 tests/resident.at   |2 +-
 tests/sh.test   |  134 ---
 tests/standalone.at |4 +-
 tests/static.at |2 +-
 tests/stresstest.at |2 +-
 tests/subproject.at |4 +-
 tests/sysroot.at|4 +-
 tests/tagtrace.test |4 +-
 tests/testsuite.at  |4 +-
 39 files changed, 200 insertions(+), 230 deletions(-)
 delete mode 100755 tests/sh.test

diff --git a/Makefile.am b/Makefile.am
index 2c6cf81..31d286e 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -557,7 +557,7 @@ changelog   = $(distdir)/ChangeLog
 # date is updated to the following year.
 changelog_start_date = 2011-01-01
 $(changelog): FORCE
-   $(AM_V_GEN)if test -d $(srcdir)/.git; then \
+   $(AM_V_GEN)if test -d '$(srcdir)/.git'; then \
  $(gitlog_to_changelog) --amend=$(git_log_fix) \
  --since=$(changelog_start_date) > '$@T'; \
  rm -f '$@'; mv '$@T' '$@'; \
@@ -874,7 +874,6 @@ COMMON_TESTS = \
tests/nomode.test \
tests/objectlist.test \
tests/quote.test \
-   tests/sh.test \
tests/suffix.test \
tests/tagtrace.test \
tests/cdemo-static.test \
diff --git a/bootstrap b/bootstrap
index 9a9e94b..4dbdf72 100755
--- a/bootstrap
+++ b/bootstrap
@@ -738,20 +738,20 @@ func_clean_unused_macros ()
 
   # We use `ls|grep' instead of `ls *.m4' to avoid exceeding
   # command line length limits in some shells.
-  for file in `cd $macro_dir && ls -1 |grep '\.m4$'`; do
+  for file in `cd "$macro_dir" && ls -1 |grep '\.m4$'`; do
 
# Remove a macro file when aclocal.m4 does not m4_include it...
 func_grep_q 'm4_include([[]'$macro_dir/$file'])' $aclocal_m4s \
-|| test ! -f $gnulib_path/m4/$file || {
+|| test ! -f "$gnulib_path/m4/$file" || {
 
   # ...and there is an identical file in gnulib...
-  if func_cmp_s $gnulib_path/m4/$file $macro_dir/$file; then
+  if func_cmp_s "$gnulib_path/m4/$file" "$macro_dir/$file"; then
 
 # ...and it's not in the precious list (`echo' is needed
 # here to squash whitespace for the match expression).
 case " "`echo $gnulib_precious`" " in
   *" $file "*) ;;
-  *) rm -f $ma