[Mingw-w64-public] Fix def file generation during parallel build

2015-11-14 Thread JonY
Patch OK?

diff --git a/mingw-w64-crt/Makefile.am b/mingw-w64-crt/Makefile.am
index a137750..48bdfd3 100644
--- a/mingw-w64-crt/Makefile.am
+++ b/mingw-w64-crt/Makefile.am
@@ -1416,7 +1416,7 @@ endif
 # ##

 %.def: %.def.in
-   $(CPP) -x c $< -Wp,-w -I$(top_srcdir)/def-include | $(SED) 's/^#/;/' >$@
+   $(MKDIR_P) $(@D) && $(CPP) -x c $< -Wp,-w -I$(top_srcdir)/def-include | 
$(SED) 's/^#/;/' > $@

 # Don't compile these, but install as is:
 #


0xD4EBC740.asc
Description: application/pgp-keys


signature.asc
Description: OpenPGP digital signature
--
___
Mingw-w64-public mailing list
Mingw-w64-public@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/mingw-w64-public


Re: [Mingw-w64-public] PATCH for autootools to use genlib

2015-11-14 Thread NightStrike
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 variables
>> there.  Move:
>> +  AM_DLLTOOLFLAGS=-o $@
>> and its twin into the respective conditional paths.  Move:
>>  if DELAY_IMPORT_LIBS
>>AM_DLLTOOLFLAGS += --output-delaylib $@.delayimp.a
>>  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 
> 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 $@.delayimp.a
>>  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 
>> wrote:
>> > 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_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 
>> 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
>> >> >  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
>> >> >> 
>> >> >> wrote:
>> >> >>>
>> >> >>> 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 for help on that :)
>> >>  >
>> >>  >
>> >>  >
>> >>  >
>> >>  >
>> --
>> >>  >
>> >>  > ___
>> >>  > Mingw-w64-public mailing list
>> >>  > Mingw-w64-public@lists.sourceforge.net
>> >>  > https://lists.sourceforge.net/lists/listinfo/mingw-w64-public
>> >>  >
>> >> 
>> >> 
>> >> 
>> >> 
>> --
>> >>  ___
>> >>  Mingw-w64-public mailing list
>> >>  Mingw-w64-public@lists.sourceforge.net
>> >>  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=/4140
>> >> >>