Add shell helper to make it easier to execute `buildozer` commands on a repository

[cgrindel]

The current README.md states that the following should work:

bazel run -- @buildifier_prebuilt//:buildozer ARGS

However, this will not work correctly. Bazel will execute the command in a sandbox, not the workspace. Hence, buildozer will never find any of the paths.

A possible fix would be to provide a shell script that performs a cd "${BUILD_WORKSPACE_DIRECTORY}" and then executes the buildozer commands.

NOTE: The buildifier would also benefit from this technique as today one must specify absolute paths for buildifier to find files and directories.

[keith]

Ah yea, the only reason I didn't do a shell script before was because that seemed unnecessary, but yea this is a good reason

[manuelnaranjo]

BTW a shell wrapper is not needed at all:

alias(
    name = "buildozer",
    actual = "@buildifier_prebuilt//:buildozer",
)

Then here is an example execution:

$ bazel run :buildozer -- 'print actual' //:buildozer
INFO: Invocation ID: 3f4494b3-882e-4c28-a33f-2e37ccb33baa
INFO: Analyzed target //:buildozer (0 packages loaded, 0 targets configured).
INFO: Found 1 target...
Target @buildifier_prebuilt~6.0.0.1//buildozer:buildozer up-to-date:
  dist/bin/external/buildifier_prebuilt~6.0.0.1/buildozer/buildozer
INFO: Elapsed time: 0.192s, Critical Path: 0.00s
INFO: 1 process: 1 internal.
INFO: Build completed successfully, 1 total action
INFO: Running command line: dist/bin/external/buildifier_prebuilt~6.0.0.1/buildozer/buildozer 'print actual' //:buildozer
@buildifier_prebuilt//:buildozer

[laurentlb]

A wrapper is indeed not needed (even without the alias): bazel run @buildifier_prebuilt//:buildozer print //:all works for me.

Modifying files also work. So it looks like this issue can be closed?

[dougthor42]

So it looks like this issue can be closed?

I'm not sure. I get a similar (the same?) issues as OP. It seems like Buildozer doesn't find any of the targets I specify. Example:

$ # running using a compiled go binary:
$ ~/go/bin/buildozer 'print name' '//src/pyle_xc/fab/package_tester/...:*'
test_data_filegroup
device_data_filegroup
device_mapping_test
...
$ running using the bazel target
$ bazel run -- @buildifier_prebuilt//:buildozer 'print name' !$
bazel run -- @buildifier_prebuilt//:buildozer 'print name' '//src/pyle_xc/fab/package_tester/...:*'
INFO: Analyzed target @@buildifier_prebuilt~//:buildozer (0 packages loaded, 0 targets configured).
INFO: Found 1 target...
Target @@buildifier_prebuilt~//buildozer:buildozer up-to-date:
  bazel-bin/external/buildifier_prebuilt~/buildozer/buildozer
INFO: Elapsed time: 0.476s, Critical Path: 0.18s
INFO: 1 process: 1 internal.
INFO: Build completed successfully, 1 total action
INFO: Running command line: bazel-bin/external/buildifier_prebuilt~/buildozer/buildozer 'print name' '//src/pyle_xc/fab/package_tester/...:*'
$ # nothing printed :-(

I tried using an alias as well as suggested a couple comments up. Same thing.

Thoughts?

Edit: trying on a different target set results in an error. Yay something new!

$ ~/go/bin/buildozer 'print name' '//:all'
python_requirements_lockfile
buildifier.check
buildifier.fix
buildozer
modules_map
gazelle_python_manifest
gazelle
$ bazel run //:buildozer -- 'print name' '//:all'
INFO: Analyzed target //:buildozer (0 packages loaded, 0 targets configured).
INFO: Found 1 target...
Target @@buildifier_prebuilt~//buildozer:buildozer up-to-date:
  bazel-bin/external/buildifier_prebuilt~/buildozer/buildozer
INFO: Elapsed time: 0.789s, Critical Path: 0.18s
INFO: 1 process: 1 internal.
INFO: Build completed successfully, 1 total action
INFO: Running command line: bazel-bin/external/buildifier_prebuilt~/buildozer/buildozer 'print name' //:all
/usr/local/google/home/dthor/.cache/bazel/_bazel_dthor/dbe74c4144b5c9a438d84a119652bef9/execroot/_main/bazel-out/k8-fastbuild/bin/external/buildifier_prebuilt~/buildozer/buildozer.runfiles/_main/BUILD: file not found or not readable

[manuelnaranjo]

For you it's running inside the sandbox and not chdir into BUILD_WORKSPACE_DIRECTORY when it starts https://blog.aspect.build/bazel-can-write-to-the-source-folder.

Maybe you can create a repro setup? It does switch to the right place for us @ booking, but we mostly pass buildozer as a tool to other binaries and those make the chdir so we may not be seeing the issue.

[dougthor42]

Thanks @manuelnaranjo. I was able to get things working with the following:

1. shell script wrapper, mostly from https://stackoverflow.com/a/53481508/1354930
2. sh_binary target
3. (optional) alias at the Bazel workspace root

I still think that the "right" way is to add a bash runner template for buildozer - there's already one for buildifier after all - but I don't consider it a super high priority.

For anyone finding this in the future, here's what I did:

### 1. Make shell wrapper script. Path: `./tools/bazel/scripts/_buildozer.sh` ```shell #!/bin/bash # # Wrapper script around @buildifier_prebuilt//:buildozer # # Most of this was taken from https://stackoverflow.com/a/53481508/1354930, the "sh_binary" # approach. # # This resolves an issue where buildozer would only run in the sandbox and not in the workspace. # See https://github.com/keith/buildifier-prebuilt/issues/9 set -Eeuo pipefail # --- begin runfiles.bash initialization v3 --- # Copy-pasted from the Bazel Bash runfiles library v3. # https://github.com/bazelbuild/bazel/blob/9d86712432fa7c6276ad5620a3185557631f36f1/tools/bash/runfiles/runfiles.bash#L70-L80 # Edited by shfmt and for linting set -uo pipefail set +e f=bazel_tools/tools/bash/runfiles/runfiles.bash # shellcheck disable=SC1090 source "${RUNFILES_DIR:-/dev/null}/$f" 2>/dev/null || source "$(grep -sm1 "^$f " "${RUNFILES_MANIFEST_FILE:-/dev/null}" | cut -f2- -d' ')" 2>/dev/null || source "$0.runfiles/$f" 2>/dev/null || source "$(grep -sm1 "^$f " "$0.runfiles_manifest" | cut -f2- -d' ')" 2>/dev/null || source "$(grep -sm1 "^$f " "$0.exe.runfiles_manifest" | cut -f2- -d' ')" 2>/dev/null || { echo >&2 "ERROR: cannot find $f" exit 1 } f= set -e # --- end runfiles.bash initialization v3 --- BUILDOZER="$(rlocation "buildifier_prebuilt/buildozer/buildozer")" readonly BUILDOZER ( cd "${BUILD_WORKSPACE_DIRECTORY}" echo "Buildozer binary: ${BUILDOZER}" echo "Args: '$*'" # Buildozer will return 3 for success & no files changed set +e ${BUILDOZER} "$@" set -e ) ``` ### 2. `sh_binary` target Path: `./tools/bazel/scripts/BUILD.bazel` ```starlark sh_binary( name = "_buildozer", srcs = ["_buildozer.sh"], data = ["@buildifier_prebuilt//:buildozer"], visibility = ["//visibility:public"], deps = ["@bazel_tools//tools/bash/runfiles"], ) ``` ### 3. (Optional) alias Path: `./BUILD.bazel` ```starlark alias( name = "buildozer", actual = "//tools/bazel/scripts:_buildozer", ) ```