Fix Windows build without --enable-distro-toolchain #564
Comments
|
The error is thrown from here: https://github.com/snowleopard/hadrian/blob/master/src/Hadrian/Haskell/Cabal/Parse.hs#L158 @alpmestan @angerman Any ideas how to fix this? Somehow, the path to Here is the list of arguments passed to |
|
@snowleopard Hmmm... I'm really confused. Given those 2 options from [...]
"--configure-option=--with-cc=C:/msys/home/nam83/ghc/inplace/mingw/bin/gcc.exe",
[...]
"--with-gcc=C:/msys/home/nam83/ghc/inplace/mingw/bin/gcc.exe",
[...]which seem just fine... why/how do we end up with Can you confirm that the path to gcc that we end up mentionning twice in |
|
@alpmestan Yes, the path in Could this be responsible: |
|
Hmm, maybe? Sounds a bit odd though, why would the install path ( |
|
@alpmestan Thanks for the suggestion to try As you can see we are calling Stage1 GHC that we built, and I think it's got an incorrect built-it path to GCC. So, the real issue is that Stage1 GHC is broken -- it cannot compile C files. Any ideas how to fix this? |
|
Just confirmed: |
|
@snowleopard Could you share the contents of your |
|
And maybe the output of |
|
@alpmestan Sure! Configuration: GHC info: As you can see, we have lots of incorrect paths there, including the cause of this issue: |
|
The configuration path to GCC on the other hand is correct: |
|
The top-level |
|
I think I figured out the problem: when GHC was built in the |
|
Hmm... That sounds like a very plausible explanation. In so... should we augment the logic that produces that value, to cover the case where we don't build inplace? I'm not sure what the additional logic should do though. The current one is implemented in I'm not entirely sure how we can say "use the C toolchain at |
|
Oh, I see you're trying to use the "enable the distro toolchain" knob, that should be even simpler =) |
|
@alpmestan Yep, let's see if it works :) Currently building on my machine too. |
|
Ha, it worked! We now hit the |
|
@Mistuke Thanks for implementing |
snowleopard
changed the title
|
@snowleopard you're correct in that the issue is happening because of the layout change. SysTools has a hard assumption about the layout of GHC's binaries. In particular it assumes: But now the layout is different, not just for the build but in the resulting tarball if you were to package up ghc. Have you thought about what the layout should be in the final binary distribution. Once you know that Also be careful with copying or changing |
@Mistuke No, I haven't thought of this and I don't think I have enough knowledge to make the right call. Could you please advise? Naively I'd suggest something along the lines @alpmestan mentioned: simply copy |
|
Sure, I'll have to take a closer look at the out of tree layout Hadrian
uses before I can comment. I'll take a look tomorrow. :)
…On Thu, Apr 12, 2018, 00:12 Andrey Mokhov ***@***.***> wrote:
Have you thought about what the layout should be in the final binary
distribution
@Mistuke <https://github.com/Mistuke> No, I haven't thought of this and I
don't think I have enough knowledge to make the right call. Could you
please advise?
Naively I'd suggest something along the lines @alpmestan
<https://github.com/alpmestan> mentioned: simply copy mingw from inplace
into the build directory, but your comment suggests that this may be not
the best solution. What are the alternatives?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#564 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ABH3KTiFW_owU_QfAX-xfy6ZGQWLpbYMks5tno3egaJpZM4TNgKS>
.
|
|
Great, thanks @Mistuke! |
This patch affects several files that affect how we detect mingw and perl
on Windows. The initial motivation is:
snowleopard/hadrian#564
where, with Hadrian building relocatable (non-inplace) GHCs, the current
detection mechanism falls short by e.g only trying $topdir/../mingw. But
in Hadrian, for reasons given in that issue, we would need to store e.g mingw
under $topdir/../../mingw except for binary distributions, where we want
to follow the existing structure, in which case $topdir/../mingw is correct. So
we need to support both, which is what this patch hopefully implements.
|
@Mistuke @snowleopard How does this look: alpmestan/ghc@f429ee8 ? Andrey, could you perhaps try out this patch? I intend to give it a spin in a Windows VM as soon as I can but I have to run away from my laptop now. |
|
To be clear, this would require |
|
Thanks Alp! I've added some feedback.
…On Fri, Apr 13, 2018, 17:45 Alp Mestanogullari ***@***.***> wrote:
To be clear, this would require hadrian to place mingw/perl under <build
root>/{mingw, perl}, except in the bindists I suppose.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#564 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ABH3KWhd6eGYvO31H7zK-g06Ndhc_T44ks5toNY1gaJpZM4TNgKS>
.
|
This patch affects several files that affect how we detect mingw and perl
on Windows. The initial motivation is:
snowleopard/hadrian#564
where, with Hadrian building relocatable (non-inplace) GHCs, the current
detection mechanism falls short by e.g only trying $topdir/../mingw. But
in Hadrian, for reasons given in that issue, we would need to store e.g mingw
under $topdir/../../mingw except for binary distributions, where we want
to follow the existing structure, in which case $topdir/../mingw is correct. So
we need to support both, which is what this patch hopefully implements.
|
@Mistuke Wow... sorry for the silly mistakes. I also refactored @snowleopard Could you perhaps try it out whenever you get a chance to? |
|
@alpmestan Thank you, this looks good to me! Shall we also do anything about the last occurrence of |
|
Oh, I didn't change this one but please do let me know if you'd like me to change how the path to |
This patch affects several files that affect how we detect mingw and perl
on Windows. The initial motivation is:
snowleopard/hadrian#564
where, with Hadrian building relocatable (non-inplace) GHCs, the current
detection mechanism falls short by e.g only trying $topdir/../mingw. But
in Hadrian, for reasons given in that issue, we would need to store e.g mingw
under $topdir/../../mingw except for binary distributions, where we want
to follow the existing structure, in which case $topdir/../mingw is correct. So
we need to support both, which is what this patch hopefully implements.
Summary:
This patch affects several files that affect how we detect mingw and perl
on Windows. The initial motivation is:
snowleopard/hadrian#564
where, with Hadrian building relocatable (non-inplace) GHCs, the current
detection mechanism falls short by e.g only trying $topdir/../mingw. But
in Hadrian, for reasons given in that issue, we would need to store e.g mingw
under $topdir/../../mingw except for binary distributions, where we want
to follow the existing structure, in which case $topdir/../mingw is correct. So
we need to support both, which is what this patch hopefully implements.
Test Plan: ./validate
Reviewers: Phyx
Subscribers:
GHC Trac Issues:
Summary:
This patch affects several files that affect how we detect mingw and perl
on Windows. The initial motivation is:
snowleopard/hadrian#564
where, with Hadrian building relocatable (non-inplace) GHCs, the current
detection mechanism falls short by e.g only trying $topdir/../mingw. But
in Hadrian, for reasons given in that issue, we would need to store e.g mingw
under $topdir/../../mingw except for binary distributions, where we want
to follow the existing structure, in which case $topdir/../mingw is correct. So
we need to support both, which is what this patch hopefully implements.
Test Plan: ./validate
Reviewers: Phyx, hvr, bgamari, erikd
Subscribers: thomie, carter
Differential Revision: https://phabricator.haskell.org/D4598
|
Got a diff up on phabricator at https://phabricator.haskell.org/D4598 -- feedback/review welcome (if/once the build passes). |
|
Thanks @alpmestan! Hope it will soon be accepted. I think relocating |
|
Sounds good to me. GHC HEAD is however broken at the moment so CI errors out on something completely unrelated. And the diff won't be reviewable/mergeable until CI passes IIRC (due to a recent phabricator update). |
|
You can pick a working build hash from my CI and rebase to that
https://github.com/Mistuke/GhcWindowsBuild/commits/master
You can also manually request a review at the bottom which will take the
issue out of draft. However I'd like to see CI build before I accept it.
Codewise it looks fine though.
…On Mon, Apr 16, 2018, 11:44 Alp Mestanogullari ***@***.***> wrote:
Sounds good to me. GHC HEAD is however broken at the moment so CI errors
out on something completely unrelated. And the diff won't be
reviewable/mergeable until CI passes IIRC.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#564 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ABH3Kbl6s6TcMw7mLE5YWeUW6oR0WVi0ks5tpHYZgaJpZM4TNgKS>
.
|
Yeah, me too! =) |
Summary:
This patch affects several files that affect how we detect mingw and perl
on Windows. The initial motivation is:
snowleopard/hadrian#564
where, with Hadrian building relocatable (non-inplace) GHCs, the current
detection mechanism falls short by e.g only trying $topdir/../mingw. But
in Hadrian, for reasons given in that issue, we would need to store e.g mingw
under $topdir/../../mingw except for binary distributions, where we want
to follow the existing structure, in which case $topdir/../mingw is correct. So
we need to support both, which is what this patch hopefully implements.
Test Plan: ./validate
Reviewers: Phyx, hvr, bgamari, erikd
Subscribers: thomie, carter
Differential Revision: https://phabricator.haskell.org/D4598
|
@snowleopard My patch passed GHC's CI and @Mistuke accepted it, so it'll land soon. As soon as it's in master, we should be able to get rid of the distro toolchain flag to configure. @Mistuke Thanks for being so responsive! |
|
@alpmestan @Mistuke Awesome, thank you both! |
Summary:
This patch affects several files that affect how we detect mingw and perl
on Windows. The initial motivation is:
snowleopard/hadrian#564
where, with Hadrian building relocatable (non-inplace) GHCs, the current
detection mechanism falls short by e.g only trying $topdir/../mingw. But
in Hadrian, for reasons given in that issue, we would need to store e.g mingw
under $topdir/../../mingw except for binary distributions, where we want
to follow the existing structure, in which case $topdir/../mingw is correct. So
we need to support both, which is what this patch hopefully implements.
Test Plan: ./validate
Reviewers: Phyx, hvr, bgamari, erikd
Reviewed By: Phyx
Subscribers: snowleopard, thomie, carter
Differential Revision: https://phabricator.haskell.org/D4598
|
No problem :) I pushed the commit as well. |
|
@snowleopard Do you want to try getting rid of |
|
@alpmestan I'm working on this -- now testing on my machine. |
|
The above commit works on my machine -- will close this if AppVeyor succeeds too. |
|
🎉 |
UPDATE: I fixed the issue below by passing
--enable-distro-toolchaintoconfigure: #565. We need to make it possible to build Hadrian without--enable-distro-toolchain, but this has lower priority.Now hitting this error on Windows:
The text was updated successfully, but these errors were encountered: