On 12/21/2012 05:31 PM, Stefano Lattarini wrote: > Hi Akim. > > This patch is basically OK, although I have some style nits below. > ACK with those addressed. Thanks. > > On 12/19/2012 02:55 PM, Akim Demaille wrote: >> * t/yacc-d-basic.sh: Comment changes. >> (generated): New. >> Use it to factor various tests. >> Check that Y_TAB_H is not issued. >> > This new checks make the test fail, right. It would be nice to say so > in the commit message: > > These new checks make the test fail, because they expose unaddressed > bugs in ylwrap. This bugs will be fixed by a follow-up change. > >> --- >> t/yacc-d-basic.sh | 51 +++++++++++++++++++++++++-------------------------- >> 1 file changed, 25 insertions(+), 26 deletions(-) >> >> diff --git a/t/yacc-d-basic.sh b/t/yacc-d-basic.sh >> index 97155a2..34d8c9d 100755 >> --- a/t/yacc-d-basic.sh >> +++ b/t/yacc-d-basic.sh >> @@ -59,8 +59,10 @@ END >> # the conversion from y.tab.c to parse.c. This was OK when Bison was >> # not issuing such an #include (up to 2.6). >> # >> -# To make sure that we perform this conversion, in bar/parse.y, use >> -# y.tab.h instead of parse.c. >> +# To make sure that we perform this conversion even with version of >> +# Bison that do not generate this include, in bar/parse.y, use y.tab.h >> +# instead of parse.h, and check the ylwrap does replace "y.tab.h" with >> +# "parse.h". >> sed -e 's/parse\.h/y.tab.h/' <foo/parse.y >bar/parse.y >> >> cat > foo/main.c << 'END' >> @@ -107,12 +109,15 @@ $AUTOMAKE baz/Makefile >> >> $MAKE >> >> -test -f foo/parse.c >> -test -f foo/parse.h >> -test -f bar/parse.c >> -test -f bar/parse.h >> -test -f baz/zardoz-parse.c >> -test -f baz/zardoz-parse.h >> +generated="foo/parse.c foo/parse.h bar/parse.c bar/parse.h >> baz/zardoz-parse.c >> +baz/zardoz-parse.h" >> > I'd like to see each file on its own line: > > generated=" > foo/parse.c > foo/parse.h > ... > baz/zardoz-parse.h > " > >> + >> +for i in $generated >> +do >> > Please keep the "do" on the same line of the "for": > > for i in $generated; do > > Ditto for similar uses below. > >> + test -f $i >> + # There must remain no obsolete header guard. >> + ! grep Y_TAB_H $generated >> > This grep should go outside the for loop, right? > >> +done >> >> # The generated C source and header files must be shipped. >> for dir in foo bar; do >> > > Thanks, > Stefano > Below is what I squashed in this patch before merging it to master:
diff --git a/t/yacc-d-basic.sh b/t/yacc-d-basic.sh index 0067b7d..a03065d 100755 --- a/t/yacc-d-basic.sh +++ b/t/yacc-d-basic.sh @@ -109,16 +109,22 @@ $AUTOMAKE baz/Makefile $MAKE -generated="foo/parse.c foo/parse.h bar/parse.c bar/parse.h baz/zardoz-parse.c -baz/zardoz-parse.h" - -for i in $generated -do +generated=" + foo/parse.c + foo/parse.h + bar/parse.c + bar/parse.h + baz/zardoz-parse.c + baz/zardoz-parse.h +" + +for i in $generated; do test -f $i - # There must remain no obsolete header guard. - ! grep Y_TAB_H $generated done +# There must remain no obsolete header guard. +grep Y_TAB_H $generated && exit 1 + # The generated C source and header files must be shipped. for dir in foo bar; do cd $dir @@ -135,8 +141,7 @@ cd .. $MAKE distdir ls -l $distdir -for i in $generated -do +for i in $generated; do test -f $distdir/$i done @@ -146,14 +151,12 @@ yl_distcheck # While we are at it, make sure that 'parse.c' and 'parse.h' are erased # by maintainer-clean, and not by distclean. $MAKE distclean -for i in $generated -do +for i in $generated; do test -f $i done ./configure # Re-create 'Makefile'. $MAKE maintainer-clean -for i in $generated -do +for i in $generated; do test ! -e $i done
