You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@iceberg.apache.org by GitBox <gi...@apache.org> on 2022/07/10 20:10:44 UTC
[GitHub] [iceberg] rdblue opened a new pull request, #5236: Build: Add iceberg-build.properties to Jars and release process
rdblue opened a new pull request, #5236:
URL: https://github.com/apache/iceberg/pull/5236
This ensures that `iceberg-build.properties` is added to Jars produced by the Gradle build, even when building from a source tarball with no `.git` directory.
This builds on the `generateGitProperties` task added in #5228 and updates the build:
* A new task, `buildInfo` ensures that `build/iceberg-build.properties` exists, either by copying it from the source root or by calling `generateGitProperties` (change in `tasks.gradle`)
* All Jars will now include `iceberg-build.properties` from the root `build` folder (change in `deploy.gradle`)
* The source release script will generate `iceberg-build.properties` and add it to the root folder for the release candidate tarball (changes in `source-release.sh`)
Note that this also changes how release tarballs are created. Previously, the `source-release.sh` script would create `version.txt` and commit the file, then tag the commit with `version.txt`. This wasn't possible for adding `iceberg-build.properties` because `iceberg-build.properties` needs to describe the git commit in the source tarball (and can't be included in the commit itself). Instead of adding the file to git, this adds both `version.txt` and `iceberg-build.properties` directly to the archive using `git archive ... --add-file`. A side benefit is that we will no longer have release tags off of the master branch.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] rdblue commented on a diff in pull request #5236: Build: Add iceberg-build.properties to Jars and release process
Posted by GitBox <gi...@apache.org>.
rdblue commented on code in PR #5236:
URL: https://github.com/apache/iceberg/pull/5236#discussion_r917443074
##########
build.gradle:
##########
@@ -51,7 +51,7 @@ try {
gitPropertiesName = 'iceberg-build.properties'
gitPropertiesResourceDir = file("${rootDir}/build")
extProperty = 'gitProps'
- failOnNoGitDirectory = false
+ failOnNoGitDirectory = true
Review Comment:
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 no `iceberg-build.properties` file.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] rdblue merged pull request #5236: Build: Add iceberg-build.properties to Jars and release process
Posted by GitBox <gi...@apache.org>.
rdblue merged PR #5236:
URL: https://github.com/apache/iceberg/pull/5236
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] danielcweeks commented on a diff in pull request #5236: Build: Add iceberg-build.properties to Jars and release process
Posted by GitBox <gi...@apache.org>.
danielcweeks commented on code in PR #5236:
URL: https://github.com/apache/iceberg/pull/5236#discussion_r917441606
##########
build.gradle:
##########
@@ -51,7 +51,7 @@ try {
gitPropertiesName = 'iceberg-build.properties'
gitPropertiesResourceDir = file("${rootDir}/build")
extProperty = 'gitProps'
- failOnNoGitDirectory = false
+ failOnNoGitDirectory = true
Review Comment:
Will this cause build verification to fail? If we're downloading the tarball and building from that will the build fail?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] kbendick commented on a diff in pull request #5236: Build: Add iceberg-build.properties to Jars and release process
Posted by GitBox <gi...@apache.org>.
kbendick commented on code in PR #5236:
URL: https://github.com/apache/iceberg/pull/5236#discussion_r917446399
##########
build.gradle:
##########
@@ -51,7 +51,7 @@ try {
gitPropertiesName = 'iceberg-build.properties'
gitPropertiesResourceDir = file("${rootDir}/build")
extProperty = 'gitProps'
- failOnNoGitDirectory = false
+ failOnNoGitDirectory = true
Review Comment:
I think this flag is more for the case that somebody overrides the `.git` directory and that overridden directory isn’t present (at least from my testing).
Ryan’s right that it doesn’t do much.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] kbendick commented on a diff in pull request #5236: Build: Add iceberg-build.properties to Jars and release process
Posted by GitBox <gi...@apache.org>.
kbendick commented on code in PR #5236:
URL: https://github.com/apache/iceberg/pull/5236#discussion_r917446399
##########
build.gradle:
##########
@@ -51,7 +51,7 @@ try {
gitPropertiesName = 'iceberg-build.properties'
gitPropertiesResourceDir = file("${rootDir}/build")
extProperty = 'gitProps'
- failOnNoGitDirectory = false
+ failOnNoGitDirectory = true
Review Comment:
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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] rdblue commented on pull request #5236: Build: Add iceberg-build.properties to Jars and release process
Posted by GitBox <gi...@apache.org>.
rdblue commented on PR #5236:
URL: https://github.com/apache/iceberg/pull/5236#issuecomment-1179793486
I created 0.14.0-rc0 to test out the `source-release.sh` script: https://dist.apache.org/repos/dist/dev/iceberg/apache-iceberg-0.14.0-rc0/
I also verified that release and everything looks good:
* The `iceberg-build.properties` and `version.txt` files are present
* All Jars that are built include the `iceberg-build.properties` file
* The tag and hash in the properties file matches the release candidate
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] rdblue commented on pull request #5236: Build: Add iceberg-build.properties to Jars and release process
Posted by GitBox <gi...@apache.org>.
rdblue commented on PR #5236:
URL: https://github.com/apache/iceberg/pull/5236#issuecomment-1179810910
@kbendick, #5237 adds this to the classpath for the tests in `iceberg-api`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] kbendick commented on a diff in pull request #5236: Build: Add iceberg-build.properties to Jars and release process
Posted by GitBox <gi...@apache.org>.
kbendick commented on code in PR #5236:
URL: https://github.com/apache/iceberg/pull/5236#discussion_r917446840
##########
dev/source-release.sh:
##########
@@ -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
Review Comment:
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 `git commit` from this script. It has caused several people issues while testing releasing in the past or when the release script needed to be rerun for the same RC candidate.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] rdblue commented on a diff in pull request #5236: Build: Add iceberg-build.properties to Jars and release process
Posted by GitBox <gi...@apache.org>.
rdblue commented on code in PR #5236:
URL: https://github.com/apache/iceberg/pull/5236#discussion_r917441564
##########
dev/source-release.sh:
##########
@@ -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
Review Comment:
This wasn't needed before because the `version.txt` file was committed. When switching back to the master branch, it would be removed because it isn't present in the master branch.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] kbendick commented on pull request #5236: Build: Add iceberg-build.properties to Jars and release process
Posted by GitBox <gi...@apache.org>.
kbendick commented on PR #5236:
URL: https://github.com/apache/iceberg/pull/5236#issuecomment-1179801921
Will this place `iceberg-build.properties` on the runtime classpath for subprojects? Including if used from the IDE?
Things in `core/build/resources/**` will be accessible from core’s runtime classpath, but I’m less sure about the top-level `build/**` files.
Possibly that’s going to be handled in a follow up though (if needed at all).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] kbendick commented on a diff in pull request #5236: Build: Add iceberg-build.properties to Jars and release process
Posted by GitBox <gi...@apache.org>.
kbendick commented on code in PR #5236:
URL: https://github.com/apache/iceberg/pull/5236#discussion_r917445685
##########
build.gradle:
##########
@@ -51,7 +51,7 @@ try {
gitPropertiesName = 'iceberg-build.properties'
gitPropertiesResourceDir = file("${rootDir}/build")
extProperty = 'gitProps'
- failOnNoGitDirectory = false
+ failOnNoGitDirectory = true
Review Comment:
I tried without the try catch and having `gitProperties` in the build file causes even simple tasks like `gradle tasks` to fail because the surrounding `gitProperties` block is not registered and has no meaning to gradle. Even if it’s not used, such as `gradle clean`.
So my observation was that the try-catch block will always be needed for any `gitProperties` block.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org