LGTM, thanks for taking the time to do it well!
On Nov 11, 2015 2:24 AM, "Martell Malone" wrote:
> You changed from --enable to --with, but left the conditional name as
>> ENABLE_GENLIB. Change to WITH_GENLIB.
>> Makefile.am:
>> Don't test WITH_GENLIB twice. Test it once and set all the variabl
Ping?
On Tuesday, November 10, 2015, Martell Malone
wrote:
> You changed from --enable to --with, but left the conditional name as
>> ENABLE_GENLIB. Change to WITH_GENLIB.
>> Makefile.am:
>> Don't test WITH_GENLIB twice. Test it once and set all the variables
>> there. Move:
>> + AM_DLLTOOLF
>
> You changed from --enable to --with, but left the conditional name as
> ENABLE_GENLIB. Change to WITH_GENLIB.
> Makefile.am:
> Don't test WITH_GENLIB twice. Test it once and set all the variables
> there. Move:
> + AM_DLLTOOLFLAGS=-o $@
> and its twin into the respective conditional paths.
configure.ac:
You missed AS_VAR_SET:
[GENLIB=$with_genlib]
[AS_VAR_SET([GENLIB], [$with_genlib])]
This handles quoting very portably, and is more usable wrt indirection
for when someone copies this code elsewhere to do a similar change.
You also forgot to remove AC_SUBST, which is redundant with
Thanks for your input and help on this.
Please review :)
On Sun, Nov 8, 2015 at 10:05 PM, NightStrike wrote:
> ...and, as such, you have to provide a way to set the name.
> --with-genlib=mygenlib. You can find examples of this elsewhere in
> the build system. This means that you don't call AC_
...and, as such, you have to provide a way to set the name.
--with-genlib=mygenlib. You can find examples of this elsewhere in
the build system. This means that you don't call AC_CHECK_TOOL until
after the with- processing, and only if with_genlib is not set to no.
AS_CASE is appropriate for that
Sorry, missed this the first time around... this should be a with-
option, not enable. genlib is an external tool for the crt, not an
internal feature that is getting compiled in. You should just have to
change to AC_ARG_WITH and from enable_ to with_.
On Mon, Nov 9, 2015 at 12:46 AM, Martell Ma
Here is the final patch after running autoreconf -fi and only applying
relevant changes.
I would like to apply this if there are no objections ?
Kind Regards
Martell
On Fri, Nov 6, 2015 at 3:39 PM, Martell Malone
wrote:
> Updated to reflect Nightstrike's feedback
>
> On Wed, Nov 4, 2015 at 2:00
Updated to reflect Nightstrike's feedback
On Wed, Nov 4, 2015 at 2:00 PM, NightStrike wrote:
> Use AM_CONDITIONAL, not DEFINE_UNQUOTED
>
> On Wed, Nov 4, 2015 at 1:56 PM, Martell Malone
> wrote:
> > Be warned I am no autotools expert.
> > A review would be very helpful. :)
> >
> > CC+ alexey fo
Use AM_CONDITIONAL, not DEFINE_UNQUOTED
On Wed, Nov 4, 2015 at 1:56 PM, Martell Malone wrote:
> Be warned I am no autotools expert.
> A review would be very helpful. :)
>
> CC+ alexey for help on that :)
>
>
> --
>
>
Be warned I am no autotools expert.
A review would be very helpful. :)
CC+ alexey for help on that :)
From cd7023eb40a970e3a8293cdbcb0639450cf4d223 Mon Sep 17 00:00:00 2001
From: Martell Malone
Date: Wed, 4 Nov 2015 18:47:54 +
Subject: [PATCH] configure.ac: add support for --enable-genlib
d
11 matches
Mail list logo
