antlr4 icon indicating copy to clipboard operation
antlr4 copied to clipboard

Head towards 4.11 release

Open parrt opened this issue 3 years ago • 20 comments
trafficstars

Heh, we have to get 4.11 out so that people can use the amazing Go target what day is done by @jimidle.

Stuff to do:

  • [ ] Make sure to generate a Windows dotnet package from Windows to get the signing right
  • [ ] Update antlr.org quick start guide to use https://github.com/antlr/antlr4-tools
  • [x] @jimidle any more Go Target stuff that needs to go in?
  • [x] Make sure 4.11.0 tag is also they're not just 4.11 (Go requirement)
  • [x] C++ issue regarding EOF symbol: https://github.com/antlr/antlr4/issues/2914
  • [x] @KvanTTT's https://github.com/antlr/antlr4/pull/3701
  • [x] https://github.com/antlr/antlr4/issues/3733 ST uses 3.5.2 antlr not most recent.
  • [x] Fix old error https://github.com/antlr/antlr4/issues/2016, merging https://github.com/antlr/antlr4/pull/2022
  • [x] https://github.com/antlr/antlr4/pull/3841
  • [x] https://github.com/antlr/antlr4/pull/3794

What else needs to go in 4.11?

@jimidle @ericvergnaud @mike-lischke @marcospassos @pboyer, @janyou, @ewanmellor, @lingyv-li have comments?

parrt avatar Aug 27 '22 18:08 parrt

I wish the following issues were resolved:

  • https://github.com/antlr/antlr4/pull/3676 It decreases the size of generated files and improves performance
  • New action-related warnings: https://github.com/antlr/antlr4/pull/3626 (it's better to reimplement them on ATN instead of grammar)
  • Fixes related to "mutually left recursive rules" errors: https://github.com/antlr/antlr4/pull/3779
  • Last but not least refactoring: https://github.com/antlr/antlr4/pull/3780

KvanTTT avatar Aug 27 '22 20:08 KvanTTT

Hmm..not sure I have the time to go through those for now. It will require a massive effort to remember all of the ATN stuff before I can evaluate those. The last one is something I probably won't accept because it's just refactoring and incurs a risk.

parrt avatar Aug 27 '22 20:08 parrt

🙌 Nothing to add for Dart.

lingyv-li avatar Aug 28 '22 06:08 lingyv-li

The EOF macro problem is tricky to solve. Either you annoy users who use C libs with ANTLR4 or you annoy existing project owners if you rename that symbol. Given that there are only very few complains I tend to keep the current solution. For the threads patch @jcking can probably help.

mike-lischke avatar Aug 28 '22 09:08 mike-lischke

Yeah, breaking all existing users' code that ref EOF is problematic; i.e., we shouldn't rename. The undef trick @jcking suggested might not even be enough since someone can stick an include of C stdio.h after every include file. Wouldn't it be reasonable to ask the few users that need C libs to add #undef EOF right after ever #include that pulls in a EOF macro? Let me see if I can do a test.

parrt avatar Aug 28 '22 19:08 parrt

I don't think those issues impact the PHP target 🚀

marcospassos avatar Aug 29 '22 12:08 marcospassos

I will try to get in the remaining fixes (such as test failures) in the next two days, then I think the go runtime is good for 4.11 and I can move to more performance analysis

jimidle avatar Aug 30 '22 04:08 jimidle

Awesome. Long weekend coming up and I hope to release the software over the weekend!

parrt avatar Aug 30 '22 15:08 parrt

@parrt Some prep and a little faffing about is needed to release the go runtime at 4.11.0:

The current v4.x.x tags do not work because only tags at v0.x.x and v1.x.x are picked up from a repo when the go.mod does not include a vn tag for the module. So currently, while we have a go.mod file, the go mod xxxx commands do not see any valid tags, so they are just syncing to last hash (HEAD) of the master branch.

So we need to:

  • Push a new tag v1.4.10 to point at the same hash as the 4.10.1 tag - this means that existing users who do not move to the v4 module are covered as they will stay on what we know as the 4.10.1 release even if they do a go get -u - in other words, backwards compatibility.
  • Delete the existing v4.x.x tags as when we move to a v4 module, those tags will be confusing (and will not work), as they won't include the change to the v4 module
  • We need to do this now so that pkg.go.dev has enough time to rebuild docs for the old release. Maybe I have permissions to do that as a member. If not then I will have to ask you of course.

That will get the current source into shape for go modules and as soon as the new tag is there, existing go modules will work properly instead of by accident. Note that pure GOPATH mode will not be supported anymore when we release. Go modules were introduced in go 1.11 and were default from go 1.13 - this should not be an issue for anyone as we have already been shipping a go.mod file, but it only partially works. This fixes it all.

Then, I will create a PR to move the module to v4:

  • The suffix v4 is used in the module name so that the import becomes github.com/antlr/antlr4/runtime/Go/antlr/v4 - this is SOP for Go when a module changes from a 1.x.x release.
  • Upgrading users will need to change their import modules
  • I will need to change any internal references to the module to also use /v4 in the module reference - this includes the codegen template and the test generator - all trivial stuff and the test setup will simplify a little as a result, which will speed go tests up a little, by avoiding some fork/exec stuff.

Then when you merge into master from dev, we need only do what you did before, which is create a tag v4.11.0 pointing to the same hash as the tag 4.11.0 (or 4.11 if you do it that way, but I think semver wants the minor even if it is 0). After that point, the tags will all work correctly in go modules.

You can see that the current tags are being ignored by go mod xxxx in two ways:

  1. When go mod tidy or go get are issued on a project using ANTLR, you will see that the version in your project's go.mod is require github.com/antlr/antlr4/runtime/Go/antlr v0.0.0-20220826213629-cd8f367ca010. In other words, it does not use the tags and the project uses the HEAD of master, which is not what we want.
  2. The pkg.go.dev system is also unable to see the v4.x.x tags and lists the version the same way go.mod does: ANTLR docs so it also lists the module as version v0.0.0.xxxxxx

I have detailed the requirements above here extensively, but you can see that this is relatively trivial:

  1. Fix existing tags
  2. Accept PR to move module to /v4
  3. Release as normal, including create a v4.11.0 tag

We should document how to move to the v4 module in the release notes. I will also document this in the published runtime docs for the new module, which will list as v4.11.0, which keeps everything in sync from here on in.

NB: Changing to the v4 module normally implies breaking the public API, but in this case, that will not happen - nobody will need to change their existing code, other than to properly specify the module number.

BTW - if you migrate to use git flow (AVH version is best), you will be able to do emergency fixes in master and automatically keep dev up to date. It is simpler than standard git commands. There is a Maven plugin that takes care of this and a git plugin for standard command lines. It does not affect github PRs etc.

jimidle avatar Aug 31 '22 03:08 jimidle

As I thought, I do not have permissions to mess with the tags. So:

image

From these, delete the two v4.x.x tags and add a new v1.4.10 tag that points to the same hash as tag 4.10.1 (which is of course the same hash as v4.10.1)

jimidle avatar Aug 31 '22 04:08 jimidle

With PR #3852, the Go runtime is now ready for release. But please see above about the existing tags and new tag on the Master branch. We need to take care of those tags, before merging this PR from dev->master

Also, when you pull in my PRs, if you would please tag the hash with v4.11.0-beta or something similar, I can test that the v4 path for the runtime is hunky dory.

jimidle avatar Aug 31 '22 09:08 jimidle

Hmm... Not sure I want to delete all of my tags. I will have to read your comment in detail but people of been telling me it works with v4.10.1 etc... OH! I get it now. Just the v versions not the 4.10.1 stuff. The v1.4.10 looks kind of weird. Are those only going in dev not master? I would hate to have a tag in master that said 1.4.10.

I wonder if we can simply get away with removing v4.10.0 instead of adding the weird v1.4.10? i.e., let's make a clean break? Most people are just pulling from master and their code will still just work, right? Nah, I'll follow your lead and add v1.4.10 for backward compatibility.

parrt avatar Aug 31 '22 21:08 parrt

From these, delete the two v4.x.x tags and add a new v1.4.10 tag that points to the same hash as tag 4.10.1 (which is of course the same hash as v4.10.1)

Done! Killed v... and added v1.4.10 at 4.10.1

I resolved the conflicting GoRunner file and will merge after tests!

Ok, merged! Thanks a million! I think we're cleaned up on Go now.

parrt avatar Aug 31 '22 22:08 parrt

Also, when you pull in my PRs, if you would please tag the hash with v4.11.0-beta or something similar, I can test that the v4 path for the runtime is hunky dory.

Added v4.11.0-go-beta for ya!

parrt avatar Aug 31 '22 23:08 parrt

OK - so to sort out the tags for go modules - sorry about the messing around, it is tricky:

  • We can put the old ones back exactly as they were, as some people seem to have used them, not realizing that they were meant for go - so be it
  • Delete the current v1.4.10 tag
  • Delete the v4.11.0-beta tag

Now we need to tag the go runtime slightly differently so that go modules embedded in the mono-repo will work and we can restore the old v4 tags as they were.

In the fully synced dev branch at the root of the repo (please note the exact paths in the commands below - they are very sensitive):

~/antlr/antlr4 (dev ✔) ᐅ git tag -a runtime/Go/antlr/v4/v4.11.0-go-beta -m "Go runtime module only" 
~/antlr/antlr4 (dev ✔) ᐅ git push origin runtime/Go/antlr/v4/v4.11.0-go-beta 

This allows go to realize that that tag is for the v4 module path

Now, in the master branch (before we pull inthe latest dev):

~/antlr/antlr4 (master ✔) ᐅ git tag -a runtime/Go/antlr/v1.4.10 -m "Go runtime module only" 
~/antlr/antlr4 (master ✔) ᐅ git push origin runtime/Go/antlr/v1.4.10 

When the release is made, then in the master branch:

~/antlr/antlr4 (master ✔) ᐅ git tag -a runtime/Go/antlr/v4/v4.11.0 -m "Go runtime module only" 
~/antlr/antlr4 (master ✔) ᐅ git push origin runtime/Go/antlr/v4/v4.11.0

This should then give us:

  • A beta tag from the dev branch, which I can test.
  • A release tag that works with go modules, which I can immediately verify
  • Existing projects that do not use modules or think that they are should work as they do now (but, if they use no modules at all, then they will autoupgrade to 4.11.0 without knowing - this seems a reasonable compromise as they will soon know

Now, a question is that we could also add a tag to the release so that if a project does not switch to the v4 module path, then a go get -u will tell them that they are now on 4.11. This seems like it might be better to do this for this release because I can add a deprecated tag to the existing go.mod telling them to switch to the v4 path and pointing at the docs.

I think we should do that, which means after you make the release in to master, as well as the v4 tag, add this tag:

~/antlr/antlr4 (master ✔) ᐅ git tag -a runtime/Go/antlr/v1.4.11 -m "Go runtime module only" 
~/antlr/antlr4 (master ✔) ᐅ git push origin runtime/Go/antlr/v1.4.11 

I think that then everyone will be happy and we do not need to change the tags in the master branch. Because we tag the submodules, the go mod will now happily ignore the previous v4.x.x tags.

Phew! That took some working out.

jimidle avatar Sep 01 '22 06:09 jimidle

@parrt Please let me know when you have fixed up the tags, I will verify that they work before you make the release.

jimidle avatar Sep 02 '22 03:09 jimidle

@jimidle done:

  • [x] We can put the old ones back exactly as they were, as some people seem to have used them, not realizing that they were meant for go - so be it
  • [x] Delete the current v1.4.10 tag
  • [x] Delete the v4.11.0-beta tag

will do the other bits next

parrt avatar Sep 02 '22 16:09 parrt

I also suggest completing clearing up of C# runtime: https://github.com/antlr/antlr4/pull/3861 Opened ANTLR4 C# runtime project looks like colorful Christmas tree in Rider. That's why I would like to fix most part of warnings.

KvanTTT avatar Sep 02 '22 17:09 KvanTTT

Ok, @jimidle you should have your v4.11.0-go-beta (dev) and v1.4.10 (master) tags and I updated the release docs to indicate more details on v tags. Can you verify I did the tags on the right branch?

parrt avatar Sep 02 '22 17:09 parrt

Ok, looks like i've checked off all the items for release. Gotta check antlr4-tools but I'll start heading towards release.

parrt avatar Sep 04 '22 17:09 parrt