> I think checking in the java files is fine and probably better then relying > on a third party package. We should make sure there are instructions on > how to regenerate them along with the PR
Micah, I just opened a pull-request to satisfy ARROW-12111. This is my first contribution to an apache project; please let me know if there is anything else that I need to do to get this past the finish line. https://github.com/apache/arrow/pull/10058 Thanks, Nate On Wed, Apr 14, 2021 at 11:45 PM Nate Bauernfeind < natebauernfe...@deephaven.io> wrote: > Hey Bob, > > Someone did publish a 1.12 version of the flatc maven plugin. I double > checked that the plugin and binaries look correct and legit.. but you know, > it's always shady to download some random executable from the internet and > run it. However, I have been using it to generate the arrow flatbuffer > files because I _really_ wanted some features that are in flatc 1.12's > runtime jar (there are performance improvements for array types in > particular). > > You can see them here: > https://search.maven.org/search?q=com.github.shinanca > The repository fork is here: https://github.com/shinanca/flatc > > On the bright side that developer appears to have published an x86_64 > windows binary which might satisfy one of your earlier complaints in the > thread. > > On the other hand, if everyone is comfortable checking in the flatc > generated files (obviously with the additional documentation on how to > re-generate should the fbs files change), it's a relatively small change to > replace the existing apache/arrow/java/format source. Based on the previous > discussion on this thread, it seems that the arrow dev team _is_ > comfortable with the check-in-the-generated-files approach. > > Although 4.0 is near the release phase, there are still a few blocking > issues that people are trying to fix (according to the arrow-sync call > earlier today). I don't mind jumping in and doing this; it appears that > there might be enough time for such a small change to make it into the > release if the work is performed and merged ASAP. > > I guess, I'm either looking for the "pull request is on the way" or the > "thumbs up - we definitely want this; I'll get the code review for you when > it's ready" style reply =D. > > On Wed, Apr 14, 2021 at 10:43 PM Bob Tinsman <bobti...@gmail.com> wrote: > >> I apologize for leaving this hanging, but it looks like 4.0 is leaving >> the station :( >> Yes, it makes sense to bump it to 1.12, but you can't do that in >> isolation, because the flatc binary which is fetched as a Maven dependency >> is only available for 1.9. I will get back onto this and finish it this >> week. >> >> FWIW, I was looking around and catalogued the various ways of generating >> flatbuffers for all the languages--you can look at it in my branch: >> https://github.com/bobtins/arrow/tree/check-in-gen-code/java/dev >> Let me know if any info is wrong or missing. >> The methods of generation are all over the map, and some have no script >> or build file, just doc. Would there be any value in making this more >> uniform? >> >> On 2021/04/14 16:36:47, Nate Bauernfeind <natebauernfe...@deephaven.io> >> wrote: >> > It would also be nice to upgrade that java flatbuffer version from 1.9 >> to >> > 1.12. Is anyone planning on doing this work (as listed in ARROW-12111)? >> > >> > If I did this work today, might it be possible to get it included in the >> > 4.0.0 release? >> > >> > On Fri, Mar 26, 2021 at 3:25 PM bobtins <bobti...@gmail.com> wrote: >> > >> > > OK, originally this was part of >> > > https://issues.apache.org/jira/browse/ARROW-12006 and I was going to >> just >> > > add some doc on flatc, but I will make this a new bug because it's a >> little >> > > bigger: https://issues.apache.org/jira/browse/ARROW-12111 >> > > >> > > On 2021/03/23 23:40:50, Micah Kornfield <emkornfi...@gmail.com> >> wrote: >> > > > > >> > > > > I have a concern, though. Four other languages (Java would be >> five) >> > > check >> > > > > in the generated flatbuffers code, and it appears (based on a >> quick >> > > scan of >> > > > > Git logs) that this is done manually. Is there a danger that the >> binary >> > > > > format could change, but some language might get forgotten, and >> thus be >> > > > > working with the old format? >> > > > >> > > > The format changes relatively slowly and any changes at this point >> should >> > > > be backwards compatible. >> > > > >> > > > >> > > > >> > > > > Or is there enough interop testing that the problem would get >> caught >> > > right >> > > > > away? >> > > > >> > > > In most cases I would expect integration tests to catch these types >> of >> > > > error. >> > > > >> > > > On Tue, Mar 23, 2021 at 4:26 PM bobtins <bobti...@gmail.com> wrote: >> > > > >> > > > > I'm happy to check in the generated Java source. I would also >> update >> > > the >> > > > > Java build info to reflect this change and document how to >> regenerate >> > > the >> > > > > source as needed. >> > > > > >> > > > > I have a concern, though. Four other languages (Java would be >> five) >> > > check >> > > > > in the generated flatbuffers code, and it appears (based on a >> quick >> > > scan of >> > > > > Git logs) that this is done manually. Is there a danger that the >> binary >> > > > > format could change, but some language might get forgotten, and >> thus be >> > > > > working with the old format? Or is there enough interop testing >> that >> > > the >> > > > > problem would get caught right away? >> > > > > >> > > > > I'm new to the project and don't know how big of an issue this is >> in >> > > > > practice. Thanks for any enlightenment. >> > > > > >> > > > > On 2021/03/23 07:39:16, Micah Kornfield <emkornfi...@gmail.com> >> wrote: >> > > > > > I think checking in the java files is fine and probably better >> then >> > > > > relying >> > > > > > on a third party package. We should make sure there are >> > > instructions on >> > > > > > how to regenerate them along with the PR >> > > > > > >> > > > > > On Monday, March 22, 2021, Antoine Pitrou <anto...@python.org> >> > > wrote: >> > > > > > >> > > > > > > >> > > > > > > Le 22/03/2021 à 20:17, bobtins a écrit : >> > > > > > > >> > > > > > >> TL;DR: The Java implementation doesn't have generated >> flatbuffers >> > > code >> > > > > > >> under source control, and the code generation depends on an >> > > > > > >> unofficially-maintained Maven artifact. Other language >> > > > > implementations do >> > > > > > >> check in the generated code; would it make sense for this to >> be >> > > done >> > > > > for >> > > > > > >> Java as well? >> > > > > > >> >> > > > > > >> I'm currently focusing on Java development; I started >> building on >> > > > > Windows >> > > > > > >> and got a failure under java/format, because I couldn't >> download >> > > the >> > > > > > >> flatbuffers compiler (flatc) to generate Java source. >> > > > > > >> The artifact for the flatc binary is provided "unofficially" >> (not >> > > by >> > > > > the >> > > > > > >> flatbuffers project), and there was no Windows version, so I >> had >> > > to >> > > > > jump >> > > > > > >> through hoops to build it and proceed. >> > > > > > >> >> > > > > > > >> > > > > > > While this does not answer the more general question of >> checking >> > > in the >> > > > > > > generated Flatbuffers code (which sounds like a good idea, >> but I'm >> > > not >> > > > > a >> > > > > > > Java developer), note that you could workaround this by >> installing >> > > the >> > > > > > > Conda-provided flatbuffers package: >> > > > > > > >> > > > > > > $ conda install flatbuffers >> > > > > > > >> > > > > > > which should get you the `flatc` compiler, even on Windows. >> > > > > > > (see https://docs.conda.io/projects/conda/en/latest/ for >> > > installing >> > > > > conda) >> > > > > > > >> > > > > > > You may also try other package managers such as Chocolatey: >> > > > > > > >> > > > > > > https://chocolatey.org/packages/flatc >> > > > > > > >> > > > > > > Regards >> > > > > > > >> > > > > > > Antoine. >> > > > > > > >> > > > > > >> > > > > >> > > > >> > > >> > >> > >> > -- >> > >> > > > -- > > --
