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

Allow enforcing transitive visibility rules with __dependents_rules__ / __dependencies_rules__ #21449

Open
huonw opened this issue Sep 24, 2024 · 0 comments

Comments

@huonw
Copy link
Contributor

huonw commented Sep 24, 2024

Is your feature request related to a problem? Please describe.

Currently visibility rules using __dependents_rules__ / __dependencies_rules__ (https://www.pantsbuild.org/stable/docs/using-pants/validating-dependencies) can only be used to enforce direct dependency constraints, e.g. "I don't want code in src/abc to write import bad_library directly", but don't allow expressing "I don't want code in src/abc to transitively depend on bad_library at all".

It'd be cool if it did.

Describe the solution you'd like

Some mechanism to express transitive-denies with those rules: suggestion !! as a prefix or "action": "deny-transitive". (I'm not sure if transitive-allows need to be a thing?)

For instance:

__dependencies_rules__(
  (
    {"type": python_sources}, # python_sources...
    "src/good/**", # ... can depend on `good_library` code...
    "!!*", # ... but not anything else transitively (i.e. if there's parts of good library that pull in other deps, we can't use them here)
  ),

  ("*", "*"),
)

__dependents_rules__(
  (
    ":target", # this target...
    (
      {"path": "bad_library/**", "action": "transitively-deny"} # ... should not depend on this library...
      "*", # ... but can depend on other things
    ),
  ),

  ("*", "*"),
)

Describe alternatives you've considered

  1. Writing custom processing as the docs suggest:

Keep in mind that visibility rules only operate on direct dependencies - they do not validate dependencies transitively. This is because it would otherwise make it impossible to use private modules. For instance, imagine you have an application stored in src.app.main. It needs to access the public modules in the shared library src.library. The methods in the public module src.library.public make calls to the private modules in that shared library, which means that src.library.public depends on src.library._private. So when we declare that src.app.main may not depend on src.library._private, we only forbid the application accessing the private modules directly, since it still needs to access the functionality they provide transitively (but only via the public interface).

If your codebase has a very complex dependency graph, you may need to make sure a given module never reaches some other modules (transitively). For instance, you may need to declare that modules in component C1 may depend on any module in component C2 as long as these modules (in C2) do not depend (transitively) on any module in component C3. This could be necessary if components are deployed separately, for instance, you may package a deployable artifact with components C1 (full) and C2 (partial) and another one with components C2 (partial) and C3 (full).

In this scenario, you may need to look into alternative solutions to codify these constraints. For example, to check for violations, you could query the dependencies of component C1 (transitively) using the dependencies goal with the --transitive option to confirm none of the modules from component C3 are listed. If there are some, you may find the paths goal helpful as it would show you exactly why a certain module from component C1 depends on a given module in component C3 if it is unclear.

It'd be nice to package this up into built-in rules, rather than require custom tests that are hard for Pants to run natively.

  1. writing a plugin that exposes the pants peek :: output as a codegen'd file that a test can read and manipulate

Additional context

Solving #12733 would make this properly reliable.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant