Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Visibility: support dict rules same way as for selectors. #20769

Merged
merged 2 commits into from
Apr 15, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 23 additions & 2 deletions docs/docs/using-pants/validating-dependencies.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ Validating your code's dependencies.

Visibility rules are the mechanism by which to control who may depend on whom. It is an implementation of Pants's dependency rules API. With these rules a dependency between two files (or targets) may be set for entire directory trees down to single files. A target may be selected not only by its file path but also by target type, name, and tags.

NB: Visibility rules are applied on a low level, so they can inspect metadata provided directly on a target and via `__defaults__`, but anything applied via `overrides` is opaque to the visibility rules. For example, a rule that selects tagged targets can find tags provided directly on the target using the `tags` field (like `target(tags=[...])`) or via `__defaults__` (like `__defaults__({target:dict(tags=[...])})`) but any tags in `overrides` (like `target(overrides={"foo": dict(tags=[...])})`) cannot be inspected by the visibility rules.

To jump right in, start with [enabling the backend](./validating-dependencies.mdx#enable-visibility-backend) and add some rules to your BUILD files.

## Example visibility rules
Expand Down Expand Up @@ -203,13 +205,13 @@ __dependencies_rules__(

The selector and rule share a common syntax (refered to as a **target rule spec**), that is a dictionary value with properties describing what targets it applies to. Together, this pair of selector(s) and rules is called a **rule set**. A rule set may have multiple selectors wrapped in a list/tuple and the rules may be spread out or grouped in any fashion. Grouping rules like this makes it easier to re-use/insert rules from macros or similar.

:::note An empty selector (`{}` or `""`) will never match anything and is as such pointless and will result in an error.
:::note An empty selector (`{}` or `""`) will never match anything and is as such pointless and is ignored.
For every dependency link, only a single set of rules will ever be applied. The first rule set
with a matching selector will be the only one used and any remaining rule sets are never
consulted.
:::

The **target rule spec** has four properties: `type`, `name`, `tags`, and `path`. From the above example, when determining which rule set to apply for the dependencies of `src/a/main.py` Pants will look for the first selector for `src/a/BUILD` that satisifies the properties `type=python_sources`, `tags=["apps"]`, and `path=src/a/main.py`. The selection is based on exclusion so only when there is a property value and it doesn't match the target's property it will move on to the next selector; the lack of a property will be considered to match anything. Consequently an empty target spec would match all targets, but this is disallowed and will raise an error if used because it is conceptually not very clear when reading the rules.
The **target rule spec** has four (five for rules) properties: `type`, `name`, `tags`, `path`, and `action` (only rules consult `action`). The `action` is one of `allow` (default if not specified), `warn`, or `deny`. From the above example, when determining which rule set to apply for the dependencies of `src/a/main.py` Pants will look for the first selector for `src/a/BUILD` that satisifies the properties `type=python_sources`, `tags=["apps"]`, and `path=src/a/main.py`. The selection is based on exclusion so only when there is a property value and it doesn't match the target's property it will move on to the next selector; the lack of a property will be considered to match anything. Consequently an empty target spec would match all targets, but this is disallowed and will raise an error if used because it is conceptually not very clear when reading the rules.
kaos marked this conversation as resolved.
Show resolved Hide resolved

The values of a **target rule spec** supports wildcard patterns (or globs) in order to have a single selector match multiple different targets, as described in [glob syntax](./key-concepts/targets-and-build-files.mdx#glob-syntax). When listing multiple values for the `tags` property, the target must have all of them in order to match. Spread the tags over multiple selectors in order to switch from _AND_ to _OR_ as required. The target `type` to match against will be that of the type used in the BUILD file, as the path (and target address) may refer to a generated target it is the target generators type that will be used during the selector matching process.

Expand Down Expand Up @@ -289,6 +291,25 @@ The previous example, using this alternative syntax for the selectors, would loo
)
```

Similarily, the rules may also be expressed using the dict syntax:

```python
(
( # Selectors block
...
),
( # Grouping rules for readability
( # Deny rules
{"path": "tests/**", "action": "deny"}, # No tests
{"path": "src/*/*/**", "action": "deny"}, # Nothing deeply nested
),
( # Allow rules
{"path": "*"}, # Allow everything else (allow is default action)
),
)
)
```

### Glob syntax

The visibility rules are all about matching globs. There are two wildcards: the `*` matches anything except `/`, and the `**` matches anything including `/`. (For paths that is non-recursive and recursive globbing respectively.)
Expand Down
33 changes: 19 additions & 14 deletions src/python/pants/backend/visibility/rule_types.py
Original file line number Diff line number Diff line change
Expand Up @@ -68,20 +68,25 @@ class VisibilityRule:
@classmethod
def parse(
cls,
rule: str,
rule: str | dict,
relpath: str,
) -> VisibilityRule:
if not isinstance(rule, str):
raise ValueError(f"expected a path pattern string but got: {rule!r}")
if rule.startswith("!"):
action = DependencyRuleAction.DENY
pattern = rule[1:]
elif rule.startswith("?"):
action = DependencyRuleAction.WARN
pattern = rule[1:]
else:
action = DependencyRuleAction.ALLOW
pattern: str | dict
if isinstance(rule, str):
if rule.startswith("!"):
action = DependencyRuleAction.DENY
pattern = rule[1:]
elif rule.startswith("?"):
action = DependencyRuleAction.WARN
pattern = rule[1:]
else:
action = DependencyRuleAction.ALLOW
pattern = rule
elif isinstance(rule, dict):
action = DependencyRuleAction(rule.get("action", "allow"))
pattern = rule
else:
raise ValueError(f"invalid visibility rule: {rule!r}")
return cls(action, TargetGlob.parse(pattern, relpath))

def match(self, address: Address, adaptor: TargetAdaptor, relpath: str) -> bool:
Expand Down Expand Up @@ -153,7 +158,7 @@ def parse(cls, build_file: str, arg: Any) -> VisibilityRuleSet:
relpath = os.path.dirname(build_file)
try:
selectors = cast("Iterator[str | dict]", flatten(arg[0], str, dict))
rules = cast("Iterator[str]", flatten(arg[1:], str))
rules = cast("Iterator[str | dict]", flatten(arg[1:], str, dict))
return cls(
build_file,
tuple(TargetGlob.parse(selector, relpath) for selector in selectors),
Expand All @@ -173,8 +178,8 @@ def peek(self) -> tuple[str, ...]:
return tuple(map(str, self.rules))

@staticmethod
def _noop_rule(rule: str) -> bool:
return not rule or rule.startswith("#")
def _noop_rule(rule: str | dict) -> bool:
return not rule or isinstance(rule, str) and rule.startswith("#")

def match(self, address: Address, adaptor: TargetAdaptor, relpath: str) -> bool:
return any(selector.match(address, adaptor, relpath) for selector in self.selectors)
Expand Down
20 changes: 19 additions & 1 deletion src/python/pants/backend/visibility/rule_types_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ def parse_address(raw: str, description_of_origin: str = repr("test"), **kwargs)
return parsed.dir_to_address() if "." not in raw else parsed.file_to_address()


def parse_rule(rule: str, relpath: str = "test/path") -> VisibilityRule:
def parse_rule(rule: str | dict, relpath: str = "test/path") -> VisibilityRule:
return VisibilityRule.parse(rule, relpath)


Expand Down Expand Up @@ -158,6 +158,7 @@ def test_flatten(expected, xs) -> None:
(True, "<target>", "src/a", ""),
(False, "<target>[src/b]", "src/a", ""),
(False, "<file>", "src/a", ""),
(True, {"path": "src/a"}, "src/a", ""),
],
)
def test_visibility_rule(expected: bool, spec: str, path: str, relpath: str) -> None:
Expand Down Expand Up @@ -206,6 +207,23 @@ def test_visibility_rule(expected: bool, spec: str, path: str, relpath: str) ->
),
("<target>", "src/*", "src/a:lib"),
),
(
VisibilityRuleSet(
build_file="test/path/BUILD",
selectors=(TargetGlob.parse("<target>", ""),),
rules=(
parse_rule("!src/*"),
parse_rule("?src/a:lib"),
parse_rule("(this, is, ok)"),
),
),
(
"<target>",
{"action": "deny", "path": "src/*"},
{"action": "warn", "path": "src/a", "name": "lib"},
{"tags": ["this", "is", "ok"]},
),
),
],
)
def test_visibility_rule_set_parse(expected: VisibilityRuleSet, arg: Any) -> None:
Expand Down
Loading