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

skylark: rule.output function patterns that expand to >1 file cause an error #2881

Closed
dslomov opened this issue Apr 25, 2017 · 2 comments
Closed
Assignees

Comments

@dslomov
Copy link
Contributor

dslomov commented Apr 25, 2017

Implicit output patterns defined in Skylark are artificially restricted to expand to at most one file. No such restriction exists for native rules such as proto_library. Consider:

$ cat t/BUILD 
proto_library(
    name = "a_proto",
    srcs = [
        "a.proto",
        "b.proto",
    ],
    cc_api_version = 2,
)

load(":inc.bzl", "my_proto_library")

my_proto_library(
    name = "my_proto_library",
    srcs = [
        "a.proto",
        "b.proto",
    ],
)

$ cat t/inc.bzl 

def _implicit_outputs(srcs):
  return {"": "%{srcs}.pb.go"}

def _implementation():
  pass

my_proto_library = rule(
    outputs = _implicit_outputs,
    implementation = _implementation,
    attrs = {
        "srcs": attr.label_list(),
    },
)

$ blaze query -k 'kind(generated, t:*)'
ERROR: google3/t/BUILD:12:1: google3/t/inc.bzl:8:20: expected one element but was: <a.pb.go, b.pb.go>
//t:b.proto.h
//t:b.pb.h
//t:b.pb.cc
//t:a.proto.h
//t:a.pb.h
//t:a.pb.cc

Notice that the native proto_library's outputs expand to [a.pb.h b.pb.h], but Blaze reports an error when my_proto_library tries to do the same thing using the "%{srcs}.pb.go" rule. I don't know why this restriction exists, but clearly it is an obstacle to Skylark rules achieving parity with native rules.

Changing my_proto_library to use a dict {"": "%{srcs}.pb.go"} instead of a function causes Blaze to emit the same error (and crash while doing so).

@dslomov dslomov added this to the 0.6 milestone Apr 25, 2017
@dslomov dslomov self-assigned this Apr 25, 2017
@dslomov
Copy link
Contributor Author

dslomov commented Apr 25, 2017

(this issue originally reported by @alandonovan)

@c-parsons
Copy link
Contributor

Lets close this, as I believe it's covered under #6241
In short, we're going to remove the output parameter of rule() altogether.

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

No branches or pull requests

3 participants