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

BXL: get stdout/stderr of actions.run() #760

Open
cbarrete opened this issue Aug 29, 2024 · 8 comments
Open

BXL: get stdout/stderr of actions.run() #760

cbarrete opened this issue Aug 29, 2024 · 8 comments

Comments

@cbarrete
Copy link
Contributor

Is it possible to get the output of an action to print it out via ctx.output.print()?

The best I could do was redirect stdout to a declared output and print out its path, but since there is no way to read that file from BXL (that I know of anyway), this requires the user to cat out this file (potentially via a wrapper script), which is a poor UX.

@cormacrelf
Copy link
Contributor

cormacrelf commented Sep 2, 2024

You should think of BXL a bit like a thing that writes a temporary .bzl + BUCK file pair, and immediately executes a build on it. There is no such thing in BUCK / bzl files as reading the output files, and so there is no such thing in BXL either. (A nice analogy is a control plane for a networking device. You never get to read the packets. You only get to write routing rules, and your rules are submitted to a big box that runs them at its leisure. In Buck that big box is Buck driving a potentially remote executor. Stdout is less simple when it could be halfway across the world, so you have to describe to Buck how it's going to get that stdout back.)

Wrapper scripts are pretty much the way to go every time. It's mildly inconvenient that you have to write your wrapper in a separate file / language, but the alternative would be for Buck to add a bunch of Python-esque IO tools to the starlark context, which would be a huge waste of time. I guess being able to dump a file to stdout would be neat (and it would mean BXL could emulate buck2 build --out - on its own). But literally anything else, eg checking if the file you dumped was in fact empty, is not feasible, so I'm not really sure there's any point. In the simplest scenario, your wrapper script can be buck2 bxl ... -- "$@" | xargs cat.

In any case, typing out a buck2 bxl invocation is tedious enough that every real world BXL deserves a wrapper script. So I recommend just leaning into it.

@Nero5023
Copy link
Contributor

Nero5023 commented Sep 2, 2024

Is it possible to get the output of an action to print it out via ctx.output.print()?

What do you mean for "output" here? The stdout of a actions.run or the output artifact(s) of a actions.run

@cbarrete
Copy link
Contributor Author

cbarrete commented Sep 2, 2024

@cormacrelf ctx.output.print and ctx.output.print_json are already existing APIs, and they make a lot of sense to me.

Stdout is less simple when it could be halfway across the world

But Buck2 already captures that stdout and prints it in the case of failures, so it already supports a subset of this functionality.

the alternative would be for Buck to add a bunch of Python-esque IO tools
checking if the file you dumped was in fact empty, is not feasible

Can you elaborate? I don't immediately see why we would need a lot of IO tools, or what would be so challenging about exposing stdout/stderr to the user. At least, having a convenient way of forwarding stdout upon success (like it is already the case when an action fails) seems perfectly doable, and returning a struct with stderr/stdout fields does not seem impossible to me (although it might not be trivial).

A nice analogy is a control plane for a networking device

But in this case, we do not have a way of routing stdout/stderr to the user, which is the core problem that I have. Being able to inspect them would be nice and helpful in some cases, but that's not the main focus here (especially if there are technical limitations that I do not understand).

Regarding wrapper scripts, I am really hoping that #86 will get some traction. I would love for buck2 to be a universal entrypoint, sort of like nix can be with flakes, but with much better ergonomics (I find Starlark/BXL a lot nicer to write than Nix derivations).

In any case, typing out a buck2 bxl invocation is tedious enough that every real world BXL deserves a wrapper script. So I recommend just leaning into it.

I have already warned some of my users that we might need to do this (and probably will have to at least at first), and the reaction was strongly negative. Buck2 can already print the stdout of actions when they fail, this seems like an arbitrary limitation, and we expect the use case of "use Buck2 to drive some underlying tools and forward their output" to be very common. Besides, BXL has nice handling of command line arguments by default, it would be a shame to have to essentially not take advantage of that by having to overlay command line parsing and generation, for every script. Another argument against wrapper scripts is that they are not hermetic, so past a certain size, we need to rely on non-hermetic scripts that call a hermetic build system, and need to solve the exact problems why we wanted to use buck2 as a universal entry point...

@Nero5023 I mean the stdout/stderr of actions.run.

@Nero5023
Copy link
Contributor

Nero5023 commented Sep 3, 2024

stdout/stderr are considered as the side effects here and relying on stdout/stderr of an action is not recommened. Each action is like pure function. Args are inputs, output are output artifacts. Each action can be cached, cache keys are the inputs and the cached outputs are the output artifacts. stdout/stderr can not be cached.
buck2 make the assumption that each action is deterministic. If relying on stdout/stderr, the following operations should be considered as nondeterministic. Same cases for bxl, we also have cache for bxl.

We often have a wrapper for the bxl script internally. We always have a buck2 target wrap that script, so that we can call it by buck2 run and in the wrapper, it calls the bxl script.

@cbarrete
Copy link
Contributor Author

cbarrete commented Sep 4, 2024

stdout/stderr can not be cached

Sort of, if I redirect them to an output file, they become an artifact like any other, and Buck could handle them the same way. But I'm willing to concede that this would add complexity.

We often have a wrapper for the bxl script internally. We always have a buck2 target wrap that script, so that we can call it by buck2 run and in the wrapper, it calls the bxl script.

This is interesting, I briefly considered that, but it seemed overengineered to me, and it seems like it would have bad ergonomics (having to handle argument management at various levels instead of one). I'm not thrilled about it, but I'll consider it again if it ends up being a better user experience. Do we agree that it is not ideal though?

In your experience, what kind of issues do you get (if any) for this "buck within buck" setup? I have experienced (temporary) lock ups when running commands in parallel, so nesting them sounds scary to me.

@cormacrelf
Copy link
Contributor

It's not buck within buck. buck2 run :some_script will simply fork/exec the script, no different from any other wrapper. Buck within buck is when you have an actions.run(buck, ...).

@Nero5023
Copy link
Contributor

Nero5023 commented Sep 5, 2024

Do we agree that it is not ideal though?

Yeah, of course it is not ideal. I am also frustrated that at last I need have a wrapper for the bxl script I wrote. And handling args converting is tedious. We don't have a plan for now to solve this issue, but I will consider it in next half.


Yeah, as @cormacrelf said. It is not buck within buck.

I have experienced (temporary) lock ups when running commands in parallel

You can different isolation dir in this case --isolation-dir. Also it may be not related to this issue, but good to know that we recently add a flag preemptible. It may could help you in some cases.

      --preemptible <PREEMPTIBLE>
          Used to configure when this command could be preempted by another command for the same isolation dir.

          Normally, when you run two commands - from different terminals, say - buck2 will attempt to run them in parallel. However, if the two commands are based on different state, that is they either have different configs or different filesystem states, buck2 cannot run them in parallel. The default behavior in
          this case is to block the second command until the first completes.

          Possible values:
          - never:            (default) When another command starts that cannot run in parallel with this one, block that command
          - always:           When another command starts, interrupt this command, *even if they could run in parallel*. There is no good reason to use this other than that it provides slightly nicer superconsole output
          - ondifferentstate: When another command starts that cannot run in parallel with this one, interrupt this command

@cbarrete
Copy link
Contributor Author

cbarrete commented Sep 5, 2024

Thanks for the clarification about buck within buck and --isolation-dir, that makes sense. I'll give this a try if my users complain too much about raw BXL ergonomics. Either way, we'll be very interested if/when you get to tackling this in Buck2 itself!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants