opened
prattmic
username_0
I've defined a Skylark rule with a label attribute referencing a tool the rule needs ([see code](https://github.com/username_0/nanopb/blob/cpp/nanopb.bzl#L160)).

It seems that this label is not handled properly when the rule is loaded from an external repository. [This minimal example](https://gist.github.com/username_0/33671d80d9242fef86c2) fails with this error:

```
$ bazel build //:test
ERROR: /tmp/external/BUILD:3:1: every rule of type nanopb_cpp_srcs implicitly depends upon the target '//generator:nanopb_cpp_generator', but this target could not be found because of: no such package 'generator': BUILD file not found on package path.
ERROR: Loading failed; build aborted.
INFO: Elapsed time: 4.662s
```

It looks like bazel is looking for //generator:nanopb_cpp_generator in my workspace, instead of the nanopb workspace. I imagine this is a known deficiency, but I couldn't find an issue tracking plans to change this (or suggested alternatives).

I found [REPOSITORY_NAME](http://bazel.io/docs/skylark/lib/globals.html#REPOSITORY_NAME), which I can use to prefix the "//util/task:status" dependency in the `nanopb_cpp_library` macro. However, it doesn't seem to be possible to use it in the global scope where actual rules are defined. i.e:

```
nanopb_cpp_srcs = rule(
    implementation=_nanopb_cpp_srcs,
    attrs={
        "_nanopb_cpp_generator": attr.label(
                executable=True,
                default=Label(REPOSITORY_NAME + "//generator:nanopb_cpp_generator"),
        ),  
...
```

This fails with `name 'REPOSITORY_NAME' is not defined.`

Is it currently possible to reference labels in this repository in rule definitions?
REPOSITORY_NAME in rule definitions
issue
created
bsilver8192
username_1
What version are you using? This sounds like what was fixed in ee9e688c3df0d5c67f8bc7a2cc1d4a0a06229c66 and 0c8049f5fc866d785dd83769fa6c38ecf771ba96.
comment
created
prattmic
username_0
I am using 0.2.0(-jdk7), which it looks like should contain those commits?
comment
created
bsilver8192
username_1
Now that I look back at it, those commits were consistently making this behavior not happen. Looks like the currently recommended way to do this is by putting a `workspace(name = 'nanopb')` in nanopb's WORKSPACE and then referring to the labels as `@nanopb//...`. See eg [this mailing list discussion](https://groups.google.com/d/topic/bazel-discuss/9mY7WLSzth0/discussion).
comment
created
prattmic
username_0
It looks like referring to a named workspace (`workspace(name = 'nanopb'`) by name does not work within that workspace in 0.2.0 (`no such package '@nanopb//util/task': error loading package 'external': The repository named 'nanopb' could not be resolved and referenced by '//tests/cpp:simple'`)

However, from the [document](https://docs.google.com/document/d/1skNx5o-8k5-YXUAyEETvr39eKoh9fecJbGUquPh5iy8/edit#heading=h.ee9yd9yu4fqu) in the mailing list discussion, it sounds like this is planned to work eventually. I'll just wait out a bit longer.
comment
created
kchodorow
username_2
//generator:nanopb_generator should really refer to the repository-local target, it sounds like a skylark bug if it is not doing that.
comment
created
bsilver8192
username_1
@username_2: That's what I thought originally too, but default-repository labels in rule attributes actually mean local to the repository the rule is used in, not the one it's created in.
comment
created
kchodorow
username_2
That is so inconsistent with how labels in other places work that, even if it's intentional, I'd argue it's a bug.
comment
created
bsilver8192
username_1
Fair enough. However, I think "relative to the repository the rule is used in" is a useful behavior. For example, this is how the Go rules locate the `go_prefix` rule. What do you think is a reasonable syntax for that?
comment
created
kchodorow
username_2
What do you think that would be useful for?
comment
created
bsilver8192
username_1
Bazel's Go rules use it now, and it seems like a reasonable way to do it.
comment
created
kchodorow
username_2
I'm kind of confused about go_prefix. Shouldn't it be different depending on which repository you're referring to?  Like, say we have local_ws, repo1, and repo2.  If you're in @repo1 and using something from @repo2, shouldn't @repo2's go_prefix be `github.com/user/repo2`, not `github.com/user/local_ws`?
comment
created
bsilver8192
username_1
Yes. I believe that's the current behavior, because the `go_library` in `@repo2` will interpret the `//:go_prefix` default as `@repo2//:go_prefix` and the one in `@repo1` will get `@repo1//:go_prefix`.

If you change labels in the default repository to do what seems more obvious at first (refer to the repository the rule is created in), then they would both end up with `@bazel_tools//:go_prefix` which doesn't really work.
comment
created
kchodorow
username_2
Okay, IIUC, we want Skylark to use a different dependency, depending on which repo is using the rule.
That seems reasonable and I don't know what the syntax would look like, but theoretically we could just modify load() to provide Skylark with the repo it's called from.
comment
created
bsilver8192
username_1
I don't think it's load that wants to get modified. Providing a variable at load time doesn't help because the RuleClass (ie thing returned by the skylark `rule` function) is the same for usages from all repositories. Allowing the function implementing late bound attributes (ie when you give a function as the default value) to access REPOSITORY_NAME (or equivalent) seems like a reasonable way to get this behavior.
comment
created
kchodorow
username_2
I was thinking that you'd have something like:

```
my_rule = rule(
    attrs = {_sensitive_dep = attr.label(REPOSITORY_NAME_OR_WHATEVER + "//foo:bar")},
    ...
)
```

So you'd actually get a different rule depending on what repo it was loaded in.
comment
created
bsilver8192
username_1
Usage looks reasonable, but I think that's much harder to implement.
comment
created
jart
username_3
@username_1 Doesn't `@//foo` exist so you can reference the main workspace from an external workspace? If so, Go's bzl rules should probably be updated to use that convention.

It makes sense to me that, if a bzl rule is defined in `@A` then `Label("//foo")` should be the same as `Label("@A//foo")`.

The fact that it doesn't currently behave this way is actually creating problems for me. I'm working on Closure build rules in a new workspace repository. I've got a script I wrote called `//closure/compiler/jschecker`. But my `closure_js_library` rule (defined as `//closure/compiler:closure_js_library.bzl`) isn't able to reference jschecker. It works from within the Closure Rules workspace. But if I import Closure Rules into a different workspace, then it breaks, because Bazel tries to find jschecker in the other workspace.
comment
created
bsilver8192
username_1
@username_3: That does work (in theory; trying it right now will likely hit #940), but that's not what the Go rules want. The prefix needs to be set differently in each repository the rules are used from, not the one which declares the rules.

I agree how it works now is counterintuitive, but I think it is necessary to have this behavior somehow. Also, just changing what the obvious syntax means will break a variety of Skylark rules (I know I have some). Not necessarily a deal-breaker, but something else to keep in mind. @username_5: Any ideas?

For context, #700 was a related bug (although before the fix for that, labels were just interpreted as being from the main repository all the time).
comment
created
jart
username_3
I took a look at the Go rules sauce and I understand now what's going on with the `go_prefix()` rule. Yeah, the behavior is totally merited. Although it probably shouldn't be the default behavior. Perhaps that behavior could be defined as `Label("//:go_prefix", in_load_workspace=True)` or something.

Do you know if a workaround exists that would allow `Label()` to reference the workspace in which it's declared?
comment
created
bsilver8192
username_1
As a workaround, going through a `bind` (so you'd create one of those in `WORKSPACE` and then use `Label(//external:thing)`) should work. Not the greatest, but not all that bad.
comment
created
jart
username_3
Unfortunately `bind()` does not work. (Why does that feature even exist?) It still resolves against the calee workspace.
comment
created
shahms
username_4
(I suspect bsilver was suggesting you  "//external:thing" and require the enclosing repository bind that name appropriately).

Based on my experimentation[1](and a bit in the docs), it seems like the currently recommended practice is to pick a canonical name for your workspace and include:

workspace(name = "com_github_user_project")  # For example.

in the project WORKSPACE file.  This requires users of your rules to use the same name when including your project as an external repository.  You can then reference all of the tools as "@com_github_user_project//path/to:tool".  While I'm not a huge fan of mandating consistent and globally unique workspace names, it is the simplest way to keep label references consistent in the current dependency model (where external repository WORKSPACE files are largely ignored, AFAIK).


1. Setting up a simple example where a repository sets its own workspace name and is then included as a `local_repository` under a different name yields the warning: 
.../external/local_skylark/WORKSPACE:1: Workspace name in .../external/local_skylark/WORKSPACE (@skylark) does not match the name given in the repository's definition (@local_skylark); this will cause a build error in future versions.
comment
created
prattmic
username_0
@username_4 I [tried](https://github.com/bazelbuild/bazel/issues/975#issuecomment-190065598) what I believe you are suggesting, and it did not seem to work. It would seem that the `@name` syntax does not work to refer to the local workspace. (i.e., the rule couldn't be used within its original workspace).
comment
created
lberki
username_5
@Brian: I think the most reasonable approach we can do is to have a symbol
that's like REPOSITORY_NAME except that it evaluates to not the repository
the .bzl is loaded in, but the repository the .bzl file is *defined* in.
That shouldn't be too hard to implement and would be a reasonably clean
solution.

I agree that in an ideal world, default repository labels in .bzl files
would refer to the repository they the .bzl rules are in. I actually think
it would be a reasonably easy change, since the current behavior is weird
enough that I can't think of any use case (other than go_prefix, but i
don't know how our Go rules actually work, so I can't tell if that's in
turn a reasonable thing)

In the short term, adding a symbol like RULE_REPOSITORY_NAME will bring the
most bang for the buck.
comment
created
damienmg
username_6
@username_5 agree, do you want to signup for it? :)

`__WORKSPACE_NAME__` ?
comment
created
lberki
username_5
Of course. Put it at the end of my TODO list... :)
comment
created
jart
username_3
Oh here's a weird case with that solution. What if I define all my rules, one per bzl file. Then I have a single `defs.bzl` that loads all my rules, so the user of my workspace only has to worry about a single deps file, rather than the location of a whole bunch of them. In that case, the `RULE_REPOSITORY_NAME` solution won't work.
comment
created
lberki
username_5
I was hoping we could make it so that RULE_REPOSITORY_NAME returns the name
of the file where the rule() function was invoked (not the .bzl file
mentioned in the BUILD file). Then this would work.
comment
created
damienmg
username_6
For error tracking, we keep the path of the file that contains the definition, we should be able to do the same thing with the label of the file which then includes the repository.
comment
created
lberki
username_5
Yep, shouldn't be too hard once Someone (cue the old tale about Somebody,
Everybody, Anybody and Nobody) decides to act.
comment
created
shahms
username_4
Along with this, it would be nice to include the name on the repository_ctx for repository rules
comment
created
damienmg
username_6
What do you mean?
comment
created
shahms
username_4
When creating a repository via repository_rule(), there is no access to the name used to create it, e.g.

def _imp(ctx):
  # There is no access to the repo name from ctx
  # (i.e. the repository_rule equivalent of `ctx.label`)

dummy_repo = repository_rule(_impl, local=True)

def setup_dummy():
  dummy_repo(name = "foo")
comment
created
damienmg
username_6
Oh I see, can you create a separate bug?
comment
created
damienmg
username_6
After a offline discussion, we decided to set the workspace name correctly when using the Label function and adds a non-default option to that function for the legacy behavior (we know of only one weird use case).
comment
closed
kchodorow
username_2
issue
created
kchodorow
username_2
This should now be fixed, the related bugs are both fixed.
comment