Re: flex beta 2.5.22 released

[Akim Demaille]

Please, always keep thing public!

| On Friday, 11 October 2002,10:22 +0200, Akim Demaille wrote:
| > I have a few comments, which are mainly related to the fact that I'm
| > used to packages where I run `./configure && make && make check &&
| > make install'.  It turns out you don't use make check, like most
| > packages, but make test.
| 
| We'd like to have it be make check, but until recently, the automake
| documentation was excessively vague on how to accomplish this. I've
| looked at the 1.7 manual and I see how to fit our stuff in, but it's
| low priority since the test suite does run. It will happen soon though.

I don't get it: your suite runs properly, so what is the problem with
my patch?


| > This revealed for instance that test-create is not shipped :(  So this
| 
| After looking at your patch for create-test, that really seems like a
| bug in automake. (But see my comments later.)

Many many things in Autoconf/Automake can look like bugs when you
don't have history.  This behavior is the right one: people can have
noinst_SCRIPTS that are not distributed, for instance if created from
create-test.in by configure.


| > BTW: Of course I was about to send a ChangeLog, but running `make
| > distcheck' to check my patch killed the ChangeLog.  This is
| > unfortunate.  Today most people write the ChangeLog and have CVS use
| > the ChangeLog as log message, not the converse.
| 
| Tell me about how people do this. I'm interested to know as I'd like
| to have a more useful ChangeLog.

The point is probably not usefulness: I was not saying that yours was
not.  But more of readability.  It is simply that reading ChangeLog is
so frequent, that it's better to have properly formatted etc.  rcs2log
does not help making it nice to the reader.

Another point is: wrapping a patch with a ChangeLog is easy.  Writing
a patch with a RCS log is not :)


| diff='^[ \t]*//' /tmp/flex-2.5.22/Makefile.am 
/home/lrde/prof/akim/src/flex-2.5.22/Makefile.am
| --- /tmp/flex-2.5.22/Makefile.am      2002-10-10 17:34:31.000000000 +0200
| +++ /home/lrde/prof/akim/src/flex-2.5.22/Makefile.am  2002-10-11 
10:00:05.000000000 +0200
| @@ -113,12 +113,6 @@
|       intl \
|       m4  \
|       examples \
| -     po
| -
| -DIST_SUBDIRS = \
| -     intl \
| -     m4  \
| -     examples \
|       po \
|       tests
| 
| What I'm doing with dist_subdirs is this: I don't want "make all" to
| recurse into the test suite. Am I doing this wrong?

It's customary to being able to run make check from the top level, so
Automake has to recurse into your tests directory.


| diff='^[ \t]*//' /tmp/flex-2.5.22/mkinstalldirs 
/home/lrde/prof/akim/src/flex-2.5.22/mkinstalldirs
| --- /tmp/flex-2.5.22/mkinstalldirs    2002-10-10 21:55:58.000000000 +0200
| +++ /home/lrde/prof/akim/src/flex-2.5.22/mkinstalldirs        2002-10-11 
09:57:04.000000000 +0200
| 
| I'm using the mkinstalldirs that autoreconf leaves me. If that's not
| the right one,then it's a bug in autoreconf.

I ran autoreconf and got this one.  Probably because I use CVS
Automake.  No worries here, I just pulling you a bit forward.


| diff='^[ \t]*//' /tmp/flex-2.5.22/tests/Makefile.am 
/home/lrde/prof/akim/src/flex-2.5.22/tests/Makefile.am
| --- /tmp/flex-2.5.22/tests/Makefile.am        2002-10-01 17:10:12.000000000 
+0200
| +++ /home/lrde/prof/akim/src/flex-2.5.22/tests/Makefile.am    2002-10-11 
10:14:00.000000000 +0200
| @@ -22,45 +22,9 @@
|  EXTRA_DIST = \
|       README
| 
| -noinst_SCRIPTS = \
| +dist_noinst_SCRIPTS = \
|       create-test
| 
| It's not at all clear that noinst_SCRIPTS will not be included in the
| distribution. (I just read the section in the automake 1.7 manual.)

The documentation should certainly be adjusted!

| Also, the tests/TEMPLATE directory does not need to be run when making
| test/check/all, etc. Am I approaching that problem in the wrong way?
| 

Sorry, I don't know what you mean here.


| @@ -98,7 +62,7 @@
| 
|  # clean up before running the test suite so we dont test old builds of test 
code
| 
| -test: clean
| +check-local: clean
|       NOK=;\
|       NFAIL=;\
|       for dir in $(SUBDIRS) ; do \
| 
| changing the "test" target to "check-local" will make the whole test
| suite show up under "make check"?
| 

Correct.


| @@ -115,4 +79,5 @@
|       done ; \
|       echo Results: ; \
|       echo Tests succeeded: `echo @ECHO_N@ "address@hidden@"|wc -c`; \
| -     echo Tests FAILED: `echo @ECHO_N@ "address@hidden@"|wc -c` ;
| +     echo Tests FAILED: `echo @ECHO_N@ "address@hidden@"|wc -c` ; \
| +     test "$$NFAIL" = "";
| 
| What are you trying to accomplish with this last line?

I'm acheiving that make check fails when the test suite fails.


| diff='^[ \t]*//' /tmp/flex-2.5.22/tests/TEMPLATE/Makefile.am 
/home/lrde/prof/akim/src/flex-2.5.22/tests/TEMPLATE/Makefile.am
| --- /tmp/flex-2.5.22/tests/TEMPLATE/Makefile.am       2002-10-10 
21:12:49.000000000 +0200
| +++ /home/lrde/prof/akim/src/flex-2.5.22/tests/TEMPLATE/Makefile.am   
2002-10-11 10:05:30.000000000 +0200
| @@ -19,10 +19,8 @@
|  # WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
|  # PURPOSE.
| 
| -BISON = @BISON@
| 
| If we dont have this, automake wants to have bison be "bison -y" we
| want bison with no options or anything. How can we get this?

You don't have to add these lines: Automake knows its jobs, and knows
that if there is an AC_SUBSTS(BISON), then it needs to insert these
lines.

|  FLEX = $(top_builddir)/flex
| 
| -builddir = @builddir@
| 
| If I don't define this, $(builddir) shows up empty, despite what the
| automake/autoconf manual says. Is that a bug in one of those two packages?

Well, in that case, you do have found a bug!  But I can't see it:

src/flex-2.5.22/tests % fgrep 'builddir =' **/Makefile.am        nostromo 15:12
src/flex-2.5.22/tests % fgrep 'builddir =' **/Makefile           nostromo Err 1
Makefile:top_builddir = ..
TEMPLATE/Makefile:top_builddir = ../..
test-array-nr/Makefile:top_builddir = ../..
test-array-r/Makefile:top_builddir = ../..
test-basic-nr/Makefile:top_builddir = ../..
test-basic-r/Makefile:top_builddir = ../..
test-bison-nr/Makefile:top_builddir = ../..
test-bison-yylloc/Makefile:top_builddir = ../..
test-bison-yylval/Makefile:top_builddir = ../..
test-c++-basic/Makefile:top_builddir = ../..
test-c-cpp-nr/Makefile:top_builddir = ../..
test-c-cpp-r/Makefile:top_builddir = ../..
test-c++-multiple-scanners/Makefile:top_builddir = ../..
test-debug-nr/Makefile:top_builddir = ../..
test-debug-r/Makefile:top_builddir = ../..
test-header-nr/Makefile:top_builddir = ../..
test-header-r/Makefile:top_builddir = ../..
test-include-by-buffer/Makefile:top_builddir = ../..
test-include-by-reentrant/Makefile:top_builddir = ../..
test-lineno-nr/Makefile:top_builddir = ../..
test-lineno-r/Makefile:top_builddir = ../..
test-mem-nr/Makefile:top_builddir = ../..
test-mem-r/Makefile:top_builddir = ../..
test-multiple-scanners-nr/Makefile:top_builddir = ../..
test-multiple-scanners-r/Makefile:top_builddir = ../..
test-posixly-correct/Makefile:top_builddir = ../..
test-posix/Makefile:top_builddir = ../..
test-prefix-nr/Makefile:top_builddir = ../..
test-prefix-r/Makefile:top_builddir = ../..
test-pthread/Makefile:top_builddir = ../..
test-reject/Makefile:top_builddir = ../..
test-string-nr/Makefile:top_builddir = ../..
test-string-r/Makefile:top_builddir = ../..
test-table-opts/Makefile:top_builddir = ../..
test-yyextra/Makefile:top_builddir = ../..