[kudu-CR] [build] Fix the java compatibility checker script
Grant Henke has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/13004 ) Change subject: [build] Fix the java compatibility checker script .. [build] Fix the java compatibility checker script This patch fixes check_compatibility.py. Primarily it changed the existing script to use the Gradle build in place of the Maven build that no longer exists. It now uses japicmp in place of java_acc because java_acc had issues unzipping the jars created by the kudu build. Additionally japicmp’s filtering seemed to work better. The patch also updates the Gradle build to ensure generated Protobuf classes are annotated with @Generated to make report filtering easier. In follow on patches I may incorporate this into the LINT job and cause failure on API breakage. Change-Id: Id482da0b3aa5ea35fcfb2b674f0d7ae413045ca3 Reviewed-on: http://gerrit.cloudera.org:8080/13004 Tested-by: Kudu Jenkins Reviewed-by: Greg Solovyev Reviewed-by: Alexey Serbin --- M build-support/check_compatibility.py M java/gradle/protobuf.gradle 2 files changed, 111 insertions(+), 91 deletions(-) Approvals: Kudu Jenkins: Verified Greg Solovyev: Looks good to me, but someone else must approve Alexey Serbin: Looks good to me, approved -- To view, visit http://gerrit.cloudera.org:8080/13004 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: Id482da0b3aa5ea35fcfb2b674f0d7ae413045ca3 Gerrit-Change-Number: 13004 Gerrit-PatchSet: 5 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Greg Solovyev Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] [build] Fix the java compatibility checker script
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/13004 ) Change subject: [build] Fix the java compatibility checker script .. Patch Set 4: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/13004 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Id482da0b3aa5ea35fcfb2b674f0d7ae413045ca3 Gerrit-Change-Number: 13004 Gerrit-PatchSet: 4 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Greg Solovyev Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Mon, 11 Jan 2021 21:54:00 + Gerrit-HasComments: No
[kudu-CR] [build] Fix the java compatibility checker script
Greg Solovyev has posted comments on this change. ( http://gerrit.cloudera.org:8080/13004 ) Change subject: [build] Fix the java compatibility checker script .. Patch Set 4: Code-Review+1 (2 comments) http://gerrit.cloudera.org:8080/#/c/13004/3/build-support/check_compatibility.py File build-support/check_compatibility.py: http://gerrit.cloudera.org:8080/#/c/13004/3/build-support/check_compatibility.py@121 PS3, Line 121: def find_client_jars(path): > I exclude jars because they are know to be non-public or application jars w Done http://gerrit.cloudera.org:8080/#/c/13004/3/build-support/check_compatibility.py@175 PS3, Line 175: if src_name == dst_name: > Currently the script uses super rudimentary arguments. I will enhance the a Done -- To view, visit http://gerrit.cloudera.org:8080/13004 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Id482da0b3aa5ea35fcfb2b674f0d7ae413045ca3 Gerrit-Change-Number: 13004 Gerrit-PatchSet: 4 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Greg Solovyev Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Mon, 11 Jan 2021 18:28:37 + Gerrit-HasComments: Yes
[kudu-CR] [build] Fix the java compatibility checker script
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/13004 ) Change subject: [build] Fix the java compatibility checker script .. Patch Set 3: (5 comments) http://gerrit.cloudera.org:8080/#/c/13004/3//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/13004/3//COMMIT_MSG@22 PS3, Line 22: incorparate > incorporate ? Done http://gerrit.cloudera.org:8080/#/c/13004/3/build-support/check_compatibility.py File build-support/check_compatibility.py: http://gerrit.cloudera.org:8080/#/c/13004/3/build-support/check_compatibility.py@109 PS3, Line 109: if os.path.exists(get_japicmp_path()): : logging.info("japicmp is already downloaded.") > Will it work as expected when a new version of the tool is available? Mayb Done http://gerrit.cloudera.org:8080/#/c/13004/3/build-support/check_compatibility.py@115 PS3, Line 115: "curl", "-o" > nit: does it make sense to add few options to match the behavior of downloa Done http://gerrit.cloudera.org:8080/#/c/13004/3/build-support/check_compatibility.py@121 PS3, Line 121: return [j for j in all_jars if ( > Nit: if I am reading this correctly, this code is excluding 9 patterns in o I exclude jars because they are know to be non-public or application jars which do not have public API. I prefer a exclusion to explicit inclusion so that by default we scan and check APIs if a new jar is added. http://gerrit.cloudera.org:8080/#/c/13004/3/build-support/check_compatibility.py@175 PS3, Line 175: # TODO(ghenke): Add support for --error-on-binary-incompatibility and --error-on-source-incompatibility. > Currently, the script produces a lot of output for modified and added metho Currently the script uses super rudimentary arguments. I will enhance the arguments in a follow on patch once I understand what types of options a scheduled job might need. -- To view, visit http://gerrit.cloudera.org:8080/13004 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Id482da0b3aa5ea35fcfb2b674f0d7ae413045ca3 Gerrit-Change-Number: 13004 Gerrit-PatchSet: 3 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Greg Solovyev Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Mon, 11 Jan 2021 17:13:04 + Gerrit-HasComments: Yes
[kudu-CR] [build] Fix the java compatibility checker script
Hello Alexey Serbin, Attila Bukor, Kudu Jenkins, Greg Solovyev, Hao Hao, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/13004 to look at the new patch set (#4). Change subject: [build] Fix the java compatibility checker script .. [build] Fix the java compatibility checker script This patch fixes check_compatibility.py. Primarily it changed the existing script to use the Gradle build in place of the Maven build that no longer exists. It now uses japicmp in place of java_acc because java_acc had issues unzipping the jars created by the kudu build. Additionally japicmp’s filtering seemed to work better. The patch also updates the Gradle build to ensure generated Protobuf classes are annotated with @Generated to make report filtering easier. In follow on patches I may incorporate this into the LINT job and cause failure on API breakage. Change-Id: Id482da0b3aa5ea35fcfb2b674f0d7ae413045ca3 --- M build-support/check_compatibility.py M java/gradle/protobuf.gradle 2 files changed, 111 insertions(+), 91 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/04/13004/4 -- To view, visit http://gerrit.cloudera.org:8080/13004 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Id482da0b3aa5ea35fcfb2b674f0d7ae413045ca3 Gerrit-Change-Number: 13004 Gerrit-PatchSet: 4 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Greg Solovyev Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] [build] Fix the java compatibility checker script
Greg Solovyev has posted comments on this change. ( http://gerrit.cloudera.org:8080/13004 ) Change subject: [build] Fix the java compatibility checker script .. Patch Set 3: (1 comment) http://gerrit.cloudera.org:8080/#/c/13004/3/build-support/check_compatibility.py File build-support/check_compatibility.py: http://gerrit.cloudera.org:8080/#/c/13004/3/build-support/check_compatibility.py@175 PS3, Line 175: # TODO(ghenke): Add support for --error-on-binary-incompatibility and --error-on-source-incompatibility. Currently, the script produces a lot of output for modified and added methods, which do not break compatibility. In order to see if anything is broken, I had to edit the script and add --only-incompatible. Perhaps, a simpler way is to pass CLI options through to the japi? -- To view, visit http://gerrit.cloudera.org:8080/13004 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Id482da0b3aa5ea35fcfb2b674f0d7ae413045ca3 Gerrit-Change-Number: 13004 Gerrit-PatchSet: 3 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Greg Solovyev Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Fri, 08 Jan 2021 22:55:47 + Gerrit-HasComments: Yes
[kudu-CR] [build] Fix the java compatibility checker script
Greg Solovyev has posted comments on this change. ( http://gerrit.cloudera.org:8080/13004 ) Change subject: [build] Fix the java compatibility checker script .. Patch Set 3: (1 comment) http://gerrit.cloudera.org:8080/#/c/13004/3/build-support/check_compatibility.py File build-support/check_compatibility.py: http://gerrit.cloudera.org:8080/#/c/13004/3/build-support/check_compatibility.py@121 PS3, Line 121: return [j for j in all_jars if ( Nit: if I am reading this correctly, this code is excluding 9 patterns in order to find 6 files. Wouldn't it be easier include instead of exclude? -- To view, visit http://gerrit.cloudera.org:8080/13004 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Id482da0b3aa5ea35fcfb2b674f0d7ae413045ca3 Gerrit-Change-Number: 13004 Gerrit-PatchSet: 3 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Greg Solovyev Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Fri, 08 Jan 2021 22:44:22 + Gerrit-HasComments: Yes
[kudu-CR] [build] Fix the java compatibility checker script
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/13004 ) Change subject: [build] Fix the java compatibility checker script .. Patch Set 3: Code-Review+1 (3 comments) Looks good overall, just a few nits. http://gerrit.cloudera.org:8080/#/c/13004/3//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/13004/3//COMMIT_MSG@22 PS3, Line 22: incorparate incorporate ? http://gerrit.cloudera.org:8080/#/c/13004/3/build-support/check_compatibility.py File build-support/check_compatibility.py: http://gerrit.cloudera.org:8080/#/c/13004/3/build-support/check_compatibility.py@109 PS3, Line 109: if os.path.exists(get_japicmp_path()): : logging.info("japicmp is already downloaded.") Will it work as expected when a new version of the tool is available? Maybe, it makes sense to switch to a filename that matches the name of the downloaded JAR? http://gerrit.cloudera.org:8080/#/c/13004/3/build-support/check_compatibility.py@115 PS3, Line 115: "curl", "-o" nit: does it make sense to add few options to match the behavior of downloading source packages in thirdparty? Something like curl --retry 3 -L -o ... -- To view, visit http://gerrit.cloudera.org:8080/13004 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Id482da0b3aa5ea35fcfb2b674f0d7ae413045ca3 Gerrit-Change-Number: 13004 Gerrit-PatchSet: 3 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Greg Solovyev Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Fri, 08 Jan 2021 22:38:55 + Gerrit-HasComments: Yes
[kudu-CR] [build] Fix the java compatibility checker script
Grant Henke has removed Adar Lieber-Dembo from this change. ( http://gerrit.cloudera.org:8080/13004 ) Change subject: [build] Fix the java compatibility checker script .. Removed reviewer Adar Lieber-Dembo. -- To view, visit http://gerrit.cloudera.org:8080/13004 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: deleteReviewer Gerrit-Change-Id: Id482da0b3aa5ea35fcfb2b674f0d7ae413045ca3 Gerrit-Change-Number: 13004 Gerrit-PatchSet: 3 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] [build] Fix the java compatibility checker script
Hello Kudu Jenkins, Adar Lieber-Dembo, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/13004 to look at the new patch set (#3). Change subject: [build] Fix the java compatibility checker script .. [build] Fix the java compatibility checker script This patch fixes check_compatibility.py. Primarily it changed the existing script to use the Gradle build in place of the Maven build that no longer exists. It now uses japicmp in place of java_acc because java_acc had issues unzipping the jars created by the kudu build. Additionally japicmp’s filtering seemed to work better. The patch also updates the Gradle build to ensure generated Protobuf classes are annotated with @Generated to make report filtering easier. In follow on patches I may incorparate this into the LINT job and cause failure on API breakage. Change-Id: Id482da0b3aa5ea35fcfb2b674f0d7ae413045ca3 --- M build-support/check_compatibility.py M java/gradle/protobuf.gradle 2 files changed, 108 insertions(+), 91 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/04/13004/3 -- To view, visit http://gerrit.cloudera.org:8080/13004 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Id482da0b3aa5ea35fcfb2b674f0d7ae413045ca3 Gerrit-Change-Number: 13004 Gerrit-PatchSet: 3 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Adar Lieber-Dembo Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] [build] Fix the java compatibility checker script
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/13004 ) Change subject: [build] Fix the java compatibility checker script .. Patch Set 1: (2 comments) http://gerrit.cloudera.org:8080/#/c/13004/1//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/13004/1//COMMIT_MSG@13 PS1, Line 13: > Nit: extra space here. Done http://gerrit.cloudera.org:8080/#/c/13004/1/build-support/check_compatibility.py File build-support/check_compatibility.py: http://gerrit.cloudera.org:8080/#/c/13004/1/build-support/check_compatibility.py@204 PS1, Line 204: # Run the build in each tree. > Hmm, would be nice if this would work on existing build output, since it'll I agree that would be a good enhancement for follow on changes. For now, given the gradle build has been around a while, this is better than nothing and can be use for releases and arbitrary commits. -- To view, visit http://gerrit.cloudera.org:8080/13004 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Id482da0b3aa5ea35fcfb2b674f0d7ae413045ca3 Gerrit-Change-Number: 13004 Gerrit-PatchSet: 1 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Adar Lieber-Dembo Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Thu, 07 Jan 2021 21:20:53 + Gerrit-HasComments: Yes
[kudu-CR] [build] Fix the java compatibility checker script
Hello Kudu Jenkins, Adar Lieber-Dembo, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/13004 to look at the new patch set (#2). Change subject: [build] Fix the java compatibility checker script .. [build] Fix the java compatibility checker script This patch fixes check_compatibility.py. Primarily it changed the existing script to use the Gradle build in place of the Maven build that no longer exists. It now uses japicmp in place of java_acc because java_acc had issues unzipping the jars created by the kudu build. Additionally japicmp’s filtering seemed to work better. The patch also updates the Gradle build to ensure generated Protobuf classes are annotated with @Generated to make report filtering easier. Change-Id: Id482da0b3aa5ea35fcfb2b674f0d7ae413045ca3 --- M build-support/check_compatibility.py M java/gradle/protobuf.gradle 2 files changed, 108 insertions(+), 91 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/04/13004/2 -- To view, visit http://gerrit.cloudera.org:8080/13004 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Id482da0b3aa5ea35fcfb2b674f0d7ae413045ca3 Gerrit-Change-Number: 13004 Gerrit-PatchSet: 2 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Adar Lieber-Dembo Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] [build] Fix the java compatibility checker script
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/13004 ) Change subject: [build] Fix the java compatibility checker script .. Patch Set 1: (2 comments) What are the next steps once this is merged? http://gerrit.cloudera.org:8080/#/c/13004/1//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/13004/1//COMMIT_MSG@13 PS1, Line 13: Nit: extra space here. http://gerrit.cloudera.org:8080/#/c/13004/1/build-support/check_compatibility.py File build-support/check_compatibility.py: http://gerrit.cloudera.org:8080/#/c/13004/1/build-support/check_compatibility.py@204 PS1, Line 204: # Run the build in each tree. Hmm, would be nice if this would work on existing build output, since it'll be a lot easier to run it on historical artifacts that way (i.e. no need to worry about releases done before we switched to Gradle). Would it be possible to allow for URLs or revisions, and if the former, download the JAR and use it directly? -- To view, visit http://gerrit.cloudera.org:8080/13004 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Id482da0b3aa5ea35fcfb2b674f0d7ae413045ca3 Gerrit-Change-Number: 13004 Gerrit-PatchSet: 1 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Sun, 14 Apr 2019 17:52:27 + Gerrit-HasComments: Yes
[kudu-CR] [build] Fix the java compatibility checker script
Grant Henke has uploaded this change for review. ( http://gerrit.cloudera.org:8080/13004 Change subject: [build] Fix the java compatibility checker script .. [build] Fix the java compatibility checker script This patch fixes check_compatibility.py. Primarily it changed the existing script to use the Gradle build in place of the Maven build that no longer exists. It now uses japicmp in place of java_acc because java_acc had issues unzipping the jars created by the kudu build. Additionally japicmp’s filtering seemed to work better. The patch also updates the Gradle build to ensure generated Protobuf classes are annotated with @Generated to make report filtering easier. Change-Id: Id482da0b3aa5ea35fcfb2b674f0d7ae413045ca3 --- M build-support/check_compatibility.py M java/gradle/protobuf.gradle 2 files changed, 84 insertions(+), 87 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/04/13004/1 -- To view, visit http://gerrit.cloudera.org:8080/13004 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: Id482da0b3aa5ea35fcfb2b674f0d7ae413045ca3 Gerrit-Change-Number: 13004 Gerrit-PatchSet: 1 Gerrit-Owner: Grant Henke