Cherrypicks for 0.3.4 release (#366)

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

@@ -146,13 +146,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`.
+
 <table class="table table-condensed table-bordered table-params">
   <colgroup>
     <col class="col-param" />
@@ -174,7 +177,7 @@ redirection of Go.
     <tr>
       <td><code>importpath</code></td>
       <td>
-        <code>String, required</code>
+        <code>String, optional</code>
         <p>An import path in Go, which also provides a default value for the
 	root of the target remote repository</p>
       </td>
@@ -183,8 +186,16 @@ redirection of Go.
       <td><code>remote</code></td>
       <td>
         <code>String, optional</code>
-        <p>The root of the target remote repository, if this differs from the
-	value of <code>importpath</code></p>
+        <p>The URI of the target remote repository, if this cannot be determined
+        from the value of <code>importpath</code>.</p>
+      </td>
+    </tr>
+    <tr>
+      <td><code>vcs</code></td>
+      <td>
+        <code>String, optional</code>
+        <p>The version control system to use for fetching the repository. Useful
+        for disabling importpath redirection if necessary.</p>
       </td>
     </tr>
     <tr>
@@ -211,7 +222,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
@@ -239,7 +250,7 @@ importpath redirection of Go.
     <tr>
       <td><code>importpath</code></td>
       <td>
-        <code>String, required</code>
+        <code>String, optional</code>
         <p>An import path in Go, which also provides a default value for the
 	root of the target remote repository</p>
       </td>
@@ -248,8 +259,15 @@ importpath redirection of Go.
       <td><code>remote</code></td>
       <td>
         <code>String, optional</code>
-        <p>The root of the target remote repository, if this differs from the
-	value of <code>importpath</code></p>
+        <p>The URI of the target remote repository, if this differs from the
+        value of <code>importpath</code></p>
+      </td>
+    </tr>
+    <tr>
+      <td><code>vcs</code></td>
+      <td>
+        <code>String, optional</code>
+        <p>The version control system to use for fetching the repository.</p>
       </td>
     </tr>
     <tr>

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 = '4190564903f61ddc94bcfda3dc2cdd32d4b330e5',
     ),
     '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 = "io_bazel_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(

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(),

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"],
 )

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)
+		}
+	}
+}

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)
 }