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 AC_CHECK_TOOL. 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. Move: if DELAY_IMPORT_LIBS AM_DLLTOOLFLAGS += --output-delaylib [email protected] endif after the WITH_GENLIB conditional. Send patches as .txt, as google screws up the encoding / mime type. On Tue, Nov 10, 2015 at 2:08 AM, Martell Malone <[email protected]> wrote: > Thanks for your input and help on this. > Please review :) > > On Sun, Nov 8, 2015 at 10:05 PM, NightStrike <[email protected]> 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_CHECK_TOOL until >> after the with- processing, and only if with_genlib is not set to no. >> AS_CASE is appropriate for that checking. >> >> On Mon, Nov 9, 2015 at 1:02 AM, NightStrike <[email protected]> wrote: >> > 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 Malone >> > <[email protected]> wrote: >> >> 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 >> >> <[email protected]> >> >> wrote: >> >>> >> >>> Updated to reflect Nightstrike's feedback >> >>> >> >>> On Wed, Nov 4, 2015 at 2:00 PM, NightStrike <[email protected]> >> >>> wrote: >> >>>> >> >>>> Use AM_CONDITIONAL, not DEFINE_UNQUOTED >> >>>> >> >>>> On Wed, Nov 4, 2015 at 1:56 PM, Martell Malone >> >>>> <[email protected]> >> >>>> wrote: >> >>>> > Be warned I am no autotools expert. >> >>>> > A review would be very helpful. :) >> >>>> > >> >>>> > CC+ alexey for help on that :) >> >>>> > >> >>>> > >> >>>> > >> >>>> > >> >>>> > ------------------------------------------------------------------------------ >> >>>> > >> >>>> > _______________________________________________ >> >>>> > Mingw-w64-public mailing list >> >>>> > [email protected] >> >>>> > https://lists.sourceforge.net/lists/listinfo/mingw-w64-public >> >>>> > >> >>>> >> >>>> >> >>>> >> >>>> ------------------------------------------------------------------------------ >> >>>> _______________________________________________ >> >>>> Mingw-w64-public mailing list >> >>>> [email protected] >> >>>> https://lists.sourceforge.net/lists/listinfo/mingw-w64-public >> >>> >> >>> >> >> >> >> >> >> >> >> ------------------------------------------------------------------------------ >> >> Presto, an open source distributed SQL query engine for big data, >> >> initially >> >> developed by Facebook, enables you to easily query your data on Hadoop >> >> in a >> >> more interactive manner. Teradata is also now providing full enterprise >> >> support for Presto. Download a free open source copy now. >> >> http://pubads.g.doubleclick.net/gampad/clk?id=250295911&iu=/4140 >> >> _______________________________________________ >> >> Mingw-w64-public mailing list >> >> [email protected] >> >> https://lists.sourceforge.net/lists/listinfo/mingw-w64-public >> >> >> >> >> ------------------------------------------------------------------------------ >> Presto, an open source distributed SQL query engine for big data, >> initially >> developed by Facebook, enables you to easily query your data on Hadoop in >> a >> more interactive manner. Teradata is also now providing full enterprise >> support for Presto. Download a free open source copy now. >> http://pubads.g.doubleclick.net/gampad/clk?id=250295911&iu=/4140 >> _______________________________________________ >> Mingw-w64-public mailing list >> [email protected] >> https://lists.sourceforge.net/lists/listinfo/mingw-w64-public > > > > ------------------------------------------------------------------------------ > > _______________________________________________ > Mingw-w64-public mailing list > [email protected] > https://lists.sourceforge.net/lists/listinfo/mingw-w64-public > ------------------------------------------------------------------------------ _______________________________________________ Mingw-w64-public mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/mingw-w64-public
