Fix #3483, set HASKELL_DIST_DIR #3697
Conversation
See https://ghc.haskell.org/trac/ghc/ticket/12494; we cargo-culted some incorrect code from base. Lift the restriction. Signed-off-by: Edward Z. Yang <ezyang@cs.stanford.edu>
|
LGTM so far. |
ezyang
force-pushed
the
pr/env-haskell-dist-dir
branch
from
0351e30 to
c1c01bc
Compare
| @@ -334,7 +335,8 @@ selfExecSetupMethod verbosity options _pkg bt mkargs = do | |||
|
|
|||
| searchpath <- programSearchPathAsPATHVar | |||
| (getProgramSearchPath (useProgramConfig options)) | |||
| env <- getEffectiveEnvironment [("PATH", Just searchpath)] | |||
There was a problem hiding this comment.
For this to work properly with the self-exec setup method we probably also want HASKELL_DIST_DIR=/foo/bar cabal $CMD to behave identically to cabal $CMD --builddir=/foo/bar.
There was a problem hiding this comment.
@23Skidoo I understand not—I'd have expected the outer cabal $CMD (say install) to override HASKELL_DIST_DIR (as it does); the change should be behavior-preserving, other than the extra variable in the environment.
There was a problem hiding this comment.
Yeah I don't understand @23Skidoo's comment either.
There was a problem hiding this comment.
FWIW, his idea would make sense if we wanted to propagate HASKELL_DIST_DIR automatically to any sub-cabal process—not what this PR is about.
In scenarios that do need that for whatever reason (assuming they exist, and maybe they do), scripts can/should use --builddir=$HASKELL_DIST_DIR.
There was a problem hiding this comment.
I retract my comment. For some reason I thought that act-as-setup --builddir is a thing and was worried about potential inconsistencies.
|
For people who want this PR, would it be possible for you to (1) test and see if it works for you, (2) help add some tests for this PR? (Probably integration-tests) Thanks. |
| @@ -304,7 +304,8 @@ internalSetupMethod verbosity options _ bt mkargs = do | |||
| info verbosity $ "Using internal setup method with build-type " ++ show bt | |||
| ++ " and args:\n " ++ show args | |||
| inDir (useWorkingDir options) $ | |||
| buildTypeAction bt args | |||
| withEnv "HASKELL_DIST_DIR" (useDistPref options) $ | |||
There was a problem hiding this comment.
I'd add a comment here that in the case of the internal setup method we're doing this for the sake of build-types Configure and Make, since the Cabal library code does not care about HASKELL_DIST_DIR.
There was a problem hiding this comment.
If you're doing ./Setup test the test script might care too. So really it needs to apply in all cases.
|
LGTM. |
ezyang
force-pushed
the
pr/env-haskell-dist-dir
branch
from
825f48e to
44fc26b
Compare
Signed-off-by: Edward Z. Yang <ezyang@cs.stanford.edu>
Signed-off-by: Edward Z. Yang <ezyang@cs.stanford.edu>
Signed-off-by: Edward Z. Yang <ezyang@cs.stanford.edu>
ezyang
force-pushed
the
pr/env-haskell-dist-dir
branch
from
44fc26b to
a0efec0
Compare
But first things first; need
unsetEnvto implementwithEnvto implement same-process environment change.