Skip to content

Protobuf 22.5 cpp #11961

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

Open
wants to merge 62 commits into
base: master
Choose a base branch
from
Open

Conversation

kannanjgithub
Copy link
Contributor

No description provided.

ejona86 and others added 30 commits March 12, 2025 13:23
…ose since it doesn't work on multiarch build arch64.
…ule instead of as a package, and this version is not available to install. Hence building cmake from source. Building the latest source of cmake needs openssl location to be specified (or have openssl turned off) so using the older version 3.13.4
…uf progress bar is causing 'access denied error' on windows console: crc-org/crc#1184
…uf progress bar is causing 'access denied error' on windows console: crc-org/crc#1184
@kannanjgithub
Copy link
Contributor Author

Windows run: fusion/0b0cb5e6-81e6-47ec-982b-ea4ba8d44f23

@kannanjgithub kannanjgithub marked this pull request as ready for review May 14, 2025 10:08
@kannanjgithub kannanjgithub requested a review from ejona86 May 14, 2025 10:10
RUN curl -Ls https://github.com/Kitware/CMake/releases/download/v3.26.3/cmake-3.26.3-linux-x86_64.tar.gz | \
tar xz -C /var/local

# Install Maven
Copy link
Member

Choose a reason for hiding this comment

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

Why add maven? I don't think we use Maven in this container. Ditto for ubuntu2004.base

Copy link
Member

Choose a reason for hiding this comment

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

Maven was pre-existing (and needed) in Dockerfile. You added it in Dockerfile.multiarch.base and Dockerfile.ubuntu2004.base. Only remove it from the places you added it. The UnsupportedClassVersionError is completely unrelated. The failure was /grpc-java/buildscripts/kokoro/unix.sh: line 74: mvn: command not found

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done. I wrongly thought the UnsupportedClassVersionError was the cause of the build failure and associated it with the maven install removal commit even though it didn't make logical sense.

set CMAKE_NAME=cmake-3.3.2-win32-x86
echo on
@rem set PROTOBUF_VER=21.7
choco install -y gradle git curl pkgconfiglite
Copy link
Member

Choose a reason for hiding this comment

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

We don't use the system-installed gradle, so I don't know why that is here. Git should already be installed; is this some different version of git? What uses curl?

I had thought we were going to be adding these installations to the base image itself. OpenJDK in particular I'm not wild about installed every invocation, as it is relatively large.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed git and gradle. git command is not available in PATH on the VM I worked with even though the build is going through on CI.

String libsList = rootProject.property('vcProtobufLibs') as String
libsList.split(',').each() { lib -> linker.args.add(lib) }
}
if (rootProject.hasProperty('vcProtobufLibPaths')) {
Copy link
Member

Choose a reason for hiding this comment

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

Why do we need this new property? It is the same as vcProtobufLibs; we can just use -PvcProtobufLibs=%VC_PROTOBUF_LIBS%,%VC_PROTOBUF_LIB_PATHS% right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

@kannanjgithub kannanjgithub requested a review from ejona86 May 15, 2025 14:58
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.

2 participants