build: Support tags[] arg for more specific build control #8233
Conversation
Where the underlying bazel primitives support tags[], envoy_() should support them. Signed-off-by: Yechiel Kalmenson <[email protected]> Signed-off-by: William Rowe <[email protected]>
|
@lizan do you mind taking a look at this? |
| @@ -90,6 +90,7 @@ def api_proto_library( | |||
| visibility = ["//visibility:private"], | |||
| srcs = [], | |||
| deps = [], | |||
| tags = [], | |||
There was a problem hiding this comment.
why do you need tags even for api_proto_library? If you're not building the library/binary depends on them they're not built anyway.
There was a problem hiding this comment.
You raise a good point, we introduced tags[] wherever we observed that the underlying bazel API's supported them, but this case may not be necessary for our purpose of excluding from Windows. It might be best to support them where they are valid, though?
There was a problem hiding this comment.
We do omit some parameter as "opinionated" wrapper of underlying bazel API and add tags/options for Envoy specific. I'm not feeling strongly on this so either way is fine.
| @@ -90,6 +90,7 @@ def api_proto_library( | |||
| visibility = ["//visibility:private"], | |||
| srcs = [], | |||
| deps = [], | |||
| tags = [], | |||
There was a problem hiding this comment.
We do omit some parameter as "opinionated" wrapper of underlying bazel API and add tags/options for Envoy specific. I'm not feeling strongly on this so either way is fine.
bazel/envoy_test.bzl
Outdated
| repository + "//test:main", | ||
| ], | ||
| deps = select({ | ||
| "@envoy//bazel:windows_x86_64": [repository + "//test:dummy_main"], |
There was a problem hiding this comment.
This is not needed at the end, and shouldn't included in this PR?
There was a problem hiding this comment.
We saw this exclusion for Apple so we added it for Windows.
We don't have a strong opinion either way and we can remove this change.
There was a problem hiding this comment.
@achasveachas the exclusion you mentioned is for fuzz I think? No platform should be excluded from normal cc_test.
Signed-off-by: William Rowe <[email protected]> Signed-off-by: Yechiel Kalmenson <[email protected]> Signed-off-by: William Rowe <[email protected]>
lizan
added
the
api-review-required
…#8233) Support tags[] arg for more specific build control. Where the underlying bazel primitives support tags[], envoy_() should support them. Risk Level: Low Testing: Local on Windows and Linux CI Signed-off-by: Yechiel Kalmenson <[email protected]>
…#8233) Support tags[] arg for more specific build control. Where the underlying bazel primitives support tags[], envoy_() should support them. Risk Level: Low Testing: Local on Windows and Linux CI Signed-off-by: Yechiel Kalmenson <[email protected]>
…#8233) Support tags[] arg for more specific build control. Where the underlying bazel primitives support tags[], envoy_() should support them. Risk Level: Low Testing: Local on Windows and Linux CI Signed-off-by: Yechiel Kalmenson <[email protected]>
Description:
Support tags[] arg for more specific build control.
Where the underlying bazel primitives support tags[], envoy_() should support them.
Risk Level: Low
Testing: Local on Windows and Linux CI
Docs Changes: None
Release Notes: None