Skip to content
This repository has been archived by the owner on May 16, 2019. It is now read-only.

Depends on protobuf-c 1.3 #4

Open
ringerc opened this issue Jun 11, 2018 · 7 comments
Open

Depends on protobuf-c 1.3 #4

ringerc opened this issue Jun 11, 2018 · 7 comments

Comments

@ringerc
Copy link

ringerc commented Jun 11, 2018

The build seems to depend on protobuf-c 1.3 or newer, since it refers to protobuf-gen-c, which AFAICS was added in 1.3. (See https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=221572).

I've tweaked it to build on 1.2 too. Unclear how important that really is, since RHEL seems to carry 1.0.2 and EL6 EPEL has 0.15 (!). But it's cheap and easy.

ringerc added a commit to 2ndQuadrant/jaeger-client-c that referenced this issue Jun 11, 2018
There were a number of case sensitivity issues in the CMake files relating
to differing case between the Hunter files and the ones in cmake/ (isaachier#1)

Also fixes finding of the protocol buffers support when protobuf 1.2 is
present (isaachier#4)
@isaachier
Copy link
Owner

protobuf-gen-c vs. protoc-c is more of a fix for Hunter than for anything else. I can change it if you'd like. As long as it supports proto3 I'm happy. For the record, the schema for messages is about to change really soon (see jaegertracing/jaeger#856).

@ringerc
Copy link
Author

ringerc commented Jun 11, 2018

I have some more changes pending in my tree, so don't work on it yet. I'm integrating Travis support for HUNTER_ENABLED=0, fixing protobuf finding, etc.

It seems like really the Hunter CMake files should expose a protobuf-c::protoc-c target and fail to do so, but it's easily worked around.

@ringerc
Copy link
Author

ringerc commented Jun 11, 2018

(You can see my WIP in my https://github.com/2ndquadrant/jaeger-client-c master branch, but beware it's full of CI-fixit spam commits and will be brutally squashed and rebased soon)

@ringerc
Copy link
Author

ringerc commented Jun 11, 2018

... and I won't waste time seeing if I can make it build with the packaged libprotobuf-c0-dev (0.15) in Ubuntu Trusty.

protobuf will have to be fetched from a PPI or backports or the like. It's amazingly poorly packaged I'm finding - not in EPEL for example. Oh well, at least it's simple to install and reasonably backward compatible.

I've already confirmed it's fine on 1.2 though, just needs a fallback implementatoin to use protoc-c that I've added.

@ringerc
Copy link
Author

ringerc commented Jun 12, 2018

1.2 does not support proto3. Only protobuf-c 1.3.0 does. jaeger-client-c currently works fine with protobuf 1.2, but I'm assuming you plan to move to proto3?

So that makes this a dependency that's impossible to satisfy on anything but bleeding edge systems except by bundling it.

Not the end of the world, since it can be a private copy. Ugly, but workable. I now see why you have the code in scripts/install-deps.sh to download and build a local copy of protobuf-c. I've switched it off when hunter is enabled, but am using it for the new non-hunter CI build.

@ringerc
Copy link
Author

ringerc commented Jun 12, 2018

Addressed in #3

@isaachier
Copy link
Owner

I've commented in #3 about this as well.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants