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

Respect go_prefix attr of dependencies #30

Merged
merged 1 commit into from
May 30, 2016
Merged

Conversation

yugui
Copy link
Contributor

@yugui yugui commented May 30, 2016

Uses go_prefix attr of individual dependencies to calculate their
importpaths. This allows users to use Bazel's external dependency
resolution -- {,new_}git_repository rules -- to manage Go libraries.

c.f. #16 (comment)

Supports case 1 and 3 in #16.

Uses go_prefix attr of individual dependencies to calculate their
importpaths.  This allows users to use Bazel's external dependency
resolution -- {,new_}git_repository rules -- to manage Go libraries.

c.f. #16 (comment)
@bazel-io
Copy link

Can one of the admins verify this patch?

@yugui
Copy link
Contributor Author

yugui commented May 30, 2016

Jenkins: test this please.

@yugui
Copy link
Contributor Author

yugui commented May 30, 2016

Hmm, "test this please" does not work.
Does it mean I need to be a member of bazel organization but not just a collaborator of rules_go to trigger CI?

@damienmg
Copy link
Contributor

Yes, you need to be member of bazelbuild and make it public (https://github.com/orgs/bazelbuild/people/yugui)

Jenkins test this please.

remote = "https://github.com/golang/glog.git",
commit = "23def4e6c14b4da8ac2ed8007337bc5eb5007998",
build_file_content = GLOG_BUILD,
)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems like a pretty easy way to do a go_vendor macro rule:

TEMPLATE = """
load("@io_bazel_rules_go//go:def.bzl", "go_prefix", "go_library")

go_prefix("github.com/%s/%s")

go_library(name = "go_default_library", srcs = glob(["**/*.go"]), visibility = ["//visibility:public"])
"""

def go_vendor(org, project, commit):
  new_git_repository(name = "com_github_%s_%s" % (org, project), remote = "https://github.com/%s/%s.git" % (org, project), commit = commit, build_file_content = TEMPLATE % (org, project))

Copy link
Contributor

@damienmg damienmg May 30, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually io_bazel_rules_go won't work currently. You have to use a trick with str(Label(...)) for now.

Copy link
Contributor Author

@yugui yugui May 30, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@damienmg
I would like to keep go_vendor separated from this PR because other go libraries requires much more work. It it acceptable for you?

The simple macro works fine with glog but other common go libraries are more complex. e.g. https://github.com/golang/oauth2 requires:

  • parsing build tags
  • dependency analysis in the repository
  • external dependency

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes sounds reasonable.

I don't think you need to handle all the cases for an initial version but sure you might want to handle some more than a basic build.

@yugui
Copy link
Contributor Author

yugui commented May 30, 2016

@damienmg
The URL https://github.com/orgs/bazelbuild/people/yugui is invalid because I'm not a member of bazel organization.
Could you add me to the organization again?

@damienmg
Copy link
Contributor

LGTM

@damienmg
Copy link
Contributor

For the org: you have a pending invitation, you should be able to accept it.

@yugui
Copy link
Contributor Author

yugui commented May 30, 2016

I found the invitation mail in my inbox. Thank you!

@yugui yugui merged commit bb7e87b into master May 30, 2016
@yugui yugui deleted the feature/resolve-external branch June 2, 2016 04:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants