From 2c046486f5febb9e475a5589b59abae77f14245e Mon Sep 17 00:00:00 2001 From: ichern Date: Fri, 2 Aug 2019 07:15:23 -0700 Subject: [PATCH] Fix git_repository rule to support fetching a commit on a tag - specified commit may only be present on a tag, not on any branch - use the verbose references specification in git fetch command - add black box test - initially implemented by https://github.com/jamiesnape in #8933, but the suggested solution with --tags flag does not work for CentOS Bazel CI configuration Closes #9059. PiperOrigin-RevId: 261316448 --- .../workspace/GitRepositoryBlackBoxTest.java | 51 ++++++++++++++++++- .../tests/workspace/GitRepositoryHelper.java | 12 +++++ tools/build_defs/repo/git_worker.bzl | 18 +++++-- 3 files changed, 77 insertions(+), 4 deletions(-) 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 index 92d626537755b2..27b0754f6ca5a3 100644 --- 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 @@ -51,7 +51,6 @@ * "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!"; @@ -192,6 +191,56 @@ public void testCheckoutOfCommitFromBranch() throws Exception { WorkspaceTestUtils.assertLinesExactly(outPath, HELLO_FROM_BRANCH); } + /** + * Tests usage of git_repository workspace rule in the particular use case, when only the commit + * hash is specified, and the commit is not in the HEAD-reachable subtree, on a separate tag and + * not on any branch. + */ + @Test + public void testCheckoutOfCommitFromTag() throws Exception { + Path repo = context().getTmpDir().resolve("tag_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 tag1"); + gitRepository.tag("tag1"); + String tagCommitHash = gitRepository.getHead(); + + gitRepository.checkout("master"); + + // delete branch, so that the last commit is not an any branch. + gitRepository.deleteBranch("demonstrate_branch"); + + 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',", tagCommitHash), + ")"); + + // 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); 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 index 0db27f38905ce6..4574fe70316fed 100644 --- 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 @@ -97,6 +97,18 @@ void createNewBranch(String branchName) throws Exception { runGit("checkout", "-b", branchName); } + /** + * Deletes the local branch with the specified name. + * + * @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 deleteBranch(String branchName) throws Exception { + runGit("branch", "-D", branchName); + } + /** * Checks out specified revision or reference. * diff --git a/tools/build_defs/repo/git_worker.bzl b/tools/build_defs/repo/git_worker.bzl index 97af0cc02d16f2..74caca0365f173 100644 --- a/tools/build_defs/repo/git_worker.bzl +++ b/tools/build_defs/repo/git_worker.bzl @@ -119,9 +119,21 @@ def add_origin(ctx, git_repo, 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") + # We need to explicitly specify to fetch all branches and tags, otherwise only + # HEAD-reachable is fetched. + # The semantics of --tags flag of git-fetch have changed in Git 1.9, from 1.9 it means + # "everything that is already specified and all tags"; before 1.9, it used to mean + # "ignore what is specified and fetch all tags". + # The arguments below work correctly for both before 1.9 and after 1.9, + # as we directly specify the list of references to fetch. + _git_maybe_shallow( + ctx, + git_repo, + "fetch", + "origin", + "refs/heads/*:refs/remotes/origin/*", + "refs/tags/*:refs/tags/*", + ) else: _git_maybe_shallow(ctx, git_repo, "fetch", "origin", git_repo.fetch_ref)