diff --git a/src/test/java/com/google/devtools/build/lib/blackbox/framework/BuilderRunner.java b/src/test/java/com/google/devtools/build/lib/blackbox/framework/BuilderRunner.java index 14892fcfb6c03d..00dc60d2f6804d 100644 --- a/src/test/java/com/google/devtools/build/lib/blackbox/framework/BuilderRunner.java +++ b/src/test/java/com/google/devtools/build/lib/blackbox/framework/BuilderRunner.java @@ -236,6 +236,22 @@ public ProcessResult build(String... args) throws Exception { return runBinary("build", args); } + /** + * Runs bazel cquery <args> and returns the result. Asserts that the process + * exit code is zero. Does not assert that the error stream is empty. + * + * @param args arguments to pass to cquery command + * @return ProcessResult with process exit code, strings with stdout and error streams contents + * @throws TimeoutException in case of timeout + * @throws IOException in case of the process startup/interaction problems + * @throws InterruptedException if the current thread is interrupted while waiting + * @throws ProcessRunnerException if the process return code is not zero or error stream is not + * empty when it was expected + */ + public ProcessResult cquery(String... args) throws Exception { + return runBinary("cquery", args); + } + /** * Runs bazel query <args> and returns the result. Asserts that the process * exit code is zero. Does not assert that the error stream is empty. 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 118a143b8e2629..b998818e3595d3 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 @@ -16,16 +16,24 @@ package com.google.devtools.build.lib.blackbox.tests.workspace; import static com.google.common.truth.Truth.assertThat; +import static com.google.common.truth.Truth.assertWithMessage; 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 static java.util.Map.entry; +import static java.util.stream.Collectors.toMap; 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.framework.ProcessResult; import com.google.devtools.build.lib.blackbox.junit.AbstractBlackBoxTest; +import com.google.devtools.build.lib.util.OS; import java.nio.file.Files; import java.nio.file.Path; +import java.util.ArrayList; +import java.util.HashMap; +import java.util.List; +import java.util.Map; import org.junit.Test; /** @@ -302,6 +310,139 @@ public void testGitRepositoryErrorMessage() throws Exception { assertThat(result.errString()).contains("fatal: Could not read from remote repository."); } + /** + * Tests that git_repository ignores a host core.autocrlf (the LF-to-CRLF checkout rewrite that is + * prevalent on Windows), supplied here via $HOME/.gitconfig. Honoring it would tie the checkout + * to the host's git setup (CRLF where autocrlf is set, LF elsewhere), so fetches would not be + * reproducible. + */ + @Test + public void testCheckoutIgnoresHostAutocrlf() throws Exception { + assertCheckoutIgnoresHostGitConfig( + scratch -> { + Path home = Files.createDirectories(scratch.resolve("home")); + Files.writeString( + home.resolve(".gitconfig"), + """ + [core] + \tautocrlf = true + """); + return Map.ofEntries( + entry("HOME", home.toString()), // git also honors HOME (over USERPROFILE) on Windows + entry("GIT_CONFIG_NOSYSTEM", "1")); // neutralize host /etc/gitconfig in this test + }); + } + + /** + * Tests that git_repository ignores a host core.eol (which sets the working-tree EOL of + * text-marked files on checkout), supplied here via a GIT_CONFIG_SYSTEM file plus a global + * text=auto attribute so the files count as text. Honoring it would tie the checkout to the + * host's git setup, so fetches would not be reproducible. + */ + @Test + public void testCheckoutIgnoresHostEol() throws Exception { + assertCheckoutIgnoresHostGitConfig( + scratch -> { + // core.eol only converts files marked text, so plant an ambient "* text=auto". + Path etc = Files.createDirectories(scratch.resolve("etc")); + Path gitattributes = etc.resolve("gitattributes"); + Files.writeString( + gitattributes, + """ + * text=auto + """); + Path gitconfig = etc.resolve("gitconfig"); + Files.writeString( + gitconfig, + """ + [core] + \teol = crlf + \tattributesFile = %s + """ + // git config would read backslashes as escapes on Windows. + .formatted(PathUtils.pathForStarlarkFile(gitattributes))); + String nullConfig = OS.getCurrent() == OS.WINDOWS ? "NUL" : "/dev/null"; + return Map.ofEntries( + entry("GIT_CONFIG_SYSTEM", gitconfig.toString()), + entry("GIT_CONFIG_GLOBAL", nullConfig)); // neutralize host ~/.gitconfig in this test + }); + } + + @FunctionalInterface + private interface HostGitConfig { + /** + * Writes a host git config under {@code scratch} and returns the environment that makes git + * read it while neutralizing the other config sources. + */ + Map generate(Path scratch) throws Exception; + } + + /** + * Commits files with assorted line endings, fetches them via git_repository under an ambient host + * git config (written per {@code hostGitConfig}), and asserts each checks out byte-for-byte. + */ + private void assertCheckoutIgnoresHostGitConfig(HostGitConfig hostGitConfig) throws Exception { + Path repo = context().getTmpDir().resolve("eol_repo"); + GitRepositoryHelper gitRepository = initGitRepository(context(), repo); + + // Write the files directly as context().write may use OS-specific newlines. + Files.writeString(repo.resolve("lf.txt"), "stays\nLF\n"); + Files.writeString(repo.resolve("crlf.txt"), "stays\r\nCRLF\r\n"); + Files.writeString(repo.resolve(".gitattributes"), "lf_to_crlf.txt eol=crlf\n"); + Files.writeString(repo.resolve("lf_to_crlf.txt"), "commit LF\ncheckout CRLF\n"); + + gitRepository.config("core.autocrlf", "false"); // ensure newlines are committed verbatim + gitRepository.addAll(); + gitRepository.commit("Add files with assorted line endings"); + gitRepository.tag("first"); + context() + .write( + "MODULE.bazel", + "git_repository = use_repo_rule(\"@bazel_tools//tools/build_defs/repo:git.bzl\"," + + " \"git_repository\")", + "git_repository(", + " name='ext',", + String.format(" remote='%s',", PathUtils.pathToFileURI(repo.resolve(".git"))), + " tag='first',", + " build_file_content='filegroup(name=\"data\", srcs=glob([\"*.txt\"]))',", + ")"); + + Path scratch = Files.createDirectories(context().getTmpDir().resolve("git_env")); + Map repoEnv = new HashMap<>(hostGitConfig.generate(scratch)); + if (OS.getCurrent() == OS.WINDOWS) { + // git needs these to run on Windows. + repoEnv.put("SYSTEMROOT", System.getenv("SYSTEMROOT")); + repoEnv.put("SYSTEMDRIVE", System.getenv("SYSTEMDRIVE")); + } + + // Make sure no ambient GIT_CONFIG_*, XDG_CONFIG_HOME, etc. leaks into the test. + List flags = new ArrayList<>(List.of("--experimental_strict_repo_env")); + repoEnv.forEach((name, value) -> flags.add("--repo_env=" + name + "=" + value)); + + BuilderRunner bazel = + WorkspaceTestUtils.bazel(context()).withFlags(flags.toArray(new String[0])); + bazel.build("@ext//:data"); + Path execRoot = context().resolveExecRootPath(bazel, ""); + Map checkout = + bazel + .cquery("@ext//:data", "--output=files") + .outString() + .trim() + .lines() + .map(execPath -> PathUtils.resolve(execRoot, execPath)) + .collect(toMap(path -> path.getFileName().toString(), path -> path)); + + assertWithMessage("host git config must not add CR") + .that(Files.readString(checkout.get("lf.txt"))) + .isEqualTo("stays\nLF\n"); + assertWithMessage("host git config must not strip CR") + .that(Files.readString(checkout.get("crlf.txt"))) + .isEqualTo("stays\r\nCRLF\r\n"); + assertWithMessage("git_repository must not shadow in-tree eol=crlf") + .that(Files.readString(checkout.get("lf_to_crlf.txt"))) + .isEqualTo("commit LF\r\ncheckout CRLF\r\n"); + } + 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 ff2dd2e082c4dc..dd71aca76d9dbd 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 @@ -75,6 +75,19 @@ void commit(String commitMessage) throws Exception { runGit("commit", "-m", commitMessage); } + /** + * Sets a repository-local git config value. + * + * @param key config key + * @param value config value + * @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 config(String key, String value) throws Exception { + runGit("config", key, value); + } + /** * Tags the HEAD commit. * diff --git a/tools/build_defs/repo/git_worker.bzl b/tools/build_defs/repo/git_worker.bzl index 884d9de613064a..ed807a7f15c386 100644 --- a/tools/build_defs/repo/git_worker.bzl +++ b/tools/build_defs/repo/git_worker.bzl @@ -309,7 +309,9 @@ _GIT_LOCAL_ENV_VARS = { def _execute(ctx, git_repo, args): # "core.fsmonitor=false" disables git from spawning a file system monitor which can cause hangs when cloning a lot. # See https://github.com/bazelbuild/bazel/issues/21438 - start = ["git", "-c", "core.fsmonitor=false"] + # "core.autocrlf=false" & "core.eol=lf" bypass host config to keep checkouts verbatim; .gitattributes still applies. + # See https://github.com/bazelbuild/bazel/issues/30026 + start = ["git", "-c", "core.fsmonitor=false", "-c", "core.autocrlf=false", "-c", "core.eol=lf"] return ctx.execute( start + args, environment = ctx.os.environ | _GIT_LOCAL_ENV_VARS,