1. Not ideal that its a single error at a time. We regularly write rules which initially trigger dozens of violations. But pants would ony show us one error per run (when they are treated as errors, warnings would log all of them). It would be great to treat the output more like a linter, which wrote all all the violations.

2. It does not appear play nicely with python automatically depending on all `__init__py`'s above it in the tree. This may be solved with documentation. But you can't easily deny everything, since the dependencies on

__init__.py

in the current and parent directory are inferred.

3. Maybe we just couldn't figure this out, but it seems you cannot use a non-file target in any of the rules. Colons in the rules seem to be interpreted literally rather than as part of an identifier for a non-file target, like a docker image or pex.

4. We figured this out eventually, but it was a bit annoying to figure out that we needed to install some default rules at the root of our repo. This may be solved with documentation.

2. Oh? I’ll have to run some labs on that. Thanks for raising it.

3. Yes, the current design is path based, with the option to filter down on target type but not by target address.

4. Could you please elaborate a bit on this one? Parts of the subtree not using these rules should have an implicit “allow all” rule applied.

Also, are these experiences pre or post https://github.com/pantsbuild/pants/pull/17588 ?

on 4 - the rules we were writing were likely covering more things than we expected. I think the error message could be a bit more verbose to say “you may need to add a “*” catch-all…” etc. we were stumped for a few minutes until we re-read the PR description and noticed the bit about needing a catch-all rule

on 3 - at least for us, filtering down by target address is critical to compete with

import-linter

(https://import-linter.readthedocs.io/en/stable/contract_types.html). we’ll typically introduce new rules before our codebase is completely in compliance to “stop the bleeding”, and set all the

ignore_imports

for existing rule violations. if we aren’t able to specify per-target exceptions then we’d need to keep

import-linter

around until we’ve either broken all the bad imports or majorly reorganized our codebase (both of which will take a long while)

there are more details logged when running with

-ldebug

though, in case you’ve not seen those yet… 🙂

Most systems don’t support that AFAIK

hmm… I’ll have a think through if maybe I can glean some stuff out any way….

oh yea, I do have the address in question… nvm me, fixing! 😄

(and thanks a million for trying this out so thoroughly early on!!)

OK, a couple of upcoming improvements regarding error messages first:

pants.engine.internals.dep_rules.DependencyRuleActionDeniedError: src/python/pants/backend/visibility/rules.py has 2 dependency violations:

  * src/python/pants/backend/explorer/BUILD: '!*' : DENY python_sources src/python/pants/backend/visibility/rules.py -> python_sources src/python/pants/backend/explorer/graphql/setup.py
  * src/python/pants/backend/explorer/BUILD: '!*' : DENY python_sources src/python/pants/backend/visibility/rules.py -> python_sources src/python/pants/backend/explorer/server/uvicorn.py

and

pants.backend.visibility.rule_types.BuildFileVisibilityRulesError: There is no matching rule from the `__dependencies_rules__` defined in src/python/pants/engine/internals/BUILD for the `python_tests` target src/python/pants/engine/internals/dep_rules_test.py:tests for the dependency on the `python_test_utils` target //conftest.py:test_utils

Consider adding the required catch-all rule at the end of the rules spec. Example adding a "deny all" at the end:

  (('*',), 'not/matching', '!this/either', '!*')

where the BUILD file has:

__dependencies_rules__(("*", "not/matching", "!this/either"),)

PR will likely be coming sometime tomorrow… any suggestions for improving the phrasing of the above, or information to include most welcome 🙂

regarding the

BuildFileVisibilityRulesError

so will it pick out the matching rule spec for the target in question based on the target globs for the rules in the BUILD file for the example in the message 😉

https://pantsbuild.slack.com/archives/C046T6T9U/p1669067240497999?thread_ts=1669067231.802249&cid=C046T6T9U @loud-laptop-17949 I’m guessing these init files are non-empty? And yes, I think having an early

"__init__.py"

rule before any deny rules to allow them could go a long way..

This might be a case where the docs need to be a big clearer, exp when providing a clean-slate deny-all policy from which you can grant back permitted imports.