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

Hard to provider rules_go examples / tests #720

Closed
mattmoor opened this issue Aug 14, 2017 · 4 comments · Fixed by #733
Closed

Hard to provider rules_go examples / tests #720

mattmoor opened this issue Aug 14, 2017 · 4 comments · Fixed by #733

Comments

@mattmoor
Copy link

Challenge

I provide Bazel rules. I want to add Go examples / tests. I do NOT want to require all users of my rules to import rules_go.

Today to use rules_go, three main elements are required.

  1. Import rules_go in WORKSPACE
  2. go_prefix in /BUILD.
  3. go_binary in .../BUILD.

It is this second requirement that is problematic because it is on the BUILD path that Bazel loads for every .bzl file in the repo.

Strawman

I think the simplest solution would be to enable folks in my situation to override the baked-in //:prefix target, so that we can move it into .../BUILD and enable rule consumers to dodge the rules_go requirement.

@ianthehat
Copy link
Contributor

go_prefix is problematic for a couple of reasons, and we would like to get rid of it rather than fix it.
go_library was modified to support importpath a while ago. If that parameter is set, the _go_prefix is ignored, but unfortunately still has to be present.
The current plan is to change gazelle to always write out the importpath for all rules, remove the _go_prefix attribute and then finally deprecate and remove the go_prefix rule.
Would that meet your requirements?

@mattmoor
Copy link
Author

Even better, is there an issue I can follow, which once done will enable me to write things in a way that meets my needs?

@jayconrod
Copy link
Contributor

I think we've mentioned deprecating go_prefix in passing in a few places, but we didn't have an issue for it. I've created #721 to actually track this.

I'd like to understand your problem a little better though. Are you providing macros that instantiate Go rules in users' repositories? If so, I see how it can be annoying to require users to define their own go_prefix when it's apparently not needed.

Even if go_prefix is not required, your users will still need to include @io_bazel_rules_go as a dependency in their WORKSPACE file. WORKSPACE is a flat list of all dependencies, including transitives. You can hide this by declaring dependencies in a macro that you provide.

@mattmoor
Copy link
Author

Are you providing macros that instantiate Go rules in users' repositories?

In some cases yes, in others no. In the cases where I am, it is absolutely my expectation that folks would include the Go rules prior to mine.

I'm happy to GVC, it might be easier to explain? My username is my GitHub handle @google.com

jayconrod pushed a commit to jayconrod/rules_go that referenced this issue Aug 16, 2017
go_prefix is used to generate Go import paths from Bazel labels. Until
now, it has been mandatory, since there's an implicit dependency from
all go_library, go_binary, and go_test rules on //:go_prefix. With
this change, that dependency is not present when the importpath
attribute is specified. This means that if all of the Go rules in a
repository specify importpath, there's no need to define a go_prefix
rule in the repository root.

Fixes bazel-contrib#720
Related bazel-contrib#721
jayconrod added a commit that referenced this issue Aug 17, 2017
go_prefix is used to generate Go import paths from Bazel labels. Until
now, it has been mandatory, since there's an implicit dependency from
all go_library, go_binary, and go_test rules on //:go_prefix. With
this change, that dependency is not present when the importpath
attribute is specified. This means that if all of the Go rules in a
repository specify importpath, there's no need to define a go_prefix
rule in the repository root.

Fixes #720
Related #721
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants