Ping? On Tuesday, November 10, 2015, Martell Malone <[email protected]> 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_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. > > Yeah I knew I done that. I thought the variable name could have stayed the > same. > Laziness on my part there. > > Anyway here is an updated patch to address your comments. > It is in .txt format :) > > On Mon, Nov 9, 2015 at 11:47 PM, NightStrike <[email protected] > <javascript:_e(%7B%7D,'cvml','[email protected]');>> wrote: > >> 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] >> <javascript:_e(%7B%7D,'cvml','[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] >> <javascript:_e(%7B%7D,'cvml','[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] >> <javascript:_e(%7B%7D,'cvml','[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] >> <javascript:_e(%7B%7D,'cvml','[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] >> <javascript:_e(%7B%7D,'cvml','[email protected]');>> >> >> >> wrote: >> >> >>> >> >> >>> Updated to reflect Nightstrike's feedback >> >> >>> >> >> >>> On Wed, Nov 4, 2015 at 2:00 PM, NightStrike <[email protected] >> <javascript:_e(%7B%7D,'cvml','[email protected]');>> >> >> >>> wrote: >> >> >>>> >> >> >>>> Use AM_CONDITIONAL, not DEFINE_UNQUOTED >> >> >>>> >> >> >>>> On Wed, Nov 4, 2015 at 1:56 PM, Martell Malone >> >> >>>> <[email protected] >> <javascript:_e(%7B%7D,'cvml','[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] >> <javascript:_e(%7B%7D,'cvml','[email protected]');> >> >> >>>> > https://lists.sourceforge.net/lists/listinfo/mingw-w64-public >> >> >>>> > >> >> >>>> >> >> >>>> >> >> >>>> >> >> >>>> >> ------------------------------------------------------------------------------ >> >> >>>> _______________________________________________ >> >> >>>> Mingw-w64-public mailing list >> >> >>>> [email protected] >> <javascript:_e(%7B%7D,'cvml','[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] >> <javascript:_e(%7B%7D,'cvml','[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] >> <javascript:_e(%7B%7D,'cvml','[email protected]');> >> >> https://lists.sourceforge.net/lists/listinfo/mingw-w64-public >> > >> > >> > >> > >> ------------------------------------------------------------------------------ >> > >> > _______________________________________________ >> > Mingw-w64-public mailing list >> > [email protected] >> <javascript:_e(%7B%7D,'cvml','[email protected]');> >> > https://lists.sourceforge.net/lists/listinfo/mingw-w64-public >> > >> >> >> ------------------------------------------------------------------------------ >> _______________________________________________ >> Mingw-w64-public mailing list >> [email protected] >> <javascript:_e(%7B%7D,'cvml','[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
