Skip to content

Conversation

@westonpace
Copy link
Member

Ubuntu 18.04 does not have a new enough version of protobuf to use the engine module.

@github-actions
Copy link

@github-actions
Copy link

⚠️ Ticket has not been started in JIRA, please click 'Start Progress'.

@westonpace westonpace force-pushed the bugfix/ARROW-15700--engine-compilation-on-ubuntu-xbow branch from 08304d9 to 87bdd53 Compare February 17, 2022 03:19
@westonpace
Copy link
Member Author

@github-actions crossbow submit test-ubuntu-*-cpp

@github-actions
Copy link

Revision: 87bdd53

Submitted crossbow builds: ursacomputing/crossbow @ actions-1644

Task Status
test-ubuntu-18.04-cpp Github Actions
test-ubuntu-20.04-cpp Github Actions
test-ubuntu-21.04-cpp Github Actions

@pitrou
Copy link
Member

pitrou commented Feb 17, 2022

Would it be reasonable to vendor the generated files instead? @lidavidm

@jvanstraten
Copy link
Contributor

jvanstraten commented Feb 17, 2022

I've already modified the Susbtrait consumer sources to work all the way down to 3.0.0 with minimal effort. It's merely a bit annoying. Alternative PR incoming. -> #12448

@pitrou Unfortunately, no. The files protoc generates demand exactly the same version of libprotobuf or they will refuse to compile, and in general only one version of libprotobuf can be linked in an application/library (even with private static linking tricks) because the library has global state and no ability to namespace anything.

@pitrou
Copy link
Member

pitrou commented Feb 17, 2022

Since Substrait is AFAIK still just a draft, this would be a good reason to try and switch it to Flatbuffers, IMHO.

@jvanstraten
Copy link
Contributor

jvanstraten commented Feb 17, 2022

That'd basically be starting over from scratch, when DuckDB already has a working extension for Substrait merged in to their main branch, there's lots of work already in progress for Ibis, there's obviously the Arrow consumer that would have to be completely rewritten, etc. I wouldn't call it "just a draft" anymore.

What we could consider doing if this continues to be a problem is switch to an alternative protobuf library, like upb, but that'd probably also be a rewrite on our end. I suppose we could even fork protobuf to just add namespacing support (if no one else has done so already). I can't imagine that being very difficult, but it's obviously maintenance work (assuming Google doesn't want to merge it in). But maybe that's a bit naive of me.

ETA: oh, it looks like upb isn't intended to be used within C(++) directly. So maybe not the best choice.

@pitrou
Copy link
Member

pitrou commented Feb 17, 2022

Yes, I don't think we want to fork protobuf :-) Well, it's a pity if Substrait considers its choice done, even though there are few convincing reasons for protobuf over flatbuffers (I think the main one is that protobuf is a bit more ubiquitous, but there are also flatbuffers implementations for many languages, and Arrow didn't have a problem with it).

@jvanstraten
Copy link
Contributor

PR for support down to libprotobuf 3.0.0 at #12448

@lidavidm
Copy link
Member

We could look into alternative Protobuf implementations for this use case, since we don't need the gRPC code generator. Presumably something like nanopb could be fully vendored and would not conflict with downstream usage.

@lidavidm
Copy link
Member

But yes, as Jeroen points out, the Google Protobuf implementation is really built around a monorepo assumption where you are in full control of all code being built and so compatibility, vendoring, etc. are not really concerns…

@westonpace
Copy link
Member Author

I'm abandoning this in favor of #12448

@westonpace westonpace closed this Feb 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants