automake-patches
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH 01/10] Add new tests on strictness and warnings precedence an


From: Stefano Lattarini
Subject: Re: [PATCH 01/10] Add new tests on strictness and warnings precedence and overriding.
Date: Sun, 2 Jan 2011 16:31:18 +0100
User-agent: KMail/1.13.3 (Linux/2.6.30-2-686; KDE/4.4.4; i686; ; )

On Sunday 02 January 2011, Ralf Wildenhues wrote:
> Hi Stefano,
> 
> I'm not sure if I'll get through the whole series today, but I gotta
> start somewhere, so here we go.  Feel free to push the patch series
> (as far as OKed) on a new branch based off of maint,
>
Hmmm.. currently my private branch is based off of master, since I
thought it would be wrong to apply such slightly backward-incompatible
changes to maint.  Rebasing on maint would require some modifications
to the patches in this series, and/or preliminary changes to maint
(which still lacks definition of e.g. `$original_AUTOMAKE' in the
testsuite).

Would be OK with you if I'd just push to a new public branch off of
master?

> if that is helpful for you.  Thanks.
> 
> * address@hidden wrote on Thu, Dec 23, 2010 at 12:27:37PM CET:
> > --- /dev/null
> > +++ b/tests/strictness-overriding.test
> 
> How about 'strictness-override.test'?
> (I'll explain in another mail why I'm keen on not too long names.)
>
I generally prefer long and clear to short and confusing.  But in this
case the name you're proposing is as clear as the one proposed by me,
so the rename is fine with me.

Similarly, I renamed warnings-overriding.test to warnings-override.test.

> > @@ -0,0 +1,116 @@
> > +#! /bin/sh
> > +# Copyright (C) 2010 Free Software Foundation, Inc.
> 
> 2011.  Sorry!
>
Fixed (here and in the other three new tests too).

I also ran "update-copyright" on tests/Makefile.am.

> > +# The strictness specified in Makefile.am:AUTOMAKE_OPTIONS should
> > +# override that specified in configure.in:AM_INIT_AUTOMAKE, and both
> > +# should override the strictness specified on the command line.
> > +# NOTE: this current semantic might not be the best one (even if it has
> > +# been in place for quite a long time); see also Automake bug #7673.
> 
> I don't think it is grammatically correct to use 'semantic' as a noun in
> singular form, similar to how 'information' works; 'semantic' is only
> ever used as adjective or adverb AFAIK.  Here, "the current semantics"
> would be fine.
> 
> > +# Update this test if the semantic is changed.
> 
> Likewise: "the current semantics are"
>
Both fixed (here and in warnings-override.test too).  Thanks.

> > +. ./defs || Exit 1
> > +
> > +set -e
> > +
> > +# We want complete control over automake options.
> > +AUTOMAKE=$original_AUTOMAKE
> > +
> > +cat > Makefile.am <<'END'
> > +AUTOMAKE_OPTIONS =
> > +END
> > +
> > +set_strictness()
> 
> Space before open parenthesis; several instances.
>
Oops, I should have known better than this by now.  Fixed here and in
the other three new test scripts.

> > +{
> > +  set +x
> 
> Curious: why turn off tracing here?
>
Because, while I was testing and debugging the test cases, those traces
turned out to be useless and a little confusing; it was much more useful
and clear to just display the contents of the edited file.  And once I
was done with debugging, I saw no reason to revert this temporary
disabling of shell traces.

> > +  sed <$2 >$2-t -e "s|^\\(AUTOMAKE_OPTIONS\\) *=.*|\\1 = $1|" \
> > +                -e "s|^\\(AM_INIT_AUTOMAKE\\).*|\\1([$1])|"
> > +  mv -f $2-t $2
> > +  set -x
> > +  cat $2
> 
> To avoid caching please 'rm -rf autom4te.cache' here.
>
OK, done here and in warnings-overide.test too.

Well, with two differences:
 - I've used 'rm -rf autom4te*.cache', for consistency with what is
   used and suggested in other tests (e.g., asm.test).
 - I've added various 'rm -rf autom4te*.cache' calls in the main code,
   not a single one in set_strictness() and/or set_wanings().

> > +}
> > +
> > +ok()
> > +{
> > +  $AUTOMAKE -Werror $*
> > +}
> > +
> > +ko()
> > +{
> > +  AUTOMAKE_fails $*
> > +  grep 'required file.*README' stderr
> > +}
> > +
> > +$ACLOCAL
> > +
> > +# Leave out only one of the required files, to avoid too much
> > +# repetitions in the error messages.
> 
> repetition
>
Fixed (here and in strictness-precedence too).

> > +touch INSTALL NEWS AUTHORS ChangeLog COPYING
> 
> Most of the above comments apply to several tests in this patch.
> 
> Thank you,
> Ralf
> 

Attached is what I've squashed in.

Thanks,
  Stefano
diff --git a/tests/Makefile.am b/tests/Makefile.am
index cd98e91..e155340 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -1,7 +1,7 @@
 ## Process this file with automake to create Makefile.in
 
 # Copyright (C) 1996, 1997, 1998, 1999, 2001, 2002, 2003, 2004, 2005,
-# 2006, 2007, 2008, 2009, 2010  Free Software Foundation, Inc.
+# 2006, 2007, 2008, 2009, 2010, 2011 Free Software Foundation, Inc.
 
 # This program is free software; you can redistribute it and/or modify
 # it under the terms of the GNU General Public License as published by
@@ -804,7 +804,7 @@ stdinc.test \
 stamph2.test \
 stdlib.test \
 stdlib2.test \
-strictness-overriding.test \
+strictness-override.test \
 strictness-precedence.test \
 strip.test \
 strip2.test \
@@ -931,7 +931,7 @@ version8.test \
 vpath.test \
 vtexi.test \
 vtexi2.test \
-warnings-overriding.test \
+warnings-override.test \
 warnings-precedence.test \
 warnopts.test \
 werror.test \
diff --git a/tests/Makefile.in b/tests/Makefile.in
index 1656962..0c0180c 100644
--- a/tests/Makefile.in
+++ b/tests/Makefile.in
@@ -16,7 +16,7 @@
 @SET_MAKE@
 
 # Copyright (C) 1996, 1997, 1998, 1999, 2001, 2002, 2003, 2004, 2005,
-# 2006, 2007, 2008, 2009, 2010  Free Software Foundation, Inc.
+# 2006, 2007, 2008, 2009, 2010, 2011 Free Software Foundation, Inc.
 
 # This program is free software; you can redistribute it and/or modify
 # it under the terms of the GNU General Public License as published by
@@ -1067,7 +1067,7 @@ stdinc.test \
 stamph2.test \
 stdlib.test \
 stdlib2.test \
-strictness-overriding.test \
+strictness-override.test \
 strictness-precedence.test \
 strip.test \
 strip2.test \
@@ -1194,7 +1194,7 @@ version8.test \
 vpath.test \
 vtexi.test \
 vtexi2.test \
-warnings-overriding.test \
+warnings-override.test \
 warnings-precedence.test \
 warnopts.test \
 werror.test \
diff --git a/tests/strictness-overriding.test b/tests/strictness-override.test
similarity index 83%
rename from tests/strictness-overriding.test
rename to tests/strictness-override.test
index 4c24ede..35f1763 100755
--- a/tests/strictness-overriding.test
+++ b/tests/strictness-override.test
@@ -1,5 +1,5 @@
 #! /bin/sh
-# Copyright (C) 2010 Free Software Foundation, Inc.
+# Copyright (C) 2011 Free Software Foundation, Inc.
 #
 # This program is free software; you can redistribute it and/or modify
 # it under the terms of the GNU General Public License as published by
@@ -17,9 +17,9 @@
 # The strictness specified in Makefile.am:AUTOMAKE_OPTIONS should
 # override that specified in configure.in:AM_INIT_AUTOMAKE, and both
 # should override the strictness specified on the command line.
-# NOTE: this current semantic might not be the best one (even if it has
+# NOTE: the current semantics might not be the best one (even if it has
 # been in place for quite a long time); see also Automake bug #7673.
-# Update this test if the semantic is changed.
+# Update this test if the semantics are changed.
 
 . ./defs || Exit 1
 
@@ -32,7 +32,7 @@ cat > Makefile.am <<'END'
 AUTOMAKE_OPTIONS =
 END
 
-set_strictness()
+set_strictness ()
 {
   set +x
   sed <$2 >$2-t -e "s|^\\(AUTOMAKE_OPTIONS\\) *=.*|\\1 = $1|" \
@@ -42,12 +42,12 @@ set_strictness()
   cat $2
 }
 
-ok()
+ok ()
 {
   $AUTOMAKE -Werror $*
 }
 
-ko()
+ko ()
 {
   AUTOMAKE_fails $*
   grep 'required file.*README' stderr
@@ -56,57 +56,66 @@ ko()
 $ACLOCAL
 
 # Leave out only one of the required files, to avoid too much
-# repetitions in the error messages.
+# repetition in the error messages.
 touch INSTALL NEWS AUTHORS ChangeLog COPYING
 
+rm -rf autom4te*.cache
 set_strictness '' Makefile.am
 set_strictness '' configure.in
 ko --gnu
 ko
 ok --foreign
 
+rm -rf autom4te*.cache
 set_strictness 'gnu' Makefile.am
 set_strictness '' configure.in
 ko --gnu
 ko
 ko --foreign
 
+rm -rf autom4te*.cache
 set_strictness '' Makefile.am
 set_strictness 'gnu' configure.in
 ko --gnu
 ko
 ko --foreign
 
+rm -rf autom4te*.cache
 set_strictness 'foreign' Makefile.am
 set_strictness '' configure.in
 ok --gnu
 ok
 ok --foreign
 
+rm -rf autom4te*.cache
 set_strictness '' Makefile.am
 set_strictness 'foreign' configure.in
 ok --gnu
 ok
 ok --foreign
 
+rm -rf autom4te*.cache
 set_strictness 'gnu' Makefile.am
 set_strictness 'gnu' configure.in
 ko --gnu
 ko
 ko --foreign
 
+rm -rf autom4te*.cache
 set_strictness 'foreign' Makefile.am
 set_strictness 'foreign' configure.in
 ok --gnu
 ok
 ok --foreign
 
+rm -rf autom4te*.cache
 set_strictness 'foreign' Makefile.am
 set_strictness 'gnu' configure.in
 ok --gnu
 ok
 ok --foreign
 
+rm -rf autom4te*.cache
 set_strictness 'gnu' Makefile.am
 set_strictness 'foreign' configure.in
 ko --gnu
diff --git a/tests/strictness-precedence.test b/tests/strictness-precedence.test
index 237b068..48086cc 100755
--- a/tests/strictness-precedence.test
+++ b/tests/strictness-precedence.test
@@ -1,5 +1,5 @@
 #! /bin/sh
-# Copyright (C) 2010 Free Software Foundation, Inc.
+# Copyright (C) 2011 Free Software Foundation, Inc.
 #
 # This program is free software; you can redistribute it and/or modify
 # it under the terms of the GNU General Public License as published by
@@ -29,7 +29,7 @@ cat > Makefile.am <<'END'
 AUTOMAKE_OPTIONS =
 END
 
-set_strictness()
+set_strictness ()
 {
   set +x
   sed <$2 >$2-t -e "s|^\\(AUTOMAKE_OPTIONS\\) *=.*|\\1 = $1|" \
@@ -39,19 +39,19 @@ set_strictness()
   cat $2
 }
 
-ok()
+ok ()
 {
   $AUTOMAKE -Werror $*
 }
 
-ko()
+ko ()
 {
   AUTOMAKE_fails $*
   grep 'required file.*README' stderr
 }
 
 # Leave out only one of the required files, to avoid too much
-# repetitions in the error messages.
+# repetition in the error messages.
 touch INSTALL NEWS AUTHORS ChangeLog COPYING
 
 $ACLOCAL --force
diff --git a/tests/warnings-overriding.test b/tests/warnings-override.test
similarity index 85%
rename from tests/warnings-overriding.test
rename to tests/warnings-override.test
index e1b8d20..fad1f12 100755
--- a/tests/warnings-overriding.test
+++ b/tests/warnings-override.test
@@ -1,5 +1,5 @@
 #! /bin/sh
-# Copyright (C) 2010 Free Software Foundation, Inc.
+# Copyright (C) 2011 Free Software Foundation, Inc.
 #
 # This program is free software; you can redistribute it and/or modify
 # it under the terms of the GNU General Public License as published by
@@ -17,9 +17,9 @@
 # The warnings specified in Makefile.am:AUTOMAKE_OPTIONS should override
 # those specified in configure.in:AM_INIT_AUTOMAKE, and both should
 # override the warnings specified on the command line.
-# NOTE: this current semantic might not be the best one (even if it has
+# NOTE: the current semantics might not be the best one (even if it has
 # been in place for quite a long time); see also Automake bug #7673.
-# Update this test if the semantic is changed.
+# Update this test if the semantics are changed.
 
 . ./defs || Exit 1
 
@@ -33,7 +33,7 @@ FOO := bar
 AUTOMAKE_OPTIONS =
 END
 
-set_warnings()
+set_warnings ()
 {
   set +x
   sed <$2 >$2-t -e "s|^\\(AUTOMAKE_OPTIONS\\) *=.*|\\1 = $1|" \
@@ -43,12 +43,12 @@ set_warnings()
   cat $2
 }
 
-ok()
+ok ()
 {
   $AUTOMAKE $*
 }
 
-ko()
+ko ()
 {
   AUTOMAKE_fails $*
   grep '^Makefile\.am:1:.*:=.*not portable' stderr
@@ -59,44 +59,52 @@ $ACLOCAL
 # Files required in gnu strictness.
 touch README INSTALL NEWS AUTHORS ChangeLog COPYING
 
+rm -rf autom4te*.cache
 set_warnings '-Wno-portability' Makefile.am
 set_warnings '' configure.in
 
 ok -Wportability
 ok
 
+rm -rf autom4te*.cache
 set_warnings '' Makefile.am
 set_warnings '-Wno-portability' configure.in
 
 ok -Wportability
 ok
 
+rm -rf autom4te*.cache
 set_warnings '-Wno-portability' Makefile.am
 set_warnings '-Wno-portability' configure.in
 
 ok -Wportability
 
+rm -rf autom4te*.cache
 set_warnings '-Wportability' Makefile.am
 set_warnings '' configure.in
 
 ko
 ko -Wno-portability
 
+rm -rf autom4te*.cache
 set_warnings '' Makefile.am
 set_warnings '-Wportability' configure.in
 
 ko
 ko -Wno-portability
 
+rm -rf autom4te*.cache
 set_warnings '-Wportability' Makefile.am
 set_warnings '-Wportability' configure.in
 ko -Wno-portability
 
+rm -rf autom4te*.cache
 set_warnings '-Wno-portability' Makefile.am
 set_warnings '-Wportability' configure.in
 ok
 ok -Wportability
 
+rm -rf autom4te*.cache
 set_warnings '-Wportability' Makefile.am
 set_warnings '-Wno-portability' configure.in
 ko
diff --git a/tests/warnings-precedence.test b/tests/warnings-precedence.test
index de60a8c..dcb078e 100755
--- a/tests/warnings-precedence.test
+++ b/tests/warnings-precedence.test
@@ -1,5 +1,5 @@
 #! /bin/sh
-# Copyright (C) 2010 Free Software Foundation, Inc.
+# Copyright (C) 2011 Free Software Foundation, Inc.
 #
 # This program is free software; you can redistribute it and/or modify
 # it under the terms of the GNU General Public License as published by
@@ -30,7 +30,7 @@ FOO := bar
 AUTOMAKE_OPTIONS =
 END
 
-set_warnings()
+set_warnings ()
 {
   set +x
   sed <$2 >$2-t -e "s|^\\(AUTOMAKE_OPTIONS\\) *=.*|\\1 = $1|" \
@@ -40,12 +40,12 @@ set_warnings()
   cat $2
 }
 
-ok()
+ok ()
 {
   $AUTOMAKE $*
 }
 
-ko()
+ko ()
 {
   AUTOMAKE_fails $*
   grep '^Makefile\.am:1:.*:=.*not portable' stderr

reply via email to

[Prev in Thread] Current Thread [Next in Thread]