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

RFC: don't allow cabal install --lib to update environment files #8483

Closed
ulysses4ever opened this issue Sep 18, 2022 · 29 comments
Closed

RFC: don't allow cabal install --lib to update environment files #8483

ulysses4ever opened this issue Sep 18, 2022 · 29 comments

Comments

@ulysses4ever
Copy link
Collaborator

ulysses4ever commented Sep 18, 2022

Problem

Repeated calls to cabal install --lib can lead to inconsistent environment files (#5559).
This may end with very confusing error messages down the road (e.g. #8473).

Proposal

Don't update environment files ever. In particular, Cabal docs say:

This [cabal install --lib] works by managing GHC package environment files.
By default, it is writing to the global environment in ~/.ghc/$ARCH-$OS-$GHCVER/environments/default

I checked and initially that file does not exist. Good! In this state, the user can say:

> cabal install --lib a b c

This will create the said environment file. Let's say now the user decided to install more:

> cabal install --lib d e f
ERROR: You are trying to install more packages into already existing package environment.
       This is currently unsupported. You have two options:
       1) Call `cabal install --lib --reset-environment` -- this will destroy previously
          installed packages and create a new environment with the requested package(s).
          NOTE: Older users of the current environment may stop working.
       2) Call `cabal install --lib --package-env /path/to/directory/to/store/new/environment`
          -- this will initialize a new environment in the said directory. If you call cabal
          or GHC while in that directory, the tools will automatically use that environment. Otherwise,
          you need to explicitly point to that directory in the subsequent calls. A good idea 
          is to use `--package-env .` in the directory where you want to experiment with the 
          packages you're trying to install: e.g. calling `ghci` (or `ghc`) directly in that
          directory will make the packages available to the compiler with no additional flags.
NOTE: `cabal install --lib` has a number of limitations. With modern versions of cabal, it is 
      discouraged and in many cases can be replaced with `cabal build` of a local package
      depending on the packages of interest. It is easy to start a new package with `cabal init`.

The said error and strategy will apply to calls with explicit --package-env too, because it's affected just as well.

@gbaz
Copy link
Collaborator

gbaz commented Sep 19, 2022

I agree on the problem -- I do not think this is the right solution. We want to make working with env files work right, for a variety of applications!

For one, maybe we should just check if an inconsistent environment file would be created, and error if so?

@gbaz
Copy link
Collaborator

gbaz commented Sep 19, 2022

More generally, I think there is a desire to redesign env file interactions into a different command than install --lib. Cf: https://github.com/haskell/cabal/projects/10

I think that would help, as install --lib is too close to the v1-style usage to imply how different it is. But in this hypothetical cabal env command, I think we'd still certainly want to be able to mutate existing env files (but only in consistent and safe ways!)

@jneira
Copy link
Member

jneira commented Sep 19, 2022

agree with @gbaz here, so I think the goal of this RFC is covered by the linked project

@ulysses4ever
Copy link
Collaborator Author

ulysses4ever commented Sep 19, 2022

There has been zero progress on getting cabal env in for a long time, and from what I see there's no volunteer on the horizon. Meanwhile, we keep getting duplicate bug reports from confused users. My proposal is a smallest possible change that (I believe) will avoid big portion of confusion, and there's a volunteer to get it done (namely, me). Moreover, this proposal don't preclude any grand plan for imaginable future cabal env that will mutate environment files in safe and consistent ways.

For one, maybe we should just check if an inconsistent environment file would be created, and error if so?

This is another proposal. I think that it may be fine to have that, but a) I think it's harder to implement, b) I think the UX it will deliver will be less straightforward than the one I propose; i.e. I doubt it will be able to explain the user, especially a novice, what's the problem and will just throw a solver error on their head, which, as we know from previous bug reports on inconsistent environments, are incredibly hard to decipher.

@jneira
Copy link
Member

jneira commented Sep 19, 2022

those are convincing arguments tbh
but that change would break actual usage of install -lib, assuming there are cases where adding more packages does not break the environment file (it usually works or at least works for some cases, no?)

@gbaz
Copy link
Collaborator

gbaz commented Sep 19, 2022

This is another proposal. I think that it may be fine to have that, but a) I think it's harder to implement, b) I think the UX it will deliver will be less straightforward than the one I propose; i.e. I doubt it will be able to explain the user, especially a novice, what's the problem and will just throw a solver error on their head, which, as we know from previous bug reports on inconsistent environments, are incredibly hard to decipher.

Outside of the complexity of implementing (which will be paid forward work in future env work) the solver issue has at least a good UX as this proposal, as it just succeeds more often (in the non-conflicted case) and where it fails can still suggest that the easiest way, if a conflicted env would be created, is instead to scrap the env for a new one, as the current proposal does.

@ulysses4ever
Copy link
Collaborator Author

From what I hear, it seems that both of you suggest that having a sometimes working feature (updating environments) is better than not having it. I disagree but I think my proposal can accommodate it by the way of another flag. Let's get back to the scenario in the OP, in particular, to the point when the user does cabal install --lib d e f (after previously installing a b c). How about still refusing to do the "sometimes working" thing and suggesting three options instead of two options as in OP (the text below is not changed except option 3) is added):

> cabal install --lib d e f
ERROR: You are trying to install more packages into already existing package environment.
       This is currently unsupported. You have three options:
       1) Call `cabal install --lib --reset-environment` -- this will destroy previously
          installed packages and create a new environment with the requested package(s).
          NOTE: Older users of the current environment may stop working.
       2) Call `cabal install --lib --package-env /path/to/directory/to/store/new/environment`
          -- this will initialize a new environment in the said directory. If you call cabal
          or GHC while in that directory, the tools will automatically use that environment. Otherwise,
          you need to explicitly point to that directory in the subsequent calls. A good idea 
          is to use `--package-env .` in the directory where you want to experiment with the 
          packages you're trying to install: e.g. calling `ghci` (or `ghc`) directly in that
          directory will make the packages available to the compiler with no additional flags.
       3) Call `cabal install --lib --unsafe-update-env` if you want to let cabal try to update existing
          environment. Note, this is known to lead to confusing errors down the road. Use at your own risk.
NOTE: `cabal install --lib` has a number of limitations. With modern versions of cabal, it is 
      discouraged and in many cases can be replaced with `cabal build` of a local package
      depending on the packages of interest. It is easy to start a new package with `cabal init`.

@Mikolaj
Copy link
Member

Mikolaj commented Sep 19, 2022

This is still not backward compatible, but it's OK to do this in a major version, with proper warnings in release notes.

But how about, instead, if that's possible, make sure newbies that don't really need --lib, don't find it? I mean, how do they even learn about --lib? Is it from an error message when cabal install fails for a library? From the manual? From google?

If we can't prevent them finding --lib before learning it's not what they want, perhaps we can warn instead of erroring out, with similar messages as yours? And add a flag --iknowwhatimdoing that turns the warnings off?

@ulysses4ever
Copy link
Collaborator Author

ulysses4ever commented Sep 20, 2022

My understanding is that they find it because cabal install tells them about it (and pretty much every online tutorial since the dawn of time and before maybe 2021 says to do cabal install):

> cabal install aeson
...
Completed    aeson-2.1.0.0 (lib)
Warning:
@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@
@ WARNING: Installation might not be completed as desired! @
@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@
The command "cabal install [TARGETS]" doesn't expose libraries.
* You might have wanted to add them as dependencies to your package. In this
case add "aeson" to the build-depends field(s) of your package's .cabal file.
* You might have wanted to add them to a GHC environment. In this case use
"cabal install --lib aeson". The "--lib" flag is provisional: see
https://github.com/haskell/cabal/issues/6481 for more information.

I don't propose disabling or hiding --lib altogether because this is a strictly more drastic change than necessary. My hope is that --lib --package-env . works reasonably well for simple use cases, and that's been my experience (and when you're up for more sophisticated use cases, maybe you are able to decide for yourself how to proceed).

Resetting environment feels like a good match for certain (also, especially novice's) use cases to me.

@tomjaguarpaw
Copy link
Member

This is is interesting because the behaviour of cabal in under these circumstances is strictly worse than the cabal v1's behaviour under --force-reinstalls. The latter gave a warning and broke at most the subset of the packages incompatible with the forcibly-reinstalled package. The cabal install --lib behaviour breaks all packages with no warning!

@Mikolaj
Copy link
Member

Mikolaj commented Sep 28, 2022

I'd be for rephrasing

* You might have wanted to add them to a GHC environment. In this case use
"cabal install --lib aeson". The "--lib" flag is provisional: see
https://github.com/haskell/cabal/issues/6481 for more information.

to something less copy-pastable, like

* In rare cases, you might have wanted to add them to a GHC environment, 
using an external tool or a built-in provisional mechanism, see
https://github.com/haskell/cabal/issues/6481 for more information.

This is obviously not enough, but may limit how often people mindlessly
copy-paste and get into trouble.

Alternatively, how about we introduce --safe-lib that does exactly what
@ulysses4ever proposes and is mentioned in the blurb (and in all docs)
instead of --lib? I know this is an option proliferation, but this is
a particularly ugly case with people hurting themselves in the wild.

I'm not firmly against breaking compatibility for --lib, but perhaps
we don't have to? @ulysses4ever, what do you think?

@jneira
Copy link
Member

jneira commented Sep 28, 2022

hmm another intermmediate approach could be:

  • cabal install --lib e f c gives a verbose and scary warning in the line of proposed ones in sdterr but success so no breaking change other than scripts waiting for a determinate output of the command (and i think there will not be much of that)
  • not adding any new option which will not have a long live
  • make the suggestion when you do a cabal install any-lib more scary, mentioning cabal install --lib caveats in addition to mark it as "experimental" (which does not sound sooo bad). And remove the copypasty part as @Mikolaj has suggested.
> cabal install --lib d e f
WARNING: You are trying to install more packages into already existing package environment.
       This is known to lead to confusing errors down the road. Use at your own risk.
       You have two safer alternatives:
       1) Call `cabal install --lib --reset-environment d e f` -- this will destroy previously
          installed packages and create a new environment with the requested package(s).
          NOTE: Older users of the current environment may stop working.
       2) Call `cabal install --lib --package-env /path/to/directory/to/store/new/environment d e f`
          -- this will initialize a new environment in the said directory. If you call cabal
          or GHC while in that directory, the tools will automatically use that environment. Otherwise,
          you need to explicitly point to that directory in the subsequent calls. A good idea 
          is to use `--package-env .` in the directory where you want to experiment with the 
          packages you're trying to install: e.g. calling `ghci` (or `ghc`) directly in that
          directory will make the packages available to the compiler with no additional flags.
NOTE: `cabal install --lib` has a number of limitations. With modern versions of cabal, it is 
      discouraged and in many cases can be replaced with `cabal build` of a local package
      depending on the packages of interest. It is easy to start a new package with `cabal init`.

Note i added the requested packages to make the snippets directly usable, it could be nice to do it way but not required.

@ulysses4ever
Copy link
Collaborator Author

Thanks everyone for your input! I want to try to summarize the story so far.

First, and foremost, my proposal is a breaking change, and there seem to be no
appetite for such changes among Cabal developers, understandably so. My argument
— that it's not really a breaking change when the feature is broken in the first
place — didn't find support. Oh, well...

Suggestions for a more cautious plan were essentially:

  • improve warnings of the existing commands;
  • make another command (e.g. cabal install --safe-lib) that would do what I
    propose (with a caveat that not everyone likes such stop-gap options).

A more concrete plan that may get people behind is roughly as follows:

  1. Make cabal install not mention --lib explicitly. And when / if
    --safe-lib arrives, mention it.
  2. Make cabal install --lib louder about possible problems down the road
    when it mutates environment files.
  3. Add cabal install --safe-lib that works as described in OP (and possibly
    make cabal install advertise it).

Personally, I think this may be a fine plan. In my opinion, it doesn't go far
enough. Naturally, I'm less inclined to work on it myself. I'd be happy
to review patches and help with it in other ways if we see a volunteer to
execute on it. Maybe I'll do some of it myself one day, but please don't hold your
breath for that.

@jneira
Copy link
Member

jneira commented Oct 2, 2022

only want to note that i assumed the feature is not broken for all cases, if that would be the situation I would go for throw an informative error directly

@ulysses4ever
Copy link
Collaborator Author

Right, my definition of something broken is when it doesn't work sometimes.

@fgaz
Copy link
Member

fgaz commented Oct 2, 2022

I agree with #8483 (comment). I oppose another intermediate option, or arbitrarily restricting --lib.

Tbh I'm surprised that nobody was upset enough by --lib to step up and implement cabal env yet :-P
It's actually surprisingly simple:

$ cloc cabal-env
       9 text files.
       5 unique files.                              
       4 files ignored.

github.com/AlDanial/cloc v 1.94  T=0.06 s (81.0 files/s, 10985.3 lines/s)
-------------------------------------------------------------------------------
Language                     files          blank        comment           code
-------------------------------------------------------------------------------
Haskell                          4            103             79            493
Markdown                         1              1              0              2
-------------------------------------------------------------------------------
SUM:                             5            104             79            495
-------------------------------------------------------------------------------

I'd expect a basic implementation (ie. just add, using fake-package) in cabal to be no bigger than that.
(note that cabal-env is gpl so whoever implements cabal env would have to either clean-room it or ask oleg to relicense it)

@ulysses4ever
Copy link
Collaborator Author

@fgaz thanks for your input! Good to know about the size of cabal env (too bad that the license doesn’t work for us though). But this thread is about the future of cabal install --lib specifically, so an existing interface that is already used by people. Do you mind sharing your view what should it be: can this interface be discontinued at some point, can it become stricter in terms of unsafe mutation of environments, etc.? Feel free to describe two scenarios for install --lib: one that happens if a brave heart implementing cabal env is found and one for the unfortunate case when we don’t have cabal env.

Personally, I’ve seen talks of cabal env for years now, and I long lost a hope for it being added. Nevertheless, we can discuss either case.

@gbaz
Copy link
Collaborator

gbaz commented Oct 3, 2022

Again: my proposal is that we just make cabal install --lib work better. We allow it to do what it does now, which is to mutate environment files. However, we also just add a check that the mutated environment file will not be inconsistent, and then error with a helpful message only if it is inconsistent.

This check doesn't seem too scary, seems like it comes closer to what the v1 cabal install of a lib did, and also is tech that could be reused in any cabal env implementation...

@ulysses4ever
Copy link
Collaborator Author

@gbaz that sounds plausible. Would you be interested in advising such a work and providing specific pointers? In particular, I understand that writing an updated env file happens here:

writeFileAtomic envFile (BS.pack contents')

Should the check go before it's written out, and how to actually go about the check? Again, the more specific the more helpful would be the advice.

@fgaz
Copy link
Member

fgaz commented Oct 3, 2022

@ulysses4ever

Do you mind sharing your view what should it be: can this interface be discontinued at some point, can it become stricter in terms of unsafe mutation of environments, etc.?

I personally think it should stay like this for now. Greater chance for someone to get annoyed and write cabal env ;)
More seriously, I think we shouldn't add complexity to this zombie-like feature, nor we should remove essential functionality until there's a (even minimal) replacement (regarding unsafe mutation, I agree with @gbaz). If we have enough resources to improve --lib substantially, we have enough resources for a minimal env.

one that happens if a brave heart implementing cabal env is found

In that case I say we kill --lib immediately

@gbaz
Copy link
Collaborator

gbaz commented Oct 3, 2022

Should the check go before it's written out, and how to actually go about the check? Again, the more specific the more helpful would be the advice.

So the first thing is to delete from the base entries anything that overlaps with the newly installed entries. (So if foo-1 is in the env file, and foo-2 is installed, foo-2 takes precedence). The second thing to do is to translate the entries back to the solver or otherwise check for their coherence. Both steps could in fact initially be taken even earlier -- before the call to projectBuildPhase at

buildOutcomes <- runProjectBuildPhase verbosity baseCtx buildCtx

But then of course the amended nonglobal entries would be the ones added later on.

@gbaz
Copy link
Collaborator

gbaz commented Oct 3, 2022

Thinking about this more -- the only source of inconsistency comes from reinstalling things that already exist.

The easy issue is if I have foo-1 and want to install foo-2 -- obviously the latter should take precedence over the former. The obvious bug afaik is that instead we get both, and an inconsistent file.

But what if I have bar-1 which depends on foo-1 and then install baz-1 which depends on foo-2? I need to run some tests to confirm, but my recollection is that this does fail fast, but it does not do so in a nice and recoverable way. (and #5559 includes both issues).

In that case we can say that this is fine, because it already prevents installing a lib that would make things inconsistent, and just make sure we give a better error message -- and that's probably fine for this ticket! Or we could try to keep track of the fact that the user wanted bar-1 still, and try to calculate a new consistent env file that had bar-1 and baz-1, perhaps through installing a new bar-1 that depended on foo-2 if constraints permitted. That approach is what I think gets us to the "cabal env" feature...

@tomjaguarpaw
Copy link
Member

But what if I have bar-1 which depends on foo-1 and then install baz-1 which depends on foo-2? I need to run some tests to confirm, but my recollection is that this does fail fast, but it does not do so in a nice and recoverable way.

Assuming you mean that an inconsistent environment file can never be created if the user never cabal install --libs two different version of the same package then I think it is worth confirming because that is not my understanding of the issue from #8473.

@gbaz
Copy link
Collaborator

gbaz commented Oct 3, 2022

Assuming you mean that an inconsistent environment file can never be created if the user never cabal install --libs two different version of the same package then I think it is worth confirming because that is not my understanding of the issue from #8473.

Ah! I see the error in my logic, I think? New theory: the solver runs against the constraints not from all packages in the environment, but only the ones that are in the global package db. So it doesn't worry about consistency of e.g. foo-1 with baz-1 as long as baz does not depend on foo. So assume packages a and b are incompatible. Now if we install package c depending on a, and d depending on b, then both a and b get put in the environment and the solver never detects that a and b are incompatible because it never tries to find a plan including both.

So we can recover the situation I propose by simply forcing all packages in the environment to be passed to the solver as "global" packages whose constraints apply universally.

@ulysses4ever
Copy link
Collaborator Author

@fgaz

I personally think it [cabal install --lib] should stay like this for now.

Well, that means we can't agree on anything:

  • should we touch --lib as of now (without full-fledged cabal env implementation) or not -- people disagree;

  • should we supply a stop-gap version of --lib along the lines proposed in this RFC -- same.

Good old cabal analysis-paralysis. I think I'm gonna throw in the towel and leave it be.


@gbaz what you describe sounds far from trivial to me and coming really close to cabal env. Also, you seem to be keen on "solver is smart, let's throw in something to it and see what happens" kind of thing. In this RFC I'm coming with an opposite perspective: let's not try to test solver's smartness, let's refuse to do anything remotely smart in cabal install --lib and leave all the smartness smartness for future cabal env.

You mentioned v1-install above: even that refused to do the dangerous thing and hid it behind the --force-reinstalls flag. I was proposing exactly the same: updating environments in the manner we do it now is not safe (may lead to ill-formed envs), so refuse to blindly update envs and require another flag, and teach users/implement safer options (use --package-env . or remove existing env and start from scratch) -- on the way.

@gbaz
Copy link
Collaborator

gbaz commented Oct 5, 2022

You mentioned v1-install above: even that refused to do the dangerous thing and hid it behind the --force-reinstalls flag. I was proposing exactly the same.

Lol. This is the nub of our disagreement. I feel like what I am proposing is closer to the same! In particular, v1-install let you install a new package that did not break the existing environment. You are proposing not only to block reinstalls but all sequences of installs.

@gbaz
Copy link
Collaborator

gbaz commented Oct 5, 2022

With regards to "what you describe sounds far from trivial to me" and your concern about analysis paralysis I'll put my money where my mouth is and self-assign #6165 which I'll try to tackle based on my understanding articulated above.

@jneira
Copy link
Member

jneira commented Oct 5, 2022

Well, i think we agreed about two things: actual situation is no good for user experience and we should take some action.
I have suggested an intermmediate approach: make a big warning to scare newcomers and drop suggestions about --lib.
Do you think that option would not improve things? Even if you think other alternatives will be better.

@Mikolaj
Copy link
Member

Mikolaj commented Oct 17, 2022

@jneira: I think your proposal is not controversial (I don't count "it could be approached in a better way") and it's immediately actionable, but perhaps we can learn something new from @gbaz's fix to #6165, so waiting a little bit before the second round of consensus forging may be beneficial. This time I will try to break ties so we don't remain deadlocked.

We are not in a rush, but an implemented partial improvement is way better than a infinitely discussed complete fix...

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

No branches or pull requests

6 participants