Skip to content

Drop the requirement of specifying upper bounds #51

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

hasufell
Copy link
Member

at least specifying lower bounds. You **SHALL* at least specify upper
bounds if there are known incompatibilities.
It is **NOT REQUIRED** to set an upper bound if all currently known
versions of a dependency are compatible, although it is **RECOMMENDED**.
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could also talk about the caret operator here, but I think that is for another PR and a different discussion.

@hasufell
Copy link
Member Author

hasufell commented Jul 15, 2023

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

@gbaz
Copy link
Contributor

gbaz commented Jul 15, 2023

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
Copy link
Member Author

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.

do not follow this convention. The aim is that before this happens, we
will put in place tool support that makes it easier to follow the
convention and less painful when dependencies are updated.
at least specifying lower bounds. You **SHALL* at least specify upper
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing *

@gbaz
Copy link
Contributor

gbaz commented Jul 15, 2023

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
Copy link
Member Author

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
Copy link
Contributor

gbaz commented Jul 15, 2023

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

@hasufell
Copy link
Member Author

hasufell commented Jul 15, 2023

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

pvp/pvp-faq.md

Lines 108 to 119 in c633fe8

This assumes that compile-success is equivalent to semantic
correctness. While it's true that a compile failure implies that a
breakage has occurred, the inverse is not true in general.
There's been already a couple of incidents (see next Q) when popular
packages changed their semantics without changing their type-signature
and thereby caused problems in packages which didn't have proper
PVP-mandated upper bounds in place.
Therefore leaving off upper bounds under the assumption that breakages
will show in form of build-failures is a dangerous erroneous belief,
as it can result in hard to detect/debug silent failures.

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
Copy link
Contributor

gbaz commented Jul 15, 2023

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
Copy link

juhp commented Jul 15, 2023

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
Copy link
Contributor

gbaz commented Jul 15, 2023

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
Copy link

endgame commented Jul 16, 2023

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
Copy link
Collaborator

phadej commented Jul 16, 2023

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

One could argue that

  • ^>=1.2.3 could mean: "We haven't tried with 1.3 version, it might or might not work".
  • and >=1.2.3 && <1.3 could mean "We tried with 1.3 version, and it doesnt' work".

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.

  • It's easy to forget, as the (slightly) incorrect metadata doesn't manifest easily.
  • It's busywork to make these revisions
  • Usual CI setups don't run builds for old releases
  • And even if they did, there aren't --allow-newer=^ relaxing all caret versions (only per-package: --allow-newer=*:^somedep

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).

  • This helps as Stackage acts as "few expert maintainers".
  • And expertise and tools are needed, because when a caret version problem with a package version in Stackage is found, someone has to check older releases too.
  • Here I think that it would be useful for Stackage, but everyone else could still consider caret versions as just syntactic sugar.

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
Copy link
Collaborator

phadej commented Jul 16, 2023

Addition to previous:

  • Without semantically diffenent ^>= bounds (syntax sugar), the package repository only needs to support revisions to correct metadata. If no mistakes are made, the amount of revisions is minimal. (Relaxing bounds is a bonus).
  • With semantically different ^>= (not known to not work), the package repository MUST support revisions, as all potentially all ^>= bounds should be eventually turned into >= .. && < (known to not work) bounds.
    • in other words, opting out of revisions should not be possible if the caret versions are used.

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
Copy link
Member Author

@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
Copy link
Member Author

@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
Copy link
Member

simplified subsumption breaking packages with looks base bounds

"loose base bounds" I guess

@endgame
Copy link

endgame commented Jul 16, 2023

simplified subsumption breaking packages with looks base bounds

"loose base bounds" I guess

Corrected, thank you.

@phadej
Copy link
Collaborator

phadej commented Jul 16, 2023

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
Copy link
Member Author

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
Copy link

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
Copy link
Member Author

hasufell commented Jul 16, 2023

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
Copy link

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

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

@hasufell
Copy link
Member Author

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
Copy link
Collaborator

phadej commented Jul 16, 2023

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
Copy link
Member Author

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
Copy link

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
Copy link

juhp commented Jul 17, 2023

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
Copy link
Member Author

There is no anarchy on Hackage

I disagree:

  • hackage does not enforce PVP, you're free to upload whatever you want
  • hackage has no CI system (anymore)... you have no idea what you break when you do/upload anything... no early warning system based on release candidates etc.
  • hackage revisions are a disaster... instead of enforcing conformance or dealing with it like a proper managed ecosystem (like stackage), it lets you upload trash and then calls the trustees to fix all errors post-release
  • the hackage-specific parts of PVP have stagnated and don't reflect the current status of Cabal anymore... it's not clear what hackage is following at the moment... practically nothing, I'd say

@andreabedini
Copy link

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:

  • Let's build that CI again, what do we need?
  • Let's build tooling that help developers

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.

💪

@hasufell
Copy link
Member Author

hasufell commented Jul 17, 2023

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
Copy link

andreabedini commented Jul 18, 2023

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.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants