Skip to content

bazel: ignore tool_deps for dependency validation#17598

Closed
keith wants to merge 1 commit intoenvoyproxy:mainfrom
keith:ks/bazel-ignore-tool_deps-for-dependency-validation
Closed

bazel: ignore tool_deps for dependency validation#17598
keith wants to merge 1 commit intoenvoyproxy:mainfrom
keith:ks/bazel-ignore-tool_deps-for-dependency-validation

Conversation

@keith
Copy link
Member

@keith keith commented Aug 5, 2021

When validating the different deps types and their use_categories, host
/ tool deps don't need to be included since those aren't part of the
built product. We hit this with rules_foreign_cc changes.

Signed-off-by: Keith Smiley keithbsmiley@gmail.com

When validating the different deps types and their use_categories, host
/ tool deps don't need to be included since those aren't part of the
built product.

Signed-off-by: Keith Smiley <keithbsmiley@gmail.com>
@lizan
Copy link
Member

lizan commented Aug 5, 2021

LGTM but defer to @envoyproxy/dependency-shepherds

@htuch
Copy link
Member

htuch commented Aug 5, 2021

LGTM. I know @moderation has some stronger opinions here though.

@moderation
Copy link
Contributor

@keith Not keen on carving some categories of dependencies out of the checks. Feels like a slippery slope. Could a sophisticated adversary somehow introduce problems in the excluded dependencies that then end up impacting the built product?

Here is the Bazel documentation - https://docs.bazel.build/versions/4.0.0/user-manual.html#query

Is this the error you are trying to work around? #17445 (comment)

Is there a way to fix upstream in rules_foreign_cc? Or configure rules_foreign_cc differently? Or change our metadata?

@moderation
Copy link
Contributor

This also works:

diff --git a/bazel/repository_locations.bzl b/bazel/repository_locations.bzl
index 59ad429af..72cf854d4 100644
--- a/bazel/repository_locations.bzl
+++ b/bazel/repository_locations.bzl
@@ -656,7 +656,7 @@ REPOSITORY_LOCATIONS_SPEC = dict(
         strip_prefix = "rules_foreign_cc-{version}",
         urls = ["https://github.com/bazelbuild/rules_foreign_cc/archive/{version}.tar.gz"],
         release_date = "2021-07-22",
-        use_category = ["build"],
+        use_category = ["build", "dataplane_core", "controlplane"],
     ),
     rules_python = dict(
         project_name = "Python rules for Bazel",

@ggreenway
Copy link
Member

/wait

@moderation
Copy link
Contributor

@wrowe wrowe self-assigned this Aug 16, 2021
@keith keith marked this pull request as draft August 25, 2021 16:36
@keith
Copy link
Member Author

keith commented Sep 21, 2021

Yea I was keeping this open as a placeholder, it might be worth investigating why that connection exists in the non-host parts of the build, but unless it causes more issue I'm not going to look at it for now

@keith keith closed this Sep 21, 2021
@keith keith deleted the ks/bazel-ignore-tool_deps-for-dependency-validation branch September 21, 2021 22:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants