haskell / pvp

Haskell Package Version Policy (PVP)
http://pvp.haskell.org/
38 stars 25 forks source link

Drop the requirement of specifying upper bounds #51

Open hasufell opened 1 year ago

hasufell commented 1 year ago

Following some discussion from https://discourse.haskell.org/t/the-operator-in-cabal-files/6938

hasufell commented 1 year ago

Of course, this is invasive, potentially controversial and possibly needs pvp spec version bump. I consider this discussion stage.

gbaz commented 1 year ago

I strongly disagree. We could at some point, as caret semantics are pinned down, put those in the PVP. But bounds of some sort (caret or otherwise) are core to the pvp for good reason. There's a lot of prior debate and discussion on these, throughout various community fora over many years, and nothing fundamentally has changed to make those considerations vanish.

The best arguments for maintaining them, most mainly unaddressed in that discussion, which was not fundamentally over keeping upper bounds at all, are in fact in the sections of text this PR deletes.

hasufell commented 1 year ago

are in fact in the sections of text this PR deletes

Well, the current text fails to mention anything about the caret operator and in light of the caret operator, it doesn't make any sense anymore.

>= 3.4.2 && < 3.5 meaning not known to be compatible with 3.5+ totally defeats the purpose of ^>=3.4.2.

Additionally, the section that attempts to explain why automation is not sufficient is somewhat ridiculous, since it disregards that you can run a test suite during CI and absolutely test semantics.

gbaz commented 1 year ago

Well, the current text fails to mention anything about the caret operator and in light of the caret operator, it doesn't make any sense anymore.

The solution is to in down the caret operator and add it to the pvp, and then require either caret-upper-bounds or caretless-upper-bounds, not eliminate upper-bounds requirements altogether.

hasufell commented 1 year ago

Well, the current text fails to mention anything about the caret operator and in light of the caret operator, it doesn't make any sense anymore.

The solution is to in down the caret operator and add it to the pvp, and then require either caret-upper-bounds or caretless-upper-bounds, not eliminate upper-bounds requirements altogether.

That is an option. But as I find the reasoning about CI automation being insufficient totally wrong, I think expressing that possibility by downgrading the requirement to a recommendation is the right decision.

A later PR can then introduce the caret operator in the spec. This PR does not attempt this and will not.

gbaz commented 1 year ago

What reasoning about CI and automation? Please link to it.

hasufell commented 1 year ago

What reasoning about CI and automation? Please link to it.

Sorry. Here: https://github.com/haskell/pvp/blob/master/pvp-faq.md#upper-bounds-can-be-inferred-by-running-build-bots-to-determine-when-breaking-changes-have-been-introduced-in-dependencies

https://github.com/haskell/pvp/blob/c633fe82f89e8f5b7fe05b53539cec0e38d7e099/pvp-faq.md?plain=1#L108-L119

Specifically "This assumes that compile-success is equivalent to semantic correctness." which seems to indicate they didn't realize you can run tests in CI, e.g. https://discourse.haskell.org/t/a-github-action-to-bump-your-cabal-dependencies/6919

gbaz commented 1 year ago

Ah gotcha. I agree that language can be improved, but I think an argument still stands -- it just needs to be more detailed.

First, for package maintainers they must have excellent test-cases to catch any possible dependency-induced regressions. In my experience, even a very good test-suite can often miss things if they were considered invariants inherited from dependencies.

Second, for package consumers this doesn't help, regardless. If I'm using foo-1.0 which depends on bar > 1.0 and then we get bar-2.0, then the maintainer CI may never run at all again (assuming foo remains unchanged) but an end-user may compile foo vs bar-2.0 and run into unanticipated errors. So all end-users would also have to run the test suites. We might imagine cross-cutting build-bots rebuilding every package when any of its deps changed -- but that would require these bots now also run test-suites, assuming such test-suites existed, etc. So I think there's still reasons this is unworkable, or even if it was, would require a farm of hardware and automation on a community infrastructural level far beyond our current capacity (even matrix, a very cut-down version of this that didn't have test suites, and only ran certain combinations on a certain basis, has fallen into disrepair).

But also, I agree that the text you pointed out doesn't really address most of that.

juhp commented 1 year ago

I agree that < X.Y should mean "known not to build with X.Y" (or possibly "expected not to build with" for a fast breaking library). But < X as a conservative upperbound might be reasonable.

The huge amount of time and effort wasted bumping and waiting for trivial upperbounds bumps of dependencies to be made in the ecosystem makes me very sad. Maybe at the time of heavy cabal dependency hell conservative upperbounds made more sense, but we have better CI, Stackage, Hackage notifications to help us now.

I think it's better to have optimistic or no upperbounds and just impose them as revisions when the need actually arises.

So yes I am generally against having mandatory upperbounds.

gbaz commented 1 year ago

I agree that < X.Y should mean "known not to build with X.Y" (or possibly "expected not to build with" for a fast breaking library). But < X as a conservative upperbound might be reasonable.

If we add a further version number, so that we have X.Y.Z and X.Y, then the above describes exactly the bounds practices the PVP recommends! In particular, that conservative upperbounds only apply to the major (two digit) component of a package version, and strict bounds only be put in place on the third digit when there is a known problem.

So perhaps some of the discomfort here is just expectation mismatch between the two-major-component practice of the PVP vs the one-major-component practice of semver.

On another note:

Maybe at the time of heavy cabal dependency hell conservative upperbounds made more sense, but we have better CI, Stackage, Hackage notifications to help us now.

Stackage notifications only occur when new stackages are released -- which means all users who are ahead of stackage gain no benefit from them. In fact, stackage maintainers themselves reap large net benefits from good bounds practices making stackage solving itself more practical and ergonomic.

On Hackage notifications, which I was involved in the design of -- these are designed precisely to make the process of managing conservative upperbounds more ergonomic, because they can notify you exactly when a new dependency is released which begins to violate said bounds, so that you can test and revise to bump bounds. The alternative notifications which are available necessarily alert you on all new releases of any dependency -- which would amount to line noise in your inbox. So having good upperbounds is what makes good, meaningful hackage notifications even possible.

endgame commented 1 year ago

As I said on the corresponding Discourse thread, please don't do this. The cleanup from the aeson-2.2.0.0/attoparsec-aeson split was bad enough without encouraging more packages to drop upper bounds. The cleanup from simplified subsumption breaking packages with loose base bounds was also really annoying. Removing upper bounds amplifies the total maintenance burden because many releases may need revising when a new release of a dependency causes breakage. It's also really frustrating when one of your dependencies refuses to build because of a new release of a transitive dependency: you can patch the problem locally with a constraint in cabal.project but you have no way of recommending that constraint to your users.

stackage maintainers themselves reap large net benefits from good bounds practices making stackage solving itself more practical and ergonomic.

This is one of the reasons I consider stack extremely rude: stackage derives a lot of benefit from package bounds, because they make it easier to build a working package set. But then stack-the-tool instantiates templates which guide people away from contributing well-bounded packages back to Hackage.

phadej commented 1 year ago

my five cents: I don't believe in ^>=1.2.3 having different semantics than >=1.2.3 && <1.3.

One could argue that

That would be great, but I don't believe it works.

Consider: There is a package version pkg-5.0.0 with build-depends: somedep ^>=1.2.3, and dep-1.3.0 is released. Maintainers tries it, and package doesn't work so they release a new version pkg-5.0.0.1 with build-depends: somedep ^>=1.2.3 || ^>=1.3.0 (as the changes are internal).

Is it enough? No, it isn't. The maintainer should also make a revision to pkg-5.0.0 from somedep ^>=1.2.3 to somedep >=1.2.3 && <1.3. And that is problematic.

Turning caret versions into strict versions would most likely be never ending job for Hackage Trustees.

The idea is great, and would work in setup where there are only few maintainers, which are all expects. But I don't believe such expert feature will be used correctly in Hackage setup where there are a lot of non-expert maintainers.

Maybe it could be made work. For example if Stackage Nightly builds were always run with caret versions relaxed, so the revisions could be made (by maintainers or trustees).

So I think that ^>= should be treated as a syntax sugar. Yes, we would lose (maybe) useful semantic difference. But the feature is simply too fancy to be universally used correctly.

phadej commented 1 year ago

Addition to previous:

A "good" thing about ^>= (not known to not work) bounds could be, that Hackage Trustees may out right refuse to do revisions for < (known to not work) version bounds, as their semantics say so: known to not work. But be less reluctant to revise base ^>=4.18.0.0 into base ^>=4.18.0.0 || base ^>=4.19.0.0.

hasufell commented 1 year ago

@gbaz

First, for package maintainers they must have excellent test-cases to catch any possible dependency-induced regressions. In my experience, even a very good test-suite can often miss things if they were considered invariants inherited from dependencies.

I don't agree. How do these things get caught otherwise in the wild? People usually try to build packages with relaxed upper bounds and run their test-suite (if you are lucky), then create a PR to relax the bounds.

It's as if we're assuming some magic happens when maintainers or end-users get stuck with overly restrictive upper bounds. But that's not the case. You're setting higher standards for automation and assume people do more than the absolute minimum anyway when they do it manually.

Instead of waiting for manual human intervention, we can just do what those maintainers do when they realize there's an upper bound issue: build and run tests.

There are ways to automate that on a platform like hackage via release candidates, automatic CI builds and notifications. PVP shouldn't be concerned with those things. And that's an entirely different topic.

hasufell commented 1 year ago

@phadej

So I think that ^>= should be treated as a syntax sugar. Yes, we would lose (maybe) useful semantic difference. But the feature is simply too fancy to be universally used correctly.

I get your point and I must admit I also never have considered updating ^>= bounds to >= && < on older versions after publishing new releases or revisions.

Following that, I guess practically speaking ^>= only carries significant meaning in the latest version of a package.

Isn't that another argument to relax the requirement of upper bounds to a recommendation, so that active maintainers have a way to opt out of constant bounds bumping?

tomjaguarpaw commented 1 year ago

simplified subsumption breaking packages with looks base bounds

"loose base bounds" I guess

endgame commented 1 year ago

simplified subsumption breaking packages with looks base bounds

"loose base bounds" I guess

Corrected, thank you.

phadej commented 1 year ago

Isn't that another argument to relax the requirement of upper bounds to a recommendation, so that active maintainers have a way to opt out of constant bounds bumping?

No. PVP is setup so if no-one does anything, nothing breaks. If there are no upper bounds and stuff breaks, someone have to do emergency revisions, most likely Hackage Trustees.

Are you willing to be on duty, dropping anything else to do revisions?

As Hackage Trustee making these emergency revisions right after truly widely breaking releases was not fun. Lately there weren't as many "disasters", let's keep it that way.

hasufell commented 1 year ago

Are you willing to be on duty, dropping anything else to do revisions?

This is a sign that our current model is already broken.

andreabedini commented 1 year ago

I disagree with the proposal. IMHO the essence of upper bounds is this: being able to compile an old unmaintained package should be a no-op.

Of course using a newer GHC might not be possible, but with upper bound one is guaranteed to have a build plan that works.

As a test, someone tell me what dependency versions I need to make wai-middleware-auth work. It's not a trick, I did spend a couple of days trying to figure it out and gave up 😊

hasufell commented 1 year ago

As a test, someone tell me what dependency versions I need to make wai-middleware-auth work. It's not a trick, I did spend a couple of days trying to figure it out and gave up

Set the hackage index state to the date the package was added. Done.

This worked well:

cabal unpack wai-middleware-auth
cd wai-middleware-auth-0.2.6.0/
# index state is when the last revision was made
cabal build -w ghc-8.10.7 --index-state=2022-07-21T10:20:17Z
andreabedini commented 1 year ago

Set the hackage index state to the date the package was added. Done.

:joy: true, that works :relaxed: But you understand this is roughtly equivalent to having upper-bound right? only that you have many more than you need.

hasufell commented 1 year ago

Set the hackage index state to the date the package was added. Done.

joy true, that works relaxed But you understand this is roughtly equivalent to having upper-bound right? only that you have many more than you need.

Yes, but it's not enforced by the spec.

An ecosystem could decide to handle rolling release in a different way than hackage (e.g. through enforced CI builds, release candidates, sign-off phases etc.) instead of absolute anarchy (like on hackage).

I don't see why the spec should be tied to hackage's historical decisions. Hackage can be stricter than the spec (and probably just should).

I want to remove this odd dependency between hackage and PVP spec.

phadej commented 1 year ago

I'd remind you, @hasufell, that PVP is the versioning policy used on Hackage now. And the readme clearly says

Formally, the PVP is maintained by the Core Libraries Committee together with the Hackage Trustees.

So if you want to make changes as if PVP were independent, you first need to make PVP and Hackage independent of each other, and only then propose your changes to non-Hackage versioning policy.

As I commented on other issue, PVP is the Hackage versioning policy. If you want/need a policy for non-Hackage, you are free to make your own. (That would cause less confusion than repurposing PVP).

hasufell commented 1 year ago

So if you want to make changes as if PVP were independent, you first need to make PVP and Hackage independent of each other, and only then propose your changes to non-Hackage versioning policy.

I don't think so. PVP is versioned. Hackage will depend on the legacy version and then is free to update to the more slimmed down version, adding its own policies to it.

andreabedini commented 1 year ago

An ecosystem could decide to handle rolling release in a different way than hackage (e.g. through enforced CI builds, release candidates, sign-off phases etc.) instead of absolute anarchy (like on hackage).

I agree with what @phadej says. There is no anarchy on Hackage, Hackage has a policy which is PVP. An ecosystem outside of Hackage can have a different policy.

juhp commented 1 year ago

Stackage notifications only occur when new stackages are released -- which means all users who are ahead of stackage gain no benefit from them. In fact, stackage maintainers themselves reap large net benefits from good bounds practices making stackage solving itself more practical and ergonomic.

That's not true - Stackage has to deal with all the upperbounds breakage day by day and we report issues frequently both in our issue tracker which alerts maintainers and sometimes also to upstream. I would say Stackage contributes significantly to fixing upperbounds issues, also thanks no little to our active contributors.

Also noting I was not writing with my Stackage hat on.

hasufell commented 1 year ago

There is no anarchy on Hackage

I disagree:

andreabedini commented 1 year ago

I disagree:

I agree that situation (whom both you and I are passinate to improve) is far from optimal. I don't understand how dropping the requirement of specifying upper bounds is meant to improve it.

Let's work on something productive:

None of this has to be built from scratch. There's plenty of prior art: pvp checkers, tools to manipulate cabal files, nightly builders, notification workflows, etc, etc.

:muscle:

hasufell commented 1 year ago

Let's work on something productive

I'm sorry, but are you saying working on the spec is not productive?

I don't see why we can't have a productive discussion about:

  1. does requiring upper bounds make any sense spec wise?
  2. are there other ways to build a healthy ecosystem without enforcing upper bounds? (the answer is yes)
  3. do these concerns belong in the spec at all?

These are the questions I'm posing here. I'm trying to resolve questions about the spec, its scope and a bit broader "what should hackage be", but that can't fully be resolved in this ticket.

If you want to work on hackage-matrix-builder, Cabal or PVP tools, you're free to do so and gather volunteers e.g. on discourse.

But please let's stay ontopic.

andreabedini commented 1 year ago

I'm sorry, but are you saying working on the spec is not productive?

That's not the message I intended to convey :-/ I would be happy to continue our discussion in private if you like.