[prev in list] [next in list] [prev in thread] [next in thread]
List: cfe-dev
Subject: Re: [cfe-dev] Merging MinGW toolchain patches to 3.7
From: Richard Smith <richard () metafoo ! co ! uk>
Date: 2015-07-27 20:27:20
Message-ID: CAOfiQqkEmy-Xpp+7Jh5UUP6oksSvdxr3rw3FNdkenvaSky0YCw () mail ! gmail ! com
[Download RAW message or body]
[Attachment #2 (multipart/alternative)]
On Mon, Jul 27, 2015 at 9:47 AM, Hans Wennborg <hans@chromium.org> wrote:
> For my own notes...
>
> On Fri, Jul 24, 2015 at 11:33 PM, Yaron Keren <yaron.keren@gmail.com>
> wrote:
> > I committed these patches to the mingw toolchain:
>
> Committed before 3.7 branch-point:
>
> > r241241 - Support mingw-w64 and mingw.org toolchains at any install
> > location.
> > r241432 - Teach mingw toolchain the msys2 mingw-w64 distribution C++
> dirs.
> > r241485 - Support -pthread in mingw toolchain.
> > r242121 - Add support for -fuse-ld= in the mingw toolchain driver.
> > r242150 - Support mingw-w64 toolchain on openSUSE and likely other
> Linuxes
>
> Already merged:
>
> > r242660 - Support mingw toolchain include and lib directories on Arch
> > Linux.
> > r242662 - Fix typo found by Joerg Sonnenberger.
> > r242667 - Remove erroneous space in "lib64" string constant.
> > r242766 - Fix mingw toolchain to honor sysroot on Linux and add tests.
>
> Not merged yet; waiting for owner:
>
> > r243098 - Add extensive tests for the mingw toolchain and remove trailing
> > slash from Arch.
> > r243101 - Try to appease clang buildbot by forcing libstdc++ in mingw.cpp
> > test.
> > r243105 - Apparently some of the bots add .svn dirs inside the
> > test/Driver/Inputs
> > r243144 - Base the sys-root/mingw/include path on sysroot and not on /usr
> > r243153 - Select the highest version of the mingw toolchain found using
> > Generic_GCC::GCCVersion
>
Thanks for the summary, approved for branch.
> Already merged:
>
> > Reid Kleckner also committed:
> > r242905 - [mingw] Add support for -rtlib option and -stdlib option to the
> > mingw driver
> >
> >
> > 2015-07-25 1:21 GMT+03:00 Richard Smith <richard@metafoo.co.uk>:
> >>
> >> On Fri, Jul 24, 2015 at 7:08 AM, Yaron Keren <yaron.keren@gmail.com>
> >> wrote:
> >>>
> >>> Hi Richard,
> >>>
> >>> Both issues implemented in r243098, r243101, r243105 per your
> >>> suggestions.
> >>
> >>
> >> Yaron, can you give a complete list of the additional revisions that
> >> should be integrated to the branch, including today's changes? Thanks!
> >>
> >>>
> >>> It may be of general interest that two builders:
> >>> sanitizer-x86_64-linux-fast and clang-x86_64-ubuntu-gdb-75 add .svn
> entries
> >>> in some (all?) directories which may cause code that does not expect
> them
> >>> there to malfunction.
> >>>
> >>> Yaron
> >>>
> >>>
> >>> 2015-07-24 2:25 GMT+03:00 Richard Smith <richard@metafoo.co.uk>:
> >>>>
> >>>> On Thu, Jul 23, 2015 at 2:56 PM, Yaron Keren <yaron.keren@gmail.com>
> >>>> wrote:
> >>>>>
> >>>>> Hi Richard,
> >>>>>
> >>>>> r242660 + r242666 + r242667 are the current mingw toolchain, should
> be
> >>>>> tested together.
> >>>>> The code test covarage is not complete:
> >>>>>
> >>>>> 1) To make the toolchain work automatically, the first stage is
> >>>>> detection of the mingw installation dir in MinGW::findGccLibDir().
> This part
> >>>>> is not covered by any tests as it probes several file system
> directories
> >>>>> outside the LLVM tree using llvm::sys::fs::directory_iterator, we
> can't
> >>>>> control that using test infrastructure.
> >>>>
> >>>>
> >>>> Please take a look at how other targets handle this (see the various
> >>>> sysroot trees under test/Driver/Inputs). If the same approach can't
> work
> >>>> here, we should figure out how to fix that. The MinGW driver support
> should
> >>>> be testable without having a MinGW toolchain installed, otherwise the
> cost
> >>>> imposed on people maintaining the driver and Clang in general is too
> high.
> >>>>
> >>>>> The most reliable way to test this toolchain would be to have a mingw
> >>>>> toolchain installed on various bots and run tests using it. I'm open
> to
> >>>>> suggestions how to test MinGW::findGccLibDir() otherwise.
> >>>>>
> >>>>> 2) The second stage generates the C and C++ include directories based
> >>>>> on GccLibDir found in stage one. This *is* covered by the tests
> introduced
> >>>>> in r242666 by injecting GccLibDir with --sysroot argument (instead of
> >>>>> probing the file system) for both Windows and Linux.
> >>>>>
> >>>>> MinGW::Arch has a trainling slash as a small optimization so Arch
> could
> >>>>> be concatenated into a path instead of using sys::path::append. If
> you find
> >>>>> that confusing I can remove the trailing slash and use
> sys::path::append
> >>>>> calls instead, it would not matter much.
> >>>>
> >>>>
> >>>> There should at least be a comment explaining this in the class
> >>>> definition. But generally, using the higher-level facilities such as
> >>>> sys::path::append should be preferred over manually inserting
> separators
> >>>> into paths, so I'd prefer you did that. Performance
> microoptimizations are
> >>>> not especially relevant for the driver, since it's such a tiny
> proportion of
> >>>> the cost of a compilation.
> >>>>
> >>>>>
> >>>>> Yaron
> >>>>>
> >>>>>
> >>>>>
> >>>>>
> >>>>> 2015-07-24 0:09 GMT+03:00 Richard Smith <richard@metafoo.co.uk>:
> >>>>>>
> >>>>>> On Thu, Jul 23, 2015 at 12:30 PM, Richard Smith <metafoo@gmail.com>
> >>>>>> wrote:
> >>>>>>>
> >>>>>>> Yes, we should get these changes into 3.7. I'll look over the
> patches
> >>>>>>> later today.
> >>>>>>
> >>>>>> I'm not very happy about applying patches with no tests to the
> branch,
> >>>>>> but, reluctantly, approved.
> >>>>>>
> >>>>>> Yaron: please add some tests for r242660 and r242667 to trunk. (I
> also
> >>>>>> find it very weird that the MinGW::Arch member has a trailing
> slash. Is
> >>>>>> there a good reason for that?)
> >>>>>>>
> >>>>>>> On 23 Jul 2015 9:56 am, "Hans Wennborg" <hans@chromium.org> wrote:
> >>>>>>>>
> >>>>>>>> +cfe-dev to keep it on the list, and Richard
> >>>>>>>>
> >>>>>>>> This sounds like a reasonable thing to merge.
> >>>>>>>>
> >>>>>>>> Richard, what's your opinion as an owner?
> >>>>>>>>
> >>>>>>>>
> >>>>>>>> On Thu, Jul 23, 2015 at 9:48 AM, Martell Malone
> >>>>>>>> <martellmalone@gmail.com> wrote:
> >>>>>>>> > I almost forgot
> >>>>>>>> >
> >>>>>>>> > Yaron also fixed all the different combination of linux
> >>>>>>>> > host/distros
> >>>>>>>> > targetting mingw with theses 3.
> >>>>>>>> > I have a feeling we will have people hitting the mailing list
> >>>>>>>> > about come the
> >>>>>>>> > 3.7 release if their distro isn't working.
> >>>>>>>> >
> >>>>>>>> > clang
> >>>>>>>> >
> >>>>>>>> > r242660 Support mingw toolchain include and lib directories on
> >>>>>>>> > Arch Linux.
> >>>>>>>> > r242667 Remove erroneous space in "lib64" string constant.
> >>>>>>>> > r242766 Fix mingw toolchain to honor sysroot on Linux and add
> >>>>>>>> > tests.
> >>>>>>>> >
> >>>>>>>> > Again as i said I've no idea what the process is or what we can
> do
> >>>>>>>> > here.
> >>>>>>>> > Hoping you can fill me in on the details
> >>>>>>>> >
> >>>>>>>> > Kind Regards
> >>>>>>>> > Martell
> >>>>>>>> >
> >>>>>>>> >
> >>>>>>>> >
> >>>>>>>> > On Thu, Jul 23, 2015 at 5:44 PM, Martell Malone
> >>>>>>>> > <martellmalone@gmail.com>
> >>>>>>>>
> >>>>>>>> > wrote:
> >>>>>>>> >>
> >>>>>>>> >> Hi Hans :)
> >>>>>>>> >>
> >>>>>>>> >> rnk and I were talking to me about the possibility of getting
> the
> >>>>>>>> >> mingw
> >>>>>>>> >> driver work back ported into the 3.7 branch.
> >>>>>>>> >>
> >>>>>>>> >> I read the thread where you branched the 3.7 release you said
> >>>>>>>> >> that if
> >>>>>>>> >> anyone has a request to have patches merged they should contact
> >>>>>>>> >> you directly
> >>>>>>>> >>
> >>>>>>>> >> When the branch was made we were in the middle of crushing the
> >>>>>>>> >> final bugs
> >>>>>>>> >> in the MINGW Driver.
> >>>>>>>> >> I had a total of about 9 patches merged but 2 missed the window
> >>>>>>>> >> unfortunately.
> >>>>>>>> >> 1 was for compiler-rt and 1 was for clang
> >>>>>>>> >>
> >>>>>>>> >> clang
> >>>>>>>> >> r242905 Add support for -rtlib option and -stdlib option to
> the
> >>>>>>>> >> mingw
> >>>>>>>> >> driver
> >>>>>>>> >>
> >>>>>>>> >> compiler-rt
> >>>>>>>> >> r242539 compiler-rt: add support for mingw-w64 in builtins
> >>>>>>>> >> r242540 Add missing chkstk.S files from r242539
> >>>>>>>> >>
> >>>>>>>> >> (r242540 is here because only half the patch was committed
> >>>>>>>> >> initially)
> >>>>>>>> >>
> >>>>>>>> >> These 2(now 3) patches complete my series and fixes everything
> to
> >>>>>>>> >> have a
> >>>>>>>> >> clang toolchain with mingw-w64 without needing gcc.
> >>>>>>>> >>
> >>>>>>>> >> I don't know what the process is for back porting or how you
> feel
> >>>>>>>> >> about
> >>>>>>>> >> these patches but I said I would email you about it an see
> where
> >>>>>>>> >> we can go
> >>>>>>>> >> from here :)
> >>>>>>>> >>
> >>>>>>>> >> Kind Regards
> >>>>>>>> >> Martell
> >>>>>>>> >
> >>>>>>>> >
> >>>>>>
> >>>>>>
> >>>>
> >>
> >
>
[Attachment #5 (text/html)]
<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Mon, \
Jul 27, 2015 at 9:47 AM, Hans Wennborg <span dir="ltr"><<a \
href="mailto:hans@chromium.org" \
target="_blank">hans@chromium.org</a>></span> wrote:<br><blockquote \
class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc \
solid;padding-left:1ex">For my own notes...<br> <span class=""><br>
On Fri, Jul 24, 2015 at 11:33 PM, Yaron Keren <<a \
href="mailto:yaron.keren@gmail.com">yaron.keren@gmail.com</a>> \
wrote:<br> > I committed these patches to the mingw toolchain:<br>
<br>
</span>Committed before 3.7 branch-point:<br>
<br>
> r241241 - Support mingw-w64 and <a \
href="https://urldefense.proofpoint.com/v2/url?u=http-3A__mingw.org&d=AwMFaQ \
&c=8hUWFZcy2Z-Za5rBPlktOQ&r=CnzuN65ENJ1H9py9XLiRvC_UQz6u3oG6GUNn7_wosSM&m=52 \
l9Zv15iRovoYHh3_gXY2KODNrvNg1B9_tBDl4KQsc&s=EWZcxXoJ-ntyUKnC9Pnd_D9Qj4EjZuZFUvHVbYdCXLg&e=" \
rel="noreferrer" target="_blank">mingw.org</a> toolchains at any \
install<br> <span class="">> location.<br>
> r241432 - Teach mingw toolchain the msys2 mingw-w64 distribution C++ \
dirs.<br> > r241485 - Support -pthread in mingw toolchain.<br>
> r242121 - Add support for -fuse-ld= in the mingw toolchain driver.<br>
> r242150 - Support mingw-w64 toolchain on openSUSE and likely other \
Linuxes<br> <br>
</span>Already merged:<br>
<span class=""><br>
> r242660 - Support mingw toolchain include and lib directories on \
Arch<br> > Linux.<br>
> r242662 - Fix typo found by Joerg Sonnenberger.<br>
> r242667 - Remove erroneous space in "lib64" string \
constant.<br> > r242766 - Fix mingw toolchain to honor sysroot on Linux \
and add tests.<br> <br>
</span>Not merged yet; waiting for owner:<br>
<span class=""><br>
> r243098 - Add extensive tests for the mingw toolchain and remove \
trailing<br> > slash from Arch.<br>
> r243101 - Try to appease clang buildbot by forcing libstdc++ in \
mingw.cpp<br> > test.<br>
> r243105 - Apparently some of the bots add .svn dirs inside the<br>
> test/Driver/Inputs<br>
> r243144 - Base the sys-root/mingw/include path on sysroot and not on \
/usr<br> > r243153 - Select the highest version of the mingw toolchain \
found using<br> > \
Generic_GCC::GCCVersion<br></span></blockquote><div><br></div><div>Thanks \
for the summary, approved for branch.</div><div> </div><blockquote \
class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc \
solid;padding-left:1ex"><span class=""> </span>Already merged:<br>
<span class=""><br>
> Reid Kleckner also committed:<br>
> r242905 - [mingw] Add support for -rtlib option and -stdlib option to \
the<br> > mingw driver<br>
><br>
><br>
</span>> 2015-07-25 1:21 GMT+03:00 Richard Smith <<a \
href="mailto:richard@metafoo.co.uk">richard@metafoo.co.uk</a>>:<br> \
>><br> >> On Fri, Jul 24, 2015 at 7:08 AM, Yaron Keren <<a \
href="mailto:yaron.keren@gmail.com">yaron.keren@gmail.com</a>><br> <span \
class="">>> wrote:<br> >>><br>
>>> Hi Richard,<br>
>>><br>
>>> Both issues implemented in r243098, r243101, r243105 per \
your<br> >>> suggestions.<br>
>><br>
>><br>
>> Yaron, can you give a complete list of the additional revisions \
that<br> >> should be integrated to the branch, including today's \
changes? Thanks!<br> >><br>
>>><br>
>>> It may be of general interest that two builders:<br>
>>> sanitizer-x86_64-linux-fast and clang-x86_64-ubuntu-gdb-75 add \
.svn entries<br> >>> in some (all?) directories which may cause \
code that does not expect them<br> >>> there to malfunction.<br>
>>><br>
>>> Yaron<br>
>>><br>
>>><br>
</span>>>> 2015-07-24 2:25 GMT+03:00 Richard Smith <<a \
href="mailto:richard@metafoo.co.uk">richard@metafoo.co.uk</a>>:<br> \
>>>><br> >>>> On Thu, Jul 23, 2015 at 2:56 PM, \
Yaron Keren <<a href="mailto:yaron.keren@gmail.com">yaron.keren@gmail.com</a>><br>
<div><div class="h5">>>>> wrote:<br>
>>>>><br>
>>>>> Hi Richard,<br>
>>>>><br>
>>>>> r242660 + r242666 + r242667 are the current mingw \
toolchain, should be<br> >>>>> tested together.<br>
>>>>> The code test covarage is not complete:<br>
>>>>><br>
>>>>> 1) To make the toolchain work automatically, the first \
stage is<br> >>>>> detection of the mingw installation dir \
in MinGW::findGccLibDir(). This part<br> >>>>> is not \
covered by any tests as it probes several file system directories<br> \
>>>>> outside the LLVM tree using \
llvm::sys::fs::directory_iterator, we can't<br> >>>>> \
control that using test infrastructure.<br> >>>><br>
>>>><br>
>>>> Please take a look at how other targets handle this (see \
the various<br> >>>> sysroot trees under test/Driver/Inputs). \
If the same approach can't work<br> >>>> here, we should \
figure out how to fix that. The MinGW driver support should<br> \
>>>> be testable without having a MinGW toolchain installed, \
otherwise the cost<br> >>>> imposed on people maintaining the \
driver and Clang in general is too high.<br> >>>><br>
>>>>> The most reliable way to test this toolchain would be \
to have a mingw<br> >>>>> toolchain installed on various \
bots and run tests using it. I'm open to<br> >>>>> \
suggestions how to test MinGW::findGccLibDir() otherwise.<br> \
>>>>><br> >>>>> 2) The second stage generates \
the C and C++ include directories based<br> >>>>> on \
GccLibDir found in stage one. This *is* covered by the tests introduced<br> \
>>>>> in r242666 by injecting GccLibDir with --sysroot \
argument (instead of<br> >>>>> probing the file system) for \
both Windows and Linux.<br> >>>>><br>
>>>>> MinGW::Arch has a trainling slash as a small \
optimization so Arch could<br> >>>>> be concatenated into a \
path instead of using sys::path::append. If you find<br> \
>>>>> that confusing I can remove the trailing slash and use \
sys::path::append<br> >>>>> calls instead, it would not \
matter much.<br> >>>><br>
>>>><br>
>>>> There should at least be a comment explaining this in the \
class<br> >>>> definition. But generally, using the \
higher-level facilities such as<br> >>>> sys::path::append \
should be preferred over manually inserting separators<br> >>>> \
into paths, so I'd prefer you did that. Performance microoptimizations \
are<br> >>>> not especially relevant for the driver, since \
it's such a tiny proportion of<br> >>>> the cost of a \
compilation.<br> >>>><br>
>>>>><br>
>>>>> Yaron<br>
>>>>><br>
>>>>><br>
>>>>><br>
>>>>><br>
</div></div>>>>>> 2015-07-24 0:09 GMT+03:00 Richard Smith \
<<a href="mailto:richard@metafoo.co.uk">richard@metafoo.co.uk</a>>:<br>
>>>>>><br>
>>>>>> On Thu, Jul 23, 2015 at 12:30 PM, Richard Smith \
<<a href="mailto:metafoo@gmail.com">metafoo@gmail.com</a>><br> <span \
class="im HOEnZb">>>>>>> wrote:<br> \
>>>>>>><br> >>>>>>> Yes, we \
should get these changes into 3.7. I'll look over the patches<br> \
>>>>>>> later today.<br> >>>>>><br>
>>>>>> I'm not very happy about applying patches with \
no tests to the branch,<br> >>>>>> but, reluctantly, \
approved.<br> >>>>>><br>
>>>>>> Yaron: please add some tests for r242660 and \
r242667 to trunk. (I also<br> >>>>>> find it very weird \
that the MinGW::Arch member has a trailing slash. Is<br> \
>>>>>> there a good reason for that?)<br> \
>>>>>>><br> </span><span class="im \
HOEnZb">>>>>>>> On 23 Jul 2015 9:56 am, "Hans \
Wennborg" <<a \
href="mailto:hans@chromium.org">hans@chromium.org</a>> wrote:<br> \
>>>>>>>><br> >>>>>>>> \
+cfe-dev to keep it on the list, and Richard<br> \
>>>>>>>><br> >>>>>>>> This \
sounds like a reasonable thing to merge.<br> \
>>>>>>>><br> >>>>>>>> \
Richard, what's your opinion as an owner?<br> \
>>>>>>>><br> >>>>>>>><br>
>>>>>>>> On Thu, Jul 23, 2015 at 9:48 AM, Martell \
Malone<br> </span><span class="im HOEnZb">>>>>>>>> \
<<a href="mailto:martellmalone@gmail.com">martellmalone@gmail.com</a>> \
wrote:<br> >>>>>>>> > I almost forgot<br>
>>>>>>>> ><br>
>>>>>>>> > Yaron also fixed all the different \
combination of linux<br> >>>>>>>> > \
host/distros<br> >>>>>>>> > targetting mingw \
with theses 3.<br> >>>>>>>> > I have a feeling \
we will have people hitting the mailing list<br> \
>>>>>>>> > about come the<br> \
>>>>>>>> > 3.7 release if their distro isn't \
working.<br> >>>>>>>> ><br>
>>>>>>>> > clang<br>
>>>>>>>> ><br>
>>>>>>>> > r242660 Support mingw toolchain \
include and lib directories on<br> >>>>>>>> > \
Arch Linux.<br> >>>>>>>> > r242667 Remove \
erroneous space in "lib64" string constant.<br> \
>>>>>>>> > r242766 Fix mingw toolchain to \
honor sysroot on Linux and add<br> >>>>>>>> > \
tests.<br> >>>>>>>> ><br>
>>>>>>>> > Again as i said I've no idea what \
the process is or what we can do<br> >>>>>>>> > \
here.<br> >>>>>>>> > Hoping you can fill me in \
on the details<br> >>>>>>>> ><br>
>>>>>>>> > Kind Regards<br>
>>>>>>>> > Martell<br>
>>>>>>>> ><br>
>>>>>>>> ><br>
>>>>>>>> ><br>
>>>>>>>> > On Thu, Jul 23, 2015 at 5:44 PM, \
Martell Malone<br> </span><div class="HOEnZb"><div \
class="h5">>>>>>>>> > <<a \
href="mailto:martellmalone@gmail.com">martellmalone@gmail.com</a>><br> \
>>>>>>>><br> >>>>>>>> > \
wrote:<br> >>>>>>>> >><br>
>>>>>>>> >> Hi Hans :)<br>
>>>>>>>> >><br>
>>>>>>>> >> rnk and I were talking to me \
about the possibility of getting the<br> >>>>>>>> \
>> mingw<br> >>>>>>>> >> driver work \
back ported into the 3.7 branch.<br> >>>>>>>> \
>><br> >>>>>>>> >> I read the thread \
where you branched the 3.7 release you said<br> \
>>>>>>>> >> that if<br> \
>>>>>>>> >> anyone has a request to have \
patches merged they should contact<br> >>>>>>>> \
>> you directly<br> >>>>>>>> >><br>
>>>>>>>> >> When the branch was made we were \
in the middle of crushing the<br> >>>>>>>> >> \
final bugs<br> >>>>>>>> >> in the MINGW \
Driver.<br> >>>>>>>> >> I had a total of \
about 9 patches merged but 2 missed the window<br> \
>>>>>>>> >> unfortunately.<br> \
>>>>>>>> >> 1 was for compiler-rt and 1 was \
for clang<br> >>>>>>>> >><br>
>>>>>>>> >> clang<br>
>>>>>>>> >> r242905 Add support for -rtlib \
option and -stdlib option to the<br> >>>>>>>> \
>> mingw<br> >>>>>>>> >> driver<br>
>>>>>>>> >><br>
>>>>>>>> >> compiler-rt<br>
>>>>>>>> >> r242539 compiler-rt: add \
support for mingw-w64 in builtins<br> >>>>>>>> \
>> r242540 Add missing chkstk.S files from r242539<br> \
>>>>>>>> >><br> \
>>>>>>>> >> (r242540 is here because only \
half the patch was committed<br> >>>>>>>> >> \
initially)<br> >>>>>>>> >><br>
>>>>>>>> >> These 2(now 3) patches complete \
my series and fixes everything to<br> >>>>>>>> \
>> have a<br> >>>>>>>> >> clang \
toolchain with mingw-w64 without needing gcc.<br> \
>>>>>>>> >><br> \
>>>>>>>> >> I don't know what the process \
is for back porting or how you feel<br> >>>>>>>> \
>> about<br> >>>>>>>> >> these patches \
but I said I would email you about it an see where<br> \
>>>>>>>> >> we can go<br> \
>>>>>>>> >> from here :)<br> \
>>>>>>>> >><br> \
>>>>>>>> >> Kind Regards<br> \
>>>>>>>> >> Martell<br> \
>>>>>>>> ><br> >>>>>>>> \
><br> >>>>>><br>
>>>>>><br>
>>>><br>
>><br>
><br>
</div></div></blockquote></div><br></div></div>
_______________________________________________
cfe-dev mailing list
cfe-dev@cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic