Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bazel run child should run with bazel invocation's current directory #3325

Open
kamalmarhubi opened this issue Jul 5, 2017 · 16 comments
Open
Assignees

Comments

@kamalmarhubi
Copy link
Contributor

Description of the problem / feature request / question:

When using the bazel run command, the spawned child has a working directory different from the directory where the command is run. This leads to bazel run //path/to/target and bazel-bin/path/to/target/target being quite different. If possible, it would be ideal to have the child inherit the bazel invocation's working directory.

If possible, provide a minimal example to reproduce the problem:

Note difference between output of last two commands.

$ cd $(mktemp -d)
$ pwd
/tmp/tmp.VzGLDvgQvk
$ touch WORKSPACE
$ mkdir -p path/to/target
$ cat <<EOF > path/to/target/BUILD
sh_binary(
    name = "target",
    srcs = ["target.sh"],
)
EOF
$ cat <<EOF > path/to/target/target.sh
#!/bin/bash
pwd
EOF
$ chmod +x path/to/target/target.sh
$ bazel run //path/to/target 2>/dev/null
/home/kamal/.cache/bazel/_bazel_kamal/f4a400f5a0927c37f590fcc3c0d7cf1c/execroot/tmp.VzGLDvgQvk/bazel-out/local-fastbuild/bin/path/to/target/target.runfiles/__main__
$ bazel-bin/path/to/target/target
/tmp/tmp.VzGLDvgQvk

Environment info

  • Operating System:
$ head -1 /etc/os-release 
PRETTY_NAME="Debian GNU/Linux 9 (stretch)"
$ uname -r
4.9.0-3-amd64
  • Bazel version (output of bazel info release):
$ bazel info release
release 0.5.1

Have you found anything relevant by searching the web?

Original bazel-discuss thread: https://groups.google.com/d/msgid/bazel-discuss/CAA%3DsxTgFY9D1kyGaGKCuqUftUBAizNdXu28o_zsKRkgA3F-ZwA

Anything else, information or logs or outputs that would be helpful?

The working directory seems to be set here to the runfiles dir:

workingDir = ensureRunfilesBuilt(env, targetToRun);

@ulfjack
Copy link
Contributor

ulfjack commented Jul 5, 2017

As I said on the other bug, I'm not sure we should change this. If you want to run your binary from a different working directory, don't use bazel run?

@kamalmarhubi
Copy link
Contributor Author

Closing this in favour of #2579; will comment there.

@kamalmarhubi
Copy link
Contributor Author

Actually reopening this as I think they're not the same issue. I don't know that the behaviour requested in #2579 is desirable.

@ulfjack

As I said on the other bug, I'm not sure we should change this. If you want to run your binary from a different working directory, don't use bazel run?

bazel run is a really nice shorthand for build and run. It makes instructions for many common tasks simpler. Is there a technical reason to have the child's working directory to be its runfiles directory? The binary cannot be written expecting to find its runfiles relative to its working directory since it can be run via the link under bazel-bin. Am I missing something else?

@kamalmarhubi kamalmarhubi reopened this Jul 5, 2017
@davido
Copy link
Contributor

davido commented Jul 5, 2017

One workaround for your problem is to use absolute path for passed files. Say, I want to check in CI build the formatting of all java files in specific commit and I have defined this java_binary rule in tools/BUILD file:

java_binary(
    name = "gjf",
    main_class = "com.google.googlejavaformat.java.Main",
    visibility = ["//visibility:public"],
    runtime_deps = ["@google_java_format//jar"],
)

and WORKSPACE is defined like this:

# TODO(davido): Switch to use vanilla upstream gjf version when one of these PRs are merged:
# https://github.com/google/google-java-format/pull/106
# https://github.com/google/google-java-format/pull/154
http_jar(
    name = "google_java_format",
    sha256 = "9fe87113a2cf27e827b72ce67c627b36d163d35d1b8a10a584e4ae024a15c854",
    url = "https://github.com/davido/google-java-format/releases/download/1.3-1-gec5ce10/google-java-format-1.3-1-gec5ce10-all-deps.jar"
)

Now, i can use bazel run as following:

$ git show --diff-filter=AM --name-only HEAD | grep java$ | \
   sed 's@^@'"$PWD"/'@' | xargs -r bazel run tools:gjf -- --dry-run
  INFO: Running command line: bazel-bin/tools/gjf --dry-run
  Need Formatting:
  [gerrit-server/src/test/java/com/google/gerrit/testutil/InMemoryH2Type.java,
   gerrit/gerrit-server/src/test/java/com/google/gerrit/testutil/NoteDbMode.java]
  ERROR: Non-zero return code '1' from command: Process exited with
  status 1.

@kchodorow
Copy link
Contributor

Is there a technical reason to have the child's working directory to be its runfiles directory?

It lets you have runtime dependencies that are different than what's in your source tree. There are a couple of nice things this gets you:

  1. You can depend on outputs by their relative path: if you build a helper tool like //foo/bar:my-tool and use it as a runtime dependency, you can refer to it in your binary as foo/bar/my-tool. In your source directory, it'll be under a configuration-dependent symlink (e.g, bazel-bin) that might not even exist.
  2. The binary doesn't "see" files that aren't runtime dependencies. If you have //foo/bar:data-test.csv and //foo/bar:data-prod.csv, your binary can have a runtime dependency on data-test.csv and then, if it's run from its runfiles directory and looks for .csv files under foo/bar, it won't see data-prod.csv.

One thing that our shell tests do that you might want to consider is starting scripts with "if $PWD doesn't end with .runfiles, cd to ${0}.runfiles." That way you'll always be in .runfiles by line 2.

@zombiezen
Copy link
Contributor

I believe you can use --run_under to work around this:

bazel run --run_under="cd $PWD && " //my:target -- ARGS

(Obviously doesn't work on Windows.)

@kamalmarhubi
Copy link
Contributor Author

kamalmarhubi commented Jul 8, 2017

@kchodorow

You can depend on outputs by their relative path.

As you point out, you can always do this by looking them up in the runfiles by searching adjacent to the executable. Since the binary might be started in ways other than bazel run, a binary should not rely on its runfiles being found relative to its working directory.

The binary doesn't "see" files that aren't runtime dependencies.

Again this seems like a binary implementor's responsibility: they should look up runtime dependencies in runfiles. It seems wrong to rely on your binary only ever being called via bazel run.

Neither of these seem like reasons to have the child from bazel run start in its runfiles directory. They are more like reasons to load expected files relative to the executable's path, which I'm completely on board with.

That way you'll always be in .runfiles by line 2.
This is what I think binaries should do if they want to always be in their runfiles directory. Doing this would not conflict with bazel run respecting the working directory I invoke it from: in that situation, running the binary from bazel-bin would do the same.

@davido, I first came across this with formatting tools as well!

One workaround for your problem is to use absolute path for passed files.

This is a pretty annoying workaround.

@davido
Copy link
Contributor

davido commented Jul 9, 2017

One workaround for your problem is to use absolute path for passed files.
This is a pretty annoying workaround.

Well, at least for format checking only, you don't need any hacks, just use Bazel's sh_test rule. For google-java-format check, this could be easily achieved with my patched gjf version:

WORKSPACE:

# TODO(davido): Switch to use vanilla upstream gjf version when one of these PRs are merged:
# https://github.com/google/google-java-format/pull/106
# https://github.com/google/google-java-format/pull/154
http_jar(
    name = "google_java_format",
    sha256 = "9fe87113a2cf27e827b72ce67c627b36d163d35d1b8a10a584e4ae024a15c854",
    url = "https://github.com/davido/google-java-format/releases/download/1.3-1-gec5ce10/google-java-format-1.3-1-gec5ce10-all-deps.jar"
)

BUILD:

java_binary(
    name = "gjf-binary",
    main_class = "com.google.googlejavaformat.java.Main",
    visibility = ["//visibility:public"],
    runtime_deps = ["@google_java_format//jar"],
)

genrule(
    name = "gjf-check-invocation",
    srcs = glob(["src/main/java/**/*.java"]),
    cmd = "find . -name \"*.java\" | xargs $(location :gjf-binary) --dry-run > $@",
    tools = [":gjf-binary"],
    outs = ["check-java-format.log"],
)

sh_test(
    name = "gjf-check",
    srcs = [":gjf-check-invocation"],
    tags = ["oauth"],
)

Now, to check the format for all files, would just be bazel test foo:

Failure:

$ bazel test gjf-check
.........
INFO: Found 1 test target...
ERROR: /home/davido/projects/oauth/BUILD:28:1: Executing genrule //:gjf-check-invocation failed: Process exited with status 123 [sandboxed].
Need Formatting:
[./src/main/java/com/googlesource/gerrit/plugins/oauth/GoogleOAuthService.java]
Use --strategy=Genrule=standalone to disable sandboxing for the failing actions.
Target //:gjf-check failed to build
Use --verbose_failures to see the command lines of failed build steps.
INFO: Elapsed time: 8.210s, Critical Path: 1.25s

Executed 0 out of 1 test: 1 fails to build.

Success:

$ bazel test gjf-check
INFO: Found 1 test target...
Target //:gjf-check up-to-date:
  bazel-genfiles/check-java-format.log
  bazel-bin/gjf-check
INFO: Elapsed time: 1.323s, Critical Path: 1.09s
//:gjf-check                                                             PASSED in 0.1s

Executed 1 out of 1 test: 1 test passes.

@ianthehat
Copy link
Contributor

I understand the current state, I think it's probably the right default choice. However there are lots of use cases where you really do want to know the place you were invoked from because it's a tool that modifies the source (formatters, automated fixers, refactoring tools etc) and at the moment the information is not recoverable.
I think it would be sufficient to have bazel run set an environment variable with the original working directory of bazel, so that tools can recover the information (or be wrapped in a sh_binary that cd's back before invoking the underlying tool for instance)

@mboes
Copy link
Contributor

mboes commented May 28, 2018

Bazel now has a --direct_run flag. It implements a similar idea to @ianthehat describes above and attempted in #3635: it sets $BUILD_WORKING_DIRECTORY to record the working directory at the point where bazel run was called. So I think this ticket can be closed.

@ulfjack ulfjack closed this as completed Nov 19, 2018
@ulfjack
Copy link
Contributor

ulfjack commented Nov 19, 2018

--direct_run is deprecated because it's enabled by default now.

gregmagolan added a commit to gregmagolan/rules_typescript that referenced this issue Aug 21, 2019
gregmagolan added a commit to gregmagolan/rules_typescript that referenced this issue Aug 21, 2019
alexeagle pushed a commit to bazelbuild/rules_typescript that referenced this issue Aug 22, 2019
alexeagle pushed a commit to alexeagle/rules_nodejs that referenced this issue Oct 17, 2020
alexeagle pushed a commit to alexeagle/rules_nodejs that referenced this issue Oct 18, 2020
bshi added a commit to benchsci/rules_kustomize that referenced this issue Apr 23, 2021
bazelbuild/bazel#3325

Tested:

    mkdir examples/tmp
    cd examples/tmp
    bazel run @com_benchsci_rules_kustomize//:kustomize -- create

Confirm a `kustomize.yaml` file is created in the working directory.
Previously, the tool would mutate the underlying runfiles directory.
bshi added a commit to benchsci/rules_kustomize that referenced this issue Apr 23, 2021
bazelbuild/bazel#3325

Tested:

    mkdir examples/tmp
    cd examples/tmp
    bazel run @com_benchsci_rules_kustomize//:kustomize -- create

Confirm a `kustomize.yaml` file is created in the working directory.
Previously, the tool would mutate the underlying runfiles directory.
@pauldraper
Copy link
Contributor

pauldraper commented Nov 1, 2022

As others have said, resetting the working directory is a weird choice. The environment variables aren't reset. Executables have to locate runfiles independent of working dir.

Fortunately, fixing this is quite simple. Add to .bazelrc:

run --run_under='cd "$BUILD_WORKING_DIRECTORY" && exec'

@alexeagle
Copy link
Contributor

@bazel-io reopen
Since --run_under always discards the analysis cache, even when the value is not a target, the workarounds on this thread don't work.

@brentleyjones brentleyjones reopened this Apr 3, 2024
@alexeagle
Copy link
Contributor

@pauldraper
Copy link
Contributor

pauldraper commented Apr 9, 2024

Since --run_under always discards the analysis cache

That is annoying (#10782).

A workaround is to always set it:

.bazelrc

build --run_under=//tools/bazel:run

tools/bazel/run.sh (executable)

#!/usr/bin/env sh
[ -z "$BUILD_WORKING_DIRECTORY" ] || cd "$BUILD_WORKING_DIRECTORY"
exec "$@"

tools/bazel/BUILD.bazel

sh_binary(
  name = "run",
  srcs = ["run.sh"],
)

(FYI I tried inlining the script into --run_under rather than use a target, but it doesn't work for tests....not sure why, seems to be some tokenization at play.)

mark-thm added a commit to theoremlp/rules_multitool that referenced this issue Apr 16, 2024
)

It's common for users of multitool to want to run tools in the current working directory. In #26, @alexeagle documented a technique we've used for a while with creating a script and symlinking to it. Our internal copy of this script is a bit more complicated to help avoid expensive calls to Bazel that simple `bazel run` calls don't really need. More refinements have been proposed in #27. All of these things are fundamentally workarounds for bazelbuild/bazel#3325.

To help simplify things, this PR adds a convenience wrapper that captures the execpath, switches to $BUILD_WORKING_DIRECTORY, and then runs the desired tool. The resulting shell script gets to use a very simple `bazel run`, should be compatible across any slew of Bazel options, and cache as well as any typical run target.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests