From 87cdda3fc0fd65c63ef0316533be03ea4956f809 Mon Sep 17 00:00:00 2001 From: Jay Conrod Date: Fri, 7 Apr 2017 13:53:25 -0400 Subject: [PATCH] Cherrypick 0.4.2 (#367) Cherrypicked the following changes: * #319 go_repository: add importpath and vcs flags. This allows us to have separate import paths and remote URLs for repositories. * #362 Fetch buildifier from new location. This allows the buildifier zip to be extracted with the new name. --- README.md | 34 ++++++--- go/private/go_repositories.bzl | 10 +-- go/private/go_repository.bzl | 13 ++-- go/tools/fetch_repo/BUILD | 15 +++- go/tools/fetch_repo/fetch_repo_test.go | 96 ++++++++++++++++++++++++++ go/tools/fetch_repo/main.go | 45 ++++++++++-- 6 files changed, 189 insertions(+), 24 deletions(-) create mode 100644 go/tools/fetch_repo/fetch_repo_test.go diff --git a/README.md b/README.md index 133a2d75b2..10f4ac4609 100644 --- a/README.md +++ b/README.md @@ -148,13 +148,16 @@ placed in the WORKSPACE. ## go\_repository ```bzl -go_repository(name, importpath, remote, commit, tag) +go_repository(name, importpath, remote, vcs, commit, tag) ``` Fetches a remote repository of a Go project, expecting it contains `BUILD` files. It is an analogy to `git_repository` but it recognizes importpath redirection of Go. +Either `importpath` or `remote` may be specified. To bypass importpath +redirection, specify both `remote` and `vcs`. + @@ -176,7 +179,7 @@ redirection of Go. @@ -185,8 +188,16 @@ redirection of Go. + + + + @@ -213,7 +224,7 @@ redirection of Go. ## new\_go\_repository ```bzl -new_go_repository(name, importpath, remote, commit, tag) +new_go_repository(name, importpath, remote, vcs, commit, tag) ``` Fetches a remote repository of a Go project and automatically generates @@ -241,7 +252,7 @@ importpath redirection of Go. @@ -250,8 +261,15 @@ importpath redirection of Go. + + + + diff --git a/go/private/go_repositories.bzl b/go/private/go_repositories.bzl index 3e303c8ab3..9eb609f749 100644 --- a/go/private/go_repositories.bzl +++ b/go/private/go_repositories.bzl @@ -15,9 +15,9 @@ load("//go/private:go_repository.bzl", "buildifier_repository_only_for_internal_use", "new_go_repository") repository_tool_deps = { - 'buildifier': struct( + 'buildtools': struct( importpath = 'github.com/bazelbuild/buildifier', - repo = 'https://github.com/bazelbuild/buildifier', + repo = 'https://github.com/bazelbuild/buildtools', commit = '81c36a8418cb803d381335c95f8bb419ad1efa27', ), 'tools': struct( @@ -33,8 +33,10 @@ def go_internal_tools_deps(): # TODO(yugui) Simply use go_repository when we drop support of Bazel 0.3.2. buildifier_repository_only_for_internal_use( name = "com_github_bazelbuild_buildifier", - commit = repository_tool_deps['buildifier'].commit, - importpath = repository_tool_deps['buildifier'].importpath, + commit = repository_tool_deps['buildtools'].commit, + importpath = repository_tool_deps['buildtools'].importpath, + remote = repository_tool_deps['buildtools'].repo, + vcs = 'git', ) new_go_repository( diff --git a/go/private/go_repository.bzl b/go/private/go_repository.bzl index f2b5d7d4f7..5d1b7f940d 100644 --- a/go/private/go_repository.bzl +++ b/go/private/go_repository.bzl @@ -27,14 +27,18 @@ def _go_repository_impl(ctx): # TODO(yugui): support submodule? # c.f. https://www.bazel.io/versions/master/docs/be/workspace.html#git_repository.init_submodules - remote = ctx.attr.remote if ctx.attr.remote else ctx.attr.importpath + remote = ctx.attr.remote + vcs = ctx.attr.vcs + importpath = ctx.attr.importpath result = ctx.execute([ fetch_repo, '--dest', ctx.path(''), '--remote', remote, - '--rev', rev]) + '--rev', rev, + '--vcs', vcs, + '--importpath', importpath]) if result.return_code: - fail("failed to fetch %s: %s" % (remote, result.stderr)) + fail("failed to fetch %s: %s" % (ctx.name, result.stderr)) def _new_go_repository_impl(ctx): @@ -58,8 +62,9 @@ def _new_go_repository_impl(ctx): _go_repository_attrs = { "build_file_name": attr.string(), - "importpath": attr.string(mandatory = True), + "importpath": attr.string(), "remote": attr.string(), + "vcs": attr.string(default="", values=["", "git", "hg", "svn", "bzr"]), "commit": attr.string(), "tag": attr.string(), "build_tags": attr.string_list(), diff --git a/go/tools/fetch_repo/BUILD b/go/tools/fetch_repo/BUILD index af27089932..acc536a2d7 100644 --- a/go/tools/fetch_repo/BUILD +++ b/go/tools/fetch_repo/BUILD @@ -1,7 +1,20 @@ -load("//go:def.bzl", "go_binary") +load("//go:def.bzl", "go_binary", "go_library", "go_test") go_binary( name = "fetch_repo", + library = ":go_default_library", +) + +go_library( + name = "go_default_library", srcs = ["main.go"], + visibility = ["//visibility:private"], + deps = ["@org_golang_x_tools//go/vcs:go_default_library"], +) + +go_test( + name = "go_default_test", + srcs = ["fetch_repo_test.go"], + library = ":go_default_library", deps = ["@org_golang_x_tools//go/vcs:go_default_library"], ) diff --git a/go/tools/fetch_repo/fetch_repo_test.go b/go/tools/fetch_repo/fetch_repo_test.go new file mode 100644 index 0000000000..d0573490e7 --- /dev/null +++ b/go/tools/fetch_repo/fetch_repo_test.go @@ -0,0 +1,96 @@ +package main + +import ( + "os" + "reflect" + "testing" + + "golang.org/x/tools/go/vcs" +) + +var ( + root = &vcs.RepoRoot{ + VCS: vcs.ByCmd("git"), + Repo: "https://github.com/bazeltest/rules_go", + Root: "github.com/bazeltest/rules_go", + } +) + +func TestMain(m *testing.M) { + // Replace vcs.RepoRootForImportPath to disable any network calls. + repoRootForImportPath = func(_ string, _ bool) (*vcs.RepoRoot, error) { + return root, nil + } + os.Exit(m.Run()) +} + +func TestGetRepoRoot(t *testing.T) { + for _, tc := range []struct { + label string + remote string + cmd string + importpath string + r *vcs.RepoRoot + }{ + { + label: "all", + remote: "https://github.com/bazeltest/rules_go", + cmd: "git", + importpath: "github.com/bazeltest/rules_go", + r: root, + }, + { + label: "different remote", + remote: "https://example.com/rules_go", + cmd: "git", + importpath: "github.com/bazeltest/rules_go", + r: &vcs.RepoRoot{ + VCS: vcs.ByCmd("git"), + Repo: "https://example.com/rules_go", + Root: "github.com/bazeltest/rules_go", + }, + }, + { + label: "only importpath", + importpath: "github.com/bazeltest/rules_go", + r: root, + }, + } { + r, err := getRepoRoot(tc.remote, tc.cmd, tc.importpath) + if err != nil { + t.Errorf("[%s] %v", tc.label, err) + } + if !reflect.DeepEqual(r, tc.r) { + t.Errorf("[%s] Expected %+v, got %+v", tc.label, tc.r, r) + } + } +} + +func TestGetRepoRoot_error(t *testing.T) { + for _, tc := range []struct { + label string + remote string + cmd string + importpath string + }{ + { + label: "importpath as remote", + remote: "github.com/bazeltest/rules_go", + }, + { + label: "missing vcs", + remote: "https://github.com/bazeltest/rules_go", + importpath: "github.com/bazeltest/rules_go", + }, + { + label: "missing remote", + cmd: "git", + importpath: "github.com/bazeltest/rules_go", + }, + } { + r, err := getRepoRoot(tc.remote, tc.cmd, tc.importpath) + if err == nil { + t.Errorf("[%s] expected error. Got %+v", tc.label, r) + } + } +} diff --git a/go/tools/fetch_repo/main.go b/go/tools/fetch_repo/main.go index 92e6d7626a..d39932025e 100644 --- a/go/tools/fetch_repo/main.go +++ b/go/tools/fetch_repo/main.go @@ -19,19 +19,50 @@ import ( ) var ( - remote = flag.String("remote", "", "Go importpath to the repository fetch") - rev = flag.String("rev", "", "target revision") - dest = flag.String("dest", "", "destination directory") + remote = flag.String("remote", "", "The URI of the remote repository. Must be used with the --vcs flag.") + cmd = flag.String("vcs", "", "Version control system to use to fetch the repository. Should be one of: git,hg,svn,bzr. Must be used with the --remote flag.") + rev = flag.String("rev", "", "target revision") + dest = flag.String("dest", "", "destination directory") + importpath = flag.String("importpath", "", "Go importpath to the repository fetch") + + // Used for overriding in tests to disable network calls. + repoRootForImportPath = vcs.RepoRootForImportPath ) +func getRepoRoot(remote, cmd, importpath string) (*vcs.RepoRoot, error) { + if (cmd == "") != (remote == "") { + return nil, fmt.Errorf("--remote should be used with the --vcs flag. If this is an import path, use --importpath instead.") + } + + if cmd != "" && remote != "" { + v := vcs.ByCmd(cmd) + if v == nil { + return nil, fmt.Errorf("invalid VCS type: %s", cmd) + } + return &vcs.RepoRoot{ + VCS: v, + Repo: remote, + Root: importpath, + }, nil + } + + // User did not give us complete information for VCS / Remote. + // Try to figure out the information from the import path. + r, err := repoRootForImportPath(importpath, true) + if err != nil { + return nil, err + } + if importpath != r.Root { + return nil, fmt.Errorf("not a root of a repository: %s", importpath) + } + return r, nil +} + func run() error { - r, err := vcs.RepoRootForImportPath(*remote, true) + r, err := getRepoRoot(*remote, *cmd, *importpath) if err != nil { return err } - if *remote != r.Root { - return fmt.Errorf("not a root of a repository: %s", *remote) - } return r.VCS.CreateAtRev(*dest, r.Repo, *rev) }
importpath - String, required + String, optional

An import path in Go, which also provides a default value for the root of the target remote repository

remote String, optional -

The root of the target remote repository, if this differs from the - value of importpath

+

The URI of the target remote repository, if this cannot be determined + from the value of importpath.

+
vcs + String, optional +

The version control system to use for fetching the repository. Useful + for disabling importpath redirection if necessary.
importpath - String, required + String, optional

An import path in Go, which also provides a default value for the root of the target remote repository

remote String, optional -

The root of the target remote repository, if this differs from the - value of importpath

+

The URI of the target remote repository, if this differs from the + value of importpath

+
vcs + String, optional +

The version control system to use for fetching the repository.