A naive review below. I do not have a working tc-native build to check. I think your patch is an adaptation of r1805943 (BZ 60290). As such, it looks good.
Several thoughts: 1) A typo in commit message and in changelog.xml I think it was meant to be s/but setting/by setting/. 2) There exists @TCNATIVE_LIBTOOL_VERSION@ What is its value? Looking into configure.in, I think it is evaluated rather early, before $LIBTOOL is set. 3) >> APR_SETIFNULL(LIBTOOL, `$apr_config --apr-libtool`) I wonder whether that --apr-libtool flag should be simply --libtool, like with --cc and -cpp a few lines above. 4) There exists $APR_LIBTOOL_LIBS. I wonder what is its value and whether it needs some update as well. 2017-08-24 0:12 GMT+03:00 <ma...@apache.org>: > Author: markt > Date: Wed Aug 23 21:12:17 2017 > New Revision: 1805960 > > URL: http://svn.apache.org/viewvc?rev=1805960&view=rev > Log: > Fix https://bz.apache.org/bugzilla/show_bug.cgi?id=60301 > When building Tomcat Native, allow the user to override the libtool specified > by APR but setting the LIBTOOL environment variable. > > Modified: > tomcat/native/trunk/native/Makefile.in > tomcat/native/trunk/native/configure.in > tomcat/native/trunk/xdocs/miscellaneous/changelog.xml > > Modified: tomcat/native/trunk/native/Makefile.in > URL: > http://svn.apache.org/viewvc/tomcat/native/trunk/native/Makefile.in?rev=1805960&r1=1805959&r2=1805960&view=diff > ============================================================================== > --- tomcat/native/trunk/native/Makefile.in (original) > +++ tomcat/native/trunk/native/Makefile.in Wed Aug 23 21:12:17 2017 > @@ -22,6 +22,7 @@ > CFLAGS = @CFLAGS@ > CPPFLAGS = @CPPFLAGS@ > CC_OLD = @CC@ > +LIBTOOL_OLD = @LIBTOOL@ > > # gets substituted into some targets > TCNATIVE_MAJOR_VERSION=@TCNATIVE_MAJOR_VERSION@ > @@ -52,6 +53,10 @@ ifneq ($(CC_OLD),$(CC)) > CC=$(CC_OLD) > endif > > +ifneq ($(LIBTOOL_OLD),$(LIBTOOL)) > + LIBTOOL=$(LIBTOOL_OLD) > +endif > + > LINK = $(LIBTOOL) $(LTFLAGS) --mode=link $(LT_LDFLAGS) $(COMPILE) > -version-info $(TCNATIVE_LIBTOOL_VERSION) $(ALL_LDFLAGS) -o $@ > CLEAN_SUBDIRS = test > > > Modified: tomcat/native/trunk/native/configure.in > URL: > http://svn.apache.org/viewvc/tomcat/native/trunk/native/configure.in?rev=1805960&r1=1805959&r2=1805960&view=diff > ============================================================================== > --- tomcat/native/trunk/native/configure.in (original) > +++ tomcat/native/trunk/native/configure.in Wed Aug 23 21:12:17 2017 > @@ -101,6 +101,11 @@ dnl > APR_SETIFNULL(CC, `$apr_config --cc`) > APR_SETIFNULL(CPP, `$apr_config --cpp`) > > +dnl > +dnl Default to the APR provided libtool but allow the user to override it > +dnl > +APR_SETIFNULL(LIBTOOL, `$apr_config --apr-libtool`) > + > AC_PROG_INSTALL > > dnl > @@ -234,6 +239,7 @@ AC_SUBST(TCNATIVE_LDFLAGS) > AC_SUBST(TCNATIVE_LIBS) > AC_SUBST(CFLAGS) > AC_SUBST(CPPFLAGS) > +AC_SUBST(LIBTOOL) > > dnl copy apr's rules.mk into our build directory. > if test ! -d ./build; then > > Modified: tomcat/native/trunk/xdocs/miscellaneous/changelog.xml > URL: > http://svn.apache.org/viewvc/tomcat/native/trunk/xdocs/miscellaneous/changelog.xml?rev=1805960&r1=1805959&r2=1805960&view=diff > ============================================================================== > --- tomcat/native/trunk/xdocs/miscellaneous/changelog.xml (original) > +++ tomcat/native/trunk/xdocs/miscellaneous/changelog.xml Wed Aug 23 21:12:17 > 2017 > @@ -66,6 +66,11 @@ > <code>CC</code> if explicitly set. Patch provided by Michael Osipov. > (markt) > </fix> > + <fix> > + <bug>60301</bug>: When building Tomcat Native, allow the user to > override > + the libtool specified by APR but setting the <code>LIBTOOL</code> > + environment variable. (markt) > + </fix> > </changelog> > </section> --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
