apache / pekko

Build highly concurrent, distributed, and resilient message-driven applications using Java/Scala
https://pekko.apache.org/
Apache License 2.0
1.18k stars 142 forks source link

source release: document use of version.sbt #339

Closed pjfanning closed 1 year ago

pjfanning commented 1 year ago

At the moment, when we use sbt to build jars, sbt plugins create a SNAPSHOT version number based on the last git tag - eg 0.0.0+26669-ec5b6764-SNAPSHOT.

When we put the source in a dir without having the dir under git control, the sbt plugins construct a version with HEAD-<date>-<time> - eg HEAD+20230524-1416

We have not fully discussed what to do as part of a release but it sort of makes sense to add a version.sbt like:

ThisBuild / version := "1.0.0"

Note that Apache releases are done manually by Release Managers (humans) and that the release won't be based on an automated job that starts when a git tag is added. The git tag is still useful but it doesn't drive the build.

If we include this file in the source release (generated by the sbt-source-dist plugin), then that affects the jar names that will be created and I think it makes sense that our '1.0.0' source release builds jars with '1.0.0' version on them.

fyi @mdedetrich, @raboof, @jrudolph

mdedetrich commented 1 year ago

A lot of the points we already discussed in https://github.com/apache/incubator-pekko/issues/130 but unless there is a formal ASF requirement I wouldn't put that much effort/energy into putting build.sbt into the source package and using it as a source of truth for the version.

As stated elsewhere, aside from verification purposes there is going to be close to no one actually downloading the source package, everyone will download the binary jars from maven via their JVM based build tool as a dependency. Also as discussed in https://github.com/apache/incubator-pekko/issues/130#issuecomment-1410210094, the reason why we wanted to use git tags is that it works completely seamlessly with sbt-dyn-ver and because of that the entire release process is very simple.

In short, if we start hardcoding the version in build.sbt at best its largely going to be ceremonial and at worst it can cause problems/conflicts. In fact sbt-dynver already states to NOT use a static version in build.sbt, directly quoting their README

Then make sure to NOT set the version setting, otherwise you will override sbt-dynver.

Note that Apache releases are done manually by Release Managers (humans) and that the release won't be based on an automated job that starts when a git tag is added. The git tag is still useful but it doesn't drive the build.

This was already discussed and its quite easy to handle this problem. Release managers are responsible for creating a git tag when a release is being announced and other committers can either refer to that tag to build the software or just download the source (which the git tag points to).

If we include this file in the source release (generated by the sbt-source-dist plugin), then that affects the jar names that will be created and I think it makes sense that our '1.0.0' source release builds jars with '1.0.0' version on them.

It will also completely break sbt-dynver which all of our snapshots rely on to work correctly.

pjfanning commented 1 year ago

It does not break sbt-dynver. Having the version explicitly declared just overrides what sbt-dynver does.

There is no reason why we can't set the version explicitly for the release (in build.sbt or version.sbt) and remove it after the release.

We need the Incubator PMC to sign off on our source releases - so making it easy for people to use the source release to build 1.0.0 jars makes a lot of sense.

mdedetrich commented 1 year ago

I understand but I re-iterate my earlier point about it being largely ceremonial and hence its just adding extra steps. This was discussed either in the earlier thread or on the mailing list, but iirc there isn't any problem in creating our own release process which can just state "clone the git repository at this tag".

Also the version just controls the filename of the jars being made, everything else about the build is the same. Again I don't really know if this breaks some ASF process, but one thing I have noticed on this journey is that a lot of "rules" are in fact just formalities/habits.

If there is an actual rule that committers/IPMC verifying a release are not allowed to clone the git repo and check out a tag then I don't have a problem temporarily setting and deleting the version even though it basically kills the premise of sbt-dynver (which is that the git tag is the source of truth). Even in the case where checking out a git repository is not allowed, you can simply set the version when starting sbt avoiding having to manually edit the build.sbt/version.sbt file, i.e.

sbt "set ThisBuild / version := \"1.0.0\"; shell"

This force the version for the project before going into the standard sbt shell. You can also do

sbt "set ThisBuild / version := \"1.0.0\"; +publishLocal"

Which will publish the project with that version locally if you want to verify the jars and then exit. Or if we want to make a source distribution

sbt "set ThisBuild / version := \"1.0.0\"; sourceDistGenerate"

Another option is to do it the Akka/Pekko way and just pass it as a -D system flag, adding this functionality is just a few lines and it has precedent in other Pekko modules. Furthermore when we get the docker build up and running is to just pass a build-arg to the Docker script which will pass it into sbt.

So my preference is to cross that bridge when we get there, I am sure if this is an actual problem then we will be promptly told when doing the release.

mdedetrich commented 1 year ago

I would also add that if we are really forced to specify the version manually in source (i.e. within build.sbt/version.sbt) then I would suggest we make it so that the version is only set based on a flag/env variable, i.e.

ThisBuild / version := {
  if (releaseVersion)
     "1.0.0"
  else
     (ThisBuild / version).value
}

This means that unless you pass in releaseVersion it will default to the version provided by sbt-dynver, then at least we don't have to worry about constantly enabling and disabling it and also a lot less confusing to those using pekko outside of the context of the git repo.

pjfanning commented 1 year ago

There is 100% a rule that ASF releases involve producing a zip and/or tgz with the source code and that that is the official release artifact. Every other artifact that we produce is a convenience. The voting is all about the source release zip/tgz.

We have no chance of getting away with saying 'check out the git repo' and build it from that.

The git tag is not the source of truth - for ASF, it is the voted on source release zip/tgz.

mdedetrich commented 1 year ago

There is 100% a rule that ASF releases involve producing a zip and/or tgz with the source code and that that is the official release artifact. Every other artifact that we produce is a convenience. The voting is all about the source release zip/tgz.

We have no chance of getting away with saying 'check out the git repo' and build it from that.

I know this, and the solutions I stated earlier full fill that requirement.

Even if you are only provided the source, its perfectly possible to set the version of sbt when running it in a terminal (as described in https://github.com/apache/incubator-pekko/issues/339#issuecomment-1561770068) or as a last resort, only use the version defined in build.sbt/version.sbt if you pass a flag to sbt (as described in https://github.com/apache/incubator-pekko/issues/339#issuecomment-1561778507).

I can right now download a tar.gz source of Pekko and with no presence of git create a "1.0.0" source distribution by doing sbt "set ThisBuild / version := \"1.0.0\"; sourceDistGenerate".

Removing and adding a hardcoded version in build.sbt/version.sbt is what I have a problem with. Not even other ASF projects do this, since if you are only reading the source in absence of git you then have no idea what the version is unless you happen to have the source at the exact same time a release is made (if our primary argument is based around not having git and only sources).

pjfanning commented 1 year ago

This is getting complicated. We need to block the release and get a release doc written, agreed on and voted on.

mdedetrich commented 1 year ago

This is getting complicated. We need to block the release and get a release doc written, agreed on and voted on.

I am not entirely sure whats complicated about it. Its actually far simpler and can be done right now without any additional changes to Pekko. You just need to simply document the following

Release Process

... etc etc In order to create an Apache Source distribution, please run the following command

sbt "set ThisBuild / version := \"<VERSION>\"; sourceDistGenerate"

where <VERSION> is the version you want to make a release for, i.e. for 1.0.0

sbt "set ThisBuild / version := \"1.0.0\"; sourceDistGenerate"

Likewise in order to publish a signed artifact to maven run the following

sbt "set ThisBuild / version := \"<VERSION>\"; +publishSigned"

... etc etc

And thats it. This will work regardless if its a git repo or just raw sources. I just tried it on my local machine and it works as expected

sbt "set ThisBuild / version := \"1.0.0\"; sourceDistGenerate"
[info] welcome to sbt 1.9.0-RC1 (Homebrew Java 11.0.19)
[info] loading global plugins from /Users/mdedetrich/.sbt/1.0/plugins
[info] loading project definition from /Users/mdedetrich/github/incubator-pekko/project/project
[info] loading settings for project incubator-pekko-build from plugins.sbt ...
[info] loading project definition from /Users/mdedetrich/github/incubator-pekko/project
[info] loading settings for project pekko from build.sbt ...
[info] resolving key references (63038 settings) ...
[info] 
[info] ________     ______ ______        
[info] ___  __ \_______  /____  /_______ 
[info] __  /_/ /  _ \_  //_/_  //_/  __ \
[info] _  ____//  __/  ,<  _  ,<  / /_/ /
[info] /_/     \___//_/|_| /_/|_| \____/   0.0.0+26670-7418680c-SNAPSHOT
[info] 
[info] Useful sbt tasks:
[info] >  compile - Compile the current project
[info] >  test - Run all the tests 
[info] >  testOnly *.AnySpec - Only run a selected test
[info] >  publishLocal - Publish current snapshot version to local ~/.ivy2 repo
[info] >  verifyCodeStyle - Verify code style
[info] >  applyCodeStyle - Apply code style
[info] >  sortImports - Sort the imports
[info] >  mimaReportBinaryIssues  - Check binary issues
[info] >  validatePullRequest  - Validate pull request
[info] >  docs/paradox - Build documentation
[info] >  docs/paradoxBrowse - Browse the generated documentation
[info] >  tips: - prefix commands with `+` to run against cross Scala versions.
[info] >  Contributing guide: - https://github.com/apache/incubator-pekko/blob/main/CONTRIBUTING.md
[info] Defining ThisBuild / version
[info] The new value will be used by Compile / doc / scalacOptions, Compile / packageBin / packageOptions and 969 others.
[info]  Run `last` for details.
[info] Reapplying settings...
[info] 
[info] ________     ______ ______        
[info] ___  __ \_______  /____  /_______ 
[info] __  /_/ /  _ \_  //_/_  //_/  __ \
[info] _  ____//  __/  ,<  _  ,<  / /_/ /
[info] /_/     \___//_/|_| /_/|_| \____/   1.0.0
[info] 
[info] Useful sbt tasks:
[info] >  compile - Compile the current project
[info] >  test - Run all the tests 
[info] >  testOnly *.AnySpec - Only run a selected test
[info] >  publishLocal - Publish current snapshot version to local ~/.ivy2 repo
[info] >  verifyCodeStyle - Verify code style
[info] >  applyCodeStyle - Apply code style
[info] >  sortImports - Sort the imports
[info] >  mimaReportBinaryIssues  - Check binary issues
[info] >  validatePullRequest  - Validate pull request
[info] >  docs/paradox - Build documentation
[info] >  docs/paradoxBrowse - Browse the generated documentation
[info] >  tips: - prefix commands with `+` to run against cross Scala versions.
[info] >  Contributing guide: - https://github.com/apache/incubator-pekko/blob/main/CONTRIBUTING.md
[info] Creating zip archive at /Users/mdedetrich/github/incubator-pekko/target/dist/incubating-pekko-src-1.0.0-20230524.zip
[info] Creating tar archive at /Users/mdedetrich/github/incubator-pekko/target/dist/incubating-pekko-src-1.0.0-20230524.tgz
pjfanning commented 1 year ago

I mean completing https://github.com/apache/incubator-pekko/issues/130 and getting that doc on cwiki.apache.org or something similar. The doc should describe what the release manager does to get a release out. It should probably also consider how the source release can be verified because there is a bit of overlap between what the release manager is doing and what someone building from the source release will be doing (this issue and #323).

mdedetrich commented 1 year ago

If you are happy with it, I can start drafting the release docs with the method described in https://github.com/apache/incubator-pekko/issues/339#issuecomment-1561848206, ill get to it after the Docker image is done. In regards to verification of the source release, at least as far as I know from Kafka release process this is quite malleable but it usually involves compiling, running the full suite of tests etc etc.

Since we have reproducible builds, we can also verify that published jars have the same hash but this won't work for Scala 3 artifacts (see https://github.com/apache/incubator-pekko/issues/325).

justinmclean commented 1 year ago

Note also that git tags are mutable, so when referring to a release it's better to refer to the commit hash.

justinmclean commented 1 year ago

Artefacts are usually named with the version number in them e.g. Apache-Foo-incubating-1.0.0-RC1.tar.gz, so in general you do know what version the source is. The README or release notes will also generally state what the version is.

mdedetrich commented 1 year ago

Note also that git tags are mutable, so when referring to a release it's better to refer to the commit hash.

Actually when using github (which Pekko does) its possible to make them immutable via the use of git tag protection rules, see https://docs.github.com/en/repositories/managing-your-repositorys-settings-and-features/managing-repository-settings/configuring-tag-protection-rules. Such a rule means that only admins (i.e ASF Infra) are able to edit or delete git tags once they have been added.

I will check with #asfinra if its possible to do this with their .asf.yaml.

EDIT: I created an issue for this at https://github.com/apache/incubator-pekko/issues/342 as well as an upstream asfinfra issue.

justinmclean commented 1 year ago

Here's an example where a release got a -1 vote because of that (and other issues). I can provide other examples if you need. I suggest you use git commit SHAs instead. https://lists.apache.org/thread/8h5fw3cpx81w7nmm385hlx261sbo9m0n

mdedetrich commented 1 year ago

Here's an example where a release got a -1 vote because of that (and other issues). I can provide other examples if you need. I suggest you use git commit SHAs instead. https://lists.apache.org/thread/8h5fw3cpx81w7nmm385hlx261sbo9m0n

Pekko already creates source distributions using version and timestamp, please see https://github.com/apache/incubator-pekko/issues/339#issuecomment-1561848206. There was never an intent to use git tags as part of the filename for the source distribution (even though the difference in reality is quite benign because the whole point of git tags is that they are named the same as the official release).

EDIT: I wasn't aware that we can include the git SHA's in the source dist filename. If thats the case @pjfanning to me it sounds reasonable to add the git SHA in the source dist filename, wdyt?

pjfanning commented 1 year ago

PR merged