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

Various additions to ctx.actions #768

Open
thoughtpolice opened this issue Sep 4, 2024 · 2 comments
Open

Various additions to ctx.actions #768

thoughtpolice opened this issue Sep 4, 2024 · 2 comments

Comments

@thoughtpolice
Copy link
Contributor

thoughtpolice commented Sep 4, 2024

After writing a bunch of rules I keep coming back to a few things that I would really like to do in a raw actions context, with no help from the Prelude, and I would like if they were added:

Write stderr/stdout of a command directly to an output

I want to do something like:

stdout = ctx.actions.declare_output(ctx.label.name + ".stdout")
ctx.actions.run(
    ["/usr/bin/echo", "hello world"],
    stdout = stdout.as_output(),
)
return [ DefaultInfo(stdout) ]

Just like any command there can be a mix of multiple as_output()s in its parameters, but at least one is necessary, and stdout/stderr should fill that role.

Rationale: There are many kinds of rules, from testing rules ("compare stderr to this file") to random intermediate rules invoking tools (output JSON then read it in another command), where you just want to put the stdout/stderr somewhere. But this is really awkward right now because you basically have to:

  • Invoke a genrule OR
  • Use ctx.actions.run() with a conditional if it's Windows so you can run some Batch script/Shell script duo
  • Some other complex thing with Python or whatever?

But I just want to write the output of the command somewhere, that's it! It's really awkward to have to take two paths for Windows/Linux (in my case I don't want them to need msys2 or some mingw/git shell installation) and it's weird to invoke genrule for this. Python is so much overkill. It feels like this is something I should intuitively be able to do and have it just work.

This isn't just a problem for me with my weird Prelude, but also anyone who has to write rules for anything, and this kind of stuff is really common with every kind of cross platform tool you can imagine.

Create a directory

dir = ctx.actions.declare_output(ctx.label.name, "cool_dir", dir = True)
out_dir = ctx.actions.make_directory(dir.as_output()) # or make_directory(ctx.label.name) itself

Rationale: That's it. Why do I want this? For the exact same reason as before, there isn't any nice cross platform way to invoke mkdir -p on Windows/Linux in a way to create nested directory hierarchies.

And you need to do this all the time actually, on all platforms, because lots of things just expect directories to exist before running them. If you can't do it this way and actually have Buck track the action dependencies, then what happens is you end up writing shell scripts that just invoke the took and run mkdir first, which goes back exactly to the first problem.

There are probably some other operations like this where we could have a similar argument, but mkdir -p is very common, and I don't think you should have to rely on a rule to do it.

Change to a directory, then run

This is the least common of the three. But, likewise, many commands expect something like a directory to be written out:

some/dir/path/
some/dir/path/file.txt

And then you want to execute the command on file.txt, but it must be executed from within some/dir/path! Some tools just work this way. In my case actually, I want to run a checksum tool, then compare the output of the tool to a trusted SHA256SUMS file I download over https (thru http_archive.)

This is kind of like an inverse of relative_to on cmd_args, so maybe it should go there somehow, instead.

Broad Justification

In all cases you can get around this by doing something like writing an _impl method somewhere, then exposing that to the rest of the codebase, importing and passing things in. So, you only have to "do it once" so they say.

But I don't think this is a good developer UX, because these are actually really common operations, and making it easier to write the correct rule "by design" is very valuable in a raw AnalysisContext. It's just a chore to maintain.

Conclusion

To be clear, I think this argument is much stronger for the first and second points. The third one is a bit more niche, but I think would make some rules nicer to write, is all.

I think this argument could be made for more basic cross platform builtins, but these are the ones I can think of now.

@thoughtpolice thoughtpolice changed the title Various additions to add to ctx.actions Various additions to ctx.actions Sep 4, 2024
@cormacrelf
Copy link
Contributor

For stderr/stdout, see thread for recent discussion. #760

dir = ctx.actions.declare_output(ctx.label.name, "cool_dir", dir = True)
out_dir = ctx.actions.make_directory(dir.as_output()) # or make_directory(ctx.label.name) itself

It doesn't make sense to have two different actions making the same artifact, so this API would only ever be able to create empty directories. You probably want an API like declare_output(dir=True, create=True).

@Nero5023
Copy link
Contributor

Nero5023 commented Sep 9, 2024

For stderr/stdout, we have a discussion on this. We all agree that it is the convenient to have such feature. It is in our backlog now, if someone of us interested it will implement it.

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