-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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
Updates to mobile/third_party/rbe_configs/cc/ #36269
Conversation
This updates files containing paths to older versions of the docker container being used in CI. This matters when clang modules are being used and some dependency (looking at absl) turns on layering checks. Not updating these files prevents abseil from including many standard headers. These files should be regenerated whenever our base docker container or toolchain is updated. These files were regenerated following the instructions in https://github.com/bazelbuild/bazel-toolchains Specifically: ``` $ ./rbe_configs_gen \ --bazel_version="6.5.0" \ --toolchain_container="docker.io/envoyproxy/envoy-build-ubuntu:f94a38f62220a2b017878b790b6ea98a0f6c5f9c" \ --exec_os="linux" \ --target_os="linux" \ --output_src_root="/path/to/envoy-mobile" \ --output_config_path="third_party/rbe_configs" \ --cpp_env_json=clang.json \ --generate_java_configs=False $ cat clang.json {"CC": "/opt/llvm/bin/clang", "CXX": "/opt/llvm/bin/clang++"} ``` Other files were also regenerated, but this commit only contains changes to files with paths. Signed-off-by: Alejandro R Sedeño <asedeno@google.com>
the code for updating the build image is here envoy/.github/workflows/envoy-dependency.yml Lines 126 to 225 in d739460
perhaps we can add something there to do toolchain regeneration at the same time |
I can look into that, with two caveats.
I'm not sure what the automation should look like yet, given the resource exhaustion experienced in #36172. Also, that PR had a series of patches undoing some of the generated changes, some of which were just tidying up, and some of which may not have been needed. |
for sure, out of scope for current change - was more following up on your comment happy to assist if you want to delve into our ci system this is one of the easier workflows to change/test/etc so probably not crazy hard to do |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm, thanks @asedeno
This updates files containing paths to older versions of the docker container being used in CI. This matters when clang modules are being used and some dependency (looking at absl) turns on layering checks. Not updating these files prevents abseil from including many standard headers.
These files should be regenerated whenever our base docker container or toolchain is updated.
These files were regenerated following the instructions in https://github.com/bazelbuild/bazel-toolchains
Specifically:
Other files were also regenerated, but this commit only contains changes to files with paths.
Risk Level: low
Testing: CI, since this specifically affects the CI.