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

docs: explain that visibility rules do not apply transitively #19476

Conversation

AlexTereshenkov
Copy link
Member

@AlexTereshenkov AlexTereshenkov commented Jul 16, 2023

Clarify that visibility rules are only for direct dependencies.

We don't use any images in the documentation yet, it seems so I didn't want to add an image just for this. I think the explanation should be clear enough, but this is what I thought.

graph BT;
src.app.main-->src.library.public
src.app.main-->src.library.private.X
src.app.main-->src.library.private.Y

src.library.public-->src.library.private.X
src.library.public-->src.library.private.Y

linkStyle default stroke:green
linkStyle 1 stroke:red
linkStyle 2 stroke:red

@AlexTereshenkov AlexTereshenkov marked this pull request as ready for review July 16, 2023 15:43
Copy link
Member

@kaos kaos left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks right to me. I wish another mind besides mine finds this legible before landing ;)

Comment on lines +161 to +189
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.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this suggest there is a missing feature here?
Not sure how easy/expensive it would be to support, though..

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for taking a look, Andreas! I don't think this is a feature that needs a dedicated implementation. We have, in fact, this implemented by providing access to transitive dependencies. I think this is a fairly rare use case, so I think providing goals that could be piped is sufficient. :)

Copy link
Contributor

@huonw huonw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks fine, although your nifty diagram would definitely be a good addition (if we have an approach to adding images/diagrams) too 👍

@AlexTereshenkov AlexTereshenkov force-pushed the docs/visibility-rules-transitively branch from 11817ad to 20b76e8 Compare July 19, 2023 22:39
@AlexTereshenkov AlexTereshenkov merged commit 00b9bfa into pantsbuild:main Jul 19, 2023
24 checks passed
@AlexTereshenkov AlexTereshenkov deleted the docs/visibility-rules-transitively branch July 19, 2023 23:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants