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

Data dependencies implicitly export files #2835

Closed
david-german-tri opened this issue Apr 17, 2017 · 9 comments
Closed

Data dependencies implicitly export files #2835

david-german-tri opened this issue Apr 17, 2017 · 9 comments
Labels
P3 We're not considering working on this, but happy to review a PR. (No assignee) type: feature request

Comments

@david-german-tri
Copy link

david-german-tri commented Apr 17, 2017

Description of the problem / feature request / question:

If rule A in package Alpha has a data dependency on a file foo.txt in package Alpha, rule B in package Beta is also permitted to have a data dependency on foo.txt, even if foo.txt has not been exported with exports_files.

We originally stumbled across this in RobotLocomotion/drake#5864 - we deleted Rule A, and suddenly Bazel would no longer accept Rule B. That was pretty surprising! It seems to me like a data dependency on unexported files from another package should always be invalid.

If possible, provide a minimal example to reproduce the problem:

https://github.com/david-german-tri/implicit_exports/blob/master/BUILD#L6

Removing that rule results in:

ERROR: /home/dgerman/implicit_exports/subdir/BUILD:4:1: no such target '//:data/foo.txt': target 'data/foo.txt' not declared in package ''; however, a source file of this name exists.  (Perhaps add 'exports_files(["data/foo.txt"])' to /BUILD?) defined by /home/dgerman/implicit_exports/BUILD and referenced by '//subdir:subprogram'.

Environment info

  • Operating System: Ubuntu 14.04

  • Bazel version (output of bazel info release): release 0.4.5

Have you found anything relevant by searching the web?

Nope.

@aj-michael
Copy link
Contributor

This behavior is described in the documentation for exports_files:

https://bazel.build/versions/master/docs/be/functions.html#exports_files

"exports_files() specifies a list of files belonging to this package that are exported to other packages but not otherwise mentioned in the BUILD file."

@aehlig
Copy link
Contributor

aehlig commented Apr 18, 2017

Assigning to our Skylark experts to decide if this is intended behaviour. It indeed looks like it can cause breakage easily.

@laurentlb laurentlb added team-Bazel General Bazel product/strategy issues and removed category: extensibility > skylark labels Nov 21, 2018
@dslomov dslomov added team-Starlark untriaged and removed team-Bazel General Bazel product/strategy issues labels Jul 23, 2019
@laurentlb laurentlb added type: feature request P3 We're not considering working on this, but happy to review a PR. (No assignee) and removed untriaged labels Jul 24, 2019
@laurentlb
Copy link
Contributor

@lberki might know more about it. Rules implicitly export the files.

I suspect it's a lot of work to change the behavior (and get all the code updated), so I don't expect we'll change it soon.

@lberki
Copy link
Contributor

lberki commented Jul 25, 2019

Rules implicitly create targets for their input files. As much as I don't like it and as @laurentlb said, that behavior is mighty hard to change.

However, the visibility of such labels is the same as the default visibility of the package containing them. So they shouldn't be visible to other packages unless you say package(default_visibility=<something else>)

@laurentlb
Copy link
Contributor

That's good to know!

So there's a difference between:

cc_library(name = "sub", srcs = ["a.cc"], visibility = ["//visibility:public"])

and:

package(default_visibility = ["//visibility:public"])

cc_library(name = "sub", srcs = ["a.cc"])

I think that's a good reason to discourage setting default_visibility to public, even if most/all the targets are public.

Is it documented anywhere?

@lberki
Copy link
Contributor

lberki commented Jul 25, 2019

I don't think so. Hell, I wrote this based on reading the code for about a minute and a half so don't take it as gospel.

@laurentlb
Copy link
Contributor

I've checked, the two code samples behave differently. In the second case, the file a.cc is visible to other packages.

I've filed #8982 to improve the documentation of visibility.

@lberki
Copy link
Contributor

lberki commented Jul 25, 2019

Well, then this is one of those times when I wish I was wrong :(

@brandjon
Copy link
Member

I just verified that the visibility of a file target is not public unless it appears in an exports_files declaration, or the package's default visibility is public. Documentation was added in #8982. Closing this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P3 We're not considering working on this, but happy to review a PR. (No assignee) type: feature request
Projects
None yet
Development

No branches or pull requests

7 participants