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

Consider releasing 0.21 (and yanking 0.20.2 and 0.20.3) due to a breaking change #294

Closed
jorendorff opened this issue Feb 2, 2024 · 10 comments

Comments

@jorendorff
Copy link

0.20.2 changed the name of function expression nodes from function to function_expression.

Broadly speaking, point releases shouldn't break downstream users in the Cargo ecosystem, and 0.20.2 broke us in GitHub code search. We have tree queries that search for functions, and function is no longer a valid thing to ask for.

This might affect other users at any time. The fix would be to re-release 0.20.3 as version 0.21.0, and then yank 0.20.2 and 0.20.3.

I guess the question of when to bump is not really settled tree-sitter ecosystem. It seems bad for a simple cargo update to break users, though. There's a proposal here to treat breaking changes to grammars as requiring a version bump.

@jorendorff jorendorff added the bug label Feb 2, 2024
@jorendorff
Copy link
Author

This got marked as a "bug" by an issue template and I can't change it. I don't mean to imply the parser has a bug.

@amaanq amaanq removed the bug label Feb 2, 2024
@amaanq
Copy link
Member

amaanq commented Feb 2, 2024

Hey, I've wanted to use semantic versioning for tree-sitter core and its grammars - but Max does not want to do that, not until at least core hits 1.0. And to be fair, the spec does say anything pre-1.0 is subject to breaking changes, even if it's a patch increase.

From the semantic versioning spec:

Major version zero (0.y.z) is for initial development. Anything MAY change at any time. The public API SHOULD NOT be considered stable.

It's also difficult to reason about what is a breaking change, it really depends on the downstream consumer's usage of it and queries. e.g. if I just use it for parsing or don't use the function node in my queries, then nothing broke technically. So to be strict about that, any change to any rule is a breaking change, which just is not feasible to work with, since only new rule additions would not be breaking technically (and could, by definition, still warrant a minor version bump).

@amaanq amaanq closed this as not planned Won't fix, can't repro, duplicate, stale Feb 2, 2024
@dcreager
Copy link
Contributor

dcreager commented Feb 2, 2024

I've wanted to use semantic versioning for tree-sitter core and its grammars - but Max does not want to do that, not until at least core hits 1.0

Is that true of the grammars too, or just the core libraries? Can you point me to a discussion or issue where that was discussed? I mildly disagree with that for the core libraries, and strongly disagree for the grammars.

@amaanq
Copy link
Member

amaanq commented Feb 2, 2024

Here's one place - tree-sitter/tree-sitter-regex#19

In another - I can't remember where, but Max wants every grammar + core to effectively be within nearby versions (0.20.x, 0.21.x, etc), so grammars wouldn't get that bump till core does, which I disagree with tbh

@jorendorff
Copy link
Author

jorendorff commented Feb 2, 2024

Hey, I've wanted to use semantic versioning for tree-sitter core and its grammars - but Max does not want to do that, not until at least core hits 1.0. And to be fair, the spec does say anything pre-1.0 is subject to breaking changes, even if it's a patch increase.

From the semantic versioning spec:

Major version zero (0.y.z) is for initial development. Anything MAY change at any time. The public API SHOULD NOT be considered stable.

Note that Cargo explicitly deviates from the semver spec for 0.y.z releases:

This compatibility convention is different from SemVer in the way it treats versions before 1.0.0. While SemVer says there is no compatibility before 1.0.0, Cargo considers 0.x.y to be compatible with 0.x.z, where y ≥ z and x > 0.

This is why cargo update broke us. In Cargo's view, 0.20.2 should not contain changes from 0.20.1 that are likely to break users.

@jorendorff
Copy link
Author

Cargo also has a surprisingly worked-out convention for what changes are considered "major" (such that, for 0.x releases, a "major" change requires bumping x).

The first bullet point in the list is

This of course refers to Rust items, not "items" in the sense of tree-sitter grammar node types. But the tree-sitter grammar is what users code against.

This is a judgment call and it's certainly up to the package maintainers. I think most Rust folks would consider this at least sketchy though.

In another - I can't remember where, but Max wants every grammar + core to effectively be within nearby versions (0.20.x, 0.21.x, etc), so grammars wouldn't get that bump till core does, which I disagree with tbh

Yeah, I can see wanting to do that, but it sure is in conflict with how Cargo uses these version numbers...

@dcreager
Copy link
Contributor

dcreager commented Feb 2, 2024

@amaanq thank you for the link! I had not seen that discussion since I'm not watching that grammar repo closely. I've moved some of the points into the higher level tree-sitter discussion in the interests of visibility: tree-sitter/tree-sitter#1768 (comment)

The main point from there that I'd bring back down here is really just to echo what @jorendorff said above: cargo (and npm and all the others) have specific interpretations of the semver spec, and we should make sure that the guarantees that we are and are not making line up with the version numbers that we publish to these package registries. If every grammar release is possibly backwards changing, we can work around that by using a =0.20.1 requirement, but that's not the default, so I anticipate that a lot of other folks will get bitten by this as well.

@maxbrunsfeld
Copy link
Contributor

Hey, I don't mean to be a blocker if there is a clear plan for improvement, and humans willing to do the work.

There was a time when tree-sitter's own language ABI changed frequently, and grammars' syntax node names changed almost constantly, so that in practice, it was much more important for the version to communicate the ABI version than it was to try to model the syntax tree changing using a version number.

The situation is somewhat different now; the ABI hasn't needed to change in a while, and there exist a lot of mature grammars that don't change that much. I'm fine with changing the policy. Do folks need anything from me to begin the work of sorting things out and instituting a new embrace of semver?

@maxbrunsfeld
Copy link
Contributor

In particular, are there crates.io or npm permissions that need to be granted to particular people at GitHub?

@amaanq
Copy link
Member

amaanq commented Feb 3, 2024

Well, I can always institute the actions I proposed initially using release-please to automatically follow semver, and generate a changelog in every grammar - it would just require using conventional commits but I think that's a great thing all around. This is what I use in all of my own grammars to follow semver and it works great.

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

No branches or pull requests

4 participants