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.

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

Fix potential bug in Stack failing to properly isolate pkg-db environment #4706

Closed
hvr opened this issue Apr 6, 2019 · 15 comments · Fixed by #4715
Closed

Fix potential bug in Stack failing to properly isolate pkg-db environment #4706

hvr opened this issue Apr 6, 2019 · 15 comments · Fixed by #4715

Comments

@hvr
Copy link

hvr commented Apr 6, 2019

Recently a Stack user complained about bugs in Stack in a public venue which as you'll surely agree isn't a suitable place to complain about bugs in Stack and even less so for getting said bugs resolved. Therefore I'm quoting their lament here in Stack's bug tracker where it actually belongs so we can quickly resolve this to everyone's benefit:

I recently decided to start a project using Cabal instead of Stack because I hadn't done that in a while. After a short while I decided to also build it with Stack. I immediately ran into a problem because of the .ghc.environment file. I only managed to rescue myself because I happened to hear about this issue before. I was not happy about the whole situation.

I don't use Stack myself nor do I remember having heard any such bug reports about Stack since this feature was added to GHC 8.0 three years ago. I also reached out to a Stack user who wasn't able to reproduce this. So even though the reported faulty Stack behaviour appears unlikely to me (given there's already four major GHC series out there with this new exciting feature and this is the first time I'm aware of a report of it suggesting a bug in Stack's implementation) I'd like to give the reported malfunction in Stack the benefit of the doubt and assume it being made in good faith, and consequently take it seriously enough to bring it to your attention.

Please let me know if you have any questions about the semantics/mechanics as well as current limitations of this powerful package environment file feature in GHC as there appears to be a lot of not-well-informed misconceptions being dispersed about it. I've been involved in supporting early adopters as well as improving its implementation in response to the technical feedback we received over time, so I most likely can answer most of the question you might have even if the documentation might be lacking and this should help to quickly address this potential bug in Stack in a most effective way.

@dbaynard
Copy link
Contributor

dbaynard commented Apr 6, 2019

Thanks for raising this, @hvr.

even if the documentation might be lacking

I'm unfamiliar with this feature, myself — would this be the place to start?

With that, I have some questions about the technical details of the problem you report. It would be helpful to have log information, from somebody able to reproduce this issue. Would you please invite the person who raised the issue to comment here, if possible?

@tfausak
Copy link
Contributor

tfausak commented Apr 7, 2019

Hi! I'm the user that @hvr is talking about here.

I originally made my comment on Reddit. You can find it here: https://np.reddit.com/r/haskell/comments/b9scx2/package_environment_files_run_counter_to/ek6u69t/

I did not open a bug report for this problem for two reasons: One, it wasn't clear to me which component was at fault, if any (Stack, Cabal, or GHC). Two, I didn't record or remember the exact actions that got me into and subsequently out of this situation.

Since my hand has more or less been forced here, I'm working on reproducing this bug.

@tfausak
Copy link
Contributor

tfausak commented Apr 7, 2019

I managed to reproduce this. For context, here is my system information:

$ uname -a
Linux adele 4.18.0-17-generic #18-Ubuntu SMP Wed Mar 13 14:34:40 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux

$ stack --version
Version 1.9.3, Git revision 40cf7b37526b86d1676da82167ea8758a854953b (6211 commits) x86_64 hpack-0.31.1

$ stack exec -- cabal --version
cabal-install version 2.4.1.0
compiled using version 2.4.1.0 of the Cabal library

To get started, set up an empty Stack project somewhere. The location and the resolver don't matter.

$ mkdir /tmp/example
$ cd /tmp/example
$ echo '{ "resolver": "lts-13.0" }' > stack.yaml

Then create a minimal package file. The name and version don't matter.

$ echo 'cabal-version: 2.4' > example.cabal
$ echo 'name: example' >> example.cabal
$ echo 'version: 0' >> example.cabal
$ echo 'library { default-language: Haskell2010 }' >> example.cabal

Then prove that getting a REPL through Stack works:

$ stack build
example-0: configure (lib)
Configuring example-0...
example-0: build (lib)
Preprocessing library for example-0..
Building library for example-0..
example-0: copy/register
Installing library in /tmp/example/.stack-work/install/x86_64-linux-tinfo6/lts-13.0/8.6.3/lib/x86_64-linux-ghc-8.6.3/example-0-AX5rovEqDIA8Qj0gJCELXw
Registering library for example-0..

$ echo ':quit' | stack exec ghci
GHCi, version 8.6.3: http://www.haskell.org/ghc/  :? for help
Loaded GHCi configuration from /home/taylor/.ghci
>>> Leaving GHCi.

Then build the project with Cabal:

$ stack exec -- cabal v2-build
cabal: Use of GHC's environment variable GHC_PACKAGE_PATH is incompatible with
Cabal. Use the flag --package-db to specify a package database (it can be used
multiple times).

$ stack exec --no-ghc-package-path -- cabal v2-build
Resolving dependencies...
Build profile: -w ghc-8.6.3 -O1
In order, the following will be built (use -v for more details):
 - example-0 (lib) (first run)
Configuring library for example-0..
Preprocessing library for example-0..
Building library for example-0..

Now see that you can not longer get a REPL through Stack:

$ stack exec ghci
GHCi, version 8.6.3: http://www.haskell.org/ghc/  :? for help
Loaded package environment from /tmp/example/.ghc.environment.x86_64-linux-8.6.3

<interactive>:1:6: error:
    Not in scope: ‘System.IO.hSetBuffering’
    No module named ‘System.IO’ is imported.

<interactive>:1:30: error:
    Not in scope: ‘System.IO.stdin’
    No module named ‘System.IO’ is imported.

<interactive>:1:46: error:
    Not in scope: data constructor ‘System.IO.NoBuffering’
    No module named ‘System.IO’ is imported.

<interactive>:1:70: error:
    Not in scope: ‘System.IO.hSetBuffering’
    No module named ‘System.IO’ is imported.

<interactive>:1:94: error:
    Not in scope: ‘System.IO.stdout’
    No module named ‘System.IO’ is imported.

<interactive>:1:111: error:
    Not in scope: data constructor ‘System.IO.NoBuffering’
    No module named ‘System.IO’ is imported.

<interactive>:1:135: error:
    Not in scope: ‘System.IO.hSetBuffering’
    No module named ‘System.IO’ is imported.

<interactive>:1:159: error:
    Not in scope: ‘System.IO.stderr’
    No module named ‘System.IO’ is imported.

<interactive>:1:176: error:
    Not in scope: data constructor ‘System.IO.NoBuffering’
    No module named ‘System.IO’ is imported.

Finally, just to drive the point home, show that removing the GHC environment file allows you to get a REPL through Stack again:

$ rm .ghc.environment.x86_64-linux-8.6.3
$ echo ':quit' | stack exec ghci
GHCi, version 8.6.3: http://www.haskell.org/ghc/  :? for help
Loaded GHCi configuration from /home/taylor/.ghci
>>> Leaving GHCi.

@snoyberg
Copy link
Contributor

snoyberg commented Apr 7, 2019

Thanks for the repro. Can you confirm that setting the GHC_ENVIRONMENT environment variable to - fixes this problem?

@tfausak
Copy link
Contributor

tfausak commented Apr 7, 2019

Yup, that works.

$ echo ':quit' | env GHC_ENVIRONMENT=- stack exec ghci
GHCi, version 8.6.3: http://www.haskell.org/ghc/  :? for help
Loaded GHCi configuration from /home/taylor/.ghci
>>> Leaving GHCi.

@snoyberg
Copy link
Contributor

snoyberg commented Apr 7, 2019

Cool, thanks. I played around with setting that environment variable before, but it seemed to break builds (ie calls into the Cabal library). We can probably treat this like the GHC_PACKAGE_PATH environment variable and simply not set it during Cabal calls. It seems like a strange design in GHC to respect an implicitly discovered file over an environment variable, but it's worth putting in a workaround for support with all current GHC versions.

@snoyberg
Copy link
Contributor

snoyberg commented Apr 7, 2019

OK, one more bit of fun. Does it also fail for you when using GHC 8.2.2 and setting that environment variable?

Afaict, there's no environment variable setting that will work across all versions of GHC out there today.

@snoyberg
Copy link
Contributor

snoyberg commented Apr 7, 2019

I've opened an issue about this against the Cabal repo: haskell/cabal#5988

@phadej
Copy link
Collaborator

phadej commented Apr 7, 2019

GHC_ENVIRONMENT=- is supported only with ghc-8.4.4 and later.


For that empty package, stack ghci doesn't work either. stack exec ghci works, as it doesn't -hide-all-packages.


@tfausak said

After a short while I decided to also build it with Stack.

I tried to stack build and stack ghci: the existence of .ghc.environment files (valid or invalid) doesn't seem to have any affect.
The only thing which breaks is stack exec ghci.

I think setting GHC_ENVIRONMENT=- for stack exec would be good enough work-around, though it works only with ghc >=8.4.4.
Yet, cabal-3.0 won't generate environment files by default (PR is merged), so the corner case would be quite tiny.
There's a chance, that cabal-N.M would generate .ghc.environment files would be generated again

it's not like like we can't change the default back once we've worked out the few remaining kinks.

But there aren't any issues, neither any other clear descriptions what those are.
I asked Herbert to create those before any changes are made.

@hvr
Copy link
Author

hvr commented Apr 7, 2019

Afaict, there's no environment variable setting that will work across all versions of GHC out there today.

You're completely wrong. If you think about it a bit more you might realize there'd be a way to set the env-var in a way that works across the GHC versions stack is currently able to support: Simply do what cabal v2-exec does to fix this very problem in stack and construct a transient ghc-environment file in whatever place you like to put it matching the contents of the pkg-db provisioned by Stack and point the GHC_ENVIRONMENT var to the transient file for stack's spawned subprocesses. Also make sure to really cover all codepaths where Stack itself invokes ghc/ghci directly as to avoid failing to fix this properly.

@snoyberg
Copy link
Contributor

snoyberg commented Apr 8, 2019

@hvr your tone is out of line, and will not be accepted on this repo. Speak with respect to others, or do not participate here. I will not tolerate this kind of tone any further.

@snoyberg
Copy link
Contributor

snoyberg commented Apr 8, 2019

@phadej thanks for the feedback. I'm leaning towards doing nothing in Stack at all, possibly with some code to detect and warn about these files. However, given how much of a corner case this is, and that the feature is changing soon in cabal, I'm not such a fan of adding a bunch of system calls and I/O to Stack's initialization procedure.

@hvr
Copy link
Author

hvr commented Apr 8, 2019

I'm leaning towards doing nothing in Stack at all, possibly with some code to detect and warn about these files. However, given how much of a corner case this is, and that the feature is changing soon in cabal, I'm not such a fan of adding a bunch of system calls and I/O to Stack's initialization procedure.

It seems there's still a worrying lack of understanding about what "the feature is changing soon in cabal" actually means. For one, once the reported issues that can be addressed on cabal and ghc's side have been resolved (and this doesn't mean that Stack refusing to become resilient in the context of a documented GHC feature will be considered a blocker to hold back enhancements in Cabal) the reasons that motivated the change of defaults become moot again; and even with 3.0's defaults you have to take into account users to turn it on globally themselves and not be confronted by Stack failing to handle one of GHC's features. What you also seem to fail to understand is environment files are generated by cabal in various situations (i.e. not only in the specific case whose default is being changed) which I expect to become relatively popular among non-Nix users (as those have a different Nix-idiomatic way at their disposal to achieve the same, so likely won't be using it), so if you want to support users that switch back and forth between cabal and stack, then you better fix this in Stack properly for good, and make sure all codepaths that can lead to ghc being invoked are properly covered. I've tried to offer a workable solution in the previous comment and pointed out the issues; I clearly can't force bugs getting fixed in Stack, but I'd rather they're fixed for good since we've now become aware of them than to have Cabal+Stack users be confused for something that should have been fixed on Stack's end. If there's still something that isn't clear to you about the technical aspects of this feel free to ask me.

@phadej
Copy link
Collaborator

phadej commented Apr 8, 2019

@hvr so do you propose for stack rather set GHC_ENVIRONMENT for GHCs which support it (GHC-8.0+), then current setting of GHC_PACKAGE_PATH.

To illustrate, currently

% cabal exec sh -- -c "env | grep GHC" 
GHC_ENVIRONMENT=/home/ogre/mess/composition-1.0.2.1/dist-newstyle/tmp/environment.-1564/.ghc.environment.x86_64-linux-8.4.4
% stack exec sh -- -c "env | grep GHC"
GHC_PACKAGE_PATH=/home/ogre/mess/composition-1.0.2.1/.stack-work/install/x86_64-linux/lts-12.26/8.4.4/pkgdb:/home/ogre/.stack/snapshots/x86_64-linux/lts-12.26/8.4.4/pkgdb:/home/ogre/.stack/programs/x86_64-linux/ghc-8.4.4/lib/ghc-8.4.4/package.conf.d

These however work differently:

cabal lists the packages, so I cannot load a module from transformers, which isn't in build-depends:

% cabal exec ghci
GHCi, version 8.4.4: http://www.haskell.org/ghc/  :? for help
Loaded package environment from /home/ogre/mess/composition-1.0.2.1/dist-newstyle/tmp/environment.-1653/.ghc.environment.x86_64-linux-8.4.4
Loaded GHCi configuration from /home/ogre/.ghci
Prelude> :m +Control.Monad.Trans.Reader

<no location info>: error:
    Could not find module ‘Control.Monad.Trans.Reader’
    It is not a module in the current program, or in any known package.
Prelude> 

But with stack I can, as all packages are exposed:

% stack exec ghci                           
GHCi, version 8.4.4: http://www.haskell.org/ghc/  :? for help
Loaded GHCi configuration from /home/ogre/.ghci
Prelude> :m +Control.Monad.Trans.Reader 
Prelude Control.Monad.Trans.Reader> 

EDIT: I accidenally left /home/ogre/.ghc/x86_64-linux-8.4.4/environments/defaultin place. But even I remove it, stack exec ghci behaves as specified.

stack could create an environment with all packages explicitly listed, but I think environment files are simply badly suited for stack use cases (EDIT: because of implicit -hide-all-packages).

I'd recommend setting GHC_ENVIRONMENT=-, it's a small change, and would make GHC-8.4.4+ situation better. (I don't see any downside).

snoyberg added a commit that referenced this issue Apr 9, 2019
This fixes #4706. Since GHC 8.0, GHC will now implicitly read in from a
.ghc.environment.* file, which can cause commands like `stack exec ghci`
to fail. Due to the use case of `stack exec`, it doesn't make sense to
try to create our own environment file, but instead tell GHC to ignore
it. Unfortunately, the ability to ignore environment files was only
added in GHC 8.4.4. This patch:

* Unsets any `GHC_ENVIRONMENT` variable already set outside of Stack
* When using GHC 8.4.4 or later, sets the variable to `-`

This will help work around situations where `cabal new-build` creates an
environment file without the user's awareness. This may be somewhat
superfluous in the future depending on changes to either GHC or
cabal-install, but shouldn't present any harm for the foreseeable future
(unless GHC changes its understanding of `GHC_ENVIRONMENT`).
@snoyberg
Copy link
Contributor

snoyberg commented Apr 9, 2019

Thanks for all the feedback @phadej, much appreciated. I've opened up #4715 based on your suggestion.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants