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

Host executable with non-empty DefaultInfo files missing from sandbox #7390

Closed
jmillikin opened this issue Feb 9, 2019 · 2 comments
Closed
Labels
P2 We'll consider working on this in future. (Assignee optional) team-Local-Exec Issues and PRs for the Execution (Local) team type: bug

Comments

@jmillikin
Copy link
Contributor

jmillikin commented Feb 9, 2019

Description of the problem / feature request:

Rules marked executable = True are allowed to return a DefaultInfo with non-empty files, which means that bazel build //some:exec will print a different output path than what gets invoked by bazel run //some:exec. This behavior is useful when writing rules for "script" binaries, when the content needed for hermetic execution might not function when copied outside of the Bazel execroot.

However, when such a binary target is used in host mode (i.e. as a dependency of an action), Bazel will construct the sandbox filesystem incorrectly. The path passed to execvp() is different than the file present on disk.

Bugs: what's the simplest, easiest way to reproduce this bug? Please provide a minimal example if possible.

# BUILD
load(":rules.bzl", "bin_rule", "out_rule")
bin_rule(name = "hello_bin")
out_rule(name = "hello_out")
# rules.bzl
def _bin_rule(ctx):
    out_a = ctx.actions.declare_file(ctx.attr.name + ".a.sh")
    out_b = ctx.actions.declare_file(ctx.attr.name + ".b.sh")
    ctx.actions.write(
        output = out_a,
        content = "#!/bin/sh\necho 'hello a' > $@",
        is_executable = True,
    )
    ctx.actions.write(
        output = out_b,
        content = "#!/bin/sh\necho 'hello b' > $@",
        is_executable = True,
    )
    return DefaultInfo(
        files = depset(direct = [out_a]),
        executable = out_b,
    )

def _out_rule(ctx):
    out = ctx.actions.declare_file(ctx.attr.name + ".txt")
    ctx.actions.run(
        # I expect this to run `hello_bin.b.sh`
        executable = ctx.executable._hello_bin,
        outputs = [out],
        arguments = [out.path],
        mnemonic = "HelloOut",
    )
    return DefaultInfo(
        files = depset(direct = [out]),
    )

bin_rule = rule(_bin_rule, executable = True)
out_rule = rule(_out_rule, attrs = {
    "_hello_bin": attr.label(
        default = "//:hello_bin",
        executable = True,
        cfg = "host",
    ),
})
$ bazel-0.22 build -s //:hello_out
[...]
SUBCOMMAND: # //:hello_out [action 'HelloOut hello_out.txt']
(cd /private/var/tmp/_bazel_john/591a601d7d8436419d69a3dcb9b24d54/execroot/__main__ && \
  exec env - \
  bazel-out/host/bin/hello_bin.b.sh bazel-out/darwin-fastbuild/bin/hello_out.txt)
ERROR: /Users/john/src/runfiles-bug/BUILD:3:1: HelloOut hello_out.txt failed (Exit 1) hello_bin.b.sh failed: error executing command bazel-out/host/bin/hello_bin.b.sh bazel-out/darwin-fastbuild/bin/hello_out.txt

Use --sandbox_debug to see verbose messages from the sandbox
src/main/tools/process-wrapper-legacy.cc:58: "execvp(bazel-out/host/bin/hello_bin.b.sh, ...)": No such file or directory
Target //:hello_out failed to build

What operating system are you running Bazel on?

I can reproduce this bug on macOS and Linux, using the sandboxed strategy.

What's the output of bazel info release?

release 0.22.0

Any other information, logs, or outputs that you want to share?

I edited src/main/tools/process-wrapper-legacy.cc to system("find .") in the error path. The output shows the expected command being placed in runfiles:

src/main/tools/process-wrapper-legacy.cc:59: "execvp(bazel-out/host/bin/hello_bin.b.sh, ...)": No such file or directory
.
./bazel-out
./bazel-out/darwin-fastbuild
./bazel-out/darwin-fastbuild/bin
./bazel-out/host
./bazel-out/host/bin
./bazel-out/host/bin/hello_bin.a.sh
./bazel-out/host/bin/hello_bin.b.sh.runfiles
./bazel-out/host/bin/hello_bin.b.sh.runfiles/__main__
./bazel-out/host/bin/hello_bin.b.sh.runfiles/__main__/hello_bin.b.sh
Target //:hello_out failed to build
@dslomov dslomov added team-Local-Exec Issues and PRs for the Execution (Local) team untriaged labels Feb 11, 2019
@jmmv jmmv added type: bug P2 We'll consider working on this in future. (Assignee optional) and removed untriaged labels Mar 3, 2019
@ccate
Copy link

ccate commented Jul 30, 2019

This issue is still manifesting. In my case I have a py_binary being used as the executable for a custom rule:

load(":rules.bzl", "gen_sql")

gen_sql(
    name = "gen_sql",
    runner = ":sql_generator"
)

py_binary(
    name = "sql_generator",
    srcs = glob(["create_sql_for_tables.py", "create_sql/**/*.py"]),
    python_version = "PY2",
    srcs_version = "PY2",
    deps = [
        "//pxavro:px_avro",
    ],
    main = "create_sql_for_tables.py"
)

when building the gen_sql target, on first run we get

src/main/tools/process-wrapper-legacy.cc:58: "execvp(bazel-out/host/bin/core/postgres/sql_generator, ...)": No such file or directory

but on second run, with no other changes, it runs happily.

contents of rules.bzl:

def _gen_sql_impl(ctx):


    sql_file = ctx.actions.declare_file('create_tables.sql')

    # Action to call the script.
    ctx.actions.run(
        outputs = [sql_file],
        arguments = [sql_file.path],
        progress_message = "Generating postgres init sql.",
        executable = ctx.executable.runner,
        tools = [ctx.executable.runner],
        use_default_shell_env=False,
    )

    return [ DefaultInfo(files = depset(items = [sql_file])) ]


gen_sql = rule(
    implementation = _gen_sql_impl,
    output_to_genfiles = True,
    attrs = {
        "runner": attr.label(
            executable = True,
            cfg = "host",
            allow_files = True,
        ),
    },
)

@jmillikin
Copy link
Contributor Author

Proposed fix: #10110

philwo pushed a commit that referenced this issue Nov 14, 2019
Fixes #7390

Summary: rules are allowed to set `DefaultInfo(files = depset(), executable = ...)`. If the resulting targets are used as tool inputs to another target, the executable will not be added to the downstream targets' action inputs. This manifests as either an `execvp()` error (from `ctx.actions.run()`) or a Bash error in genrules.

This PR fixes that so the use case of non-default executables works as expected. This is useful for building scripts with complex runfiles, where `bazel run //:something` and `bazel build //:something && bazel-bin/something` cannot be supported by the same output file.

cc @ccate @jmmv

Closes #10110.

PiperOrigin-RevId: 280170524
philwo pushed a commit that referenced this issue Nov 20, 2019
Fixes #7390

Summary: rules are allowed to set `DefaultInfo(files = depset(), executable = ...)`. If the resulting targets are used as tool inputs to another target, the executable will not be added to the downstream targets' action inputs. This manifests as either an `execvp()` error (from `ctx.actions.run()`) or a Bash error in genrules.

This PR fixes that so the use case of non-default executables works as expected. This is useful for building scripts with complex runfiles, where `bazel run //:something` and `bazel build //:something && bazel-bin/something` cannot be supported by the same output file.

cc @ccate @jmmv

Closes #10110.

PiperOrigin-RevId: 280170524
philwo pushed a commit that referenced this issue Nov 25, 2019
Fixes #7390

Summary: rules are allowed to set `DefaultInfo(files = depset(), executable = ...)`. If the resulting targets are used as tool inputs to another target, the executable will not be added to the downstream targets' action inputs. This manifests as either an `execvp()` error (from `ctx.actions.run()`) or a Bash error in genrules.

This PR fixes that so the use case of non-default executables works as expected. This is useful for building scripts with complex runfiles, where `bazel run //:something` and `bazel build //:something && bazel-bin/something` cannot be supported by the same output file.

cc @ccate @jmmv

Closes #10110.

PiperOrigin-RevId: 280170524
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P2 We'll consider working on this in future. (Assignee optional) team-Local-Exec Issues and PRs for the Execution (Local) team type: bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants