Re: [PATCH] coverage: test semantics of "dummy" per-object flags

[Stefano Lattarini]

Hi Ralf.  I haven't pushed yes, as I still have a nit/doubt ...

On Saturday 22 January 2011, Ralf Wildenhues wrote:
> * Stefano Lattarini wrote on Sat, Jan 22, 2011 at 06:14:36PM CET:
> > On Saturday 22 January 2011, Ralf Wildenhues wrote:
> > 
> > > specflg11.test comes to mind as name, although I'm sure you
> > > won't like it.  ;-)
> > >
> > Indeed.  But maybe `specflg-dummy.test' is a good compromise :-)
> 
> Ahh, good one.
> 
> Nice patch by the way.  It is ok with nits addressed.
>
> > Subject: [PATCH] coverage: test semantics of "dummy" per-object flags
> 
> Please use "per-target" instead of "per-object" throughout, as that is
> the term the Automake manual uses;
>
And is also clearer and more natural BTW.

> and also, these flags are not
> modifiable per object, unlike, say, a GNU make-specific
> 
>   foo.o: CFLAGS = -O0
> 
> per-object override.
> 
> > * tests/specflg-dummy.test: Check that we can "fool" automake
> > into thinking that per-object CFLAGS are used by simply doing
> > `foo_CFLAGS = $(AM_CFLAGS)', even if AM_CFLAGS is undefined.
> 
> I don't think it is so much "fooling", as the semantics are quite
> clearly defined in the manual, and per-target flags are documented
> in several places.  See 'Renamed Objects' and 'Objects created both
> with libtool and without' for quite explicit mention of these
> semantics.
>
Ah, but there I only see examples of "real" per-target flags, while
my test is meant to check that even "dummy" ones triggers the use of
renamed objects.

> How about this?
> 
>   Ensure per-target flags `foo_CFLAGS = $(AM_CFLAGS)' enable
>   renamed objects.
>
What about this squash-in?

-*-*-*-

  diff --git a/ChangeLog b/ChangeLog
  index 28666e0..43f22f0 100644
  --- a/ChangeLog
  +++ b/ChangeLog
  @@ -1,11 +1,8 @@
   2011-01-22  Stefano Lattarini  <address@hidden>
 
  -       coverage: test semantics of "dummy" per-object flags
  -       * tests/specflg-dummy.test: Check that we can "fool" automake
  -       into thinking that per-object CFLAGS are used by simply doing
  -       `foo_CFLAGS = $(AM_CFLAGS)', even if AM_CFLAGS is undefined.
  -       Similarly for other *FLAGS variables (such as CXXFLAGS, YFLAGS,
  -       LDFLAGS, ...).
  +       coverage: test semantics of "dummy" per-target flags
  +       * tests/specflg-dummy.test: New test, ensuring that even "dummy"
  +       per-target flags triggers the use of renamed objects.
          * tests/Makefile.am (TESTS): Update.
          Suggestion by Ralf Wildenhues.
 
  diff --git a/tests/specflg-dummy.test b/tests/specflg-dummy.test
  index bdabb49..d9ac573 100755
  --- a/tests/specflg-dummy.test
  +++ b/tests/specflg-dummy.test
  @@ -14,9 +14,10 @@
   # You should have received a copy of the GNU General Public License
   # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
  -# Check that we can "fool" automake into thinking that per-object
  -# CFLAGS are used by simply doing `foo_CFLAGS = $(AM_CFLAGS)', even
  -# if AM_CFLAGS is undefined. Similarly for other *FLAGS variables
  +# Check that even "dummy" per-target flags triggers the use of renamed
  +# objects.  I.e., a definition like `foo_CFLAGS = $(AM_CFLAGS)' should
  +# always cause Automake to trigger the semantics for per-target CFLAGS,
  +# even if AM_CFLAGS is undefined. Similarly for other *FLAGS variables
   # (CXXFLAGS, YFLAGS, LDFLAGS, ...)
 
   . ./defs || Exit 1

-*-*-*-

> > Similarly for other *FLAGS variables (such as CXXFLAGS, YFLAGS,
> > LDFLAGS, ...).
> > * tests/Makefile.am (TESTS): Update.
> 
> > --- /dev/null
> > +++ b/tests/specflg-dummy.test
> 
> > +# Check that we can "fool" automake into thinking that per-object
> > +# CFLAGS are used by simply doing `foo_CFLAGS = $(AM_CFLAGS)', even
> > +# if AM_CFLAGS is undefined. Similarly for other *FLAGS variables
> > +# (CXXFLAGS, YFLAGS, LDFLAGS, ...)
> 
> Likewise.
>
See above.

> > +. ./defs || Exit 1
> > +
> > +set -e
> > +
> > +# Disable shell globbing if possible.
> > +(set +f) >/dev/null 2>&1 && set +f
> > +
> > +oIFS=$IFS
> > +nl='
> > +'
> 
> Please go ahead and commit a separate patch to maint that initializes
> IFS with space, tab, newline, in that order, so an unset IFS (which is
> Posix-conforming) doesn't mess up IFS changes.  Prior to this patch.
> Thank you.
>
Will do.

> > +matches=
> > +add_match ()
> > +{
> > +  matches="$matches$nl$1"
> > +}
> > +
> > +do_check ()
> > +{
> > +  IFS=$nl
> > +  for string in $matches; do
> > +    IFS=$oIFS
> > +    $FGREP "$string" Makefile.in
> > +  done
> > +  IFS=$oIFS
> > +}
> > +
> > +# Fake libtool presence, so that we won't have to require it.
> > +cat > acinclude.m4 <<END
> > +AC_DEFUN([AC_PROG_LIBTOOL], [AC_SUBST([LIBTOOL], [dummy])])
> > +END
> > +: > ltmain.sh
> > +
> > +cat >> configure.in << 'END'
> > +AC_PROG_CC
> > +AC_PROG_CXX
> > +AC_PROG_YACC
> > +AC_PROG_LEX
> > +AC_PROG_F77
> > +AC_PROG_FC
> > +AM_PROG_GCJ
> > +AC_PROG_RANLIB
> > +AC_PROG_LIBTOOL
> > +AM_PROG_UPC
> > +AC_PROG_OBJC
> > +AM_PROG_CC_C_O
> > +END
> > +
> > +cat > Makefile.am <<'END'
> > +bin_PROGRAMS =
> > +lib_LIBRARIES =
> > +lib_LTLIBRARIES =
> > +END
> > +
> > +$ACLOCAL
> > +
> > +cat >> Makefile.am <<'END'
> > +bin_PROGRAMS += prog1
> > +prog1_SOURCES = source1.c
> > +prog1_CFLAGS = $(AM_CFLAGS)
> > +END
> > +
> > +add_match 'prog1-source1.$(OBJEXT)'
> [...]
> 
> > +add_match 'linkprog_LINK ='
> > +
> > +cat >> Makefile.am <<'END'
> > +lib_LIBRARIES += lib-static.a
> 
> That's really weird to see a library name begin with a hyphen ;-)
> (you'd use -l-static to link against it, which looks very much
> like a typo).  I would lose that here and below.
>
Agreed.  And similarly for libtool libraries below, I guess ...

> > +lib_static_a_SOURCES = static123.c
> > +lib_static_a_CFLAGS = $(AM_CFLAGS)
> > +END
> > +
> > +add_match 'lib_static_a-static123.$(OBJEXT)'
> > +
> > +cat >> Makefile.am <<'END'
> > +lib_LTLIBRARIES += lib-shared1.la
> > +lib_shared1_la_SOURCES = shared1.c
> > +lib_shared1_la_LIBTOOLFLAGS = $(AM_LIBTOOLFLAGS)
> > +END
> > +
> > +add_match 'lib_shared1_la-shared1.lo'
> > +
> > +cat >> Makefile.am <<'END'
> > +lib_LTLIBRARIES += lib-shared2.la
> > +lib_shared2_la_SOURCES = shared2.cc
> > +lib_shared2_la_CXXFLAGS = $(AM_CXXFLAGS)
> > +END
> > +
> > +add_match 'lib_shared2_la-shared2.lo'
> > +
> > +cat >> Makefile.am <<'END'
> > +bin_PROGRAMS += parse1
> > +parse1_SOURCES = parse.y
> > +parse1_YFLAGS = $(AM_YFLAGS)
> > +END
> > +
> > +add_match 'parse1-parse.c'
> > +
> > +cat >> Makefile.am <<'END'
> > +bin_PROGRAMS += parse2
> > +parse2_SOURCES = parse.ypp
> > +parse2_YFLAGS = $(AM_YFLAGS)
> > +END
> > +
> > +add_match 'parse2-parse.cpp'
> > +
> > +cat >> Makefile.am <<'END'
> > +bin_PROGRAMS += lexer1
> > +lexer1_SOURCES = lex.l
> > +lexer1_LFLAGS = $(AM_YFLAGS)
> > +END
> > +
> > +add_match 'lexer1-lex.c'
> > +
> > +cat >> Makefile.am <<'END'
> > +bin_PROGRAMS += lexer2
> > +lexer2_SOURCES = lexer.ll
> > +lexer2_LFLAGS = $(AM_YFLAGS)
> > +END
> > +
> > +add_match 'lexer2-lexer.cc'
> > +
> > +# For debugging.
> > +cat Makefile.am
> 
> That seems fairly redundant to me; oh well.
> 
> > +$AUTOMAKE -a
> > +do_check
> > +
> > +sed 's|^\(.*\)_SOURCES *= *|sub_\1_SOURCES = srcsub/|' Makefile.am >> t1
> > +sed 's|^\(.*\)PROGRAMS *+= *|\1PROGRAMS += sub/|' t1 >> t2
> > +sed 's|^\(.*\)LIBRARIES *+= *|\1LIBRARIES += sub/|' t2 >> t3
> > +sed 's|^\(.*\)FLAGS *=|sub_\1FLAGS =|' t3 >> t4
> 
> sed '
>   s|^\(.*\)_SOURCES *= *|sub_\1_SOURCES = srcsub/|
>   s|^\(.*\)PROGRAMS *+= *|\1PROGRAMS += sub/|
>   s|^\(.*\)LIBRARIES *+= *|\1LIBRARIES += sub/|
>   s|^\(.*\)FLAGS *=|sub_\1FLAGS =|
>   ' Makefile.am > t
>
Ouch, I could have I missed that!  Now I feel a bit like an idiot ...

> and adjust for 't' below.
> 
> > +cat - t4 > Makefile.am <<'END'
> > +AUTOMAKE_OPTIONS = subdir-objects
> > +END
> > +
> > +rm -f t[0-9]
> > +
> > +# For debugging.
> > +cat Makefile.am
> > +
> > +$AUTOMAKE
> > +do_check
> 
>

Thanks,
  Stefano