Make Setup.hs configure more CWD-independent. #5397
Conversation
|
Just for reference, we have merged #4857, #4859, #4874, #4875, #4885, #4891, which were all intended to reduce the global CWD in cabal, and essential to allowing hadrian to run cabal functions in parallel without stepping onto each others toes due to the global CWD. The overall idea was to be able to represent all packages that constitute ghc in cabal*, and try to make out-of-tree builds possible, so that multiple configurations can be built from the same source tree at the same time; and allow to build relocatable ghcs (without the need for wrapper scripts) with hadrian for windows, linux and macOS. |
Previously, we were checking the package with a hard-coded root directory of ".". This was not a problem before, but with haskell#5372 we have started to expand globs while checking packages, which breaks if the CWD is not the directory containing the `.cabal` file and causes snowleopard/hadrian#634. Luckily, this is an easy fix: the correct directory is easy to determine. Writing a test and making sure it's tickling the failing case took longer than writing the fix! "." is hard-coded as the root directory passed to `checkPackageFiles` in a few other places, but those are (a) non-trivial to test, and (b) already in places that have other assumptions about their CWD, so I have simply documented the CWD requirement for those.
quasicomputational
force-pushed
the
cwd-independent-check
branch
from
f3b612e to
7f0e066
Compare
|
I managed to build a stage-1 GHC with Hadrian with this patch, and @alpmestan got through a whole build with it; I've squashed it down for neatness and it's ready for review. |
|
The failing AppVeyor test is that annoying flaky one. If you grep for 'Hadrian' in the log, you'll see that the added test is passing. Travis's 7.10 builders failed because 7.10 is fussy about some unused imports; that was from master, not related to this PR. |
|
OK, this has been up for a little while and it is quite a simple fix; the test failures aren't relevant. The Hadrian people have been using this PR in their CI set-up and they haven't reported any breakage. Since it has been causing some pain for Hadrian I am going to go ahead and merge. |
Previously, we were checking the package with a hard-coded root
directory of ".". This was not a problem before, but with #5372 we
have started to expand globs while checking packages, which breaks if
the CWD is not the directory containing the
.cabalfile and causessnowleopard/hadrian#634.
Luckily, this is an easy fix: the correct directory is easy to
determine. Writing a test and making sure it's tickling the failing
case took longer than writing the fix!
"." is hard-coded as the root directory passed to
checkPackageFilesin a few other places, but those are (a) non-trivial to test, and (b)
already in places that have other assumptions about their CWD, so I
have simply documented the CWD requirement for those.
Please include the following checklist in your PR:
[ci skip]is used to avoid triggering the build bots.Please also shortly describe how you tested your change. Bonus points for added tests!
New test added, made sure it tickled the bad case and now it passes. Currently building GHC with Hadrian to verify the failure; will ensure this patch fixes it.