Re: Proposed Build System Patch.

On Mon, Sep 03, 2001 at 02:30:42PM -0700, Greg Stein wrote:
> On Mon, Sep 03, 2001 at 02:11:55PM -0400, Kevin Pilch-Bisson wrote:
> > Since I got such an overwhelming reponse the last time I posted this patch
> > for consideration, I thought I would repost it.
>
> Sorry 'bout that...

No prob.

>
> >...
> > --- ./SVN/text-base/Makefile.in Fri Aug 31 09:50:56 2001
> > +++ ./Makefile.in Fri Aug 31 13:54:51 2001
> > @@ -21,8 +21,11 @@
> > prefix = @prefix@
> > exec_prefix = @exec_prefix@
> > libdir = @libdir@
> > +fs_libdir= @libdir@
> > sbindir = @sbindir@
> > +fs_sbindir=@sbindir@
>
> We can toss the sbin stuff.
>
Thought of that too, but I thought it would make it easier, if we ever do
decide we need some root-only utility (not that I can think of what that is).
> > bindir = @bindir@
> > +fs_bindir=@bindir@
> > includedir = @includedir@
> >
> > ### should search for these...
> > @@ -50,6 +53,11 @@
> > APACHE_TARGET = @APACHE_TARGET@
> > INSTALL_APACHE_RULE = @INSTALL_APACHE_RULE@
> >
> > +INSTALL_RULES = @INSTALL_RULES@
>
> While we're at this, let's just toss the $(INSTALL_APACHE_RULE) and use
> @INSTALL_RULES@. The only reason for the former was that we didn't have the
> latter :-)

Good idea.

>
> > +TEST_DEPS = $(NON_FS_TEST_DEPS) @FS_TEST_DEPS@
> > +TEST_PROGRAMS = $(NON_FS_TEST_PROGRAMS) @FS_TEST_PROGRAMS@
>
> We can minimize change by using:
>
> check: $(TEST_DEPS) @FS_TEST_DEPS@
>
> And similarly for the use TEST_PROGRAMS. This way, we create TEST_DEPS from
> the test_deps variable, and FS_TEST_DEPS from the fs_test_deps variable.
> Nice and clean.

I thought of this, but I didn't (and still don't) understand why minimizing
change should be a bigger priority than clarity. (Although I just caught
myself, since I have $libdir and $fs_libdir, why can't I have TEST_DEPS and
FS_TEST_DEPS. Alright, I'll make this change (which is what I did first
anyway).
>
> > +### temporary hack. Neon does not have an "extraclean" and neither does db
> > +### If we don't have extraclean -- do the next best thing.
> > external-extraclean:
> > @list='$(EXTERNAL_PROJECT_DIRS)'; \
> > for i in $$list; do \
> > - if test "$$i" != "neon"; then \
> > - echo "------ making extraclean in $$i"; \
> > - (cd $$i && $(MAKE) extraclean) || exit 1; \
> > - echo "------ completed extraclean in $$i"; \
> > + if test "$$i" != "neon" && test "$$i" != "db/dist"; then \
> > + echo "------ making extraclean in $$i"; \
> > + (cd $$i && $(MAKE) extraclean) || exit 1; \
> > + echo "------ completed extraclean in $$i"; \
> > + else \
> > + echo "------ making distclean(no extraclean) in $$i"; \
> > + (cd $$i && $(MAKE) distclean) || exit 1; \
> > + echo "------ completed distclean(no extraclean) in $$i"; \
>
> Indentation and maybe using positive logic for the if test? That is:
Doh, need 'set expandtab' in ~/.vimrc. (Speaking of which, I am planning on
coming up with a comment string that will set the coding style for vim and
adding it to files in the near future.)
>
> if test "$$i" = "neon" || test "$$i" = "db/dist"; then

Yeah, will do.
>
> >...
> > @@ -95,8 +95,10 @@
> > if test "$status" = "builtin"; then
> > # Use the include and lib files in the build dir.
> > dbdir=`cd db/dist ; pwd`
> > - CPPFLAGS="$CPPFLAGS -I$dbdir"
> > - LIBS="$LIBS -L$dbdir -ldb"
> > + SVN_DB_INCLUDES="-I$dbdir"
> > + # Note that once we upgrade to libtool 1.4 this should be changed to
> > + # SVN_DB_LIBS="$dbdir/libdb-3.3.la"
> > + SVN_DB_LIBS="-L$dbdir/.libs -ldb-3.3"
>
> Ah. Good. That .m4 file shouldn't have modified CPPFLAGS and LIBS like that.

No indeed.
>
>
> Looks good to me! Nice work.
Thanks
>
> Cheers,
> -g
> --
> Greg Stein, http://www.lyra.org/
>

-- 
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Kevin Pilch-Bisson                    http://www.pilch-bisson.net
     "Historically speaking, the presences of wheels in Unix
     has never precluded their reinvention." - Larry Wall
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~