Repository: bazelbuild/bazel
Issue: Support changing the working directory of repository_ctx.execute
Labels: type: feature request
P2

### Description of the feature request:

I want to be able to direct `repository_ctx.execute` to run in a particular directory.  I believe right now it executes under `external/` vs. `external/<repo_name>`, which is where it should be putting files.  So unless every tool we invoke supports changing directories, this is inadequate.

This can be worked around by trampolining through a script that changes into the appropriate directory.

@damienmg FYI

Issue comments:

Pull request review comments:

Patch that resolved the issue:
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/repository/skylark/SkylarkRepositoryContext.java b/src/main/java/com/google/devtools/build/lib/bazel/repository/skylark/SkylarkRepositoryContext.java
index 077f2229be1950..9fdc2ca74cf39f 100644
--- a/src/main/java/com/google/devtools/build/lib/bazel/repository/skylark/SkylarkRepositoryContext.java
+++ b/src/main/java/com/google/devtools/build/lib/bazel/repository/skylark/SkylarkRepositoryContext.java
@@ -46,6 +46,7 @@
 import com.google.devtools.build.lib.syntax.SkylarkType;
 import com.google.devtools.build.lib.util.OsUtils;
 import com.google.devtools.build.lib.util.StringUtilities;
+import com.google.devtools.build.lib.vfs.FileSystem;
 import com.google.devtools.build.lib.vfs.FileSystemUtils;
 import com.google.devtools.build.lib.vfs.Path;
 import com.google.devtools.build.lib.vfs.PathFragment;
@@ -296,8 +297,9 @@ public SkylarkExecutionResult execute(
       Integer timeout,
       SkylarkDict<String, String> environment,
       boolean quiet,
+      String workingDirectory,
       Location location)
-      throws EvalException, RepositoryFunctionException {
+      throws EvalException, RepositoryFunctionException, InterruptedException {
     WorkspaceRuleEvent w =
         WorkspaceRuleEvent.newExecuteEvent(
             arguments,
@@ -309,10 +311,15 @@ public SkylarkExecutionResult execute(
             rule.getLabel().toString(),
             location);
     env.getListener().post(w);
-    createDirectory(outputDirectory);
+
+    Path workingDirectoryPath = outputDirectory;
+    if (workingDirectory != null && !workingDirectory.isEmpty()) {
+      workingDirectoryPath = getPath("execute()", workingDirectory).getPath();
+    }
+    createDirectory(workingDirectoryPath);
     return SkylarkExecutionResult.builder(osObject.getEnvironmentVariables())
         .addArguments(arguments)
-        .setDirectory(outputDirectory.getPathFile())
+        .setDirectory(workingDirectoryPath.getPathFile())
         .addEnvironmentVariables(environment)
         .setTimeout(Math.round(timeout.longValue() * 1000 * timeoutScaling))
         .setQuiet(quiet)
diff --git a/src/main/java/com/google/devtools/build/lib/skylarkbuildapi/repository/SkylarkRepositoryContextApi.java b/src/main/java/com/google/devtools/build/lib/skylarkbuildapi/repository/SkylarkRepositoryContextApi.java
index d9485cb6c839ec..a648c7775553f3 100644
--- a/src/main/java/com/google/devtools/build/lib/skylarkbuildapi/repository/SkylarkRepositoryContextApi.java
+++ b/src/main/java/com/google/devtools/build/lib/skylarkbuildapi/repository/SkylarkRepositoryContextApi.java
@@ -230,14 +230,22 @@ public void createFileFromTemplate(
             defaultValue = "True",
             named = true,
             doc = "If stdout and stderr should be printed to the terminal."),
+        @Param(
+            name = "working_directory",
+            type = String.class,
+            defaultValue = "\"\"",
+            named = true,
+            doc = "Working directory for command execution.\n"
+                + "Can be relative to the repository root or absolute."),
       })
   public SkylarkExecutionResultApi execute(
       SkylarkList<Object> arguments,
       Integer timeout,
       SkylarkDict<String, String> environment,
       boolean quiet,
+      String workingDirectory,
       Location location)
-      throws EvalException, RepositoryFunctionExceptionT;
+      throws EvalException, RepositoryFunctionExceptionT, InterruptedException;
 
   @SkylarkCallable(
       name = "which",


Test patch:
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 b3560591e00e2a..2ebf4ce697bdc3 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
@@ -33,13 +33,13 @@
  */
 public class RepoWithRuleWritingTextGenerator {
 
-  private static final String HELPER_FILE = "helper.bzl";
-  private static final String RULE_NAME = "write_to_file";
+  static final String HELPER_FILE = "helper.bzl";
+  static final String RULE_NAME = "write_to_file";
   static final String HELLO = "HELLO";
   static final String TARGET = "write_text";
   static final String OUT_FILE = "out";
 
-  private static final String WRITE_TEXT_TO_FILE =
+  static final String WRITE_TEXT_TO_FILE =
       "def _impl(ctx):\n"
           + "  out = ctx.actions.declare_file(ctx.attr.filename)\n"
           + "  ctx.actions.write(out, ctx.attr.text)\n"
diff --git a/src/test/java/com/google/devtools/build/lib/blackbox/tests/workspace/WorkspaceBlackBoxTest.java b/src/test/java/com/google/devtools/build/lib/blackbox/tests/workspace/WorkspaceBlackBoxTest.java
index 6b6a022114fbea..0b4d6a867756f3 100644
--- a/src/test/java/com/google/devtools/build/lib/blackbox/tests/workspace/WorkspaceBlackBoxTest.java
+++ b/src/test/java/com/google/devtools/build/lib/blackbox/tests/workspace/WorkspaceBlackBoxTest.java
@@ -14,11 +14,16 @@
 
 package com.google.devtools.build.lib.blackbox.tests.workspace;
 
+import static com.google.common.truth.Truth.assertThat;
+
 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 com.google.devtools.build.lib.util.OS;
+import java.nio.file.Files;
 import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.util.List;
 import org.junit.Test;
 
 /** End to end test of workspace-related functionality. */
@@ -68,11 +73,74 @@ public void testNotInMsys() throws Exception {
 
     BuilderRunner bazel = WorkspaceTestUtils.bazel(context());
     // The build using "bash" should fail on Windows, and pass on Linux and Mac OS
-    if (OS.WINDOWS.equals(OS.getCurrent())) {
+    if (isWindows()) {
       bazel.shouldFail();
     }
     bazel.build("check");
   }
+  //echo %cd%
+
+  @Test
+  public void testExecuteInWorkingDirectory() throws Exception {
+    String pwd = isWindows() ? "['cmd', '/c', 'echo %cd%']" : "['pwd']";
+    String buildFileText = "\"\"\"" + String.join("\n",
+        RepoWithRuleWritingTextGenerator.loadRule("@main"),
+        RepoWithRuleWritingTextGenerator.callRule("debug_me", "out", "%s")) +"\"\"\" % stdout";
+    context().write("repo_rule.bzl",
+        "def _impl(rctx):",
+        String.format(
+            "  result = rctx.execute(%s, working_directory=rctx.attr.working_directory)", pwd),
+        "  if result.return_code != 0:",
+        "    fail('Execute failed: ' + result.stderr)",
+        // we want to compare the real paths,
+        // otherwise it is not clear how to verify the relative path variant
+        "  wd = str(rctx.path(rctx.attr.working_directory))",
+        // pwd returns the path with '\n' in the end of the line; cut it
+        "  stdout = result.stdout.strip(' \\n\\r').replace('\\\\', '/')",
+        "  if wd != stdout:",
+        "    fail('Wrong current directory: **%s**, expecting **%s**' % (stdout, wd))",
+        // create BUILD file with a target so we can call it;
+        // rule of a target is defined in the main repository
+        "  rctx.file('BUILD', " + buildFileText + ")",
+        "check_wd = repository_rule(implementation = _impl,",
+        "  attrs = { 'working_directory': attr.string() }",
+        ")");
+
+    context().write(RepoWithRuleWritingTextGenerator.HELPER_FILE,
+        RepoWithRuleWritingTextGenerator.WRITE_TEXT_TO_FILE);
+    context().write("BUILD");
+
+    Path tempDirectory = Files.createTempDirectory("temp-execute");
+    context().write(WORKSPACE,
+        "workspace(name = 'main')",
+        "load(':repo_rule.bzl', 'check_wd')",
+        "check_wd(name = 'relative', working_directory = 'relative')",
+        "check_wd(name = 'relative2', working_directory = '../relative2')",
+        String.format("check_wd(name = 'absolute', working_directory = '%s')",
+            PathUtils.pathForStarlarkFile(tempDirectory)),
+        String.format("check_wd(name = 'absolute2', working_directory = '%s')",
+            PathUtils.pathForStarlarkFile(tempDirectory.resolve("non_existent_child")))
+        );
+
+    BuilderRunner bazel = WorkspaceTestUtils.bazel(context());
+    bazel.build("@relative//:debug_me");
+    Path outFile = context().resolveBinPath(bazel, "external/relative/out");
+    assertThat(outFile.toFile().exists()).isTrue();
+    List<String> lines = PathUtils.readFile(outFile);
+    assertThat(lines.size()).isEqualTo(1);
+    assertThat(Paths.get(lines.get(0)).endsWith(Paths.get("external/relative/relative"))).isTrue();
+
+    bazel.build("@relative2//:debug_me");
+    bazel.build("@absolute//:debug_me");
+
+    bazel.build("@absolute2//:debug_me");
+    Path outFile2 = context().resolveBinPath(bazel, "external/absolute2/out");
+    assertThat(outFile2.toFile().exists()).isTrue();
+    List<String> lines2 = PathUtils.readFile(outFile2);
+    assertThat(lines2.size()).isEqualTo(1);
+    assertThat(Paths.get(lines2.get(0)).equals(
+        tempDirectory.resolve("non_existent_child"))).isTrue();
+  }
 
   @Test
   public void testWorkspaceChanges() throws Exception {
@@ -113,6 +181,9 @@ public void testPathWithSpace() throws Exception {
     bazel.help();
   }
 
+  private boolean isWindows() {
+    return OS.WINDOWS.equals(OS.getCurrent());
+  }
   // TODO(ichern) move other tests from workspace_test.sh here.
 
 }