-
Notifications
You must be signed in to change notification settings - Fork 2.2k
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
Build: Add iceberg-build.properties to Jars and release process #5236
Changes from 2 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -87,11 +87,12 @@ tagrc="${tag}-rc${rc}" | |
|
||
echo "Preparing source for $tagrc" | ||
|
||
echo "Adding version.txt and tagging release..." | ||
echo "Generating version.txt and iceberg-build.properties..." | ||
echo $version > $projectdir/version.txt | ||
git add $projectdir/version.txt | ||
git commit -m "Add version.txt for release $version" $projectdir/version.txt | ||
./gradlew generateGitProperties | ||
cp $projectdir/build/iceberg-build.properties $projectdir/iceberg-build.properties | ||
|
||
echo "Creating release candidate tag: $tagrc..." | ||
set_version_hash=`git rev-list HEAD 2> /dev/null | head -n 1 ` | ||
git tag -am "Apache Iceberg $version" $tagrc $set_version_hash | ||
|
||
|
@@ -109,7 +110,11 @@ fi | |
# archive (identical hashes) using the scm tag | ||
echo "Creating tarball ${tarball} using commit $release_hash" | ||
tarball=$tag.tar.gz | ||
git archive $release_hash --worktree-attributes --prefix $tag/ -o $projectdir/$tarball | ||
git archive $release_hash --worktree-attributes --prefix $tag/ --add-file $projectdir/version.txt --add-file $projectdir/iceberg-build.properties -o $projectdir/$tarball | ||
|
||
# remove the uncommitted build files so they don't affect the current working copy | ||
rm $projectdir/version.txt | ||
rm $projectdir/iceberg-build.properties | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This wasn't needed before because the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Will there be any commit specifically for the release or will it simply be the last commit that goes into the release branch? Previously the version.txt commit wound up being the source release commit. To be clear, I am definitely in agreement on removing the |
||
|
||
echo "Signing the tarball..." | ||
[[ -n "$keyid" ]] && keyopt="-u $keyid" | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will this cause build verification to fail? If we're downloading the tarball and building from that will the build fail?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, this actually doesn't do much. The plugin is configured in a try/catch after the Palantir version plugin. So if there is not .git directory, this code doesn't even run.
Eventually, we should see if this can be moved outside of the try/catch, which was my intent here. I think that if
generateGitProperties
is called and it can't, then we want to have a failure. That's why this introduces a new task,buildInfo
that will use the properties file in the build root if it is present. The failure we want is when you have no.git
directory and have noiceberg-build.properties
file.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tried without the try catch and having
gitProperties
in the build file causes even simple tasks likegradle tasks
to fail because the surroundinggitProperties
block is not registered and has no meaning to gradle. Even if it’s not used, such asgradle clean
.So my observation was that the try-catch block will always be needed for any
gitProperties
block.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this flag is more for the case that somebody overrides the
.git
directory - which the plug-in allows - and that overridden directory isn’t present (at least from my testing). But git is still present so the plug-in is able to be applied in general.Ryan’s right that it doesn’t do much and the flag won’t cause the build to fail when using the tarball.