From f897bb44d7ce2d66c0d800c9f1e66f5cb3d989bb Mon Sep 17 00:00:00 2001 From: Martin Medler Date: Mon, 22 Apr 2024 19:07:52 +0200 Subject: [PATCH] Enable skipping analysis of targets from external workspaces --- README.md | 11 ++++++++++- examples/MODULE.bazel | 10 ++++++++++ examples/WORKSPACE | 8 ++++++++ examples/aspect.bzl | 1 + examples/skipping_targets/BUILD | 6 ++++++ examples/skipping_targets/README.md | 4 ++++ .../use_broken_external_dependency.h | 1 + examples/support/external_targets.bzl | 6 ++++++ examples/support/external_targets/BUILD | 11 +++++++++++ examples/support/external_targets/MODULE.bazel | 1 + examples/support/external_targets/WORKSPACE | 0 examples/support/external_targets/foo.h | 0 examples/support/external_targets/has_unused_dep.h | 0 examples/test.py | 4 ++++ src/aspect/dwyu.bzl | 7 +++++++ src/aspect/factory.bzl | 5 +++++ test/aspect/MODULE.bazel | 6 ++++++ test/aspect/WORKSPACE | 3 +++ test/aspect/recursion/f.h | 4 ++++ test/aspect/skip_external_targets/BUILD | 0 test/aspect/skip_external_targets/aspect.bzl | 3 +++ test/aspect/skip_external_targets/external_dep.bzl | 6 ++++++ .../skip_external_targets/external_dep/BUILD | 5 +++++ .../external_dep/MODULE.bazel | 1 + .../skip_external_targets/external_dep/WORKSPACE | 0 .../external_dep/broken_dep.h | 6 ++++++ .../skip_external_targets/external_dep/lib/BUILD | 12 ++++++++++++ .../skip_external_targets/external_dep/lib/lib_a.h | 5 +++++ .../skip_external_targets/external_dep/lib/lib_b.h | 3 +++ .../test_skip_broken_target.py | 14 ++++++++++++++ 30 files changed, 142 insertions(+), 1 deletion(-) create mode 100644 examples/skipping_targets/use_broken_external_dependency.h create mode 100644 examples/support/external_targets.bzl create mode 100644 examples/support/external_targets/BUILD create mode 100644 examples/support/external_targets/MODULE.bazel create mode 100644 examples/support/external_targets/WORKSPACE create mode 100644 examples/support/external_targets/foo.h create mode 100644 examples/support/external_targets/has_unused_dep.h create mode 100644 test/aspect/recursion/f.h create mode 100644 test/aspect/skip_external_targets/BUILD create mode 100644 test/aspect/skip_external_targets/aspect.bzl create mode 100644 test/aspect/skip_external_targets/external_dep.bzl create mode 100644 test/aspect/skip_external_targets/external_dep/BUILD create mode 100644 test/aspect/skip_external_targets/external_dep/MODULE.bazel create mode 100644 test/aspect/skip_external_targets/external_dep/WORKSPACE create mode 100644 test/aspect/skip_external_targets/external_dep/broken_dep.h create mode 100644 test/aspect/skip_external_targets/external_dep/lib/BUILD create mode 100644 test/aspect/skip_external_targets/external_dep/lib/lib_a.h create mode 100644 test/aspect/skip_external_targets/external_dep/lib/lib_b.h create mode 100644 test/aspect/skip_external_targets/test_skip_broken_target.py diff --git a/README.md b/README.md index 0555baa4..e516dd05 100644 --- a/README.md +++ b/README.md @@ -154,7 +154,16 @@ You can also configure the aspect to skip targets based on a custom list of tags your_aspect = dwyu_aspect_factory(skipped_tags = ["tag1_marking_skipping", "tag2_marking_skipping"]) ``` -This is demonstrated in the [skipping_targets example](/examples/skipping_targets). +Another possibility is skipping all targets from external workspaces. +Often external dependencies are not our under control and thus analyzing them is of little value. +This option is mostly useful in combination with the recursive analysis. +You configure it like this: + +```starlark +your_aspect = dwyu_aspect_factory(skip_external_targets = True) +``` + +Both options are demonstrated in the [skipping_targets example](/examples/skipping_targets). ## Recursion diff --git a/examples/MODULE.bazel b/examples/MODULE.bazel index accb44a8..2500e7dd 100644 --- a/examples/MODULE.bazel +++ b/examples/MODULE.bazel @@ -20,3 +20,13 @@ python = use_extension( dev_dependency = True, ) python.toolchain(python_version = "3.8") + +# +# Support to make the examples work +# + +bazel_dep(name = "external_targets", dev_dependency = True) +local_path_override( + module_name = "external_targets", + path = "support/external_targets", +) diff --git a/examples/WORKSPACE b/examples/WORKSPACE index 1766bb37..0bab054a 100644 --- a/examples/WORKSPACE +++ b/examples/WORKSPACE @@ -36,3 +36,11 @@ python_register_toolchains( name = "python", python_version = "3.8", ) + +# +# Support to make the examples work +# + +load("//:support/external_targets.bzl", "load_external_targets") + +load_external_targets() diff --git a/examples/aspect.bzl b/examples/aspect.bzl index c8ee9c23..342cd4ab 100644 --- a/examples/aspect.bzl +++ b/examples/aspect.bzl @@ -3,6 +3,7 @@ load("@depend_on_what_you_use//:defs.bzl", "dwyu_aspect_factory") dwyu = dwyu_aspect_factory(use_implementation_deps = True) dwyu_recursive = dwyu_aspect_factory(recursive = True) +dwyu_recursive_skip_external = dwyu_aspect_factory(recursive = True, skip_external_targets = True) dwyu_custom_skipping = dwyu_aspect_factory(skipped_tags = ["my_tag"]) # We need to explicitly pass labels as passing strings does not work with a bzlmod setup. diff --git a/examples/skipping_targets/BUILD b/examples/skipping_targets/BUILD index 826c1bf2..709f2132 100644 --- a/examples/skipping_targets/BUILD +++ b/examples/skipping_targets/BUILD @@ -28,3 +28,9 @@ cc_library( name = "lib_b", hdrs = ["lib_b.h"], ) + +cc_library( + name = "use_broken_external_dependency", + hdrs = ["use_broken_external_dependency.h"], + deps = ["@external_targets//:has_unused_dep"], +) diff --git a/examples/skipping_targets/README.md b/examples/skipping_targets/README.md index f1f4149c..7fd55e00 100644 --- a/examples/skipping_targets/README.md +++ b/examples/skipping_targets/README.md @@ -13,3 +13,7 @@ succeeds due to the target being tagged with `no-dwyu`. Executing
`bazel build --aspects=//:aspect.bzl%dwyu_custom_skipping --output_groups=dwyu //skipping_targets:bad_target_custom_skip`
showcases the same skipping behavior, but for the tag `mmy_tag` which we configured in [aspect.bzl](../aspect.bzl). + +Executing
+`bazel build --aspects=//:aspect.bzl%dwyu_recursive_skip_external --output_groups=dwyu //skipping_targets:use_broken_external_dependency`
+showcases how we can ignore targets from external workspaces. diff --git a/examples/skipping_targets/use_broken_external_dependency.h b/examples/skipping_targets/use_broken_external_dependency.h new file mode 100644 index 00000000..26fc219f --- /dev/null +++ b/examples/skipping_targets/use_broken_external_dependency.h @@ -0,0 +1 @@ +#include "has_unused_dep.h" diff --git a/examples/support/external_targets.bzl b/examples/support/external_targets.bzl new file mode 100644 index 00000000..9f11231d --- /dev/null +++ b/examples/support/external_targets.bzl @@ -0,0 +1,6 @@ +# buildifier: disable=unnamed-macro +def load_external_targets(): + native.local_repository( + name = "external_targets", + path = "support/external_targets", + ) diff --git a/examples/support/external_targets/BUILD b/examples/support/external_targets/BUILD new file mode 100644 index 00000000..11e2427a --- /dev/null +++ b/examples/support/external_targets/BUILD @@ -0,0 +1,11 @@ +cc_library( + name = "has_unused_dep", + hdrs = ["has_unused_dep.h"], + visibility = ["//visibility:public"], + deps = [":foo"], +) + +cc_library( + name = "foo", + hdrs = ["foo.h"], +) diff --git a/examples/support/external_targets/MODULE.bazel b/examples/support/external_targets/MODULE.bazel new file mode 100644 index 00000000..a35fff9f --- /dev/null +++ b/examples/support/external_targets/MODULE.bazel @@ -0,0 +1 @@ +module(name = "external_targets") diff --git a/examples/support/external_targets/WORKSPACE b/examples/support/external_targets/WORKSPACE new file mode 100644 index 00000000..e69de29b diff --git a/examples/support/external_targets/foo.h b/examples/support/external_targets/foo.h new file mode 100644 index 00000000..e69de29b diff --git a/examples/support/external_targets/has_unused_dep.h b/examples/support/external_targets/has_unused_dep.h new file mode 100644 index 00000000..e69de29b diff --git a/examples/test.py b/examples/test.py index 2ac59922..b36a4c0d 100755 --- a/examples/test.py +++ b/examples/test.py @@ -57,6 +57,10 @@ class Example: build_cmd="--aspects=//:aspect.bzl%dwyu_custom_skipping --output_groups=dwyu //skipping_targets:bad_target_custom_skip", expected_success=True, ), + Example( + build_cmd="--aspects=//:aspect.bzl%dwyu_recursive_skip_external --output_groups=dwyu //skipping_targets:use_broken_external_dependency", + expected_success=True, + ), Example( build_cmd="--aspects=//:aspect.bzl%dwyu_map_specific_deps --output_groups=dwyu //target_mapping:use_lib_b", expected_success=True, diff --git a/src/aspect/dwyu.bzl b/src/aspect/dwyu.bzl index 0b9ca26b..65c20b5a 100644 --- a/src/aspect/dwyu.bzl +++ b/src/aspect/dwyu.bzl @@ -3,6 +3,9 @@ load("@bazel_tools//tools/cpp:toolchain_utils.bzl", "find_cpp_toolchain") load("@depend_on_what_you_use//src/cc_info_mapping:cc_info_mapping.bzl", "DwyuCcInfoRemappingsInfo") load("@rules_cc//cc:defs.bzl", "CcInfo", "cc_common") +def _is_external(ctx): + return ctx.label.workspace_root.startswith("external") + def _get_target_sources(rule): public_files = [] private_files = [] @@ -195,6 +198,10 @@ def dwyu_aspect_impl(target, ctx): if not ctx.rule.kind in ["cc_binary", "cc_library", "cc_test"]: return [] + # If configured, skip external targets + if ctx.attr._skip_external_targets and _is_external(ctx): + return [] + # Skip targets which explicitly opt-out if any([tag in ctx.attr._skipped_tags for tag in ctx.rule.attr.tags]): return [] diff --git a/src/aspect/factory.bzl b/src/aspect/factory.bzl index bad66628..16973bc8 100644 --- a/src/aspect/factory.bzl +++ b/src/aspect/factory.bzl @@ -4,6 +4,7 @@ load(":dwyu.bzl", "dwyu_aspect_impl") def dwyu_aspect_factory( ignored_includes = None, recursive = False, + skip_external_targets = False, skipped_tags = None, target_mapping = None, use_implementation_deps = False, @@ -16,6 +17,7 @@ def dwyu_aspect_factory( nothing is specified, the standard library headers are ignored by default. recursive: If true, execute the aspect on all transitive dependencies. If false, analyze only the target the aspect is being executed on. + skip_external_targets: If a target is from an external workspace DWYU skips analyzing it. skipped_tags: Do not execute the aspect on targets with at least one of those tags. By default skips the analysis for targets tagged with 'no-dwyu'. target_mapping: A target providing a map of target labels to alternative CcInfo provider objects for those @@ -67,6 +69,9 @@ def dwyu_aspect_factory( "_recursive": attr.bool( default = recursive, ), + "_skip_external_targets": attr.bool( + default = skip_external_targets, + ), "_skipped_tags": attr.string_list( default = aspect_skipped_tags, ), diff --git a/test/aspect/MODULE.bazel b/test/aspect/MODULE.bazel index f3653794..9a51727c 100644 --- a/test/aspect/MODULE.bazel +++ b/test/aspect/MODULE.bazel @@ -55,6 +55,12 @@ local_path_override( path = "complex_includes/ext_repo", ) +bazel_dep(name = "skip_external_deps_test_repo", dev_dependency = True) +local_path_override( + module_name = "skip_external_deps_test_repo", + path = "skip_external_targets/external_dep", +) + ## ## The Migration phase using WORKSPACE.bzlmod and MODULE.bazel together does not support properly loading the implicit ## Bazel dependencies. Thus, we need to load some basic things directly. This should become superfluous when we are diff --git a/test/aspect/WORKSPACE b/test/aspect/WORKSPACE index fbb862d1..767da38d 100644 --- a/test/aspect/WORKSPACE +++ b/test/aspect/WORKSPACE @@ -49,7 +49,10 @@ python_register_multi_toolchains( load("//complex_includes:ext_repo.bzl", "load_complex_includes_repo") load("//external_repo:repo.bzl", "load_external_repo") +load("//skip_external_targets:external_dep.bzl", "load_external_dep") load_external_repo() load_complex_includes_repo() + +load_external_dep() diff --git a/test/aspect/recursion/f.h b/test/aspect/recursion/f.h new file mode 100644 index 00000000..7e09faa6 --- /dev/null +++ b/test/aspect/recursion/f.h @@ -0,0 +1,4 @@ +#ifndef F_H +#define F_H + +#endif diff --git a/test/aspect/skip_external_targets/BUILD b/test/aspect/skip_external_targets/BUILD new file mode 100644 index 00000000..e69de29b diff --git a/test/aspect/skip_external_targets/aspect.bzl b/test/aspect/skip_external_targets/aspect.bzl new file mode 100644 index 00000000..14701b7b --- /dev/null +++ b/test/aspect/skip_external_targets/aspect.bzl @@ -0,0 +1,3 @@ +load("@depend_on_what_you_use//:defs.bzl", "dwyu_aspect_factory") + +dwyu_skip_external = dwyu_aspect_factory(skip_external_targets = True) diff --git a/test/aspect/skip_external_targets/external_dep.bzl b/test/aspect/skip_external_targets/external_dep.bzl new file mode 100644 index 00000000..92c946c2 --- /dev/null +++ b/test/aspect/skip_external_targets/external_dep.bzl @@ -0,0 +1,6 @@ +# buildifier: disable=unnamed-macro +def load_external_dep(): + native.local_repository( + name = "skip_external_deps_test_repo", + path = "skip_external_targets/external_dep", + ) diff --git a/test/aspect/skip_external_targets/external_dep/BUILD b/test/aspect/skip_external_targets/external_dep/BUILD new file mode 100644 index 00000000..9f31205f --- /dev/null +++ b/test/aspect/skip_external_targets/external_dep/BUILD @@ -0,0 +1,5 @@ +cc_library( + name = "broken_dep", + hdrs = ["broken_dep.h"], + deps = ["//lib:a"], # Missing dependency to //lib:b +) diff --git a/test/aspect/skip_external_targets/external_dep/MODULE.bazel b/test/aspect/skip_external_targets/external_dep/MODULE.bazel new file mode 100644 index 00000000..cf76cd70 --- /dev/null +++ b/test/aspect/skip_external_targets/external_dep/MODULE.bazel @@ -0,0 +1 @@ +module(name = "skip_external_deps_test_repo") diff --git a/test/aspect/skip_external_targets/external_dep/WORKSPACE b/test/aspect/skip_external_targets/external_dep/WORKSPACE new file mode 100644 index 00000000..e69de29b diff --git a/test/aspect/skip_external_targets/external_dep/broken_dep.h b/test/aspect/skip_external_targets/external_dep/broken_dep.h new file mode 100644 index 00000000..58495548 --- /dev/null +++ b/test/aspect/skip_external_targets/external_dep/broken_dep.h @@ -0,0 +1,6 @@ +#include "lib/lib_a.h" +#include "lib/lib_b.h" + +int useLibWithoutDirectDep() { + return doLibA() + doLibB(); +} diff --git a/test/aspect/skip_external_targets/external_dep/lib/BUILD b/test/aspect/skip_external_targets/external_dep/lib/BUILD new file mode 100644 index 00000000..0cc7511d --- /dev/null +++ b/test/aspect/skip_external_targets/external_dep/lib/BUILD @@ -0,0 +1,12 @@ +cc_library( + name = "a", + hdrs = ["lib_a.h"], + visibility = ["//visibility:public"], + deps = [":b"], +) + +cc_library( + name = "b", + hdrs = ["lib_b.h"], + visibility = ["//visibility:public"], +) diff --git a/test/aspect/skip_external_targets/external_dep/lib/lib_a.h b/test/aspect/skip_external_targets/external_dep/lib/lib_a.h new file mode 100644 index 00000000..2cfe0acb --- /dev/null +++ b/test/aspect/skip_external_targets/external_dep/lib/lib_a.h @@ -0,0 +1,5 @@ +#include "lib/lib_b.h" + +int doLibA() { + return doLibB() + 42; +} diff --git a/test/aspect/skip_external_targets/external_dep/lib/lib_b.h b/test/aspect/skip_external_targets/external_dep/lib/lib_b.h new file mode 100644 index 00000000..43fe9469 --- /dev/null +++ b/test/aspect/skip_external_targets/external_dep/lib/lib_b.h @@ -0,0 +1,3 @@ +int doLibB() { + return 1337; +} diff --git a/test/aspect/skip_external_targets/test_skip_broken_target.py b/test/aspect/skip_external_targets/test_skip_broken_target.py new file mode 100644 index 00000000..f5167685 --- /dev/null +++ b/test/aspect/skip_external_targets/test_skip_broken_target.py @@ -0,0 +1,14 @@ +from result import ExpectedResult, Result +from test_case import TestCaseBase + + +class TestCase(TestCaseBase): + def execute_test_logic(self) -> Result: + # If we would not skip al external targets the analysis would find an issue with the broken dependency + expected = ExpectedResult(success=True) + actual = self._run_dwyu( + target="@skip_external_deps_test_repo//:broken_dep", + aspect="//skip_external_targets:aspect.bzl%dwyu_skip_external", + ) + + return self._check_result(actual=actual, expected=expected)