Mailing-List: contact dev-help@tomcat.apache.org; run by ezmlm
Precedence: bulk
Reply-To: "Tomcat Developers List" <dev@tomcat.apache.org>
Subject: Re: svn commit: r1805960 - in /tomcat/native/trunk:
 native/Makefile.in native/configure.in xdocs/miscellaneous/changelog.xml
To: Tomcat Developers List <dev@tomcat.apache.org>
References: <20170823211217.9D30C3A002A@svn01-us-west.apache.org>
 <CABzHfV=1nYCd29vPGCsOQ77hsFOmR=dYzfeQ1JTWCkWti-GtLg@mail.gmail.com>
From: Mark Thomas <markt@apache.org>
Message-ID: <d1744d7c-00f4-1b0a-452b-8847739c9041@apache.org>
Date: Thu, 24 Aug 2017 19:33:19 +0100
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101
 Thunderbird/52.2.1
MIME-Version: 1.0
In-Reply-To: <CABzHfV=1nYCd29vPGCsOQ77hsFOmR=dYzfeQ1JTWCkWti-GtLg@mail.gmail.com>
Content-Type: text/plain; charset=utf-8
Content-Language: en-GB
Content-Transfer-Encoding: 7bit
archived-at: Thu, 24 Aug 2017 18:33:25 -0000

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  <markt@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-unsubscribe@tomcat.apache.org
> For additional commands, e-mail: dev-help@tomcat.apache.org
> 


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org