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

cel: fix gcc contrib build #31814

Merged
merged 1 commit into from
Jan 12, 2024
Merged

cel: fix gcc contrib build #31814

merged 1 commit into from
Jan 12, 2024

Conversation

kyessenov
Copy link
Contributor

Commit Message: Fixes #31760

Signed-off-by: Kuat Yessenov <[email protected]>
@repokitteh-read-only repokitteh-read-only bot added the deps Approval required for changes to Envoy's external dependencies label Jan 12, 2024
Copy link

CC @envoyproxy/dependency-shepherds: Your approval is needed for changes made to (bazel/.*repos.*\.bzl)|(bazel/dependency_imports\.bzl)|(api/bazel/.*\.bzl)|(.*/requirements\.txt)|(.*\.patch).
envoyproxy/dependency-shepherds assignee is @RyanTheOptimist

🐱

Caused by: #31814 was opened by kyessenov.

see: more, trace.

@kyessenov kyessenov assigned phlax and unassigned RyanTheOptimist Jan 12, 2024
Copy link
Member

@phlax phlax left a comment

Choose a reason for hiding this comment

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

lgtm, thanks @kyessenov

lets try and move as much of this patch upstream as we can after release

@repokitteh-read-only repokitteh-read-only bot removed the deps Approval required for changes to Envoy's external dependencies label Jan 12, 2024
@phlax phlax enabled auto-merge (squash) January 12, 2024 21:22
@kyessenov
Copy link
Contributor Author

Will file a bug internally. Google doesn't build with GCC - and this is a GCC weirdness.

@phlax phlax merged commit 21c4a2c into envoyproxy:main Jan 12, 2024
54 checks passed
ggreenway added a commit to ggreenway/envoy that referenced this pull request Feb 1, 2024
This reverts commit 21c4a2c.

Signed-off-by: Greg Greenway <[email protected]>
krinkinmu added a commit to krinkinmu/cel-cpp that referenced this pull request Nov 4, 2024
gcc complains about FieldBackedMapImpl::ListKeys when overloaded-virtual
check is enabled. When using -Werror, warnings like this are converted
to errors breaking builds.

That's what happened to gcc build of Envoy a while back. The issue in
Envoy was fixed by introducing a patch for cel-cpp that was applied to
the source during Envoy build (see
envoyproxy/envoy#31814).

Unfortunately during cel-cpp version update in Envoy instead of
updating the patch, dropping the parts that are not needed anymore, we
dropped the whole cep-cpp patch all together in
envoyproxy/envoy#36661. And now gcc can't build
Envoy again.

This change makes the change to silence the gcc warning in the cel-cpp
codebase, which hopefully will be a bit more durable than maintaing a
patch on the Envoy side.

Signed-off-by: Mikhail Krinkin <[email protected]>
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.

Recent commit about CEL causes GCC contrib/golang build failure
3 participants