-
Notifications
You must be signed in to change notification settings - Fork 704
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
Comments
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? |
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 |
agree with @gbaz here, so I think the goal of this RFC is covered by the linked project |
There has been zero progress on getting
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. |
those are convincing arguments tbh |
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. |
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
|
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 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 |
My understanding is that they find it because
I don't propose disabling or hiding Resetting environment feels like a good match for certain (also, especially novice's) use cases to me. |
This is is interesting because the behaviour of cabal in under these circumstances is strictly worse than the cabal v1's behaviour under |
I'd be for rephrasing
to something less copy-pastable, like
This is obviously not enough, but may limit how often people mindlessly Alternatively, how about we introduce I'm not firmly against breaking compatibility for |
hmm another intermmediate approach could be:
Note i added the requested packages to make the snippets directly usable, it could be nice to do it way but not required. |
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 Suggestions for a more cautious plan were essentially:
A more concrete plan that may get people behind is roughly as follows:
Personally, I think this may be a fine plan. In my opinion, it doesn't go far |
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 |
Right, my definition of something broken is when it doesn't work sometimes. |
I agree with #8483 (comment). I oppose another intermediate option, or arbitrarily restricting Tbh I'm surprised that nobody was upset enough by
I'd expect a basic implementation (ie. just |
@fgaz thanks for your input! Good to know about the size of Personally, I’ve seen talks of |
Again: my proposal is that we just make This check doesn't seem too scary, seems like it comes closer to what the v1 |
@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:
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. |
I personally think it should stay like this for now. Greater chance for someone to get annoyed and write cabal env ;)
In that case I say we kill |
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
But then of course the amended nonglobal entries would be the ones added later on. |
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... |
Assuming you mean that an inconsistent environment file can never be created if the user never |
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. |
Well, that means we can't agree on anything:
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 You mentioned |
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. |
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. |
Well, i think we agreed about two things: actual situation is no good for user experience and we should take some action. |
@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... |
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:
I checked and initially that file does not exist. Good! In this state, the user can say:
This will create the said environment file. Let's say now the user decided to install more:
The said error and strategy will apply to calls with explicit
--package-env
too, because it's affected just as well.The text was updated successfully, but these errors were encountered: