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

Add new strict_visibility_value= attribute to allow exposing strict dependencies to some scopes #649

Merged

Conversation

dmivankov
Copy link
Contributor

Note that using package default_visibility doesn't always work so explicit visiblity attributes are needed
bazelbuild/bazel#13681

Somewhat related to #648

In combination this for example allows to create a class_name -> maven artifact resolver for
transitive dependency classes while still keeping strict visibility (making transitive dependencies
visible only to the resolve cli tool)

Copy link
Collaborator

@shs96c shs96c left a comment

Choose a reason for hiding this comment

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

That's an amazing Bazel bug. Thanks for spotting this and creating this PR. Could you also please run ./scripts/generate_docs.sh to make sure that the new flag is properly documented?

private/rules/maven_install.bzl Outdated Show resolved Hide resolved
@dmivankov dmivankov force-pushed the add_fine_grained_strict_visibility branch from d07173d to 73b2da0 Compare January 5, 2022 12:39
@dmivankov
Copy link
Contributor Author

Tests failed, will have a look

@dmivankov dmivankov force-pushed the add_fine_grained_strict_visibility branch 2 times, most recently from a73c630 to fbe093a Compare January 5, 2022 13:04
@dmivankov dmivankov requested a review from shs96c January 5, 2022 13:07
Copy link
Collaborator

@shs96c shs96c left a comment

Choose a reason for hiding this comment

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

Thank you!

docs/api.md Outdated Show resolved Hide resolved
@dmivankov
Copy link
Contributor Author

funny, cla check thinks @shs96c hasn't signed it even being a codeowner

…ependencies to some scopes

Note that using package default_visibility doesn't always work so explicit visiblity attributes are needed
bazelbuild/bazel#13681

Somewhat related to bazel-contrib#648

In combination this for example allows to create a class_name -> maven artifact resolver for
transitive dependency classes while still keeping strict visibility (making transitive dependencies
visible only to the resolve cli tool)
@dmivankov dmivankov force-pushed the add_fine_grained_strict_visibility branch from afb5416 to fbbaee4 Compare January 18, 2022 08:08
@shs96c
Copy link
Collaborator

shs96c commented Jan 18, 2022

@dmivankov could you please remove my commit from the history? The CLA bot isn't happy since it came from the GitHub UI and doesn't have my proper email address. I'll merge the PR when it gives me the green light.

@dmivankov dmivankov force-pushed the add_fine_grained_strict_visibility branch from fbbaee4 to 85e3bad Compare January 18, 2022 12:19
@dmivankov
Copy link
Contributor Author

done, squashed and reathored commits related to toc

@shs96c shs96c merged commit 3ab3d1a into bazel-contrib:master Jan 19, 2022
@shs96c
Copy link
Collaborator

shs96c commented Jan 19, 2022

Merged! Thank you @dmivankov!

dmivankov added a commit to dmivankov/rules_jvm_external that referenced this pull request Jan 26, 2022
dmivankov added a commit to dmivankov/rules_jvm_external that referenced this pull request Jan 26, 2022
dmivankov added a commit to dmivankov/rules_jvm_external that referenced this pull request Jan 26, 2022
@Kernald
Copy link

Kernald commented Sep 23, 2022

Because of another Bazel bug (bazelbuild/bazel#13553), accessing the jar files directly was quite useful - see Bencodes/bazel_issue_13553#1. This was possible because of the default_visibility at the package level, which is now commented out. Is there any way to still access the jar files directly?

@dmivankov
Copy link
Contributor Author

@Kernald jar files should still be accessible, do you have a more detailed repro case?
genrule converting downloaded files (like @javax_inject_javax_inject_1//file:file) to @maven//... jars should have same visibility as before. You may also try using downloaded jars directly (bazel query --output build @maven//....jar)

@Kernald
Copy link

Kernald commented Sep 27, 2022

Such a query doesn't seem to yield anything, but pinning seems to fix the visibility issue. Which seems to point to a slightly different behaviour between 4.3 and 4.4.2 without pinning, but similar behaviour with pinning (and pinning is a totally acceptable solution as far as I'm concerned, I should have done that from the start anyway).

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

Successfully merging this pull request may close these issues.

3 participants