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

Provide a way to redirect stdout/stderr into a file #5511

Open
ash2k opened this issue Jul 4, 2018 · 20 comments
Open

Provide a way to redirect stdout/stderr into a file #5511

ash2k opened this issue Jul 4, 2018 · 20 comments
Labels
P3 We're not considering working on this, but happy to review a PR. (No assignee) team-Rules-API API for writing rules/aspects: providers, runfiles, actions, artifacts type: feature request

Comments

@ash2k
Copy link
Contributor

ash2k commented Jul 4, 2018

Description of the problem / feature request:

It is common for programs in unix world to output the result of an invocation to stdout. The current way to capture that output is to either use ctx.actions.run_shell() or a wrapping shell script to redirect it to a file. This requires spawning an extra process and hence adds some overhead. Perhaps it makes sense to provide a way for ctx.actions.run() to redirect stdout and/or stderr into a file.

Feature requests: what underlying problem are you trying to solve with this feature?

I have a tool that does what I described above - writes the result of its invocation to stdout. I need to capture that output and put it into a file - it is the output of the rule. Currently I have to wrap it in a shell script and I thought it might be good to suggest this feature. Thanks.

@lfpino
Copy link
Contributor

lfpino commented Jul 4, 2018

Assigning to @dslomov for triage.

@jin jin added team-Local-Exec Issues and PRs for the Execution (Local) team untriaged and removed team-Execution labels Jan 14, 2019
@jmmv jmmv added team-Starlark and removed team-Local-Exec Issues and PRs for the Execution (Local) team labels Jan 24, 2019
@laurentlb laurentlb added type: feature request P2 We'll consider working on this in future. (Assignee optional) and removed untriaged labels Jan 25, 2019
@laszlocsomor
Copy link
Contributor

laszlocsomor commented May 14, 2019

/sub

I wish this feature existed. It's important for bazelbuild/bazel-skylib#149

@laurentlb
Copy link
Contributor

I agree this would be useful. The current workaround is to use the shell. This is unfortunate because we need two implementations (bash / windows). So, something similar to https://github.com/bazelbuild/bazel-skylib/blob/master/rules/private/copy_file_private.bzl

You also need to be careful about quoting. We have https://github.com/bazelbuild/bazel-skylib/blob/master/lib/shell.bzl for the Linux shell, but nothing for cmd yet.

@laszlocsomor
Copy link
Contributor

Quoting arguments is difficult with cmd. I typically put paths into envvars instead.

@joshua-leyshon-canva
Copy link

Has any progress been made on this? Or, can anyone share a general wrapper that could be used to achieve the same goal?

@alandonovan
Copy link
Contributor

I agree that extending ctx.run to allow redirecting the standard output to a file is a reasonable feature. I don't think it's necessary for stderr because its contents are unstructured, and by redirecting it, build failure messages would become hidden.

@aiuto
Copy link
Contributor

aiuto commented Jul 10, 2020

Redirecting stderr is useful in the case where you are building tests for error messages. For example

  • I have a tool
  • I create a genrule that runs that tool with known bad input, and captures stderr to an output
  • I add a diff_test or golden_file_test against that output.

@alandonovan
Copy link
Contributor

I create a genrule that runs that tool with known bad input, and captures stderr to an output

If you're creating a genrule, you're using a shell, so redirection is trivial.

@aiuto
Copy link
Contributor

aiuto commented Jul 10, 2020

I'm jumping the gun a bit. What I would like to see out of genrule is the capability to raw run a command without having bash. Something more like an execv.

@alandonovan
Copy link
Contributor

I agree, we should remove all dependencies on Bash from the core, which would mean replacing all uses of genrule with either:
(1) a function or rule that wraps process creation and sets envp, argv, and stdout, without a shell. This is possible today using ctx.actions.run, but we need to expose it to BUILD files in a form that doesn't require adding a new bzl file for each usage.
(2) a rule defined in a library that executes a Bash script, and works on Windows too (this means it would add a dependency). (Tony, I've sent you a quick sketch of # 2 in CL 320646234.)

@ElectricRCAircraftGuy
Copy link
Contributor

ElectricRCAircraftGuy commented Sep 30, 2020

It appears the output of bazel commands is already output to a file, no? In command.log, located some place like this: /home/username/.cache/bazel/_bazel_username/3e8af127f8b488324cdf41111355ff4c/command.log.

See: https://stackoverflow.com/questions/46529412/save-terminal-bazel-build-output/46530169#46530169

This file is also accessible via something like this, assuming you're in your git repo's root dir: build/bin/../../../../../command.log.

But yes, I agree, an easy way to output this file where we want it on a given run would be very much desired.

@alandonovan
Copy link
Contributor

alandonovan commented Jan 14, 2021

Alternative approaches:

  1. "Process wrapper in skylib" (https://docs.google.com/document/d/13M9ArI_Qdj3PuJk2kXSRm6YISp8uceq_lEG8_I7kXcA)
    We should evaluate whether it's possible to implement this approach without the C++ toolchain dependency by writing the Windows implementation in the CMD.exe script language. (The code needn't be pleasant, just correct.)
  2. "Windows: Bash-less alternative for genrule" (Windows: Bash-less alternative for genrule #7503)
  3. My Google internal pending change cr/320646234:
bazel: sketch of bash_command, replacement for genrule

This CL contains a rough sketch of a design for a replacement of genrule
that meets the following criteria:

1) defined entirely in Starlark.
2) provides equivalent functionality to $(location) using Bash 4 hash tables.
3) allows the rule to add args and environment variables from Starlark values.
4) makes explicit the use of Bash. On Windows, this would entail a dependency
   to a ported executable.
...

@brandjon brandjon added P4 This is either out of scope or we don't have bandwidth to review a PR. (No assignee) team-Build-Language and removed P2 We'll consider working on this in future. (Assignee optional) team-Starlark labels Feb 16, 2021
SanjayVas added a commit to world-federation-of-advertisers/common-jvm that referenced this issue Jun 23, 2021
This will make it easier to use a toolchain configuration for CUE in the future. The `dump` CUE command had to be replaced with a `dumpFile` one due to bazelbuild/bazel#5511.

Bug: 168620448
Bug: 171800001
Change-Id: I54dffdc414ba86592fe088570723e9bb3d455960
SanjayVas added a commit to world-federation-of-advertisers/common-jvm that referenced this issue Jun 25, 2021
This will make it easier to use a toolchain configuration for CUE in the future. The `dump` CUE command had to be replaced with a `dumpFile` one due to bazelbuild/bazel#5511.

Bug: 168620448
Bug: 171800001
Change-Id: I54dffdc414ba86592fe088570723e9bb3d455960
TLATER added a commit to TLATER/rules_kotlin that referenced this issue Sep 21, 2021
This removes the python dependency in favor of a heavier reliance the
shell.

While not ideal right now, a resolution of
bazelbuild/bazel#5511 would allow us to do
the file grepping in starlark

We could use entirely native rules this way, only calling out to the
`jar` binary directly, which should be included with any jdk, and
therefore result in a rule that is much leaner on dependencies while
also more portable.

Note: This hasn't been tested yet. I've thought a few times I did
using the JS example, but each time it has turned out bazel ended up
using different rules. This is a start, and an approach, but not a
ready patch.

This also still crucially lacks an implementation for .js.map files,
which is technically trivial, but I'd like to ideally do it without
invoking more shells.
TLATER added a commit to TLATER/rules_kotlin that referenced this issue Sep 21, 2021
This removes the python dependency in favor of a heavier reliance the
shell.

While not ideal right now, a resolution of
bazelbuild/bazel#5511 would allow us to do
the file grepping in starlark

We could use entirely native rules this way, only calling out to the
`jar` binary directly, which should be included with any jdk, and
therefore result in a rule that is much leaner on dependencies while
also more portable.

Note: This hasn't been tested yet. I've thought a few times I did
using the JS example, but each time it has turned out bazel ended up
using different rules. This is a start, and an approach, but not a
ready patch.

This also still crucially lacks an implementation for .js.map files,
which is technically trivial, but I'd like to ideally do it without
invoking more shells.
TLATER added a commit to TLATER/rules_kotlin that referenced this issue Sep 21, 2021
This removes the python dependency in favor of a heavier reliance the
shell.

While not ideal right now, a resolution of
bazelbuild/bazel#5511 would allow us to do
the file grepping in starlark

We could use entirely native rules this way, only calling out to the
`jar` binary directly, which should be included with any jdk, and
therefore result in a rule that is much leaner on dependencies while
also more portable.

Note: This hasn't been tested yet. I've thought a few times I did
using the JS example, but each time it has turned out bazel ended up
using different rules. This is a start, and an approach, but not a
ready patch.

This also still crucially lacks an implementation for .js.map files,
which is technically trivial, but I'd like to ideally do it without
invoking more shells.
SanjayVas added a commit to world-federation-of-advertisers/rules_cue that referenced this issue Feb 1, 2022
This will make it easier to use a toolchain configuration for CUE in the future. The `dump` CUE command had to be replaced with a `dumpFile` one due to bazelbuild/bazel#5511.

Bug: 168620448
Bug: 171800001
Change-Id: I54dffdc414ba86592fe088570723e9bb3d455960
@cloudhan
Copy link

cloudhan commented Mar 25, 2022

I am curious why a simple non-breaking, improve everyone's life feature, can be that hard to get addressed in every project. The ctx.actions side deside not to support stdout/err redirection. The bazel command insists on capture all action stdout and garbage fill users terminal with INFO: From Executing blahblah. And the f*****g vendor tool side insists on garbage fill users' stdout. ...

If this feature is not implemented, once rules move to starlark, I'd like too see users terminal is always garbage filled, with progress_message buried in.

  1. https://stackoverflow.com/questions/1453767/prevent-cl-exe-from-printing-the-compiled-source-file

@UlrichEckhardt
Copy link

Stumbled over the same issue. As alternative to (nonportable) shell code and (big & hairy) C++ compilers, there's also Python:

from optparse import OptionParser
import os
import sys
import subprocess

if __name__ == '__main__':
    parser = OptionParser()
    parser.add_option('--output',
                      dest = 'output',
                      help = 'filename where to redirect stdout')
    parser.add_option('--verbose',
                      action = 'store_true',
                      default = False,
                      dest = 'verbose',
                      help = 'provide diagnostic output')
    (options, args) = parser.parse_args()

    # run actual program
    if options.verbose:
        print(f'Running binary (argv={repr(args)}).')
    process = subprocess.run(args,
                             capture_output = True,
                             universal_newlines = True)
    if options.verbose:
        print('Stdout:')
        print(''.join('  ' + l for l in process.stdout.splitlines(keepends = True)))
        print('Stderr:')
        print(''.join('  ' + l for l in process.stderr.splitlines(keepends = True)))
        print(f'Process finished with exit code {process.returncode}.')

    # write results to the output file
    with open(options.output, 'w') as f:
        f.write(process.stdout)

    sys.exit(process.returncode)

Put this wrapper into a py_binary and then you can use it in an action. Just use [ctx.executable._the_wrapper, "--output", out_file.path, "--"] as args, followed by the argv of the program you want to wrap. Note the "--", which serves as a separator between the two argument lists.

@alanfalloon
Copy link
Contributor

alanfalloon commented Jun 29, 2022

Stumbled over the same issue. As alternative to (nonportable) shell code and (big & hairy) C++ compilers, there's also Python:

@UlrichEckhardt that is fine and effective Python, but I think in this case Python has a number of issues:

  • Far more systems ship with a Bourne compatible shell than a Python interpreter so carefully written shell code (shellcheck can help here) will be a more portable option.
  • Startup times for Python are really bad, especially in Bazel. Even on a good day, Python takes a long time to even execute the first line of the module, but Bazel makes this far less efficient in BRE or sandboxing because it needs to drag in the Python interpreter and its massive set of stdlib files before it can even execute the action.

All this means that on short-running actions, your wrapper becomes the major cost. If one must use a wrapper, keeping it very fast to startup, with few runfiles, is key. A small shell script or preferably a statically linked native binary would be my choice.

However, all this is moot. The point of this issue is that it would be trivial for Bazel add input and output redirection to/from File objects in ctx.actions.run and that would be a huge usability improvement and be much more efficient than any wrapper.

@cloudhan
Copy link

The additional external dependency is a deal breaker, because it hurts availability. The effectiveness, however, only affect the experience. The build system is actually quite complex nowadays, a wrapper just adds some accidental complexity.

@jonathan-enf
Copy link

Another advantage of the integral redirect would be that a build action that fails could then automatically dump the redirected log to the console, similar to the behavior of --test_output=errors. This can all be accomplished by wrapper scripts, but getting it right efficiently and consistently should make this a first-class bazel feature.

@brandjon brandjon added untriaged team-Rules-API API for writing rules/aspects: providers, runfiles, actions, artifacts and removed team-Build-Language labels Nov 4, 2022
@comius comius added P3 We're not considering working on this, but happy to review a PR. (No assignee) and removed P4 This is either out of scope or we don't have bandwidth to review a PR. (No assignee) untriaged labels Aug 21, 2023
Copy link

Thank you for contributing to the Bazel repository! This issue has been marked as stale since it has not had any activity in the last 1+ years. It will be closed in the next 90 days unless any other activity occurs. If you think this issue is still relevant and should stay open, please post any comment here and the issue will no longer be marked as stale.

@github-actions github-actions bot added the stale Issues or PRs that are stale (no activity for 30 days) label Oct 25, 2024
@ilya-bobyr
Copy link

I think this is still a desired functionality.

@bcsgh
Copy link

bcsgh commented Oct 25, 2024

I second the motion.

Just because the request is so fully described there's nothing more to say and the team hasn't gotten around to implementing it in 6 years doesn't make it stale.

@github-actions github-actions bot removed the stale Issues or PRs that are stale (no activity for 30 days) label Oct 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P3 We're not considering working on this, but happy to review a PR. (No assignee) team-Rules-API API for writing rules/aspects: providers, runfiles, actions, artifacts type: feature request
Projects
None yet
Development

No branches or pull requests