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

Update C++ dependencies, roughly matching ones used in python. #5472

Merged
merged 14 commits into from
May 28, 2024

Conversation

jcferretti
Copy link
Member

The goal is actually to build python dependencies with debug info; this is a step.

The change to base-images happened first here: deephaven/deephaven-base-images#116

@jcferretti jcferretti added this to the 3. May 2024 milestone May 10, 2024
@jcferretti jcferretti requested a review from kosak May 10, 2024 03:54
kosak
kosak previously approved these changes May 10, 2024
@devinrsmith
Copy link
Member

Execution failed for task ':cpp-client:compareProtobuf'.
Sources do not match expected, re-run :cpp-client:updateProtobuf

@devinrsmith
Copy link
Member

I think the version of protobuf will need to be updated in concert with the version grpc supports ie #5299

[Task :proto:proto-backplane-grpc:compileJava FAILED] cannot access GeneratedMessageV3
                Status.newBuilder().setCode(statusCode.getNumber()).setMessage(details).build());
                      ^
  class file for com.google.protobuf.GeneratedMessageV3 not found
./gradlew -q proto:proto-backplane-grpc:dependencyInsight --configuration compileClasspath --dependency proto-google-common-protos 
com.google.api.grpc:proto-google-common-protos:2.22.0
  Variant compile:
    | Attribute Name                 | Provided | Requested    |
    |--------------------------------|----------|--------------|
    | org.gradle.status              | release  |              |
    | org.gradle.category            | library  | library      |
    | org.gradle.libraryelements     | jar      | classes      |
    | org.gradle.usage               | java-api | java-api     |
    | org.gradle.dependency.bundling |          | external     |
    | org.gradle.jvm.environment     |          | standard-jvm |
    | org.gradle.jvm.version         |          | 8            |

com.google.api.grpc:proto-google-common-protos:2.22.0
\--- io.grpc:grpc-protobuf:1.58.0
     +--- compileClasspath (requested io.grpc:grpc-protobuf)
     +--- org.apache.arrow:flight-core:13.0.0 (requested io.grpc:grpc-protobuf:1.56.0)
     |    \--- compileClasspath
     \--- io.grpc:grpc-bom:1.58.0
          \--- compileClasspath

@devinrsmith
Copy link
Member

grpc/grpc-java#11015

@devinrsmith
Copy link
Member

Looks like grpc-java is sticking with 3.x at this time due to known issues. The advice right now is

For those watching at home and want to be prepared, use protobuf-java 3.25 and protoc 25.

protocolbuffers/protobuf#16452 (comment)

chipkent
chipkent previously approved these changes May 21, 2024
chipkent
chipkent previously approved these changes May 24, 2024
# Previously used version: v1.2.11
git clone $GIT_FLAGS -b v1.2.13 --depth 1 "${GITHUB_BASE_URL}/madler/zlib"
# Previously used version: v1.2.13
git clone $GIT_FLAGS -b v1.3 --depth 1 "${GITHUB_BASE_URL}/madler/zlib"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need to specify the patch version, or are we ok with that changing?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a specific version, is just the name they gave, in git terms is just some arbitrary string. Mean as far as I understand how the universe works, that won't change when they create a new version.

Comment on lines +565 to +566
# Previously used version: v3.21.2
git clone $GIT_FLAGS -b v25.3 --depth 1 "${GITHUB_BASE_URL}/protocolbuffers/protobuf.git"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. Were we ultra out of date?
  2. Did they change their versioning convention?
  3. Do we need to specify the patch version?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. No, they changed in weird ways they numbering, and managed to confuse a lot of people in the process. They also introduced some backwards incompatibilities which is why we can't go directly to 26
  2. Yes, see above.
  3. No, like the previous answer, that tag should not change.

Copy link
Contributor

@alexpeters1208 alexpeters1208 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

R stuff looks good

@jcferretti jcferretti requested a review from chipkent May 28, 2024 21:40
@jcferretti jcferretti merged commit dbf2b2d into deephaven:main May 28, 2024
15 checks passed
@jcferretti jcferretti deleted the cfs-cpp-deps-240509 branch May 28, 2024 22:01
@github-actions github-actions bot locked and limited conversation to collaborators May 28, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants