router: use new config_impl_test_lib instead of synthetic build target#6871
router: use new config_impl_test_lib instead of synthetic build target#6871lizan merged 4 commits intoenvoyproxy:masterfrom
Conversation
Signed-off-by: Frank Fort <ffort@google.com>
|
Looks like there was somewhere else relying upon these _lib targets that I missed, let me take a look. |
Signed-off-by: Frank Fort <ffort@google.com>
|
I'm not very thrilled with this approach though not a strong opinion, this is the issue how Google internal The alternative would be either:
@htuch thoughts? |
…rgets Signed-off-by: Frank Fort <ffort@google.com>
Done, see updated PR description. Basically:
|
|
As an aside it may be worth fixing some our uses of |
|
/retest |
|
🔨 rebuilding |
| data = data, | ||
| external_deps = external_deps, | ||
| deps = deps, | ||
| deps = deps + [repository + "//test/test_common:printers_includes"], |
There was a problem hiding this comment.
Why this change? What do we gain by replacing envoy_cc_test_library?
There was a problem hiding this comment.
We cannot continue to use envoy_cc_test_library here since it declares all targets created with it as //visibility:public, which means we cannot restrict the visibility of the _lib_internal_only targets to @envoy//test/coverage:__pkg__.
| tags = test_lib_tags, | ||
| copts = copts, | ||
| # Restrict only to the code coverage tools. | ||
| visibility = ["@envoy//test/coverage:__pkg__"], |
There was a problem hiding this comment.
Can you verify that the fuzzers targets needed at https://github.com/google/oss-fuzz/blob/master/projects/envoy/build.sh#L62 still build? I.e. the _driverless variants of each fuzz target. This isn't caught by CI.
There was a problem hiding this comment.
Done. Verified that bazel build $(bazel query 'filter("_driverless", kind(".* rule", //...))') progresses to the linking stage (despite encountering a library error there) and that bazel build $(bazel query 'filter("_corpus$", kind(".* rule", //...))') finishes successfully.
Signed-off-by: Frank Fort <ffort@google.com>
This PR makes all macros used solely within envoy_build_system.bzl as 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 macro envoy_cc_test_infrastructure_library was 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/... Signed-off-by: Frank Fort <ffort@google.com>
Signed-off-by: Frank Fort ffort@google.com
Description: The build target
//test/common/router:config_impl_test_staticwas relying upon the_libtarget that is created by theenvoy_cc_testin envoy_build_system.bzl. This target should not be relied upon since it is an implementation detail of the Envoy build system rather than a public build target that we want to allow dependencies upon.This PR restricts the visibility of these synthetic targets to only the code coverage tools and renames them to end with
_lib_INTERNAL_ONLYinstead. See #6725 for where we started using these synthetic targets in ourBUILDfiles.This PR also hides the
envoy_cc_test_infrastructure_librarybuild macro by renaming it to_envoy_cc_test_infrastructure_libraryas per https://docs.bazel.build/versions/master/build-ref.html#load. This macro was not being used outside of envoy_build_system.bzl.Note this does not break coverage builds since coverage pulls in the synthetic targets via the
coverage_test_libtag, see generated_coverage_BUILD.txt for the generated coverageBUILDfile.Risk Level: Low
Testing: Ran
bazel test //test/common/.... and rantest/run_envoy_bazel_coverage.sh.Docs Changes: N/A
Release Notes: N/A