Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

ignore an externally set $GHC_ENVIRONMENT #7792

Open
fgaz opened this issue Nov 2, 2021 · 6 comments
Open

ignore an externally set $GHC_ENVIRONMENT #7792

fgaz opened this issue Nov 2, 2021 · 6 comments
Labels
attention: pr-welcome cabal-install: environment-files type: enhancement

Comments

@fgaz
Copy link
Member

fgaz commented Nov 2, 2021

Currently a bogus $GHC_ENVIRONMENT can make cabal fail. Cabal itself could trigger this issue if it's called inside a cabal exec (though this is probably a terrible idea in the first place). Local and default environments instead do not cause any interference.

Should we always pass -package-env=- to ghc, or set $GHC_ENVIRONMENT to -?

I think yes:

  • Cabal manages the set of packages it passes to ghc on its own, so why should it be affected by external environments?
  • I don't see any use for passing an environment to cabal
  • it can cause inscrutable errors

on the other hand:

  • resetting a variable that may heve been explicitly set by the user does not sound good
@Mikolaj
Copy link
Member

Mikolaj commented Nov 2, 2021

I'm for ignoring the env var, if possible.

@phadej
Copy link
Collaborator

phadej commented Nov 2, 2021 •
edited
Loading

Hmm, in https://ghc.gitlab.haskell.org/ghc/doc/users_guide/packages.html#package-environments

Additionally, unless -hide-all-packages is specified ghc will also look for the package environment in the following locations:

File .ghc.environment.arch-os-version if it exists in the current directory or any parent directory (but not the user’s home directory).
File $XDG_DATA_HOME/ghc/arch-os-version/environments/default if it exists.

But the $GHC_ENVIRONMENT is not ignored.

Note that -package-env=- doesn't work on all GHCs which support -package-env flag.

I'd say it's simpler for cabal to always reset environment clearing GHC_ENVIRONMENT, rather then trying to undo its effects with flags.

@phadej phadej mentioned this issue Jan 3, 2022
Open
@fgaz
Copy link
Member Author

fgaz commented Jan 13, 2022

Given the positive response I'm going to turn this ticket from "discussion" to "enhancement"+"pr welcome".

Some stuff to pay attention to when implementing:

  • should it be reset in cabal-install or Cabal? (probably the latter)
  • if we want to keep tools such as doctest working, the variable should only be reset for ghc calls, not globally or for other executables

@fgaz fgaz changed the title Should cabal ignore $GHC_ENVIRONMENT? ignore $GHC_ENVIRONMENT Jan 13, 2022
@fgaz fgaz changed the title ignore $GHC_ENVIRONMENT ignore an externally set $GHC_ENVIRONMENT Jan 13, 2022
@andreabedini
Copy link
Collaborator

Why not treating GHC_ENVIRONMENT like GHC_PACKAGE_PATH and die.

@sternenseemann
Copy link

Why not treating GHC_ENVIRONMENT like GHC_PACKAGE_PATH and die.

This would break quite a few test suites that rely on cabal exec setting GHC_ENVIRONMENT so that a test suite can invoke ghc/ghci and have build depends available. One such example is doctest.

Cabal 3.12 caused the check for GHC_PACKAGE_PATH to be executed when running test suites, so it seems you now would need to use GHC_ENVIRONMENT. (An alternative could be to use --test-wrapper to sidestep this check, but this seems like an unsatifactory solution.)

Not saying GHC_ENVIRONMENt being disallowed is wrong per se, but I don't see an alternative for this use case at the moment. The existing real world usage of this is significant (i.e. anything using doctest would also be affected).

@mouse07410
Copy link
Collaborator

This is a years-old discussion, and frankly I don't see it's purpose.

If GHC_ENVIRONMENT that user set has bad value - then Cabal would fail, which is to be expected. If that value makes sense - fine.

Where's the nail that this discussion/hammer is trying to hit? What's the actual problem?

sternenseemann added a commit to NixOS/nixpkgs that referenced this issue May 26, 2024
@sternenseemann
haskellPackages.mkDerivation: limit GHC_PACKAGE_PATH to test suite
120f242 
Previously, we would set GHC_PACKAGE_PATH after configure, the reasons
being that

1. Setup.hs configure forbids this from being set since it can make a
   build fail that would otherwise succeed (since it influences how
   GHC behaves when invoked by Cabal).
2. Setting GHC_PACKAGE_PATH being set is sound in our case, since
   we set it precisely to the packages available to Cabal at configure
   time, so there should be no room for a mismatch.
3. Some test suites require GHC_PACKAGE_PATH or GHC_ENVIRONMENT to be
   set, so they can invoke GHC(i) with build dependencies available.
   Cabal >= 3.12 forbids GHC_PACKAGE_PATH from being set after
   <haskell/cabal@d6e38041a7c778fadf8f416>.
   Setting GHC_ENVIRONMENT would be possible, but is cumbersome without
   cabal-install (which has the handy cabal exec command which takes
   care of that). Additionally, it is not clear if it'll remain possible
   to do that: <haskell/cabal#7792>.

Our solution to Cabal 3.12's change is to be more targeted about setting
GHC_PACKAGE_PATH: We _just_ set it for the actual test suite executable.
This can be achieved by using --test-wrapper which when given is invoked
by Cabal to run the test suite. Here we can set any environment
variables after Cabal has already done its environment checks. As long
as we don't do anything stupid, this should be unproblematic.

Users can also arbitrarily influence what GHC_PACKAGE_PATH will contain
using the NIX_GHC_PACKAGE_PATH_FOR_TEST environment variable. This is
un(der)documented for now, since I want to keep some wiggle room for
changing stuff in the coming weeks. Also it's rarely necessary to
actually touch this variable.
sternenseemann added a commit to sternenseemann/nixpkgs that referenced this issue Jun 8, 2024
@sternenseemann
haskellPackages.mkDerivation: limit GHC_PACKAGE_PATH to test suite
615821c 
Previously, we would set GHC_PACKAGE_PATH after configure, the reasons
being that

1. Setup.hs configure forbids this from being set since it can make a
   build fail that would otherwise succeed (since it influences how
   GHC behaves when invoked by Cabal).
2. Setting GHC_PACKAGE_PATH being set is sound in our case, since
   we set it precisely to the packages available to Cabal at configure
   time, so there should be no room for a mismatch.
3. Some test suites require GHC_PACKAGE_PATH or GHC_ENVIRONMENT to be
   set, so they can invoke GHC(i) with build dependencies available.
   Cabal >= 3.12 forbids GHC_PACKAGE_PATH from being set after
   <haskell/cabal@d6e38041a7c778fadf8f416>.
   Setting GHC_ENVIRONMENT would be possible, but is cumbersome without
   cabal-install (which has the handy cabal exec command which takes
   care of that). Additionally, it is not clear if it'll remain possible
   to do that: <haskell/cabal#7792>.

Our solution to Cabal 3.12's change is to be more targeted about setting
GHC_PACKAGE_PATH: We _just_ set it for the actual test suite executable.
This can be achieved by using --test-wrapper which when given is invoked
by Cabal to run the test suite. Here we can set any environment
variables after Cabal has already done its environment checks. As long
as we don't do anything stupid, this should be unproblematic.

Users can also arbitrarily influence what GHC_PACKAGE_PATH will contain
using the NIX_GHC_PACKAGE_PATH_FOR_TEST environment variable. This is
un(der)documented for now, since I want to keep some wiggle room for
changing stuff in the coming weeks. Also it's rarely necessary to
actually touch this variable.

(cherry picked from commit 120f242)
sternenseemann added a commit to NixOS/nixpkgs that referenced this issue Jun 17, 2024
@sternenseemann
haskellPackages.mkDerivation: limit GHC_PACKAGE_PATH to test suite
7ffac6b 
Previously, we would set GHC_PACKAGE_PATH after configure, the reasons
being that

1. Setup.hs configure forbids this from being set since it can make a
   build fail that would otherwise succeed (since it influences how
   GHC behaves when invoked by Cabal).
2. Setting GHC_PACKAGE_PATH being set is sound in our case, since
   we set it precisely to the packages available to Cabal at configure
   time, so there should be no room for a mismatch.
3. Some test suites require GHC_PACKAGE_PATH or GHC_ENVIRONMENT to be
   set, so they can invoke GHC(i) with build dependencies available.
   Cabal >= 3.12 forbids GHC_PACKAGE_PATH from being set after
   <haskell/cabal@d6e38041a7c778fadf8f416>.
   Setting GHC_ENVIRONMENT would be possible, but is cumbersome without
   cabal-install (which has the handy cabal exec command which takes
   care of that). Additionally, it is not clear if it'll remain possible
   to do that: <haskell/cabal#7792>.

Our solution to Cabal 3.12's change is to be more targeted about setting
GHC_PACKAGE_PATH: We _just_ set it for the actual test suite executable.
This can be achieved by using --test-wrapper which when given is invoked
by Cabal to run the test suite. Here we can set any environment
variables after Cabal has already done its environment checks. As long
as we don't do anything stupid, this should be unproblematic.

Users can also arbitrarily influence what GHC_PACKAGE_PATH will contain
using the NIX_GHC_PACKAGE_PATH_FOR_TEST environment variable. This is
un(der)documented for now, since I want to keep some wiggle room for
changing stuff in the coming weeks. Also it's rarely necessary to
actually touch this variable.

(cherry picked from commit 120f242)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
attention: pr-welcome cabal-install: environment-files type: enhancement
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
6 participants
@phadej @andreabedini @Mikolaj @sternenseemann @mouse07410 @fgaz