search

bazelbuild / bazel

a fast, scalable, multi-language and extensible build system
https://bazel.build
Apache License 2.0
22.99k stars 4.03k forks source link

FR: allow genrules to produce directories (tree artifacts) #19030

Open ghost opened 1 year ago

ghost commented 1 year ago

Description of the feature request:

It'd be cool if Bazel officially supported directories as first-class outputs of genrules. If including directories in outs is undesirable for some reason, perhaps they could be declared using a separate outdirs attribute, each of which would roughly map to a regular rule's ctx.actions.declare_directory.

Alternatively, would a Starlark impl of genrule (w/ separate outdirs semantics) in Skylib make sense? (We don't have to worry about Windows users yet, but how would cmd_bat/cmd_ps map to ctx.actions.run/run_shell? Would there be a separate FR for run_bat+run_ps?)

What underlying problem are you trying to solve with this feature?

I have to support novice Bazel users who are comfortable writing genrules but are intimidated by Bazel's rule implementations, especially when they only need a rule for a single target. (I.e. there is no need for reuse.) These users are not build engineers, so they don't want to invest time in learning about Bazel's execution model, constraints for RBE, etc. On their own, genrule's srcs, outs, and cmd are easy enough for most folks to understand. At the same time, I can't be the SPOF the instant they need to update one of their genrules to produce a directory instead of a file.

To date, we've been in the habit of exporting directories as zips (using a custom wrapper around Bazel's zipper for reproducibility), and then unzipping them somewhere else. This is clumsy when feeding outputs to @rules_pkg//pkg:mappings.bzl%pkg_files, since we have to

  1. generate a directory tree (this is the bulk of the genrule's cmd);
  2. zip it w/ custom tool (boilerplate appearing at end of cmd);
  3. unzip it (and assign it a target name) to a TreeArtifact using an internal unzip rule; and then
  4. feed that TreeArtifact to pkg_files.srcs.

I'd rather send a TreeArtifact directly from a genrule to pkg_files.

The most common use case for this is any sort of generated code or documentation. This seems like a common use case across Bazel's entire userbase (e.g. issue #1025).

Which operating system are you running Bazel on?

No response

What is the output of bazel info release?

release 6.0.0-vmware

If bazel info release returns development version or (@non-git), tell us how you built Bazel.

No response

What's the output of git remote get-url origin; git rev-parse master; git rev-parse HEAD ?

No response

Have you found anything relevant by searching the web?

I reviewed GitHub issues by searching for "in:title genrule". No relevant hits.

Other relevant refs:

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

No response

coeuvre commented 1 year ago

cc @tjgq

tjgq commented 1 year ago

I am supportive of this idea, although I'm ultimately not the one to make a call wrt expanding the genrule API. I do agree that, if this were to be implemented, it would have to be a separate outdirs attribute.

One issue with a pure Starlark implementation (in addition to the already mentioned Windows support) is that it wouldn't be possible to associate a label with the elements of outdirs as it currently happens with genrule.outs, since all of the mechanisms to declare a label (attr.output; attr.output_list; the outputs argument to rule(...)) produce the equivalent of ctx.actions.declare_file, not ctx.actions.declare_directory. We'd have to implement https://github.com/bazelbuild/bazel/issues/14647 first. (But not having a label associated with the output might also be okay.)

comius commented 1 year ago

I believe treeartifacts are hard to fully/correctly support within the rules. As such I wouldn’t like to introduce problems when they are exposed to targets.

If they don’t work perfectly they would also scare away the users like the rules do. :)

alexeagle commented 7 months ago

Note that bazel-skylib run_binary has this problem as well. I updated the genrule documentation to point most users at run_binary instead.

https://github.com/aspect-build/bazel-lib/blob/main/docs/run_binary.md has a fork of run_binary to workaround that.