Repository: bazelbuild/bazel
Issue: Bazel CI: Downstream jobs are broken by git_repository refactoring
Labels: type: bug
P1
breakage
team-ExternalDeps

https://buildkite.com/bazel/bazel-at-head-plus-downstream/builds/1035

Bazel Codelabs
```
ERROR: An error occurred during the fetch of repository 'org_golang_google_grpc':
--
  | Traceback (most recent call last):
  | File "/var/lib/buildkite-agent/.cache/bazel/_bazel_buildkite-agent/9dba31623650bde83bfd5e4fc944f26a/external/bazel_tools/tools/build_defs/repo/git.bzl", line 158
  | _clone_or_update(ctx)
  | File "/var/lib/buildkite-agent/.cache/bazel/_bazel_buildkite-agent/9dba31623650bde83bfd5e4fc944f26a/external/bazel_tools/tools/build_defs/repo/git.bzl", line 31, in _clone_or_update
  | git_repo(ctx, directory)
  | File "/var/lib/buildkite-agent/.cache/bazel/_bazel_buildkite-agent/9dba31623650bde83bfd5e4fc944f26a/external/bazel_tools/tools/build_defs/repo/git_worker.bzl", line 91, in git_repo
  | _update(ctx, git_repo)
  | File "/var/lib/buildkite-agent/.cache/bazel/_bazel_buildkite-agent/9dba31623650bde83bfd5e4fc944f26a/external/bazel_tools/tools/build_defs/repo/git_worker.bzl", line 104, in _update
  | reset(ctx, git_repo)
  | File "/var/lib/buildkite-agent/.cache/bazel/_bazel_buildkite-agent/9dba31623650bde83bfd5e4fc944f26a/external/bazel_tools/tools/build_defs/repo/git_worker.bzl", line 124, in reset
  | _git(ctx, git_repo, "reset", "--hard", git_re...)
  | File "/var/lib/buildkite-agent/.cache/bazel/_bazel_buildkite-agent/9dba31623650bde83bfd5e4fc944f26a/external/bazel_tools/tools/build_defs/repo/git_worker.bzl", line 142, in _git
  | _error(ctx.name, (start + list(args)), st.s...)
  | File "/var/lib/buildkite-agent/.cache/bazel/_bazel_buildkite-agent/9dba31623650bde83bfd5e4fc944f26a/external/bazel_tools/tools/build_defs/repo/git_worker.bzl", line 164, in _error
  | fail(("error running '%s' while worki...)))
  | error running 'git reset --hard 2fdaae294f38ed9a121193c51ec99fecd3b13eb7' while working with @org_golang_google_grpc:
  | fatal: Could not parse object '2fdaae294f38ed9a121193c51ec99fecd3b13eb7'.

```
Culprit: 50a2d6c9b9c55776ec747a656a2edd971e723771
https://buildkite.com/bazel/culprit-finder/builds/165

/cc @irengrig 

FYI @buchgr for Green Team rotation.

Issue comments:
@philwo found out this important fix is lost after refactoring: https://github.com/bazelbuild/bazel/commit/39688b2454965eb45aa2f02a3f078adeee06ccd1
The breakage has been fixed by rollback

Pull request review comments:

Patch that resolved the issue:
diff --git a/tools/build_defs/repo/git.bzl b/tools/build_defs/repo/git.bzl
index 22692baf618b9d..22c07918bc39c7 100644
--- a/tools/build_defs/repo/git.bzl
+++ b/tools/build_defs/repo/git.bzl
@@ -13,7 +13,8 @@
 # limitations under the License.
 """Rules for cloning external git repositories."""
 
-load(":utils.bzl", "patch", "update_attrs", "workspace_and_buildfile")
+load("@bazel_tools//tools/build_defs/repo:utils.bzl", "patch", "update_attrs", "workspace_and_buildfile")
+load("@bazel_tools//tools/build_defs/repo:git_worker.bzl", "git_repo")
 
 def _clone_or_update(ctx):
     if ((not ctx.attr.tag and not ctx.attr.commit and not ctx.attr.branch) or
@@ -21,102 +22,22 @@ def _clone_or_update(ctx):
         (ctx.attr.tag and ctx.attr.branch) or
         (ctx.attr.commit and ctx.attr.branch)):
         fail("Exactly one of commit, tag, or branch must be provided")
-    shallow = ""
-    if ctx.attr.commit:
-        ref = ctx.attr.commit
-    elif ctx.attr.tag:
-        ref = "tags/" + ctx.attr.tag
-        shallow = "--depth=1"
-    else:
-        ref = ctx.attr.branch
-        shallow = "--depth=1"
-    directory = str(ctx.path("."))
+
+    root = ctx.path(".")
+    directory = str(root)
     if ctx.attr.strip_prefix:
         directory = directory + "-tmp"
-    if ctx.attr.shallow_since:
-        if ctx.attr.tag:
-            fail("shallow_since not allowed if a tag is specified; --depth=1 will be used for tags")
-        if ctx.attr.branch:
-            fail("shallow_since not allowed if a branch is specified; --depth=1 will be used for branches")
-        shallow = "--shallow-since=%s" % ctx.attr.shallow_since
-
-    ctx.report_progress("Cloning %s of %s" % (ref, ctx.attr.remote))
-    if (ctx.attr.verbose):
-        print("git.bzl: Cloning or updating %s repository %s using strip_prefix of [%s]" %
-              (
-                  " (%s)" % shallow if shallow else "",
-                  ctx.name,
-                  ctx.attr.strip_prefix if ctx.attr.strip_prefix else "None",
-              ))
-    bash_exe = ctx.os.environ["BAZEL_SH"] if "BAZEL_SH" in ctx.os.environ else "bash"
-    st = ctx.execute([bash_exe, "-c", """
-cd {working_dir}
-set -ex
-( cd {working_dir} &&
-    if ! ( cd '{dir_link}' && [[ "$(git rev-parse --git-dir)" == '.git' ]] ) >/dev/null 2>&1; then
-      rm -rf '{directory}' '{dir_link}'
-      git clone '{shallow}' '{remote}' '{directory}' || git clone '{remote}' '{directory}'
-    fi
-    git -C '{directory}' reset --hard {ref} || \
-    ((git -C '{directory}' fetch '{shallow}' origin {ref}:{ref} || \
-      git -C '{directory}' fetch origin {ref}:{ref}) && git -C '{directory}' reset --hard {ref})
-      git -C '{directory}' clean -xdf )
-  """.format(
-        working_dir = ctx.path(".").dirname,
-        dir_link = ctx.path("."),
-        directory = directory,
-        remote = ctx.attr.remote,
-        ref = ref,
-        shallow = shallow,
-    )], environment = ctx.os.environ)
 
-    if st.return_code:
-        fail("error cloning %s:\n%s" % (ctx.name, st.stderr))
+    git_ = git_repo(ctx, directory)
 
     if ctx.attr.strip_prefix:
         dest_link = "{}/{}".format(directory, ctx.attr.strip_prefix)
         if not ctx.path(dest_link).exists:
             fail("strip_prefix at {} does not exist in repo".format(ctx.attr.strip_prefix))
+        ctx.delete(root)
+        ctx.symlink(dest_link, root)
 
-        ctx.symlink(dest_link, ctx.path("."))
-    if ctx.attr.init_submodules:
-        ctx.report_progress("Updating submodules")
-        st = ctx.execute([bash_exe, "-c", """
-set -ex
-(   git -C '{directory}' submodule update --init --checkout --force )
-  """.format(
-            directory = ctx.path("."),
-        )], environment = ctx.os.environ)
-    if st.return_code:
-        fail("error updating submodules %s:\n%s" % (ctx.name, st.stderr))
-
-    ctx.report_progress("Recording actual commit")
-
-    # After the fact, determine the actual commit and its date
-    actual_commit = ctx.execute([
-        bash_exe,
-        "-c",
-        "(git -C '{directory}' log -n 1 --pretty='format:%H')".format(
-            directory = ctx.path("."),
-        ),
-    ]).stdout
-    shallow_date = ctx.execute([
-        bash_exe,
-        "-c",
-        "(git -C '{directory}' log -n 1 --pretty='format:%cd' --date=raw)".format(
-            directory = ctx.path("."),
-        ),
-    ]).stdout
-    return {"commit": actual_commit, "shallow_since": shallow_date}
-
-def _remove_dot_git(ctx):
-    # Remove the .git directory, if present
-    bash_exe = ctx.os.environ["BAZEL_SH"] if "BAZEL_SH" in ctx.os.environ else "bash"
-    ctx.execute([
-        bash_exe,
-        "-c",
-        "rm -rf '{directory}'".format(directory = ctx.path(".git")),
-    ])
+    return {"commit": git_.commit, "shallow_since": git_.shallow_since}
 
 def _update_git_attrs(orig, keys, override):
     result = update_attrs(orig, keys, override)
@@ -227,13 +148,13 @@ def _new_git_repository_implementation(ctx):
     update = _clone_or_update(ctx)
     workspace_and_buildfile(ctx)
     patch(ctx)
-    _remove_dot_git(ctx)
+    ctx.delete(ctx.path(".git"))
     return _update_git_attrs(ctx.attr, _new_git_repository_attrs.keys(), update)
 
 def _git_repository_implementation(ctx):
     update = _clone_or_update(ctx)
     patch(ctx)
-    _remove_dot_git(ctx)
+    ctx.delete(ctx.path(".git"))
     return _update_git_attrs(ctx.attr, _common_attrs.keys(), update)
 
 new_git_repository = repository_rule(
diff --git a/tools/build_defs/repo/git_worker.bzl b/tools/build_defs/repo/git_worker.bzl
new file mode 100644
index 00000000000000..c7e9148f148daa
--- /dev/null
+++ b/tools/build_defs/repo/git_worker.bzl
@@ -0,0 +1,169 @@
+# Copyright 2019 The Bazel Authors. All rights reserved.
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#    http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+"""Code for interacting with git binary to get the file tree checked out at the specified revision.
+"""
+
+_GitRepoInfo = provider(
+    doc = "Provider to organize precomputed arguments for calling git.",
+    fields = {
+        "directory": "Working directory path",
+        "shallow": "Defines the depth of a fetch. Either empty, --depth=1, or --shallow-since=<>",
+        "reset_ref": """Reference to use for resetting the git repository.
+Either commit hash, tag or branch.""",
+        "fetch_ref": """Reference for fetching. Can be empty (HEAD), tag or branch.
+Can not be a commit hash, since typically it is forbidden by git servers.""",
+        "remote": "URL of the git repository to fetch from.",
+        "init_submodules": """If True, submodules update command will be called after fetching
+and resetting to the specified reference.""",
+    },
+)
+
+def git_repo(ctx, directory):
+    """ Fetches data from git repository and checks out file tree.
+
+    Called by git_repository or new_git_repository rules.
+
+    Args:
+        ctx: Context of the calling rules, for reading the attributes.
+        Please refer to the git_repository and new_git_repository rules for the description.
+        directory: Directory where to check out the file tree.
+    Returns:
+        The struct with the following fields:
+        commit: Actual HEAD commit of the checked out data.
+        shallow_since: Actual date and time of the HEAD commit of the checked out data.
+    """
+    if ctx.attr.shallow_since:
+        if ctx.attr.tag:
+            fail("shallow_since not allowed if a tag is specified; --depth=1 will be used for tags")
+        if ctx.attr.branch:
+            fail("shallow_since not allowed if a branch is specified; --depth=1 will be used for branches")
+
+    shallow = "--depth=1"
+    if ctx.attr.commit:
+        # We can not use the commit value in --shallow-since;
+        # And since we are fetching HEAD in this case, we can not use --depth=1
+        shallow = ""
+
+    # Use shallow-since if given
+    if ctx.attr.shallow_since:
+        shallow = "--shallow-since=%s" % ctx.attr.shallow_since
+
+    reset_ref = ""
+    fetch_ref = ""
+    if ctx.attr.commit:
+        reset_ref = ctx.attr.commit
+    elif ctx.attr.tag:
+        reset_ref = "tags/" + ctx.attr.tag
+        fetch_ref = "tags/" + ctx.attr.tag + ":tags/" + ctx.attr.tag
+    elif ctx.attr.branch:
+        reset_ref = "origin/" + ctx.attr.branch
+        fetch_ref = ctx.attr.branch
+
+    git_repo = _GitRepoInfo(
+        directory = ctx.path(directory),
+        shallow = shallow,
+        reset_ref = reset_ref,
+        fetch_ref = fetch_ref,
+        remote = ctx.attr.remote,
+        init_submodules = ctx.attr.init_submodules,
+    )
+
+    ctx.report_progress("Cloning %s of %s" % (reset_ref, ctx.attr.remote))
+    if (ctx.attr.verbose):
+        print("git.bzl: Cloning or updating %s repository %s using strip_prefix of [%s]" %
+              (
+                  " (%s)" % shallow if shallow else "",
+                  ctx.name,
+                  ctx.attr.strip_prefix if ctx.attr.strip_prefix else "None",
+              ))
+
+    _update(ctx, git_repo)
+    ctx.report_progress("Recording actual commit")
+    actual_commit = _get_head_commit(ctx, git_repo)
+    shallow_date = _get_head_date(ctx, git_repo)
+
+    return struct(commit = actual_commit, shallow_since = shallow_date)
+
+def _update(ctx, git_repo):
+    ctx.delete(git_repo.directory)
+
+    init(ctx, git_repo)
+    add_origin(ctx, git_repo, ctx.attr.remote)
+    fetch(ctx, git_repo)
+    reset(ctx, git_repo)
+    clean(ctx, git_repo)
+
+    if git_repo.init_submodules:
+        ctx.report_progress("Updating submodules")
+        update_submodules(ctx, git_repo)
+
+def init(ctx, git_repo):
+    cl = ["git", "init", git_repo.directory]
+    st = ctx.execute(cl, environment = ctx.os.environ)
+    if st.return_code != 0:
+        _error(ctx.name, cl, st.stderr)
+
+def add_origin(ctx, git_repo, remote):
+    _git(ctx, git_repo, "remote", "add", "origin", remote)
+
+def fetch(ctx, git_repo):
+    if not git_repo.fetch_ref:
+        # We need to explicitly specify to fetch all branches, otherwise only HEAD-reachable
+        # is fetched.
+        _git_maybe_shallow(ctx, git_repo, "fetch", "--all")
+    else:
+        _git_maybe_shallow(ctx, git_repo, "fetch", "origin", git_repo.fetch_ref)
+
+def reset(ctx, git_repo):
+    _git(ctx, git_repo, "reset", "--hard", git_repo.reset_ref)
+
+def clean(ctx, git_repo):
+    _git(ctx, git_repo, "clean", "-xdf")
+
+def update_submodules(ctx, git_repo):
+    _git(ctx, git_repo, "submodule", "update", "--init", "--checkout", "--force")
+
+def _get_head_commit(ctx, git_repo):
+    return _git(ctx, git_repo, "log", "-n", "1", "--pretty=format:%H")
+
+def _get_head_date(ctx, git_repo):
+    return _git(ctx, git_repo, "log", "-n", "1", "--pretty=format:%cd", "--date=raw")
+
+def _git(ctx, git_repo, command, *args):
+    start = ["git", command]
+    st = _execute(ctx, git_repo, start + list(args))
+    if st.return_code != 0:
+        _error(ctx.name, start + list(args), st.stderr)
+    return st.stdout
+
+def _git_maybe_shallow(ctx, git_repo, command, *args):
+    start = ["git", command]
+    args_list = list(args)
+    if git_repo.shallow:
+        st = _execute(ctx, git_repo, start + [git_repo.shallow] + args_list)
+        if st.return_code == 0:
+            return
+    st = _execute(ctx, git_repo, start + args_list)
+    if st.return_code != 0:
+        _error(ctx.name, start + args_list, st.stderr)
+
+def _execute(ctx, git_repo, args):
+    return ctx.execute(
+        args,
+        environment = ctx.os.environ,
+        working_directory = str(git_repo.directory),
+    )
+
+def _error(name, command, stderr):
+    fail("error running '%s' while working with @%s:\n%s" % (" ".join(command).strip(), name, stderr))


Test patch:
diff --git a/src/test/java/com/google/devtools/build/lib/blackbox/framework/BlackBoxTestContext.java b/src/test/java/com/google/devtools/build/lib/blackbox/framework/BlackBoxTestContext.java
index 0525f34f82533f..98583f05b7fee9 100644
--- a/src/test/java/com/google/devtools/build/lib/blackbox/framework/BlackBoxTestContext.java
+++ b/src/test/java/com/google/devtools/build/lib/blackbox/framework/BlackBoxTestContext.java
@@ -1,16 +1,17 @@
-// Copyright 2018 The Bazel Authors. All rights reserved.
+// Copyright 2019 The Bazel Authors. All rights reserved.
 //
 // Licensed under the Apache License, Version 2.0 (the "License");
 // you may not use this file except in compliance with the License.
 // You may obtain a copy of the License at
 //
-// http://www.apache.org/licenses/LICENSE-2.0
+//    http://www.apache.org/licenses/LICENSE-2.0
 //
 // Unless required by applicable law or agreed to in writing, software
 // distributed under the License is distributed on an "AS IS" BASIS,
 // WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 // See the License for the specific language governing permissions and
 // limitations under the License.
+//
 
 package com.google.devtools.build.lib.blackbox.framework;
 
@@ -224,6 +225,32 @@ public BuilderRunner bazel() {
         workDir, binaryPath, getProcessTimeoutMillis(-1), commonEnv, executorService);
   }
 
+  /**
+   * Runs external binary in the specified working directory. See {@link BuilderRunner}
+   *
+   * @param workingDirectory working directory for running the binary
+   * @param processToRun path to the binary to run
+   * @param expectEmptyError if <code>true</code>, no text is expected in the error stream,
+   * otherwise, ProcessRunnerException is thrown.
+   * @param arguments arguments to pass to the binary
+   * @return ProcessResult execution result
+   */
+  public ProcessResult runBinary(Path workingDirectory,
+      String processToRun,
+      boolean expectEmptyError,
+      String... arguments)
+      throws Exception {
+    ProcessParameters parameters =
+        ProcessParameters.builder()
+            .setWorkingDirectory(workingDirectory.toFile())
+            .setName(processToRun)
+            .setTimeoutMillis(getProcessTimeoutMillis(-1))
+            .setArguments(arguments)
+            .setExpectedEmptyError(expectEmptyError)
+            .build();
+    return new ProcessRunner(parameters, executorService).runSynchronously();
+  }
+
   /**
    * Take the value from environment variable and assert that it is a path, and the file or
    * directory, specified by this path, exists.
diff --git a/src/test/java/com/google/devtools/build/lib/blackbox/tests/workspace/BUILD b/src/test/java/com/google/devtools/build/lib/blackbox/tests/workspace/BUILD
index ecf953667d55f6..349610d63496a1 100644
--- a/src/test/java/com/google/devtools/build/lib/blackbox/tests/workspace/BUILD
+++ b/src/test/java/com/google/devtools/build/lib/blackbox/tests/workspace/BUILD
@@ -51,11 +51,30 @@ java_test(
     ],
 )
 
+java_test(
+    name = "GitRepositoryBlackBoxTest",
+    timeout = "moderate",
+    srcs = [
+        "GitRepositoryBlackBoxTest.java",
+        "GitRepositoryHelper.java",
+        "RepoWithRuleWritingTextGenerator.java",
+        "WorkspaceTestUtils.java",
+    ],
+    tags = ["black_box_test"],
+    deps = common_deps + [
+        "//src/main/java/com/google/devtools/build/lib:build-base",
+        "//src/main/java/com/google/devtools/build/lib:bazel-repository",
+        "//src/main/java/com/google/devtools/build/lib/vfs",
+        "//src/test/java/com/google/devtools/build/lib:foundations_testutil",
+    ],
+)
+
 test_suite(
     name = "ws_black_box_tests",
     tags = ["black_box_test"],
     tests = [
         "BazelEmbeddedSkylarkBlackBoxTest",
+        "GitRepositoryBlackBoxTest",
         "RepoWithRuleWritingTextGeneratorTest",
         "WorkspaceBlackBoxTest",
     ],
diff --git a/src/test/java/com/google/devtools/build/lib/blackbox/tests/workspace/GitRepositoryBlackBoxTest.java b/src/test/java/com/google/devtools/build/lib/blackbox/tests/workspace/GitRepositoryBlackBoxTest.java
new file mode 100644
index 00000000000000..2ba2d43a9470ab
--- /dev/null
+++ b/src/test/java/com/google/devtools/build/lib/blackbox/tests/workspace/GitRepositoryBlackBoxTest.java
@@ -0,0 +1,200 @@
+// Copyright 2019 The Bazel Authors. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//    http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+//
+
+package com.google.devtools.build.lib.blackbox.tests.workspace;
+
+import static com.google.devtools.build.lib.blackbox.tests.workspace.RepoWithRuleWritingTextGenerator.callRule;
+import static com.google.devtools.build.lib.blackbox.tests.workspace.RepoWithRuleWritingTextGenerator.loadRule;
+
+import com.google.devtools.build.lib.blackbox.framework.BlackBoxTestContext;
+import com.google.devtools.build.lib.blackbox.framework.BuilderRunner;
+import com.google.devtools.build.lib.blackbox.framework.PathUtils;
+import com.google.devtools.build.lib.blackbox.junit.AbstractBlackBoxTest;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import org.junit.Test;
+
+/**
+ * Black box tests for git_repository/new_git_repository.
+ * On Windows, runs without MSYS {@link WorkspaceTestUtils#bazel}
+ *
+ * General approach to testing:
+ *
+ * We use {@link GitRepositoryHelper} and {@link RepoWithRuleWritingTextGenerator} helper classes.
+ *
+ * 1. We are creating some git repository with the preset contents, which will be used for
+ * fetching contents for the test. We plan to fetch contents specifying either commit hash,
+ * tag, or branch.
+ * For all test variants, we are creating the same repository, as the same HEAD commit
+ * is marked with a tag, and can be addressed with commit hash, master branch, and tag name.
+ *
+ * 2. The contents of the git repository working tree is generated by
+ * {@link RepoWithRuleWritingTextGenerator}. We pass some certain text to that generator;
+ * that exact text should appear as a result of the build of the generated target
+ * "call_write_text" in the file "out.txt".
+ *
+ * 3. We generate the new_git_repository repository rule, which refers to the git repository,
+ * created in #1, specifying different git repository attributes in each test.
+ * We call 'bazel build' for the "call_write_text" target of the external repository,
+ * and asserting the contents in the "out.txt" file.
+ */
+public class GitRepositoryBlackBoxTest extends AbstractBlackBoxTest {
+
+  private static final String HELLO_FROM_EXTERNAL_REPOSITORY = "Hello from GIT repository!";
+  private static final String HELLO_FROM_BRANCH = "Hello from branch!";
+
+  /**
+   * Tests usage of new_git_repository workspace rule with the "tag" attribute.
+   * Please see the general approach description in the class javadoc comment.
+   */
+  @Test
+  public void testCloneAtTag() throws Exception {
+    Path repo = context().getTmpDir().resolve("ext_repo");
+    setupGitRepository(context(), repo);
+
+    String buildFileContent = String.format("%s\n%s",
+        loadRule(""),
+        callRule("call_write_text", "out.txt", HELLO_FROM_EXTERNAL_REPOSITORY));
+    context().write("WORKSPACE",
+        "load(\"@bazel_tools//tools/build_defs/repo:git.bzl\", \"new_git_repository\")",
+        "new_git_repository(",
+        "  name='ext',",
+        String.format("  remote='%s',", PathUtils.pathToFileURI(repo.resolve(".git"))),
+        "  tag='first',",
+        String.format("  build_file_content=\"\"\"%s\"\"\",", buildFileContent),
+        ")");
+
+    // This creates Bazel without MSYS, see implementation for details.
+    BuilderRunner bazel = WorkspaceTestUtils.bazel(context());
+    bazel.build("@ext//:call_write_text");
+    Path outPath = context().resolveBinPath(bazel, "external/ext/out.txt");
+    WorkspaceTestUtils.assertLinesExactly(outPath, HELLO_FROM_EXTERNAL_REPOSITORY);
+  }
+
+  /**
+   * Tests usage of new_git_repository workspace rule with the "commit" attribute.
+   * Please see the general approach description in the class javadoc comment.
+   */
+  @Test
+  public void testCloneAtCommit() throws Exception {
+    Path repo = context().getTmpDir().resolve("ext_repo");
+    String commit = setupGitRepository(context(), repo);
+
+    String buildFileContent = String.format("%s\n%s",
+        loadRule(""),
+        callRule("call_write_text", "out.txt", HELLO_FROM_EXTERNAL_REPOSITORY));
+    context().write("WORKSPACE",
+        "load(\"@bazel_tools//tools/build_defs/repo:git.bzl\", \"new_git_repository\")",
+        "new_git_repository(",
+        "  name='ext',",
+        String.format("  remote='%s',", PathUtils.pathToFileURI(repo.resolve(".git"))),
+        String.format("  commit='%s',", commit),
+        String.format("  build_file_content=\"\"\"%s\"\"\",", buildFileContent),
+        ")");
+
+    // This creates Bazel without MSYS, see implementation for details.
+    BuilderRunner bazel = WorkspaceTestUtils.bazel(context());
+    bazel.build("@ext//:call_write_text");
+    Path outPath = context().resolveBinPath(bazel, "external/ext/out.txt");
+    WorkspaceTestUtils.assertLinesExactly(outPath, HELLO_FROM_EXTERNAL_REPOSITORY);
+  }
+
+  /**
+   * Tests usage of new_git_repository workspace rule with the "branch" attribute.
+   * Please see the general approach description in the class javadoc comment.
+   */
+  @Test
+  public void testCloneAtMaster() throws Exception {
+    Path repo = context().getTmpDir().resolve("ext_repo");
+    setupGitRepository(context(), repo);
+
+    String buildFileContent = String.format("%s\n%s",
+        loadRule(""),
+        callRule("call_write_text", "out.txt", HELLO_FROM_EXTERNAL_REPOSITORY));
+    context().write("WORKSPACE",
+        "load(\"@bazel_tools//tools/build_defs/repo:git.bzl\", \"new_git_repository\")",
+        "new_git_repository(",
+        "  name='ext',",
+        String.format("  remote='%s',", PathUtils.pathToFileURI(repo.resolve(".git"))),
+        "  branch='master',",
+        String.format("  build_file_content=\"\"\"%s\"\"\",", buildFileContent),
+        ")");
+
+    // This creates Bazel without MSYS, see implementation for details.
+    BuilderRunner bazel = WorkspaceTestUtils.bazel(context());
+    bazel.build("@ext//:call_write_text");
+    Path outPath = context().resolveBinPath(bazel, "external/ext/out.txt");
+    WorkspaceTestUtils.assertLinesExactly(outPath, HELLO_FROM_EXTERNAL_REPOSITORY);
+  }
+
+  @Test
+  public void testCheckoutOfCommitFromBranch() throws Exception {
+    Path repo = context().getTmpDir().resolve("branch_repo");
+    GitRepositoryHelper gitRepository = initGitRepository(context(), repo);
+
+    context().write(repo.resolve("master.marker").toString());
+    gitRepository.addAll();
+    gitRepository.commit("Initial commit");
+
+    gitRepository.createNewBranch("demonstrate_branch");
+
+    RepoWithRuleWritingTextGenerator generator = new RepoWithRuleWritingTextGenerator(repo);
+    generator.withOutputText(HELLO_FROM_BRANCH).setupRepository();
+
+    gitRepository.addAll();
+    gitRepository.commit("Commit in branch");
+    String branchCommitHash = gitRepository.getHead();
+
+    gitRepository.checkout("master");
+    generator.withOutputText(HELLO_FROM_EXTERNAL_REPOSITORY).setupRepository();
+    gitRepository.addAll();
+    gitRepository.commit("Commit in master");
+
+    context().write("WORKSPACE",
+        "load(\"@bazel_tools//tools/build_defs/repo:git.bzl\", \"git_repository\")",
+        "git_repository(",
+        "  name='ext',",
+        String.format("  remote='%s',", PathUtils.pathToFileURI(repo.resolve(".git"))),
+        String.format("  commit='%s',", branchCommitHash),
+        ")");
+
+    // This creates Bazel without MSYS, see implementation for details.
+    BuilderRunner bazel = WorkspaceTestUtils.bazel(context());
+    bazel.build("@ext//:write_text");
+    Path outPath = context().resolveBinPath(bazel, "external/ext/out");
+    WorkspaceTestUtils.assertLinesExactly(outPath, HELLO_FROM_BRANCH);
+  }
+
+  private static String setupGitRepository(BlackBoxTestContext context, Path repo) throws Exception {
+    GitRepositoryHelper gitRepository = initGitRepository(context, repo);
+
+    RepoWithRuleWritingTextGenerator generator = new RepoWithRuleWritingTextGenerator(repo);
+    generator.skipBuildFile().setupRepository();
+
+    gitRepository.addAll();
+    gitRepository.commit("Initial commit");
+    gitRepository.tag("first");
+    return gitRepository.getHead();
+  }
+
+  private static GitRepositoryHelper initGitRepository(BlackBoxTestContext context, Path repo)
+      throws Exception {
+    PathUtils.deleteTree(repo);
+    Files.createDirectories(repo);
+    GitRepositoryHelper gitRepository = new GitRepositoryHelper(context, repo);
+    gitRepository.init();
+    return gitRepository;
+  }
+}
diff --git a/src/test/java/com/google/devtools/build/lib/blackbox/tests/workspace/GitRepositoryHelper.java b/src/test/java/com/google/devtools/build/lib/blackbox/tests/workspace/GitRepositoryHelper.java
new file mode 100644
index 00000000000000..5b3000bdbaef05
--- /dev/null
+++ b/src/test/java/com/google/devtools/build/lib/blackbox/tests/workspace/GitRepositoryHelper.java
@@ -0,0 +1,122 @@
+// Copyright 2019 The Bazel Authors. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//    http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+//
+
+package com.google.devtools.build.lib.blackbox.tests.workspace;
+
+import com.google.devtools.build.lib.blackbox.framework.BlackBoxTestContext;
+import com.google.devtools.build.lib.blackbox.framework.ProcessResult;
+import java.nio.file.Path;
+
+/**
+ * Helper class for working with local git repository in tests.
+ * Should not be used outside ot tests.
+ */
+class GitRepositoryHelper {
+  private final BlackBoxTestContext context;
+  private final Path root;
+
+  /**
+   * Constructs the helper.
+   * @param context {@link BlackBoxTestContext} for running git process
+   * @param root working directory for running git process, expected to be existing.
+   */
+  GitRepositoryHelper(BlackBoxTestContext context, Path root) {
+    this.context = context;
+    this.root = root;
+  }
+
+  /**
+   * Calls 'git init' and 'git config' for specifying test user and email.
+   * @throws Exception related to the invocation of the external git process
+   * (like IOException or TimeoutException) or
+   * ProcessRunnerException if the process returned not expected return code.
+   */
+  void init() throws Exception {
+    runGit("init");
+    runGit("config", "user.email", "me@example.com");
+    runGit("config", "user.name", "E X Ample");
+  }
+
+  /**
+   * Recursively updates git index for all the files and directories under the working directory.
+   *
+   * @throws Exception related to the invocation of the external git process
+   * (like IOException or TimeoutException) or
+   * ProcessRunnerException if the process returned not expected return code.
+   */
+  void addAll() throws Exception {
+    runGit("add", ".");
+  }
+
+  /**
+   * Commits all staged changed.
+   *
+   * @param commitMessage commit message
+   * @throws Exception related to the invocation of the external git process
+   * (like IOException or TimeoutException) or
+   * ProcessRunnerException if the process returned not expected return code.
+   */
+  void commit(String commitMessage) throws Exception {
+    runGit("commit", "-m", commitMessage);
+  }
+
+  /**
+   * Tags the HEAD commit.
+   * @param tagName tag name
+   * @throws Exception related to the invocation of the external git process
+   * (like IOException or TimeoutException) or
+   * ProcessRunnerException if the process returned not expected return code.
+   */
+  void tag(String tagName) throws Exception {
+    runGit("tag", tagName);
+  }
+
+  /**
+   * Creates the new branch with the specified name at HEAD.
+   * @param branchName branch name
+   * @throws Exception related to the invocation of the external git process
+   * (like IOException or TimeoutException) or
+   * ProcessRunnerException if the process returned not expected return code.
+   */
+  void createNewBranch(String branchName) throws Exception {
+    runGit("checkout", "-b", branchName);
+  }
+
+  /**
+   * Checks out specified revision or reference.
+   * @param ref reference to check out
+   * @throws Exception related to the invocation of the external git process
+   * (like IOException or TimeoutException) or
+   * ProcessRunnerException if the process returned not expected return code.
+   */
+  void checkout(String ref) throws Exception {
+    runGit("checkout", ref);
+  }
+
+  /**
+   * Returns the HEAD's commit hash.
+   * @throws Exception related to the invocation of the external git process
+   * (like IOException or TimeoutException) or
+   * ProcessRunnerException if the process returned not expected return code.
+   */
+  String getHead() throws Exception {
+    return runGit("rev-parse", "--short", "HEAD");
+  }
+
+  private String runGit(String... arguments) throws Exception {
+    ProcessResult result = context.runBinary(root, "git", false, arguments);
+    return result.outString();
+  }
+}
diff --git a/src/test/java/com/google/devtools/build/lib/blackbox/tests/workspace/RepoWithRuleWritingTextGenerator.java b/src/test/java/com/google/devtools/build/lib/blackbox/tests/workspace/RepoWithRuleWritingTextGenerator.java
index 2ebf4ce697bdc3..a7b4a4419a0634 100644
--- a/src/test/java/com/google/devtools/build/lib/blackbox/tests/workspace/RepoWithRuleWritingTextGenerator.java
+++ b/src/test/java/com/google/devtools/build/lib/blackbox/tests/workspace/RepoWithRuleWritingTextGenerator.java
@@ -58,6 +58,7 @@ public class RepoWithRuleWritingTextGenerator {
   private String target;
   private String outputText;
   private String outFile;
+  private boolean generateBuildFile;
 
   /**
    * Generator constructor
@@ -69,6 +70,7 @@ public class RepoWithRuleWritingTextGenerator {
     this.target = TARGET;
     this.outputText = HELLO;
     this.outFile = OUT_FILE;
+    generateBuildFile = true;
   }
 
   /**
@@ -104,6 +106,16 @@ RepoWithRuleWritingTextGenerator withOutFile(String name) {
     return this;
   }
 
+  /**
+   * Specifies that BUILD file should not be generated
+   *
+   * @return this generator
+   */
+  RepoWithRuleWritingTextGenerator skipBuildFile() {
+    generateBuildFile = false;
+    return this;
+  }
+
   /**
    * Generates the repository: WORKSPACE, BUILD, and helper.bzl files.
    *
@@ -113,13 +125,15 @@ RepoWithRuleWritingTextGenerator withOutFile(String name) {
   Path setupRepository() throws IOException {
     Path workspace = PathUtils.writeFileInDir(root, "WORKSPACE");
     PathUtils.writeFileInDir(root, HELPER_FILE, WRITE_TEXT_TO_FILE);
-    PathUtils.writeFileInDir(
-        root,
-        "BUILD",
-        "load(\"@bazel_tools//tools/build_defs/pkg:pkg.bzl\", \"pkg_tar\")",
-        loadRule(""),
-        callRule(target, outFile, outputText),
-        String.format("pkg_tar(name = \"%s\", srcs = glob([\"*\"]),)", getPkgTarTarget()));
+    if (generateBuildFile) {
+      PathUtils.writeFileInDir(
+          root,
+          "BUILD",
+          "load(\"@bazel_tools//tools/build_defs/pkg:pkg.bzl\", \"pkg_tar\")",
+          loadRule(""),
+          callRule(target, outFile, outputText),
+          String.format("pkg_tar(name = \"%s\", srcs = glob([\"*\"]),)", getPkgTarTarget()));
+    }
     return workspace.getParent();
   }
 
diff --git a/src/test/shell/bazel/skylark_git_repository_test.sh b/src/test/shell/bazel/skylark_git_repository_test.sh
index c4e14982ef57b3..ca4840e1f6e09b 100755
--- a/src/test/shell/bazel/skylark_git_repository_test.sh
+++ b/src/test/shell/bazel/skylark_git_repository_test.sh
@@ -170,7 +170,7 @@ function test_git_repository_strip_prefix() {
 
 function test_git_repository_shallow_since() {
     # This date is the day the commit was made.
-    do_git_repository_test "52f9a3f87a2dd17ae0e5847bbae9734f09354afd" "" "2016-07-16"
+    do_git_repository_test "52f9a3f87a2dd17ae0e5847bbae9734f09354afd" "" "2015-07-16"
 }
 function test_new_git_repository_with_build_file() {
   do_new_git_repository_test "0-initial" "build_file"
@@ -640,7 +640,7 @@ EOF
 
   bazel fetch //planets:planet-info >& $TEST_log \
     || echo "Expect run to fail."
-  expect_log "error cloning"
+  expect_log "error running 'git fetch origin' while working with @pluto"
 }
 
 run_suite "skylark git_repository tests"