Skip to content
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

Clarify usage of “patch” component #64

Closed
wants to merge 3 commits into from

Conversation

ffaf1
Copy link

@ffaf1 ffaf1 commented Nov 4, 2024

The spec and the FAQ contain the expression “patch-level”.

It is confusing for some users,see this Discourse thread. The confusion is understandable: “patch-level” is nor defined nor mandated by the policy; PVP leaves any component beyond the third in the hands of the maintainer.

This PR makes rectifies this, using a more consistent and clearer vocabulary.

@hasufell
Copy link
Member

hasufell commented Nov 4, 2024

I'm not sure we can retroactively change old spec versions?

@Bodigrim
Copy link
Collaborator

Bodigrim commented Nov 4, 2024

Target v1.1 please.

@@ -143,14 +143,14 @@ Now consider this scenario:
version number bumped, as required by the PVP.
2. Package B, which can still work with the old and new version of
package A, changes its dependency on package A to allow for both
versions. Package B only gets a patch-level bump.
3. Package C might or might not compile, depending on which patch-level
Copy link
Collaborator

@phadej phadej Nov 4, 2024

Choose a reason for hiding this comment

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

This is incorrect change.

Please read carefully through the scenario. In fact, here patch-level bump could be replaced with patch-level bump or revision.

Here, package B could be say parser-combinators, and the revision is from base < 4.21 to base <4.22. And we could pretend that base-4.21 removed Alternative IO instance. Consider then that parser-combinators exported a combinator based on Alternative (as it do)...

Copy link
Member

Choose a reason for hiding this comment

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

(Revisions are not part of PVP. Lets keep them out of it.)

Copy link
Author

Choose a reason for hiding this comment

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

The problem I wanted to solve with this PR is that “patch-level” is not defined and hence confusing. Despite D being often used as SemVer-patch-level and Hackage having revision, neither are in the spec, hence the rewording.

Does it make sense, @phadej?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Rewording doesn't make sense. Defined patch-level. Don't touch this example.

Copy link
Collaborator

Choose a reason for hiding this comment

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

(Revisions are not part of PVP. Lets keep them out of it.)

FAQ could mention them nevertheless. I think similar change was in FAQ section,

[^1]: In PVP, components beyond the third are optional, to be used in any
way the package maintainer sees fit. In practice, four-component
versioning (`A.B.C.D`) is standard.

Copy link
Contributor

Choose a reason for hiding this comment

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

Is this true? My recollection is hackage and/or cabal enforce we have no more than four components.

Copy link
Member

@hasufell hasufell Nov 4, 2024

Choose a reason for hiding this comment

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

That's irrelevant to the spec what cabal/hackage enforce.

Edit: Oh... although to be fair... in the FAQ we can be more lenient and consider hackag/cabal.

Copy link
Author

@ffaf1 ffaf1 Nov 4, 2024

Choose a reason for hiding this comment

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

The spec allows for more:

A package version number SHOULD have the form A.B.C, and MAY optionally have any number of additional components, […]

Cabal allows for more:

Build profile: -w ghc-9.6.3 -O1
In order, the following will be built (use -v for more details):
 - prova-0.1.0.0.18 (exe:prova) (first run)
Warning: this is a debug build of cabal-install with assertions enabled.
Configuring executable 'prova' for prova-0.1.0.0.18...
Warning: this is a debug build of cabal-install with assertions enabled.
Preprocessing executable 'prova' for prova-0.1.0.0.18...
Building executable 'prova' for prova-0.1.0.0.18...
[1 of 1] Compiling Main             ( app/Main.hs, dist-newstyle/build/x86_64-linux/ghc-9.6.3/prova-0.1.0.0.18/x/prova/build/prova/prova-tmp/Main.o, dist-newstyle/build/x86_64-linux/ghc-9.6.3/prova-0.1.0.0.18/x/prova/build/prova/prova-tmp/Main.dyn_o )
[2 of 2] Linking dist-newstyle/build/x86_64-linux/ghc-9.6.3/prova-0.1.0.0.18/x/prova/build/prova/prova
~$ 

I have not tried Hackage!

Copy link
Author

Choose a reason for hiding this comment

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

More generally, what I wanted to convey is “the standard says A.B.C, in real life you will 99% find A.B.C.D”. If it is not clear, I welcome rewording suggestions.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Copy link

@tbidne tbidne Nov 4, 2024

Choose a reason for hiding this comment

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

Hate to be a language lawyer, but I find it confusing for the opposite direction. To me, the line:

In PVP, components beyond the third are optional

implies that at least three components are required. But PVP says:

A package version number SHOULD have the form A.B.C, and MAY optionally have any number of additional components

where SHOULD means recommended. In practice, quite a few packages only use as many components "as-needed" e.g. hlint, optics. In fact, I'm not sure PVP excludes using a single component, and just bumping it for every change.

My interpretation is that PVP does not require placeholders, so anything beyond the A is optional. You still have to follow PVP when releasing new versions, though, so a minor update to A could be e.g. A.B.C, A.B, A+1.

@hasufell
Copy link
Member

hasufell commented Nov 4, 2024

I guess what we could do is: Mention in the spec itself that components after C are patch-level (e.g. after "A.B is known as the major version number, and C the minor version number").

I personally would like to move the "Special situations" and decision tree out of the spec and then not deal with this additional terminology at all.

ffaf1 added 3 commits November 4, 2024 09:10
In the instances example, use “minor version number” instead of
“patch-level”.

PVP mandates at least three components: two major ones (A, B),
and a minor one (C).  “Patch-level” is not defined in the PVP.

Using “minor version numer” in the example is consistent and
clearer.
Four-component versioning (A.B.C.D) with `D` being patch-level change
(lower risk than minor-level change) is ubiquitous, but not mandated
by the PVP.
@ffaf1
Copy link
Author

ffaf1 commented Nov 4, 2024

Target v1.1 please.

Sure!

I guess what we could do is: Mention in the spec itself that components after C are patch-level […].

As now any component beyond C is optional, its semantic left to the maintainer. If we mention “patch-level”, we need to reword that bit of the spec and also define what patch-level means and applies to.

I personally would like […] not deal with this additional terminology at all.

Me too, hence this PR. The example works totally fine with A.B.C and without requiring new jargon.
Adding new definitions is OK too, as long as they fix what happened in that Discourse thread (i.e. they are user friendly, clear and unambiguos).

@@ -146,14 +146,14 @@ Now consider this scenario:
version number bumped, as required by the PVP.
2. Package B, which can still work with the old and new version of
package A, changes its dependency on package A to allow for both
versions. Package B only gets a patch-level bump.
3. Package C might or might not compile, depending on which patch-level
versions. Package B only gets a minor version number bump.
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is still wrong. "Minor version bump" semantics suggests there are new features. Compatibility patch (e.g. relaxing bounds, or even smal lcode changes like adding CPP for compatibility), doesn't require minor version bump.

@phadej
Copy link
Collaborator

phadej commented Nov 4, 2024

If we mention “patch-level”, we need to reword that bit of the spec and also define what patch-level means and applies to.

We don't. PVP doesn't give any semantics to patch-level, but could have name for them. Note, the patch level is only mentioned in an example. That example could also say *gets bump in a 4th component (as the bump is not required in any previous one)", but patch-level is indeed a used name for 4th component, so going miles to avoid that name usage has no real benefit.

@ffaf1
Copy link
Author

ffaf1 commented Nov 4, 2024

If we mention “patch-level”, we need to reword that bit of the spec and also define what patch-level means and applies to.

We don't. PVP doesn't give any semantics to patch-level, but could have name for them.

That would really be the worst of both worlds, wouldn’t it? Within a spec, using a term that already has some kind connotation, without stating precisely what it means.

This is not an inane speculation of mine, there is the Discourse thread: a user — a user with multiple years of professional Haskell experience — unsure what the spec refers to with “patch-level”.
Compare with PVP referring to RFC2119 to define "MUST", "SHALL", etc. No space for amiguity there.

Same for it being “only mentioned in an example”. Example, yes, but still a spec document. No space for being sloppy in a spec. So I am partial to “define it or get rid of it”.

Let's see if we can gather more feedback from the other participants, this for sure is not urgent.

@phadej
Copy link
Collaborator

phadej commented Nov 4, 2024

“define it or get rid of it”.

PVP doesn't give any semantics to patch level version numbers. It could explicitly say that these version digits don't give any semantics.

If that's "define" in your sense, than I agree. But giving any other semantics to patch level is changing PVP in a way I don't want it to be changed.

without stating precisely what it means.

I.e. PVP could say that patch version don't mean anything (and versions which differ only in patch digits should be compatible with each other). But in my humble opinion that is already clear from current spec. Someone has to be really clueless to imagine something else. It could be explained in FAQ, given few examples what these can be used for if really needed.

@phadej
Copy link
Collaborator

phadej commented Nov 4, 2024

As now any component beyond C is optional, its semantic left to the maintainer.

Yes. That's exactly the point. PVP doesn't say which changes may or may not occur patch level. It says which changes MUST change major or minor version. And what doesn't need to change major or minor version could change patch version, or be a revision. Even is @hasufell doesn't want revisions mentioned in spec, they exist; cf. patch version exist, but IMO doesn't need to be mentioned in spec any more than revisions. (Note: what revisions can change is implementation defined, but it is possible to change things in metadata which would result a PVP violation: users needs to be aware of that and not do such revisions - an example is when packages re-exports definitions).

I don't understand how that is not clear from the current specification.

@hasufell
Copy link
Member

hasufell commented Nov 5, 2024

That would really be the worst of both worlds, wouldn’t it? Within a spec, using a term that already has some kind connotation, without stating precisely what it means.

This is how specs are. They are often purposefully vague. Introducing terminology doesn't mean that we apply semantics to them. We can debate whether "patch-level" is the right terminology for something we don't want to apply semantics to.

An alternative is just calling them "optional version components". But that's even worse, since technically even B and C are optional. Then we could call them "undefined version components".

I think "patch-level" is fine. It doesn't say what a "patch" is.


The only alternative I see is that we start cleaning up the spec and move everything Haskell related to another document that is not part of the actual spec (basically everything after the "Version numbers" section). Then we can just define patch-level or whatever else in that non-spec document.

But people have been rather resistant to this idea.

@ffaf1
Copy link
Author

ffaf1 commented Nov 18, 2024

I see there is little appetite for this proposal, hence I am closing it.

@ffaf1 ffaf1 closed this Nov 18, 2024
@ffaf1 ffaf1 deleted the patch-level branch November 18, 2024 09:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants