On 24/08/17 11:19, Konstantin Kolinko wrote: > 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/.
Thanks. Fixed. I think Rainer has answered the libtool questions. Mark > > 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 > --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
