dan-t / cabal-bounds

Set the version bounds of dependencies in a cabal file
Other
66 stars 7 forks source link

Behaviour change: when updating, never drop valid bounds #2

Closed bennofs closed 10 years ago

bennofs commented 10 years ago
dan-t commented 10 years ago

On Wed, Apr 23, 2014 at 05:57:25AM -0700, Benno Fünfstück wrote:

• Don't add lower bounds when only updating upper bounds. There is no need to do that, the user can achieve the same by just updating both bounds. • If there already is a lower bound/upper bound that is more permissive, don't change it. Why should the package no longer build with the old bound? If you still want to do that, you can achieve the same by first dropping the bounds for that package and then updating again.

I'm fine with these ones.

• Don't drop upper bounds when only updating lower bounds. The same effect can be achieved by using the drop feature.

But if the new lower bound is greater than the upper bound, then the upper bound should be dropped.

Is this case already handled by your change?

What does the use of 'Test.Tasty.Golden.Manage' gives you?

Greetings, Daniel

dan-t commented 10 years ago

Can you please resolve the merge conflicts by rebasing your branch?

bennofs commented 10 years ago

But if the new lower bound is greater than the upper bound, then the upper bound should be dropped. Is this case already handled by your change?

If the new lower bound is greater than the upper bound, I won't change the lower bound at all. (Assuming the the lower bound is less than the upper bound). So there is no need to drop the upper bound, is there? If you want to raise the upper bound, just use -U. But now that I think about this, is there ever a case where one only wants to raise upper or lower bounds? I think when you know that a package builds with a given version of a dependency, the bounds should be adjusted so that this version of the dependency is valid, regardless of whether that requires raising upper bounds or lowering lower bounds.

What does the use of 'Test.Tasty.Golden.Manage' gives you?

It gives the tests the --accept mode, which can be used to recreate the golden files.

Can you please resolve the merge conflicts by rebasing your branch?

Ok, I'll do this.

bennofs commented 10 years ago

Oh, I overlooked a case where you might not want to update upper/lower bounds. Of course it makes sense to only update one kind of bound when you don't want upper bounds at all, because then updating both would create upper bounds.

dan-t commented 10 years ago

On Wed, Apr 23, 2014 at 10:10:06AM -0700, Benno Fünfstück wrote:

If the new lower bound is greater than the upper bound, I won't change the lower bound at all. (Assuming the the lower bound is less than the upper bound).

I thought about the case that the dependency only has an upper bound (e.g. lens <3) and now cabal-bounds is called with 'update --lower' and the currently used lens version is 4, then the most sensible thing seems to be to change the dependency to: lens >=4.

But now that I think about this, is there ever a case where one only wants to raise upper or lower bounds? I think when you know that a package builds with a given version of a dependency, the bounds should be adjusted so that this version of the dependency is valid, regardless of whether that requires raising upper bounds or lowering lower bounds.

I think it's fine to have this as the default bahaviour, but I would still like to keep the ability to explicitly update the bounds with the currently used versions.

If e.g. your package doesn't build anymore with a newer version of a dependency, then you would like to raise the lower bound explicitly. Sure you could do this by dropping both bounds from the dependency, but why not support an additional workflow?

Perhaps using the flags '--upper' and '--lower' would give you the explicit behaviour and otherwise the bounds would be adjusted as needed.

I have to think about this one.

dan-t commented 10 years ago

On Wed, Apr 23, 2014 at 10:33:39AM -0700, Benno Fünfstück wrote:

Oh, I oversaw a case where you might not want to update upper/lower bounds. Of course it makes sense to only update one kind of bound when you don't want upper bounds at all, because then updating both would create upper bounds.

Yes, that's definite one use case.

dan-t commented 10 years ago

I think the two major use cases for cabal-bounds are:

The first use case isn't affected in any way by your changes and the second use case doesn't benefit that much by your changes.

If you want to raise the upper bounds, then you have to drop the upper bounds from the dependencies, otherwise cabal won't build with newer versions, and you have to exclude all dependencies with wider bounds (e.g. base) explicitly, because these dependencies shoudn't be changed.

Now when updating the dependencies you have again to exclude the dependencies that shoudn't be changed. Your changes would allow to leave out the exclusion, because if the bounds are already wider, they wouldn't be changed.

That's the only benefit I can see for the most common use cases. But if I would like to keep the ability to explicitly set the bounds of the dependencies, then the implemention and especially the user interface have to get more complex, without IMHO giving you that much.

• Don't add lower bounds when only updating upper bounds. There is no need to do that, the user can achieve the same by just updating both bounds.

Yes, I think that's a good change and makes the user interface even more clear.

• If there already is a lower bound/upper bound that is more permissive, don't change it. Why should the package no longer build with the old bound? If you still want to do that, you can achieve the same by first dropping the bounds for that package and then updating again.

Like I said, I don't think that the increased complexity for the user interface is worth it, because you still have to exclude the dependencies with the wider bounds for the dropping and could just reuse the excluded dependencies for the updating.

• Don't drop upper bounds when only updating lower bounds. The same effect can be achieved by using the drop feature.

I'm fine with this one, but if the new lower bound is greater than the upper bound, then the upper bound should be dropped.

bennofs commented 10 years ago

Oops, there is still a bug in dropping the upper bounds when the new lower bound is greater. I'll fix it and add a golden test.

dan-t commented 10 years ago

After making some changes for Cabal 1.20 it was easier to just copy your changes in 'Update.hs' instead of trying to merge your branch. Your changes are now in cabal-bounds 0.4.

Thanks!