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/08 08:55:28 UTC
[GitHub] [iceberg] kbendick opened a new pull request, #5228: [WIP] Build - Add Git Properties Information via Plugin
kbendick opened a new pull request, #5228:
URL: https://github.com/apache/iceberg/pull/5228
This adds a git properties file, generated using the https://github.com/n0mer/gradle-git-properties (many thanks to @nastra) for both the top level project, as well as for each individual subproject.
The top level project file is needed so that the source tarball has a file that it can add to the tarball before releasing in `dev/source-release.sh`.
The individual project level ones are needed for debugging using runtime jars, such as in the IDE when using git property information for things like version headers etc.
This is an alternative implementation to https://github.com/apache/iceberg/pull/5186
I've tested this, and we might want to remove some of the git property information but otherwise this seems to work. The top-level project properties file still needs to be added to the dev/source-release.sh script properly.
--
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 #5228: Build - Add Git Build Properties File via Plugin
Posted by GitBox <gi...@apache.org>.
rdblue commented on PR #5228:
URL: https://github.com/apache/iceberg/pull/5228#issuecomment-1179779446
To clear up some of the confusion, I talked with @kbendick directly about file paths and breaking down the problem.
The first step is to be able to generate the file when the `.git` directory is present with a gradle task. The file is at `build/iceberg-build.properties` so that we can [pull it into Jars using `include`](https://github.com/apache/iceberg/blob/master/deploy.gradle#L56-L57) like we do for `LICENSE` and `NOTICE`. We only need one copy for that. The reason why we put it in `build` is that it is a temporary file generated for the build.
The next steps are:
1. Add the file to all Jars using `include`
2. Add the file to the root of the source tarball when generating a source build
3. Add a task that either generates the content from `.git` or copies the file from the root of the build (like the current version.txt handling)
4. Add the commit to expose version information in a class and set the version header
--
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 #5228: [WIP] Build - Add Git Properties Information via Plugin
Posted by GitBox <gi...@apache.org>.
kbendick commented on code in PR #5228:
URL: https://github.com/apache/iceberg/pull/5228#discussion_r917001112
##########
build.gradle:
##########
@@ -73,6 +83,22 @@ subprojects {
apply plugin: 'nebula.dependency-recommender'
apply plugin: 'java-library'
+ // apply this plugin in a try-catch block so that we can handle cases without .git directory
+ // apply to subprojects so that git properties information is available for runtime debugging and tests
+ // e.g. sending build SHA as headers
+ try {
+ apply plugin: 'com.gorylenko.gradle-git-properties'
+ gitProperties {
+ gitPropertiesName = "${project.name}-version.properties"
+ extProperty = 'gitProps'
+ failOnNoGitDirectory = false
Review Comment:
I tried removing the `.git` directory entirely using `git clone --depth=1 --branch=kb-gradle-git-properties-plugin-file-generation git@github.com:kbendick/iceberg.git iceberg-version-no-git && cd iceberg-version-no-git && rm -rf .git && ./gradlew tasks` and the build errors out because `gitProperties` isn't recognized (because the plugin apply didn't work).
So I'm assuming that property is more for cases where there's no git directory, but JGit is still available. But JGit isn't available without the `.git` directory.
I think this flag is probably more intended for cases where the user overrides the `.git` directory, but there's still a `.git` directory in the repository root like normal (e.g. maybe they override it to do a shallower clone for a smaller git folder).
--
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 #5228: Build - Add Git Build Properties File via Plugin
Posted by GitBox <gi...@apache.org>.
kbendick commented on PR #5228:
URL: https://github.com/apache/iceberg/pull/5228#issuecomment-1179441910
> @kbendick I'm not sure which path was settled on for the properties file. I see both `/build/iceberg-build.properties` and `core/build/iceberg-build.properties` paths referenced.
That’s a historical artifact of the PR. At one point, the PR implemented a git attributes file generated for every subproject. And the previous version of the PR implemented similar functionality but without a plugin for just the core subproject, placing it into `core/build/resources/main`).
If the plugin is applied to the `core` subproject, then the git attributes file would be placed into `core/build/resources/main/${fileName}` by default (or some variant thereof - where $fileName can be specified per subproject or configured however we want, as iceberg-build.properties which is what was suggested here).
As mentioned, placing the file into `file("${rootDir}/core/build/resources/main")` will put the file in a place where it will be
included in the `core` subproject's output jar and by extension the bundle jars that use `iceberg-core` (e.g. for usage from Spark etc).
> We can either produce a top-level iceberg-build.properties and then later copy it into location for the jar to pick it up or we can set the location to gitPropertiesResourceDir = file("${rootDir}/core/build/resources/main"), which will put it in a place where it will be included.
Originally, I had the plugin also configured to generate a top-level git attributes file (which would go into `file("${rootDir}/build/resources/main")` or wherever else we requested it).
The reasoning for this is that when the artifacts are built from the tarball, the `.git` folder isn't included. So the `generateGitProperties` action won't work at all and the file won't be generated via a simple `./gradlew build` or even calling the action directly.
So when pushing the generated tarball of the project to svn, we'd want to have pre-generated the file and then we could include it here during the `deploy` phase: https://github.com/apache/iceberg/blob/b0937a4951a2d4a40b67eda484d49c4a4d32566e/deploy.gradle#L54-L59
I could make a gradle task that copies the generated file, though if we want the SHA to match the release we'll need to call the task as part of `dev/source-release.sh` the tag and SHA are correct (as the last commit is done in `dev/source-release.sh`).
But yeah, in the case that the artifacts are built from svn / the generated tarball, the `generateGitProperties` command won't work.
So we can add a task which will generate and copy the file over to a location that will be included in the tarball.
I think `core/build/resources/main` is good as that's where it's needed to be accessible from within the `core` code at runtime.
Let me find links to my older commits that have a few ways of doing it.
--
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 #5228: Build - Add Git Build Properties File via Plugin
Posted by GitBox <gi...@apache.org>.
rdblue merged PR #5228:
URL: https://github.com/apache/iceberg/pull/5228
--
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 #5228: [WIP] Build - Add Git Properties Information via Plugin
Posted by GitBox <gi...@apache.org>.
kbendick commented on code in PR #5228:
URL: https://github.com/apache/iceberg/pull/5228#discussion_r917067617
##########
deploy.gradle:
##########
@@ -55,6 +55,7 @@ subprojects {
task.from(rootDir) {
include 'LICENSE'
include 'NOTICE'
+ include 'iceberg-version.properties'
Review Comment:
This file won't exist in the tarball as the PR is (with the properties file in the requested `build/iceberg-version.properties`), so I've removed this line from deploy.gradle and we can add it in a subsequent PR.
--
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 #5228: [WIP] Build - Add Git Properties Information via Plugin
Posted by GitBox <gi...@apache.org>.
kbendick commented on code in PR #5228:
URL: https://github.com/apache/iceberg/pull/5228#discussion_r917001112
##########
build.gradle:
##########
@@ -73,6 +83,22 @@ subprojects {
apply plugin: 'nebula.dependency-recommender'
apply plugin: 'java-library'
+ // apply this plugin in a try-catch block so that we can handle cases without .git directory
+ // apply to subprojects so that git properties information is available for runtime debugging and tests
+ // e.g. sending build SHA as headers
+ try {
+ apply plugin: 'com.gorylenko.gradle-git-properties'
+ gitProperties {
+ gitPropertiesName = "${project.name}-version.properties"
+ extProperty = 'gitProps'
+ failOnNoGitDirectory = false
Review Comment:
I tried removing the `.git` directory entirely using `git clone --depth=1 --branch=kb-gradle-git-properties-plugin-file-generation git@github.com:kbendick/iceberg.git iceberg-version-no-git && cd iceberg-version-no-git && rm -rf .git && ./gradlew tasks` and the build errors out because `gitProperties` isn't recognized (because the plugin apply didn't work).
So I'm assuming that property is more for cases where there's no git directory, but JGit is still available.
--
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 #5228: [WIP] Build - Add Git Properties Information via Plugin
Posted by GitBox <gi...@apache.org>.
kbendick commented on code in PR #5228:
URL: https://github.com/apache/iceberg/pull/5228#discussion_r917053908
##########
dev/source-release.sh:
##########
@@ -87,11 +87,29 @@ tagrc="${tag}-rc${rc}"
echo "Preparing source for $tagrc"
+# Generate git properties file as git info will not be as accessible in svn during final release
+# This SHA will technically be one behind the final release as we need to commit the file
+#echo "Generating git properties file for tarball"
+# $projectdir/./gradlew generateGitProperties --no-daemon
+# mv $projectdir/build/resources/main/iceberg-version.properties $projectdir/iceberg-version.properties
+
echo "Adding version.txt and tagging release..."
echo $version > $projectdir/version.txt
+# git add $projectdir/version.txt $projectdir/iceberg-version.properties
+# git commit -m "Add version.txt for release $version" $projectdir/version.txt $projectdir/iceberg-version.properties
git add $projectdir/version.txt
git commit -m "Add version.txt for release $version" $projectdir/version.txt
+# Regenerate the git properties file so its up to date, then move it out of build/ (which is in the .gitattributes
+# file and thus will be ignored from the archive). The file will be added to the release tarball, without committing,
+# so it has the correct SHA (from the previous step).
+#
+# TODO - Add a second task that generates _only_ the root properties file with just a subset of the info (so .dirty
+# and user emails and hostnames don't appear or consider a virtual file in the git archive via --add-virtual-file).
+$projectdir/./gradlew generateGitProperties --no-daemon
+mv $projectdir/build/resources/main/iceberg-version.properties $projectdir/iceberg-version.properties
+git_props_file=$projectdir/iceberg-version.properties
+
Review Comment:
Most of this stuff got pushed when running the `dev/source-release.sh` script to test this out.
--
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 #5228: [WIP] Build - Add Git Properties Information via Plugin
Posted by GitBox <gi...@apache.org>.
kbendick commented on code in PR #5228:
URL: https://github.com/apache/iceberg/pull/5228#discussion_r917069766
##########
build.gradle:
##########
@@ -73,6 +83,22 @@ subprojects {
apply plugin: 'nebula.dependency-recommender'
apply plugin: 'java-library'
+ // apply this plugin in a try-catch block so that we can handle cases without .git directory
+ // apply to subprojects so that git properties information is available for runtime debugging and tests
+ // e.g. sending build SHA as headers
+ try {
+ apply plugin: 'com.gorylenko.gradle-git-properties'
Review Comment:
This has been removed. Note that from my previous testing, the file in `build/iceberg-build.properties` will not be available on the classpath when running in the IDE or during gradle tests. We can resolve that in subsequent PRs if need be though.
Also per the stack overflow that was the basis of https://github.com/apache/iceberg/pull/5186
--
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 #5228: [WIP] Build - Add Git Properties Information via Plugin
Posted by GitBox <gi...@apache.org>.
rdblue commented on code in PR #5228:
URL: https://github.com/apache/iceberg/pull/5228#discussion_r917048043
##########
build.gradle:
##########
@@ -73,6 +83,22 @@ subprojects {
apply plugin: 'nebula.dependency-recommender'
apply plugin: 'java-library'
+ // apply this plugin in a try-catch block so that we can handle cases without .git directory
+ // apply to subprojects so that git properties information is available for runtime debugging and tests
+ // e.g. sending build SHA as headers
+ try {
+ apply plugin: 'com.gorylenko.gradle-git-properties'
Review Comment:
We only need one file, `build/iceberg-build.properties`. Can you remove the others?
--
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 #5228: [WIP] Build - Add Git Properties Information via Plugin
Posted by GitBox <gi...@apache.org>.
rdblue commented on code in PR #5228:
URL: https://github.com/apache/iceberg/pull/5228#discussion_r917047120
##########
deploy.gradle:
##########
@@ -55,6 +55,7 @@ subprojects {
task.from(rootDir) {
include 'LICENSE'
include 'NOTICE'
+ include 'iceberg-version.properties'
Review Comment:
I think I said this should be `iceberg-build.properties`.
--
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 #5228: Build - Add Git Build Properties File via Plugin
Posted by GitBox <gi...@apache.org>.
kbendick commented on PR #5228:
URL: https://github.com/apache/iceberg/pull/5228#issuecomment-1179802732
> Thanks, @kbendick! This worked in my testing.
Great! Let me know if you want me to open PRs for any of the above steps. I have code that does all of them as I was testing the plug-in out. Though I like your improvements in https://github.com/apache/iceberg/pull/5236 (but I can’t say with certainty they’d work with my code 100% as is).
--
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 #5228: Build - Add Git Properties Information via Plugin
Posted by GitBox <gi...@apache.org>.
kbendick commented on code in PR #5228:
URL: https://github.com/apache/iceberg/pull/5228#discussion_r917052767
##########
build.gradle:
##########
@@ -73,6 +83,22 @@ subprojects {
apply plugin: 'nebula.dependency-recommender'
apply plugin: 'java-library'
+ // apply this plugin in a try-catch block so that we can handle cases without .git directory
+ // apply to subprojects so that git properties information is available for runtime debugging and tests
+ // e.g. sending build SHA as headers
+ try {
+ apply plugin: 'com.gorylenko.gradle-git-properties'
Review Comment:
From my testing, we would need it for local development or using a JAR built locally (such as copying over one `iceberg-core.jar` or even the spark bundle jar for local testing).
But I can remove this.
--
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 #5228: [WIP] Build - Add Git Properties Information via Plugin
Posted by GitBox <gi...@apache.org>.
kbendick commented on code in PR #5228:
URL: https://github.com/apache/iceberg/pull/5228#discussion_r917001112
##########
build.gradle:
##########
@@ -73,6 +83,22 @@ subprojects {
apply plugin: 'nebula.dependency-recommender'
apply plugin: 'java-library'
+ // apply this plugin in a try-catch block so that we can handle cases without .git directory
+ // apply to subprojects so that git properties information is available for runtime debugging and tests
+ // e.g. sending build SHA as headers
+ try {
+ apply plugin: 'com.gorylenko.gradle-git-properties'
+ gitProperties {
+ gitPropertiesName = "${project.name}-version.properties"
+ extProperty = 'gitProps'
+ failOnNoGitDirectory = false
Review Comment:
I tried removing the `.git` directory entirely using `git clone --depth=1 --branch=kb-gradle-git-properties-plugin-file-generation git@github.com:kbendick/iceberg.git iceberg-version-no-git && rm -rf .git` and the build errors out because `gitProperties` isn't recognized (because the plugin apply didn't work).
So I'm assuming that property is more for cases where there's no git directory, but JGit is still available.
--
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 #5228: [WIP] Build - Add Git Properties Information via Plugin
Posted by GitBox <gi...@apache.org>.
kbendick commented on code in PR #5228:
URL: https://github.com/apache/iceberg/pull/5228#discussion_r917047461
##########
deploy.gradle:
##########
@@ -55,6 +55,7 @@ subprojects {
task.from(rootDir) {
include 'LICENSE'
include 'NOTICE'
+ include 'iceberg-version.properties'
Review Comment:
I will update that.
--
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 #5228: Build - Add Git Build Properties File via Plugin
Posted by GitBox <gi...@apache.org>.
kbendick commented on PR #5228:
URL: https://github.com/apache/iceberg/pull/5228#issuecomment-1179413831
cc @rdblue this PR now has only the minimal changes for the one top level git properties file.
I've saved all of my previous changes and can pull them out when needed.
--
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 #5228: [WIP] Build - Add Git Properties Information via Plugin
Posted by GitBox <gi...@apache.org>.
rdblue commented on code in PR #5228:
URL: https://github.com/apache/iceberg/pull/5228#discussion_r917048704
##########
dev/source-release.sh:
##########
@@ -87,11 +87,29 @@ tagrc="${tag}-rc${rc}"
echo "Preparing source for $tagrc"
+# Generate git properties file as git info will not be as accessible in svn during final release
+# This SHA will technically be one behind the final release as we need to commit the file
+#echo "Generating git properties file for tarball"
+# $projectdir/./gradlew generateGitProperties --no-daemon
+# mv $projectdir/build/resources/main/iceberg-version.properties $projectdir/iceberg-version.properties
+
echo "Adding version.txt and tagging release..."
echo $version > $projectdir/version.txt
+# git add $projectdir/version.txt $projectdir/iceberg-version.properties
+# git commit -m "Add version.txt for release $version" $projectdir/version.txt $projectdir/iceberg-version.properties
git add $projectdir/version.txt
git commit -m "Add version.txt for release $version" $projectdir/version.txt
+# Regenerate the git properties file so its up to date, then move it out of build/ (which is in the .gitattributes
+# file and thus will be ignored from the archive). The file will be added to the release tarball, without committing,
+# so it has the correct SHA (from the previous step).
+#
+# TODO - Add a second task that generates _only_ the root properties file with just a subset of the info (so .dirty
+# and user emails and hostnames don't appear or consider a virtual file in the git archive via --add-virtual-file).
+$projectdir/./gradlew generateGitProperties --no-daemon
+mv $projectdir/build/resources/main/iceberg-version.properties $projectdir/iceberg-version.properties
+git_props_file=$projectdir/iceberg-version.properties
+
Review Comment:
Please remove these unrelated changes. This PR should only contain the changes needed to add the `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] danielcweeks commented on pull request #5228: Build - Add Git Build Properties File via Plugin
Posted by GitBox <gi...@apache.org>.
danielcweeks commented on PR #5228:
URL: https://github.com/apache/iceberg/pull/5228#issuecomment-1179432740
@kbendick I'm not sure which path was settled on for the properties file. I see both `/build/iceberg-build.properties` and `core/build/iceberg-build.properties` paths referenced.
@rdblue I think you may have meant `core/build/iceberg-build.properties` which you referenced in the other PR, but that's not sufficient to get into a jar.
We can either produce a top-level `iceberg-build.properties` and then later copy it into location for the jar to pick it up or we can set the location to `gitPropertiesResourceDir = file("${rootDir}/core/build/resources/main")`, which will put it in a place where it will be included.
We could also update the Jar task to include the top-level file (assuming you can reference from the project root).
Whether we want to do it in this PR or not, we should make sure that building the jar should generate the file so we don't require calling a specific task to get the build information included.
--
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 #5228: [WIP] Build - Add Git Properties Information via Plugin
Posted by GitBox <gi...@apache.org>.
kbendick commented on PR #5228:
URL: https://github.com/apache/iceberg/pull/5228#issuecomment-1179300349
Ok. This PR is now only adding the git properties file from the plugin to `build/iceberg-build.properties`.
```bash
➜ iceberg: ./gradlew :generateGitProperties
BUILD SUCCESSFUL in 4s
1 actionable task: 1 executed
➜ iceberg: cat build/iceberg-build.properties
git.branch=kb-gradle-git-properties-plugin-file-generation
git.build.host=Kyles-MacBook-Pro-2.local
git.build.user.email=kjbendickson@gmail.com
git.build.user.name=Kyle Bendickson
git.build.version=0.14.0-SNAPSHOT
git.closest.tag.commit.count=680
git.closest.tag.name=release-base-0.13.0
git.commit.id=c324a7e5636181768d5b099d2992ae74844324ed
git.commit.id.abbrev=c324a7e
git.commit.id.describe=release-base-0.13.0-680-gc324a7e
git.commit.message.full=use file in gitPropertiesResourceDir\n
git.commit.message.short=use file in gitPropertiesResourceDir
git.commit.time=2022-07-08T12\:25\:54-0700
git.commit.user.email=kjbendickson@gmail.com
git.commit.user.name=Kyle Bendickson
git.dirty=false
git.remote.origin.url=git@github.com\:kbendick/iceberg.git
git.tags=
git.total.commit.count=2938
```
--
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 #5228: Build - Add Git Build Properties File via Plugin
Posted by GitBox <gi...@apache.org>.
rdblue commented on PR #5228:
URL: https://github.com/apache/iceberg/pull/5228#issuecomment-1179789489
Thanks, @kbendick! This worked in my testing.
--
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 #5228: [WIP] Build - Add Git Properties Information via Plugin
Posted by GitBox <gi...@apache.org>.
kbendick commented on code in PR #5228:
URL: https://github.com/apache/iceberg/pull/5228#discussion_r917070400
##########
dev/source-release.sh:
##########
@@ -87,11 +87,29 @@ tagrc="${tag}-rc${rc}"
echo "Preparing source for $tagrc"
+# Generate git properties file as git info will not be as accessible in svn during final release
+# This SHA will technically be one behind the final release as we need to commit the file
+#echo "Generating git properties file for tarball"
+# $projectdir/./gradlew generateGitProperties --no-daemon
+# mv $projectdir/build/resources/main/iceberg-version.properties $projectdir/iceberg-version.properties
+
echo "Adding version.txt and tagging release..."
echo $version > $projectdir/version.txt
+# git add $projectdir/version.txt $projectdir/iceberg-version.properties
+# git commit -m "Add version.txt for release $version" $projectdir/version.txt $projectdir/iceberg-version.properties
git add $projectdir/version.txt
git commit -m "Add version.txt for release $version" $projectdir/version.txt
+# Regenerate the git properties file so its up to date, then move it out of build/ (which is in the .gitattributes
+# file and thus will be ignored from the archive). The file will be added to the release tarball, without committing,
+# so it has the correct SHA (from the previous step).
+#
+# TODO - Add a second task that generates _only_ the root properties file with just a subset of the info (so .dirty
+# and user emails and hostnames don't appear or consider a virtual file in the git archive via --add-virtual-file).
+$projectdir/./gradlew generateGitProperties --no-daemon
+mv $projectdir/build/resources/main/iceberg-version.properties $projectdir/iceberg-version.properties
+git_props_file=$projectdir/iceberg-version.properties
+
Review Comment:
Removed these changes.
--
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 #5228: [WIP] Build - Add Git Properties Information via Plugin
Posted by GitBox <gi...@apache.org>.
kbendick commented on code in PR #5228:
URL: https://github.com/apache/iceberg/pull/5228#discussion_r917052767
##########
build.gradle:
##########
@@ -73,6 +83,22 @@ subprojects {
apply plugin: 'nebula.dependency-recommender'
apply plugin: 'java-library'
+ // apply this plugin in a try-catch block so that we can handle cases without .git directory
+ // apply to subprojects so that git properties information is available for runtime debugging and tests
+ // e.g. sending build SHA as headers
+ try {
+ apply plugin: 'com.gorylenko.gradle-git-properties'
Review Comment:
From my testing, we would need it for local development or using a JAR built locally (such as copying over one `iceberg-core.jar` or even the spark bundle file).
But I can remove this.
--
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 #5228: Build - Add Git Build Properties File via Plugin
Posted by GitBox <gi...@apache.org>.
kbendick commented on PR #5228:
URL: https://github.com/apache/iceberg/pull/5228#issuecomment-1179442356
It's probably easier if I just push a branch that reflects the older state of this PR, which would have the generated git properties file both:
1. In the runtime classspath of `iceberg-core` and
2. Accessible when building the tarball for tagged releases, so that users could `./gradlew build` the project without the `.git` directory and still have access to the git properties file (by including it in the tarball).
I pushed the older version to my fork. Forgive me if it's a bit messy as I just stashed those changes at a point in time:
https://github.com/kbendick/iceberg/pull/87
--
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 #5228: [WIP] Build - Add Git Properties Information via Plugin
Posted by GitBox <gi...@apache.org>.
kbendick commented on code in PR #5228:
URL: https://github.com/apache/iceberg/pull/5228#discussion_r917049887
##########
dev/source-release.sh:
##########
@@ -87,11 +87,29 @@ tagrc="${tag}-rc${rc}"
echo "Preparing source for $tagrc"
+# Generate git properties file as git info will not be as accessible in svn during final release
+# This SHA will technically be one behind the final release as we need to commit the file
+#echo "Generating git properties file for tarball"
+# $projectdir/./gradlew generateGitProperties --no-daemon
+# mv $projectdir/build/resources/main/iceberg-version.properties $projectdir/iceberg-version.properties
+
echo "Adding version.txt and tagging release..."
echo $version > $projectdir/version.txt
+# git add $projectdir/version.txt $projectdir/iceberg-version.properties
+# git commit -m "Add version.txt for release $version" $projectdir/version.txt $projectdir/iceberg-version.properties
git add $projectdir/version.txt
git commit -m "Add version.txt for release $version" $projectdir/version.txt
+# Regenerate the git properties file so its up to date, then move it out of build/ (which is in the .gitattributes
+# file and thus will be ignored from the archive). The file will be added to the release tarball, without committing,
+# so it has the correct SHA (from the previous step).
+#
+# TODO - Add a second task that generates _only_ the root properties file with just a subset of the info (so .dirty
+# and user emails and hostnames don't appear or consider a virtual file in the git archive via --add-virtual-file).
+$projectdir/./gradlew generateGitProperties --no-daemon
+mv $projectdir/build/resources/main/iceberg-version.properties $projectdir/iceberg-version.properties
+git_props_file=$projectdir/iceberg-version.properties
+
Review Comment:
These are needed to get the file into the tarball so that it can be added in `deploy.gradle`. So I'm admittedly still experimenting with the process so that the output is correct.
Will revert these changes and we can add in another PR.
--
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 #5228: [WIP] Build - Add Git Properties Information via Plugin
Posted by GitBox <gi...@apache.org>.
kbendick commented on PR #5228:
URL: https://github.com/apache/iceberg/pull/5228#issuecomment-1179253108
Example output of the `iceberg-version.properties` file from the tarball (after running `dev/source-release.sh`). I verified that the git SHA is the same as the one from the `version.txt` commit and that the tag is correct.
I ran this as apache-iceberg-808-rc1, but via the `git.commit.message` and the `git.commit.describe` it can be verified that this information is correct.
The only issue is that the tag will be from the final release candidate, not the actual release version. We can simply exclude that info or run a shell command over the file to remove the `rc` from inside `deploy.gradle`.
I'd also like to remove some of the properties given that it leaks a lot of information about the build user etc (such as their email, which might not be the one used for the signing keys and might be hidden on github, etc).
```
cat apache-iceberg-0.808/iceberg-version.properties
git.branch=kb-gradle-git-properties-plugin-file-generation
git.build.host=Kyles-MacBook-Pro-2.local
git.build.user.email=kjbendickson@gmail.com
git.build.user.name=Kyle Bendickson
git.build.version=0.808
git.closest.tag.commit.count=0
git.closest.tag.name=apache-iceberg-0.808-rc1
git.commit.id=e2aeac66dfdbbcf82a8dbc240e3908a02e0af637
git.commit.id.abbrev=e2aeac6
git.commit.id.describe=apache-iceberg-0.808-rc1
git.commit.message.full=Add version.txt for release 0.808\n
git.commit.message.short=Add version.txt for release 0.808
git.commit.time=2022-07-08T11\:09\:02-0700
git.commit.user.email=kjbendickson@gmail.com
git.commit.user.name=Kyle Bendickson
git.dirty=false
git.remote.origin.url=git@github.com\:kbendick/iceberg.git
git.tags=apache-iceberg-0.808-rc1
git.total.commit.count=2937
```
--
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] nastra commented on a diff in pull request #5228: [WIP] Build - Add Git Properties Information via Plugin
Posted by GitBox <gi...@apache.org>.
nastra commented on code in PR #5228:
URL: https://github.com/apache/iceberg/pull/5228#discussion_r916616541
##########
build.gradle:
##########
@@ -73,6 +83,22 @@ subprojects {
apply plugin: 'nebula.dependency-recommender'
apply plugin: 'java-library'
+ // apply this plugin in a try-catch block so that we can handle cases without .git directory
+ // apply to subprojects so that git properties information is available for runtime debugging and tests
+ // e.g. sending build SHA as headers
+ try {
+ apply plugin: 'com.gorylenko.gradle-git-properties'
+ gitProperties {
+ gitPropertiesName = "${project.name}-version.properties"
+ extProperty = 'gitProps'
+ failOnNoGitDirectory = false
Review Comment:
nit: I wonder if the try-catch is necessary with `failOnNoGitDirectory=false`. Did you have a chance to try this without a .git directory?
--
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 #5228: [WIP] Build - Add Git Properties Information via Plugin
Posted by GitBox <gi...@apache.org>.
kbendick commented on code in PR #5228:
URL: https://github.com/apache/iceberg/pull/5228#discussion_r916999710
##########
build.gradle:
##########
@@ -73,6 +83,22 @@ subprojects {
apply plugin: 'nebula.dependency-recommender'
apply plugin: 'java-library'
+ // apply this plugin in a try-catch block so that we can handle cases without .git directory
+ // apply to subprojects so that git properties information is available for runtime debugging and tests
+ // e.g. sending build SHA as headers
+ try {
+ apply plugin: 'com.gorylenko.gradle-git-properties'
+ gitProperties {
+ gitPropertiesName = "${project.name}-version.properties"
+ extProperty = 'gitProps'
+ failOnNoGitDirectory = false
Review Comment:
Yeah it's still necessary.
--
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