-
Notifications
You must be signed in to change notification settings - Fork 4.6k
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
bazel updates to generate go-bindata #3689
bazel updates to generate go-bindata #3689
Conversation
This is awesome. One gotcha - this doesn't update the bindata.go in the directory AFAICT. (Rather it updates a generated copy). Not entirely sure what we should do about that, TBH. I suspect we should probably just accept that checking in these files is good neither for a Makefile nor bazel. But OTOH kops should be |
@justinsb so https://docs.bazel.build/versions/master/be/general.html#genrule.outs is saying we cannot do that by default. We can output to bin dir or the generated sources dir. Am I missing something? I am guessing we would need to write a custom rule. I would rather not. This is kinda how bazel works, and we should not change that just to have it work the way we had it working before. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bazel will prefer the bindata.go
generated by the genrule over the version in the source tree.
upup/models/BUILD.bazel
Outdated
|
||
genrule( | ||
name = "bindata", | ||
srcs = glob(["*","*/*","*/*/*","*/*/*/*","*/*/*/*/*"], exclude=["bindata.go", "vfs.go"]), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you can just do glob(["**"], exclude=...)
here for the same effect.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks
federation/model/BUILD.bazel
Outdated
|
||
genrule( | ||
name = "bindata", | ||
srcs = glob(["*"], exclude=["bindata.go"]), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe exclude the BUILD.bazel
file here too
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missed that
federation/model/BUILD.bazel
Outdated
$(location //vendor/github.com/jteeuwen/go-bindata/go-bindata:go-bindata) \ | ||
-o "$(OUTS)" -pkg model \ | ||
-prefix $$(pwd) \ | ||
-ignore bindata.go -ignore BUILD.bazel \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
... which would probably let you omit this.
f418f9d
to
4179c76
Compare
/lgtm |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: justinsb The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these OWNERS Files:
You can indicate your approval by writing |
/test all [submit-queue is verifying that this PR is safe to merge] |
Automatic merge from submit-queue. |
Fixes: #3626