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

Patch cel-cpp to not break build #31894

Closed
wants to merge 1 commit into from
Closed

Conversation

ravenblackx
Copy link
Contributor

Commit Message: Patch cel-cpp to not break build
Additional Description: This is google/cel-cpp#558
Fixes #31856
Risk Level: Low, does nothing except when build would be broken anyway.
Testing: Tried the broken build as

$ bazel build --define boringssl=fips //source/exe:envoy-static

before and after this change. Before it fails as in #31856, after it builds.
Docs Changes: n/a
Release Notes: n/a
Platform Specific Features: n/a

@repokitteh-read-only repokitteh-read-only bot added the deps Approval required for changes to Envoy's external dependencies label Jan 18, 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: #31894 was opened by ravenblackx.

see: more, trace.

@ravenblackx ravenblackx assigned phlax and unassigned RyanTheOptimist Jan 18, 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.

minor patch nits

@@ -0,0 +1,44 @@
From 09a072b4bb5a75e1df15beba29a9f13b1948ff8b Mon Sep 17 00:00:00 2001
Copy link
Member

Choose a reason for hiding this comment

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

i think we dont want the commit header - just the diff

patches = ["@envoy//bazel:cel-cpp.patch"],
patches = [
"@envoy//bazel:cel-cpp.patch",
"@envoy//bazel:cel-cpp-memory.patch",
Copy link
Member

Choose a reason for hiding this comment

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

lets just use existing patch - no?

@ravenblackx
Copy link
Contributor Author

Not needed after #31872

@ravenblackx ravenblackx deleted the cel branch January 18, 2024 17:28
archlinux-github pushed a commit to archlinux/aur that referenced this pull request Mar 6, 2024
- add patch from envoyproxy/envoy#31894 in lieu of disabling `-fno-sized-deallocation`
- update to upstream release v1.29.1
- sync patches
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
deps Approval required for changes to Envoy's external dependencies
Projects
None yet
Development

Successfully merging this pull request may close these issues.

DevContainer: build failing on main
3 participants