GHC environments (v2) #3936
GHC environments (v2) #3936
Conversation
|
@dcoutts, thanks for your PR! By analyzing the history of the files in this pull request, we identified @luite, @23Skidoo and @christiaanb to be potential reviewers. |
dcoutts
force-pushed
the
ghc-environments
branch
from
7c5c71c
to
eaf08b7
Compare
|
Updated post backpack merge |
| @@ -65,20 +66,26 @@ ghcVersionImplInfo ver = GhcImplInfo | |||
| , flagProfAuto = v >= [7,4] | |||
| , flagPackageConf = v < [7,5] | |||
| , flagDebugInfo = v >= [7,10] | |||
| , supportsPkgEnvFiles = v >= [8,0,2] -- broken in 8.0.1, fixed in 8.0.2 | |||
There was a problem hiding this comment.
Note: to test with current GHC 8.0.x branch you need to edit this to be
v > [8,0,1]
indeed, perhaps it should just be that anyway.
There was a problem hiding this comment.
@dcoutts if we know the snapshot date (== committer date of the respective commit) when it was fixed, we could say e.g.
v >= [8,0,1,20161002]That's the current date-stamped version of my ghc-8.0.2 ppa snapshot:
$ /opt/ghc/8.0.2/bin/ghc --numeric-version
8.0.1.20161002
$ /opt/ghc/8.0.2/bin/ghc --info | grep Git
,("Project Git commit id","a24092ff501028ca1245b508320493f394378495")
|
Testing appreciated. Requires a GHC 8.0.x branch from September or later. |
| @@ -730,6 +736,7 @@ buildOrReplLib forRepl verbosity numJobs pkg_descr lbi lib clbi = do | |||
| && ghcVersion < mkVersion [7,8] | |||
| then toFlag sharedLibInstallPath | |||
| else mempty, | |||
| ghcOptHideAllPackages = toFlag True, | |||
There was a problem hiding this comment.
...do we want to backport this?
| runProgram verbosity ghcProg ["-c", testcfile, | ||
| "-o", testofile] | ||
| runProgram verbosity ghcProg | ||
| [ "-hide-all-packages" |
There was a problem hiding this comment.
There is probably some history to this -hide-all-packages flag...
There was a problem hiding this comment.
is that a question?
There was a problem hiding this comment.
Yes; what happened that prompted you to add the flag here?
There was a problem hiding this comment.
Right, so the ghc behaviour with these environment files is that it uses them by default unless you're using -hide-all-packages to get a clean environment. There's a couple places in Cabal where we invoke ghc where we don't want to pick up any local environment. Most of the time doing so would be benign, but if your local environment file is borked then we still want to work, by ignoring it. So that's what these few patches do.
A test would look like making a .ghc.env file that has bogus entries and then use Cabal to do things. That's essentially the situation that provoked me to make these fixes.
There was a problem hiding this comment.
Don't tell me; write it in the code ;)
This new ghc feature was actually added in ghc 7.10 but very limited, then extended significantly in 8.0.1 but with a severe bug for our use case and then should be working fine in 8.0.2 onwards. This patch adds basic support for writing .ghc.environment files. This feature will be used later in cabal-install, but it makes sense for the functionality to live in Cabal next to the other code that knows all too much about GHC. .ghc.environment file support works in ghc 8.0.2 onwards It was actually added in ghc 7.10 but very limited, then extended significantly in 8.0.1 but with a severe bug for our use case and then should be working fine in 8.0.2 onwards.
When we invoke ghc in Cabal it's impostant that we control the environment. It's already the case that ghc ignores the .ghc.env files when we use -hide-all-packages, but there were a few places where we were not using this.
This is most of the way there but needs testing in practice with ghc-8.0.2.
Change the minimum version we use to decide if ghc supports .ghc.env files. Previously we declared that it required 8.0.2, but 8.0.2 is not out yet so this makes things hard to test. It was fixed in the 8.0.x branch at the end of August, so if ghc declares itself to be 8.0.1.$date from September or later, then it's ok.
|
I've rebased the PR branch; its conflicts where only because of the already merged #3920 |
|
@dcoutts Sure, will do tomorrow. |
New version of the ghc environments stuff, now based on the project build status infrastructure. This makes the implementation simpler and should also be much more accurate and useful.
What this really needs is testing with ghc-8.0.2. There's only so much I can do with looking at what the output would look like for older ghc versions.
Ignore the first patch, it's covered by PR #3920.