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

incompatible_no_implicit_file_export: implicitly exported files have private visibility #10225

Open
aehlig opened this issue Nov 13, 2019 · 9 comments
Labels
incompatible-change Incompatible/breaking change not stale Issues or PRs that are inactive but not considered stale P4 This is either out of scope or we don't have bandwidth to review a PR. (No assignee) team-Loading-API BUILD file and macro processing: labels, package(), visibility, glob type: process

Comments

@aehlig
Copy link
Contributor

aehlig commented Nov 13, 2019

If the flag --incompatible_no_implicit_file_export is enabled, files that are implicitly exported because they are mentioned in the BUILD have private visibility regardless of the default visibility of the package.

Migration: add an explicit exports_files for all files that should be used outside the package they belong to.

Design document: https://github.com/bazelbuild/proposals/blob/master/designs/2019-10-24-file-visibility.md

@aehlig aehlig added incompatible-change Incompatible/breaking change migration-ready labels Nov 13, 2019
@philwo philwo changed the title --incompatible_no_implicit_file_export: implicitly exported files have private visibility --incompatible_no_implicit_file_export: implicitly exported files have private visibility Dec 3, 2019
@philwo philwo changed the title --incompatible_no_implicit_file_export: implicitly exported files have private visibility incompatible_no_implicit_file_export: implicitly exported files have private visibility Dec 3, 2019
@drigz
Copy link
Contributor

drigz commented Dec 20, 2019

This flag causes a build error in an empty workspace with Bazel 2.0:

$ echo > WORKSPACE
$ bazel build @bazel_tools//tools/test/CoverageOutputGenerator/java/com/google/devtools/coverageoutputgenerator:all_lcov_merger_lib --incompatible_no_implicit_file_export
ERROR: /path/to/bazel/cache/10d1bb0ae85cffec5f45c68b4660ed75/external/bazel_tools/tools/test/CoverageOutputGenerator/java/com/google/devtools/coverageoutputgenerator/BUILD:4:1: in java_import rule @bazel_tools//tools/test/CoverageOutputGenerator/java/com/google/devtools/coverageoutputgenerator:all_lcov_merger_lib: target '@remote_coverage_tools//:all_lcov_merger_tools_deploy.jar' is not visible from target '@bazel_tools//tools/test/CoverageOutputGenerator/java/com/google/devtools/coverageoutputgenerator:all_lcov_merger_lib'. Check the visibility declaration of the former target if you think the dependency is legitimate
ERROR: Analysis of target '@bazel_tools//tools/test/CoverageOutputGenerator/java/com/google/devtools/coverageoutputgenerator:all_lcov_merger_lib' failed; build aborted: Analysis of target '@bazel_tools//tools/test/CoverageOutputGenerator/java/com/google/devtools/coverageoutputgenerator:all_lcov_merger_lib' failed; build aborted

I've also seen this error referring to @remote_coverage_tools//:all_lcov_merger_tools_deploy.jar.

This causes our build to fail for reasons I don't fully understand. Do @remote_coverage_tools and @bazel_tools need to be fixed before we can try applying this flag to our repositories, or should we remove the dependency on the broken targets?

See also: bazelbuild/continuous-integration#887

This was referenced Dec 26, 2019
kornholi pushed a commit to kornholi/bazel-toolchain that referenced this issue Jun 17, 2020
This allows someone, for example, to depend on lib/libclang.so. A good
portion of these files are already implicitly exported, but that
behaviour will be changing soon. See
bazelbuild/bazel#10225.
kornholi added a commit to kornholi/bazel-toolchain that referenced this issue Jun 17, 2020
This allows someone, for example, to depend on lib/libclang.so. A good
portion of these files are already implicitly exported, but that
behaviour will be changing soon. See
bazelbuild/bazel#10225.
kornholi added a commit to kornholi/bazel-toolchain that referenced this issue Jun 17, 2020
This allows someone, for example, to depend on `lib/libclang.so`.

A good portion of these files are already implicitly exported, but that
behaviour will be changing soon. See
bazelbuild/bazel#10225.
siddharthab pushed a commit to kornholi/bazel-toolchain that referenced this issue Jun 17, 2020
This allows someone, for example, to depend on `lib/libclang.so`.

A good portion of these files are already implicitly exported, but that
behaviour will be changing soon. See
bazelbuild/bazel#10225.
siddharthab pushed a commit to bazel-contrib/toolchains_llvm that referenced this issue Jun 17, 2020
This allows someone, for example, to depend on `lib/libclang.so`.

A good portion of these files are already implicitly exported, but that
behaviour will be changing soon. See
bazelbuild/bazel#10225.
@brandjon brandjon added P4 This is either out of scope or we don't have bandwidth to review a PR. (No assignee) team-Build-Language and removed team-Starlark labels Feb 19, 2021
@martis42
Copy link

This is labeled with breaking-change-5.0, which I interpret as the intention to flip this with Bazel 5.0.0.
The flag is however still inactive by default:
https://github.com/bazelbuild/bazel/blob/5.0.0/src/main/java/com/google/devtools/build/lib/packages/semantics/BuildLanguageOptions.java#L426

Please update the issue status.

@brandjon
Copy link
Member

I'd be interested in possibly flipping this for Bazel 7.0.

Note that there appears to be a edge case bug in the current implementation. //foo:BUILD is always considered a source file target of //foo, even if this BUILD file does not contain a mention of its own label. Yet if //foo:BUILD does not appear in an exports_files declaration, it will inherit the package's default visibility (not private visibility), even if this flag is enabled. Seems to me the correct behavior is to apply the flag to :BUILD file targets as well.

@brandjon brandjon added team-Loading-API BUILD file and macro processing: labels, package(), visibility, glob and removed team-Build-Language labels Nov 4, 2022
@dws
Copy link
Contributor

dws commented Jul 20, 2023

I tried turning this on in a repo that I'm working with and discovered (with Bazel 6.2.0):

target '@bazel_tools//tools/cpp:cc_toolchain_config_lib.bzl' is not visible

so it appears that tools/cpp/BUILD.tools in the Bazel sources needs to be fixed. It appears that tools/cpp/BUILD in the Bazel sources already has an exports_files(glob(["*.bzl"])) line, but this addition did not make it into the other BUILD.* files (even in the current master).

Copy link

Thank you for contributing to the Bazel repository! This issue has been marked as stale since it has not had any activity in the last 1+ years. It will be closed in the next 90 days unless any other activity occurs. If you think this issue is still relevant and should stay open, please post any comment here and the issue will no longer be marked as stale.

@github-actions github-actions bot added the stale Issues or PRs that are stale (no activity for 30 days) label Sep 23, 2024
@fmeum
Copy link
Collaborator

fmeum commented Sep 23, 2024

@bazelbuild/triage not stale

@iancha1992 iancha1992 added not stale Issues or PRs that are inactive but not considered stale and removed stale Issues or PRs that are stale (no activity for 30 days) labels Sep 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
incompatible-change Incompatible/breaking change not stale Issues or PRs that are inactive but not considered stale P4 This is either out of scope or we don't have bandwidth to review a PR. (No assignee) team-Loading-API BUILD file and macro processing: labels, package(), visibility, glob type: process
Projects
None yet
Development

No branches or pull requests