Skip to content

Commit

Permalink
Enable skipping analysis of targets from external workspaces
Browse files Browse the repository at this point in the history
  • Loading branch information
martis42 committed Apr 22, 2024
1 parent 768f7a0 commit b817c22
Show file tree
Hide file tree
Showing 29 changed files with 141 additions and 1 deletion.
11 changes: 10 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
10 changes: 10 additions & 0 deletions examples/MODULE.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -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",
)
8 changes: 8 additions & 0 deletions examples/WORKSPACE
Original file line number Diff line number Diff line change
Expand Up @@ -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()
1 change: 1 addition & 0 deletions examples/aspect.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
6 changes: 6 additions & 0 deletions examples/skipping_targets/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -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"],
)
4 changes: 4 additions & 0 deletions examples/skipping_targets/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,3 +13,7 @@ succeeds due to the target being tagged with `no-dwyu`.
Executing <br>
`bazel build --aspects=//:aspect.bzl%dwyu_custom_skipping --output_groups=dwyu //skipping_targets:bad_target_custom_skip` <br>
showcases the same skipping behavior, but for the tag `mmy_tag` which we configured in [aspect.bzl](../aspect.bzl).

Executing <br>
`bazel build --aspects=//:aspect.bzl%dwyu_recursive_skip_external --output_groups=dwyu //skipping_targets:use_broken_external_dependency` <br>
showcases how we can ignore targets from external workspaces.
1 change: 1 addition & 0 deletions examples/skipping_targets/use_broken_external_dependency.h
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
#include "has_unused_dep.h"
6 changes: 6 additions & 0 deletions examples/support/external_targets.bzl
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
# buildifier: disable=unnamed-macro
def load_external_targets():
native.local_repository(
name = "external_targets",
path = "support/external_targets",
)
11 changes: 11 additions & 0 deletions examples/support/external_targets/BUILD
Original file line number Diff line number Diff line change
@@ -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"],
)
1 change: 1 addition & 0 deletions examples/support/external_targets/MODULE.bazel
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
module(name = "external_targets")
Empty file.
Empty file.
Empty file.
4 changes: 4 additions & 0 deletions examples/test.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
7 changes: 7 additions & 0 deletions src/aspect/dwyu.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -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 = []
Expand Down Expand Up @@ -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 []
Expand Down
5 changes: 5 additions & 0 deletions src/aspect/factory.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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
Expand Down Expand Up @@ -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,
),
Expand Down
6 changes: 6 additions & 0 deletions test/aspect/MODULE.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
3 changes: 3 additions & 0 deletions test/aspect/WORKSPACE
Original file line number Diff line number Diff line change
Expand Up @@ -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()
4 changes: 4 additions & 0 deletions test/aspect/recursion/f.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
#ifndef F_H
#define F_H

#endif
Empty file.
3 changes: 3 additions & 0 deletions test/aspect/skip_external_targets/aspect.bzl
Original file line number Diff line number Diff line change
@@ -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)
6 changes: 6 additions & 0 deletions test/aspect/skip_external_targets/external_dep.bzl
Original file line number Diff line number Diff line change
@@ -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",
)
16 changes: 16 additions & 0 deletions test/aspect/skip_external_targets/external_dep/BUILD
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
cc_library(
name = "broken_dep",
hdrs = ["broken_dep.h"],
deps = [":a"], # Missing dependency to //lib:b
)

cc_library(
name = "a",
hdrs = ["lib/a.h"],
deps = [":b"],
)

cc_library(
name = "b",
hdrs = ["lib/b.h"],
)
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
module(name = "skip_external_deps_test_repo")
Empty file.
6 changes: 6 additions & 0 deletions test/aspect/skip_external_targets/external_dep/broken_dep.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
#include "lib/a.h"
#include "lib/b.h"

int useLibWithoutDirectDep() {
return doLibA() + doLibB();
}
5 changes: 5 additions & 0 deletions test/aspect/skip_external_targets/external_dep/lib/a.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
#include "lib/lib_b.h"

int doLibA() {
return doLibB() + 42;
}
3 changes: 3 additions & 0 deletions test/aspect/skip_external_targets/external_dep/lib/b.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
int doLibB() {
return 1337;
}
14 changes: 14 additions & 0 deletions test/aspect/skip_external_targets/test_skip_broken_target.py
Original file line number Diff line number Diff line change
@@ -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)

0 comments on commit b817c22

Please sign in to comment.