Skip to content

Fix build version for local build and snapshot publishing#4326

Closed
ajantha-bhat wants to merge 1 commit intoapache:masterfrom
ajantha-bhat:master
Closed

Fix build version for local build and snapshot publishing#4326
ajantha-bhat wants to merge 1 commit intoapache:masterfrom
ajantha-bhat:master

Conversation

@ajantha-bhat
Copy link
Member

@ajantha-bhat ajantha-bhat commented Mar 14, 2022

Today I observed that our local build and daily snapshots are still pointing to 0.13.0-SNAPSHOT even though we released 0.13.1, it should point to the next version 0.13.2-SNAPSHOT

After debugging found that in this code
we are looking for last annotated tag and it seems our release owners are not having annotated tag as they are creating a new branch for release instead of tagging from the master branch.

As we cannot amend the commit and create annotated tag, using the version.txt pointing to the next version in the master branch now.

@ajantha-bhat
Copy link
Member Author

cc: @rdblue , @RussellSpitzer , @jackye1995 , @nastra

@rdblue
Copy link
Contributor

rdblue commented Mar 14, 2022

We don't put a version.txt file in the main branch. How is this being determined? If we need to add a tag, we can do that.

@kbendick
Copy link
Contributor

We don't put a version.txt file in the main branch. How is this being determined? If we need to add a tag, we can do that.

It comes from this function in build.gradle. As you can see, we have a hardcoded .0-SNAPSHOT on it after parsing out the first two numbers (0 and 13 in this case):

iceberg/build.gradle

Lines 619 to 639 in 7f07ed4

String getProjectVersion() {
if (versionFileExists()) {
return getVersionFromFile()
}
try {
// we're fetching the version from the latest tag (prefixed with 'release-base-'),
// which can look like this: '0.13.0-2-g805400f0.dirty' but we're only interested in the MAJOR.MINOR.PATCH part
String version = gitVersion(prefix: 'release-base-')
Pattern pattern = Pattern.compile("^([0-9]+)\\.([0-9]+)\\.([0-9]+)(.*)?\$")
Matcher matcher = pattern.matcher(version)
if (matcher.matches()) {
// bump the MINOR version and always set the PATCH version to 0
return matcher.group(1) + "." + (Integer.valueOf(matcher.group(2)) + 1) + ".0-SNAPSHOT"
}
return version
} catch (Exception e) {
throw new Exception("Neither version.txt nor git version exists: " + e.getMessage(), e)
}
}

@rdblue
Copy link
Contributor

rdblue commented Mar 14, 2022

Okay, so we just need to tag release-base-0.13.0 and the main branch should start building as 0.14.0-SNAPSHOT then?

@kbendick
Copy link
Contributor

kbendick commented Mar 14, 2022

Okay, so we just need to tag release-base-0.13.0 and the main branch should start building as 0.14.0-SNAPSHOT then?

Yes I believe so. The plugin adds a gradle task printVersion.

This is equivalent to git describe --tags --always --first-parent, though we pass in the release-base- so it filters on that.

To test your idea, I tried tagging locally.

➜   ./gradlew printVersion

> Task :printVersion
0.13.0-SNAPSHOT

➜  git describe --tags --always --first-parent
release-base-0.12.0-646-g7f07ed451

➜  git tag release-base-0.13.0
➜  git describe --tags --always --first-parent
release-base-0.13.0
➜  ./gradlew printVersion

> Task :printVersion
0.14.0-SNAPSHOT

So if we wanted to publish a snapshot using the next minor version number (14 in this case), and ignore the fact that we might ship a patch now and then, what you suggested would be correct @rdblue.

If we wanted to take into account the patch, we could match on it. But presently we're more or less always working towards the next minor version, and then occasionally releasing a patch that only cherry-picks.

So the build against master represents the build for the next minor version (and not the next patch) in my opinion.

TLDR - tagging release-base-0.13.0 will start building as 0.14.0-SNAPSHOT

@kbendick
Copy link
Contributor

So it seems we need to update the release instructions to include a tag of release-base-MAJOR-MINOR-PATCH, in addition to the tag of apache-iceberg-MATCH.MINOR.PATCH.

We can likely remove all but the latest release-version-* tags as well.

@kbendick
Copy link
Contributor

Alternatively, we can update the gitVersion prefix to be apache-iceberg-, which would get us the same results without having to push an additional tag (just the normal release tag).

But downstream people who fork this might also be using this in their workflows, which might be where the release-base- came into play.

@ajantha-bhat
Copy link
Member Author

ajantha-bhat commented Mar 15, 2022

Alternatively, we can update the gitVersion prefix to be apache-iceberg-, which would get us the same results without having to push an additional tag (just the normal release tag).

@kbendick: I already tried this before raising the PR. This doesn't work, gitVersion needs an annotated tag on the master branch. The last annotated tag on master we had is on 0.12.0. The tags created after 0.12.0 is not an annotated tag on master.

git log --oneline --decorate=short | grep 'tag\:' | head -n3

Screenshot 2022-03-14 at 10 58 15 PM

So, I think new release tags are created from release branch instead of master.

Also we may not able to create tags from the master branch as commits are not same as release branch. Tag should match release SHA ?
Considering all this I went with version.txt

@rdblue: What is the problem with version.txt ?, we can still update it after every release on master branch? Can you elaborate the problem ?

@kbendick
Copy link
Contributor

I didn’t try the apache-iceberg- but I’m surprised it wouldn’t work.

Let me try it locally. But if we do switch to using version.txt file, can we remove the function that’s referenced?

@ajantha-bhat
Copy link
Member Author

ajantha-bhat commented Mar 15, 2022

I didn’t try the apache-iceberg- but I’m surprised it wouldn’t work.

As mentioned, there is no tag from master branch starting with apache-iceberg-, those tags are created from the release branch. Hence it doesn't work. You can try.

@rdblue
Copy link
Contributor

rdblue commented Mar 15, 2022

If I remember correctly, this is why we added the release-base-0.12.0 tag to master. That way we have a tag to pick up.

@kbendick
Copy link
Contributor

kbendick commented Mar 15, 2022

Ahh I understand now.

version.txt aside though, I don't quite agree that 0.13.2-SNAPSHOT is the correct version when building from master. The changes in master (at least the way we use them) are reflective of the next new release. The patches are always just bug fixes with a few things cherry-picked, that we cherry-pick back onto the state of the current minor version.

So I think 0.14.0-SNAPSHOT would be the correct thing to publish in the nightly releases etc, as code built from master contains many more changes than would ever make it into a patch release. People who were pulling nightly releases to test out 0.13.2-SNAPSHOT as a preview would get far more changes than an actual 0.13.2 release would bring.

@ajantha-bhat
Copy link
Member Author

@kbendick: I feel publishing as 0.13.2-SNAPSHOT but doing an actual release of 0.14.0 is ok than publishing 0.14.0-SNAPSHOT and doing a release of 0.13.2, because it looks like we are going back on the release version.
But.. I will leave it to the decision of the community.

@ajantha-bhat
Copy link
Member Author

ajantha-bhat commented Mar 15, 2022

If I remember correctly, this is why we added the release-base-0.12.0 tag to master. That way we have a tag to pick up.

@rdblue : Yes. But only problem is that we may not be able to create an exact tag on master branch for that release as we have released from the release branch and commits may not be same. So, we may end up creating two tags per version (one for release, one dummy tag for build). which looks odd.
But still ok for me.

So, please create a build tag release-base-0.13.0 on master branch and add note about this on release steps (As we have to do it on every release). I can close this PR after that. Thanks.

@kbendick
Copy link
Contributor

That build tag has already been created. If you fetch latest master and build, it should be 0.14.0-SNAPSHOT. That's what I get when I run ./gradlew printVersion at least.

@ajantha-bhat
Copy link
Member Author

@kbendick :

That build tag has already been created. If you fetch latest master and build, it should be 0.14.0-SNAPSHOT. That's what I get when I run ./gradlew printVersion at least.

I can't see it in here (https://github.com/apache/iceberg/tags)

or here:
image

@kbendick
Copy link
Contributor

kbendick commented Mar 15, 2022

Maybe you need to pull from upstream / apache? I'm not sure what your origin remote points to. But I was able to build a 0.14.0-SNAPSHOT for spark-runtime after pulling from apache/iceberg

$ git describe --tags --always --first-parent
release-base-0.13.0-2-g1d95424d0

image

@rdblue
Copy link
Contributor

rdblue commented Mar 15, 2022

I created the tag. Can you both fetch tags and try it again?

@ajantha-bhat
Copy link
Member Author

I created the tag. Can you both fetch tags and try it again?

I can see it now after this. Thanks for resolving this @rdblue

@nastra
Copy link
Contributor

nastra commented Mar 15, 2022

If I remember correctly, this is why we added the release-base-0.12.0 tag to master. That way we have a tag to pick up.

yep correct, that was the reason we added this tag (since we didn't want to maintain a version.txt file on master)

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.

4 participants