haskell-infra / hackage-trustees

Issue tracker for Hackage maintainance and trustee operations
https://hackage.haskell.org/packages/trustees/
42 stars 7 forks source link

Do or not do major-major revisions #375

Open phadej opened 8 months ago

phadej commented 8 months ago

E.g. in #373 @andreasabel made major-major revision, i.e. https://hackage.haskell.org/package/slist-0.2.1.0/revisions/

->=4.10.1.0 && <4.18
+>=4.10.1.0 && <5

I don't like that practice. I hope that @andreasabel (or whoever does similar revisions as a Hackage Trustee) puts these packages on their own watch list, so when next major base, containers, hedgehog etc is released they check that the package still compiles fine, and make correcting revisions in case it's not fine ASAP, perfectly before anyone notices.

phadej commented 8 months ago

In particular, in this case looks like Andreas "took over" bounds maintenership of some Kowainik packages (slist, colourista, trial which are dependencies of stan (and nothing else). It would be healthier if @tomjaguarpaw took over these packages if he is the new maintainer of stan, or worked on dropping these (unmaintained?) dependencies from stan.

phadej commented 8 months ago

In particular our policy says (related to relaxing revisions)

Trustees are expected to use this power judiciously and make sure they understand the packages involved and their APIs.

I'd expect the understanding be thoughtful if such super-major relaxations are made.

endgame commented 8 months ago

I disagree with base <5 in revisions for the same reasons that I disagree with it in package uploads: it allows new versions which have explicit permission to make breaking changes.

tomjaguarpaw commented 8 months ago

For the record, I have requested co-maintainership of the entire Kowaink ecosystem, but only been granted maintainership of stan.

andreasabel commented 8 months ago

When you are placing an upper bound, you are making a bet either way.

  1. If you place a tight upper bound, like <4.20, you are betting that the package will break when 4.20 comes out. (Pessimistic view.)
  2. If you place a loose upper bound, like <5, you are betting that the package will work when 4.20 comes out, and also with 4.21 and maybe with 4.22 etc. (Optimistic view.)

In my limited experience so far, I found that the optimistic view is more "correct" for stable packages in the sense that I bet correctly most of the time, while more caution is required for unstable or feature-rich packages.

One should also look at the damage caused by either way of placing the upper bound.

  1. A tight upper bound that is not diligently updated will cause the package to be stuck in the past render the package unusable (or at least hard to use) by others. Often there is no material reason for the tight upper bound, i.e., the package turns out to work the same for many major bumps of its dependencies. The package needs a revision to get unstuck.
  2. A loose upper bound that loses the bet leads to compilation errors, forcing users to investigate what the reason of the breakage is. The package needs a revision, and often previous versions of it. The latter is what lay maintainer often forget.

Either way, an upper bound that does not reflect the truth causes damage. Either way, the package needs attentive maintainership. The question is how to minimize the work and the damage.

For stable packages, my own resolution is that the optimistic strategy has an overall better performance.

phadej commented 8 months ago

If you place a tight upper bound, like <4.20, you are betting that the package will break when 4.20 comes out. (Pessimistic view.)

It's not betting, it's playing by the rules (of PVP).

That is not pessimistic. It's realistic. PVP doesn't give more guarantees. Maybe CLC will suddenly change their stance and start making vast changes to base (say, a miracle refactoring tool appears which would allow migrating code swiftly, as long as someone migrates it).

For stable packages, my own resolution is that the optimistic strategy has an overall better performance.

That's non-sense.

base <5 bounds are meaningless, there are no widely agreed semantics to < super bounds (PVP doesn't mention any). You can as well put something random like base <4.24 or base <100. It is silly that Cabal / Hackage force you to put some base bound, where you really want to not have any.

Once a revision to package-version had made by Hackage Trustee, the responsibility of keeping that package building is on all of Hackage Trustees.

You can make them as individual to packages you (co-) maintain, say Agda or cassava. But I don't want Hackage Trustees making such revisions as Hackage Trustees.

If you don't feel comfortable leaving the upper bound out completely, don't make major-major relaxations either.

davean commented 8 months ago

There is no bet, you're saying it's unknown. It is in fact unknown.

On Sun, Oct 15, 2023 at 9:52 PM Oleg Grenrus @.***> wrote:

If you place a tight upper bound, like <4.20, you are betting that the package will break when 4.20 comes out. (Pessimistic view.)

It's not betting, it's playing by the rules (of PVP).

That is not pessimistic. It's realistic. PVP doesn't give any other guarantees. Maybe CLC will suddenly change their stance and start making vast changes to base (say, a miracle refactoring tool appears which would allow migrating code swiftly, as long as someone migrates it).

For stable packages, my own resolution is that the optimistic strategy has an overall better performance.

That's non-sense.

base <5 bounds are meaningless, there are no widely agreed semantics to < super bounds (PVP doesn't mention any). You can as well put something random like base <4.24 or base <100. It is silly that Cabal / Hackage force you to put some base bound, where you really want to not have any.

Once a revision to package-version had made by Hackage Trustee, the responsibility of keeping that package building is on all of Hackage Trustees.

You can make them as individual to packages you (co-) maintain, say Agda or cassava. But I don't want Hackage Trustees making such revisions as Hackage Trustees.

If you don't feel comfortable leaving the upper bound out completely, don't make major-major relaxations either.

— Reply to this email directly, view it on GitHub https://github.com/haskell-infra/hackage-trustees/issues/375#issuecomment-1763606588, or unsubscribe https://github.com/notifications/unsubscribe-auth/AABZLS5TG5XKJB34VJVR5BTX7SHNPAVCNFSM6AAAAAA6ASKXRKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTONRTGYYDMNJYHA . You are receiving this because you are subscribed to this thread.Message ID: @.***>

gbaz commented 8 months ago

Oleg's right here. We can make our own choices for our own packages, though there's good arguments for tight bounds regardless. However, for trustee revisions, trustee policies dictate that tight and not speculative bounds be used.

ivanperez-keera commented 7 months ago

I have to agree with @phadej here. Revising the package to use base < 5 is not conformant with PVP and I'd be unhappy if my package was revised to introduce that change.

hasufell commented 7 months ago

Revising the package to use base < 5 is not conformant with PVP

I disagree. As usual, the PVP spec is not well worded.

The relevant section is here:

When publishing a Cabal package, you SHALL ensure that your dependencies in the build-depends field are accurate. This means specifying not only lower bounds, but also upper bounds on every dependency.

So "accurate dependencies" as per the spec just means "has lower and upper bounds". It doesn't say it must be tight bounds. In fact, it says later that even tight bounds can break compilation:

To minimize breakage when new package versions are released, you can use dependencies that are insensitive to minor version changes (e.g. foo >= 1.2.1 && < 1.3). However, note that this approach is slightly risky: when a package exports more things than before, there is a chance that your code will fail to compile due to new name-clash errors.

Note the wording "you can". So this is not a requirement.

I can't comment on the hackage trustee policy part.

juhp commented 7 months ago

Maybe CLC will suddenly change their stance and start making vast changes to base (say, a miracle refactoring tool appears which would allow migrating code swiftly, as long as someone migrates it).

That should then be base >= 5 in my opinion.

For stable packages, my own resolution is that the optimistic strategy has an overall better performance.

I agree with Andreas

Tightening loose bounds is easier than loosening overtight ones.

Overtight bounds prevent building packages that legitimately can build, just because they might not...

hasufell commented 7 months ago

Also note that CLC requires impact assessments and I believe these would (at least partly) catch issues with too loose upper bounds.

This is the way: testing reverse dependencies and then notifying maintainers of upcoming issues instead of specifying pessimistic upper bounds.

But hackage doesn't facilitate this workflow.

So this really boils down to social trust, policies of individual maintainers and off-hackage communication. This is why I believe enforcing upper bounds is a misguided attempt at fixing a much larger issue.

ivanperez-keera commented 7 months ago

Overtight bounds prevent building packages that legitimately can build, just because they might not...

Overtight bounds pass the responsibility of checking the work on the developer of the package in question.

There's a chance, however slim, that a function might change its meaning between majors without altering its signature. A package that depends on such a function may therefore build, giving a false sense of being compatible with a version of base, when in reality it behaves incorrectly. Such errors may be very tricky to detect.

I think there're better approaches at helping package devs coordinate with GHC devs so that they quickly release new versions of their respective libraries after a GHC release.

hasufell commented 7 months ago

I think there're better approaches at helping package devs coordinate with GHC devs so that they quickly release new versions of their respective libraries after a GHC release.

Yes, instead of shoving everything into head.hackage for no one to know, maintainers could actually be notified. But we're digressing.

Either way is perfectly PVP compatible.

ivanperez-keera commented 7 months ago

But we're digressing.

I don't think we are. Bumping < 4.X to < 5 is a matter of convenience because developers don't publish updated versions quickly.

If we manage to improve communication and give developers better ways to update their packages very quickly, there'll be no need to bump to < 5 for them.

hasufell commented 7 months ago

That is likely a larger project that needs funding by the HF, so I don't think anyone is going to solve it short term and hackage trustees will need to keep making decisions.

gbaz commented 7 months ago

The pvp faq makes quite clear that the intended meaning of the upper bounds policy is tight upper bounds, and makes some arguments as to why:

https://pvp.haskell.org/faq/#upper-bounds

Loose or "speculative" upper bounds have the problem that a single update of an upstream package can break those bounds for all versions of the target package, and thus require n revisions. Meanwhile tight upper bounds require typically one revision per breakage (of the latest version alone), which is among the many reasons tight upper bounds are the policy.

There are other places to debate this policy, but I want to be clear on what it is. While this policy stands, individual users may of course choose to abide by it or not, but hackage trustees are obliged, in their actions as hackage trustees, to abide by it.

That said, let me respond to this:

Yes, instead of shoving everything into head.hackage for no one to know, maintainers could actually be notified.

In fact, hackage now has opt-in email notifications if any package you maintain has a dependency updated such that it breaks its bounds, which is a huge step forward!

The builders of course don't automatically attempt to bump the bound and test it for you, but nonetheless this should hopefully provide a convenient way to stay on top of such things.

hasufell commented 7 months ago

The pvp faq

PVP faq is not the spec text and contains all sorts of questionable statements.

gbaz commented 7 months ago

yes but it explains the intended meaning of the text, which i believe corresponds to the body of the text quite clearly. you may question the statements, but that does not invalidate them. i'm not interested in rules-lawyering this. as someone who has been involved in the pvp body and faq both, as well as discussions around it for some time, as well as someone who is responsible for the hackage trustee process, i'm just clarifying what the policies are. you may not like them, you may not think they are justified, or written clearly, or justified clearly. i'm not arguing about any of that. i'm just saying what they are.

gbaz commented 7 months ago

if we do want to lawyer, here is the contested text again:

When publishing a Cabal package, you SHALL ensure that your dependencies in the build-depends field are accurate. This means specifying not only lower bounds, but also upper bounds on every dependency.

Accurate bounds are necessarily tight bounds, not speculative bounds. The subsequent sentence does not amend the prior one to make it less restrictive. it just clarifies that the scope of the prior sentence includes upper and lower bounds both.

Here is the other contested language:

To minimize breakage when new package versions are released, you can use dependencies that are insensitive to minor version changes.

The "you can" means that "accurate" bounds may be insensitive to minor version bumps. It does not invalidate the "shall" requirement earlier.

hasufell commented 7 months ago

yes but it explains the intended meaning

The intended meaning belongs into the spec text, not the FAQ. The FAQ is there to guide users on the application of the spec with additional examples.

There is no way to conclude from the spec text alone that it demands tight upper bounds. Please raise a PR if you think the spec text is wrong.

The FAQ is not meant to be a prose patchset to fix unintentional (which is up to debate) ambiguity of the spec.

hasufell commented 7 months ago

I think that is an interesting discussion wrt PVP, but I suggest to continue it here: https://github.com/haskell/pvp/pull/57

This thread is more about hackage trustees policies, in my view, and they can make whatever decision they please.