You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by GitBox <gi...@apache.org> on 2021/04/15 16:56:44 UTC
[GitHub] [arrow] nbauernfeind opened a new pull request #10058: ARROW-12111: [Java] Generate flatbuffer files using flatc 1.12.0
nbauernfeind opened a new pull request #10058:
URL: https://github.com/apache/arrow/pull/10058
This is a resolution of ARROW-12111, which came up on the dev mailing list titled "[Java] Source control of generated flatbuffers code", the main complaint was that the plugin used to generate the flatbuffer files was unavailable for windows developers.
I, personally, would also like to see this upgraded to flatbuffers 1.12 from 1.9.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] nbauernfeind commented on a change in pull request #10058: ARROW-12111: [Java] Generate flatbuffer files using flatc 1.12.0
Posted by GitBox <gi...@apache.org>.
nbauernfeind commented on a change in pull request #10058:
URL: https://github.com/apache/arrow/pull/10058#discussion_r614264302
##########
File path: java/pom.xml
##########
@@ -36,8 +36,8 @@
<dep.netty.version>4.1.48.Final</dep.netty.version>
<dep.jackson.version>2.11.4</dep.jackson.version>
<dep.hadoop.version>2.7.1</dep.hadoop.version>
- <dep.fbs.version>1.9.0</dep.fbs.version>
- <dep.flatc.version>1.9.0</dep.flatc.version>
+ <dep.fbs.version>1.12.0</dep.fbs.version>
+ <dep.flatc.version>1.12.0</dep.flatc.version>
Review comment:
The `dep.flatc.version` is not; good catch. (the `dep.fbs.version` is, however). I have force pushed the change to remove 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] emkornfield commented on a change in pull request #10058: ARROW-12111: [Java] Generate flatbuffer files using flatc 1.12.0
Posted by GitBox <gi...@apache.org>.
emkornfield commented on a change in pull request #10058:
URL: https://github.com/apache/arrow/pull/10058#discussion_r614262888
##########
File path: java/README.md
##########
@@ -64,26 +64,46 @@ and arrow-format into a single JAR. Using the classifier "shade-format-flatbuff
pom.xml will make use of this JAR, you can then exclude/resolve the original dependency to
a version of your choosing.
+### Updating the flatbuffers generated code
+
+To update the checked-in flatbuffer generated code perform the following:
+
+```bash
+cd $ARROW_HOME
+
+# remove the existing files
+rm -rf java/format/src
+
+# regenerate from the .fbs files
+flatc --java -o java/format/src/main/java format/*.fbs
Review comment:
it is probably good to document version here?
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] github-actions[bot] commented on pull request #10058: ARROW-12111: [Java] Generate flatbuffer files using flatc 1.12.0
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #10058:
URL: https://github.com/apache/arrow/pull/10058#issuecomment-820606937
https://issues.apache.org/jira/browse/ARROW-12111
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] bobtins commented on pull request #10058: ARROW-12111: [Java] Generate flatbuffer files using flatc 1.12.0
Posted by GitBox <gi...@apache.org>.
bobtins commented on pull request #10058:
URL: https://github.com/apache/arrow/pull/10058#issuecomment-820698674
@kiszk if the generated code didn't work on big-endian then it would be a flatbuffers bug because it's in charge of enforcing the binary format.
@nbauernfeind thanks for jumping in! Sorry for not following through on this; I got pulled off on other things.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] emkornfield commented on a change in pull request #10058: ARROW-12111: [Java] Generate flatbuffer files using flatc 1.12.0
Posted by GitBox <gi...@apache.org>.
emkornfield commented on a change in pull request #10058:
URL: https://github.com/apache/arrow/pull/10058#discussion_r614262710
##########
File path: java/README.md
##########
@@ -64,26 +64,46 @@ and arrow-format into a single JAR. Using the classifier "shade-format-flatbuff
pom.xml will make use of this JAR, you can then exclude/resolve the original dependency to
a version of your choosing.
+### Updating the flatbuffers generated code
+
+To update the checked-in flatbuffer generated code perform the following:
+
+```bash
+cd $ARROW_HOME
+
+# remove the existing files
+rm -rf java/format/src
+
+# regenerate from the .fbs files
+flatc --java -o java/format/src/main/java format/*.fbs
+
+# prepend license header
+find java/format/src -type f | while read file; do
+ (cat header | while read line; do echo "// $line"; done; cat $file) > $file.tmp
Review comment:
is header checked in? maybe add a line on how it was extracted?
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] emkornfield commented on pull request #10058: ARROW-12111: [Java] Generate flatbuffer files using flatc 1.12.0
Posted by GitBox <gi...@apache.org>.
emkornfield commented on pull request #10058:
URL: https://github.com/apache/arrow/pull/10058#issuecomment-820605522
LGTM once CI passes.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] kiszk commented on pull request #10058: ARROW-12111: [Java] Generate flatbuffer files using flatc 1.12.0
Posted by GitBox <gi...@apache.org>.
kiszk commented on pull request #10058:
URL: https://github.com/apache/arrow/pull/10058#issuecomment-820595477
Let me jump in this PR. flatbuffers 1.9 for s390x is not officially prepared. Thus, we privately build and provide it on own artifact. You can see [here](https://github.com/apache/arrow/blob/master/ci/scripts/java_build.sh#L33-L40).
If this approach works well, can we remove this workaround for s390x, too?
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] nbauernfeind commented on pull request #10058: ARROW-12111: [Java] Generate flatbuffer files using flatc 1.12.0
Posted by GitBox <gi...@apache.org>.
nbauernfeind commented on pull request #10058:
URL: https://github.com/apache/arrow/pull/10058#issuecomment-820597282
@kiszk I believe so! With this patch there is no need to have access to a `flatc` binary during any phase of the Java build. Shall I axe-those lines in the `java_build.sh` file you've pointed at?
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] nbauernfeind commented on a change in pull request #10058: ARROW-12111: [Java] Generate flatbuffer files using flatc 1.12.0
Posted by GitBox <gi...@apache.org>.
nbauernfeind commented on a change in pull request #10058:
URL: https://github.com/apache/arrow/pull/10058#discussion_r614269387
##########
File path: java/README.md
##########
@@ -64,26 +64,46 @@ and arrow-format into a single JAR. Using the classifier "shade-format-flatbuff
pom.xml will make use of this JAR, you can then exclude/resolve the original dependency to
a version of your choosing.
+### Updating the flatbuffers generated code
+
+To update the checked-in flatbuffer generated code perform the following:
+
+```bash
+cd $ARROW_HOME
+
+# remove the existing files
+rm -rf java/format/src
+
+# regenerate from the .fbs files
+flatc --java -o java/format/src/main/java format/*.fbs
Review comment:
updated the readme to include a step to verify `flatc` matches the declared dependency version
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] kiszk commented on pull request #10058: ARROW-12111: [Java] Generate flatbuffer files using flatc 1.12.0
Posted by GitBox <gi...@apache.org>.
kiszk commented on pull request #10058:
URL: https://github.com/apache/arrow/pull/10058#issuecomment-820627338
@nbauernfeind Let us wait for dropping it before seeing the test result.
I am curious whether the Java code generated on the little-endian platform also works for the big-endian platform.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] nbauernfeind commented on a change in pull request #10058: ARROW-12111: [Java] Generate flatbuffer files using flatc 1.12.0
Posted by GitBox <gi...@apache.org>.
nbauernfeind commented on a change in pull request #10058:
URL: https://github.com/apache/arrow/pull/10058#discussion_r614263386
##########
File path: java/README.md
##########
@@ -64,26 +64,46 @@ and arrow-format into a single JAR. Using the classifier "shade-format-flatbuff
pom.xml will make use of this JAR, you can then exclude/resolve the original dependency to
a version of your choosing.
+### Updating the flatbuffers generated code
+
+To update the checked-in flatbuffer generated code perform the following:
+
+```bash
+cd $ARROW_HOME
+
+# remove the existing files
+rm -rf java/format/src
+
+# regenerate from the .fbs files
+flatc --java -o java/format/src/main/java format/*.fbs
+
+# prepend license header
+find java/format/src -type f | while read file; do
+ (cat header | while read line; do echo "// $line"; done; cat $file) > $file.tmp
Review comment:
The header is already checked into the root of apache/arrow.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] emkornfield commented on a change in pull request #10058: ARROW-12111: [Java] Generate flatbuffer files using flatc 1.12.0
Posted by GitBox <gi...@apache.org>.
emkornfield commented on a change in pull request #10058:
URL: https://github.com/apache/arrow/pull/10058#discussion_r614261910
##########
File path: java/pom.xml
##########
@@ -36,8 +36,8 @@
<dep.netty.version>4.1.48.Final</dep.netty.version>
<dep.jackson.version>2.11.4</dep.jackson.version>
<dep.hadoop.version>2.7.1</dep.hadoop.version>
- <dep.fbs.version>1.9.0</dep.fbs.version>
- <dep.flatc.version>1.9.0</dep.flatc.version>
+ <dep.fbs.version>1.12.0</dep.fbs.version>
+ <dep.flatc.version>1.12.0</dep.flatc.version>
Review comment:
is this still used?
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] emkornfield closed pull request #10058: ARROW-12111: [Java] Generate flatbuffer files using flatc 1.12.0
Posted by GitBox <gi...@apache.org>.
emkornfield closed pull request #10058:
URL: https://github.com/apache/arrow/pull/10058
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] kiszk edited a comment on pull request #10058: ARROW-12111: [Java] Generate flatbuffer files using flatc 1.12.0
Posted by GitBox <gi...@apache.org>.
kiszk edited a comment on pull request #10058:
URL: https://github.com/apache/arrow/pull/10058#issuecomment-820595477
Let me jump in this PR. flatbuffers 1.9 for s390x is not officially prepared. Thus, we privately build and provide it on own artifact. You can see [here](https://github.com/apache/arrow/blob/master/ci/scripts/java_build.sh#L33-L40).
If this approach works well, can we remove this workaround for s390x that uses big-endian, too?
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org