envoy_build_system: use private bazel macros where possible#6926
envoy_build_system: use private bazel macros where possible#6926htuch merged 7 commits intoenvoyproxy:masterfrom
Conversation
Signed-off-by: Frank Fort <ffort@google.com>
htuch
left a comment
There was a problem hiding this comment.
LGTM modulo a minor comment.
/wait
bazel/envoy_build_system.bzl
Outdated
| }) | ||
|
|
||
| def envoy_select_force_libcpp(if_libcpp, default = None): | ||
| def _envoy_select_force_libcpp(if_libcpp, default = None): |
There was a problem hiding this comment.
I think most of the selects here are intended for export. If not, maybe we can regroup them elsewhere and give them a name with "internal" in it to avoid confusion with the exported select macros.
There was a problem hiding this comment.
The envoy_select_foo macros that I have marked as private with an _ are all used only within envoy_build_system.bzl, at least within the Envoy code base (and google3 as well). Are you saying that we intentionally want to leave these as part of the public API of envoy_build_system.bzl so that external developers building upon Envoy may rely on them?
There was a problem hiding this comment.
No, I'm suggesting you move them to elsewhere in this file and rename them, since they are not really in the same category as the ones we are exporting, and it's confusing to colocate these in the same section of the file.
There was a problem hiding this comment.
Ah gotcha. I moved all the envoy_select_* macros to the bottom and append _internal to them. I'm planning a larger refactor of this file so I didn't move/rename all the private macros.
Signed-off-by: Frank Fort <ffort@google.com>
Signed-off-by: Frank Fort <ffort@google.com>
Signed-off-by: Frank Fort <ffort@google.com>
Signed-off-by: Frank Fort <ffort@google.com>
Signed-off-by: Frank Fort ffort@google.com
Description: This PR makes all macros used solely within
envoy_build_system.bzlas private by prefixing the macro name with an_as per https://docs.bazel.build/versions/master/build-ref.html#load. For more background, see #6871 where Envoy's build macroenvoy_cc_test_infrastructure_librarywas made private.We should strive to restrict visibility of our build macros where possible so that we don't create dependencies on the internal details of our build system where we do not intend to.
Risk Level: Low
Testing: Ran
bazel test //test/common/... //test/exe/...Docs Changes: N/A
Release Notes: N/A