Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rebase repository #18

Closed
wants to merge 48 commits into from

Conversation

jpkrohling
Copy link

See #17 for context: if this looks good, I'll merge master and push it directly, as I'm sure squashing this PR will only cause more problems. For this case, we certainly want a proper rebase/merge.

rvolosatovs and others added 30 commits February 24, 2020 13:41
Use cmake for significantly faster builds. Introduce glibc for
execution of protoc-gen-dart binary with glibc dependency. Use shared
library build of grpc and optimize upx to skip files smaller than 40k.
The embedded Dart runtime in executables generated by dart2native will
not work if they are modified after generation - for example by
stripping symbols or compressing.
…fig do not seem to be looking in /usr/lib64 for the other shared libraries.
rvolosatovs and others added 18 commits June 17, 2020 15:36
Resolving shared libraries not found errors
Build fails with
```
Step 14/70 : RUN mkdir -p /grpc-java &&     curl -sSL https://api.github.com/repos/grpc/grpc-java/tarball/v${GRPC_JAVA_VERSION} | tar xz --strip 1 -C /grpc-java &&     cd /grpc-java &&     g++         -I. -I/usr/include         compiler/src/java_plugin/cpp/*.cpp         -L/usr/lib64         -lprotoc -lprotobuf -lpthread --std=c++0x -s         -o protoc-gen-grpc-java &&     install -Ds protoc-gen-grpc-java /out/usr/bin/protoc-gen-grpc-java
 ---> Running in e32c5284be9a
compiler/src/java_plugin/cpp/java_plugin.cpp:46:12: error: 'uint64_t JavaGrpcGenerator::GetSupportedFeatures() const' marked 'override', but does not override
   46 |   uint64_t GetSupportedFeatures() const override {
      |            ^~~~~~~~~~~~~~~~~~~~
compiler/src/java_plugin/cpp/java_plugin.cpp: In member function 'uint64_t JavaGrpcGenerator::GetSupportedFeatures() const':
compiler/src/java_plugin/cpp/java_plugin.cpp:47:12: error: 'FEATURE_PROTO3_OPTIONAL' was not declared in this scope
   47 |     return FEATURE_PROTO3_OPTIONAL;
      |            ^~~~~~~~~~~~~~~~~~~~~~~
The command '/bin/sh -c mkdir -p /grpc-java &&     curl -sSL https://api.github.com/repos/grpc/grpc-java/tarball/v${GRPC_JAVA_VERSION} | tar xz --strip 1 -C /grpc-java &&     cd /grpc-java &&     g++         -I. -I/usr/include         compiler/src/java_plugin/cpp/*.cpp         -L/usr/lib64         -lprotoc -lprotobuf -lpthread --std=c++0x -s         -o protoc-gen-grpc-java &&     install -Ds protoc-gen-grpc-java /out/usr/bin/protoc-gen-grpc-java' returned a non-zero code: 1
```
…ertracing#3)

* Sync dependencies with main repo and only load required modules

Signed-off-by: Annanay <[email protected]>

* Address comments, test image to verify files generated match with Jaeger repo

Signed-off-by: Annanay <[email protected]>
* Fix github actions workflow

Signed-off-by: Annanay <[email protected]>

* Change master build tag to latest

Signed-off-by: Annanay <[email protected]>
…ertracing#4)

* Update README with language specific code generation examples

Signed-off-by: Annanay <[email protected]>

* Add cli options list

Signed-off-by: Annanay <[email protected]>

* Update README, remove go file

Signed-off-by: Annanay <[email protected]>

* Explain CLI flags

Signed-off-by: Annanay <[email protected]>
* Test PR to verify GitHub Actions works

Signed-off-by: Annanay <[email protected]>

* Test change

Signed-off-by: Annanay <[email protected]>
* Push a single tag from releases

Signed-off-by: Annanay <[email protected]>

* Nit, tab to space

Signed-off-by: Annanay <[email protected]>
* Document how to generate dependencies

Signed-off-by: Pavol Loffay <[email protected]>

* Change find command

Signed-off-by: Pavol Loffay <[email protected]>
* Update versions

* GRPC uses cmake now
* linux-headers is necessary as abseil is used as dependency
* Use cmake instead of make (deprecated)

Signed-off-by: Kraemer, Benjamin <[email protected]>

* Use separate protoc for C#

Signed-off-by: Kraemer, Benjamin <[email protected]>

* Improved argument check

Signed-off-by: Kraemer, Benjamin <[email protected]>

* Add review comments

* Disable unused plugins
* Use generic version for copying protoc

Signed-off-by: Kraemer, Benjamin <[email protected]>
Signed-off-by: Juraci Paixão Kröhling <[email protected]>
@jpkrohling jpkrohling self-assigned this Sep 2, 2020
@jpkrohling
Copy link
Author

Image with this PR: quay.io/jpkroehling/jaeger-docker-protobuf:latest

@yurishkuro
Copy link
Member

lgtm, but could you please describe the testing plan / steps.

@annanay25
Copy link
Member

Thanks for this PR @jpkrohling! Could you please fix the conflicts? That will make it easier to review.

@jpkrohling
Copy link
Author

Thanks for this PR @jpkrohling! Could you please fix the conflicts? That will make it easier to review.

I don't think it's possible to fix it in a PR, but I'll try. The best would be to checkout this branch locally and run a git diff master.

@Falco20019
Copy link
Contributor

Falco20019 commented Sep 8, 2020

I don't think it's possible to fix it in a PR, but I'll try.

You should be able to fix it by merging master into this branch and solve the conflicts then.

@pavolloffay
Copy link
Member

just FYI if somebody is interested I have built an image with this PR pavolloffay/jaeger-docker-protobuf:PR18

@jpkrohling
Copy link
Author

I'm closing this, as we'll probably not work on rebasing it this way.

@jpkrohling jpkrohling closed this Oct 21, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants