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

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

Open
ghost opened this issue Jul 24, 2023 · 4 comments
Open

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

ghost opened this issue Jul 24, 2023 · 4 comments
Labels
P4 This is either out of scope or we don't have bandwidth to review a PR. (No assignee) team-Rules-API API for writing rules/aspects: providers, runfiles, actions, artifacts type: feature request

Comments

@ghost
Copy link

ghost commented Jul 24, 2023

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
Copy link
Member

coeuvre commented Jul 24, 2023

cc @tjgq

@tjgq
Copy link
Contributor

tjgq commented Jul 24, 2023

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 #14647 first. (But not having a label associated with the output might also be okay.)

@iancha1992 iancha1992 added the team-Remote-Exec Issues and PRs for the Execution (Remote) team label Jul 24, 2023
@tjgq tjgq added team-Rules-API API for writing rules/aspects: providers, runfiles, actions, artifacts and removed team-Remote-Exec Issues and PRs for the Execution (Remote) team labels Jul 25, 2023
@comius comius added P4 This is either out of scope or we don't have bandwidth to review a PR. (No assignee) and removed untriaged labels Aug 22, 2023
@comius
Copy link
Contributor

comius commented Aug 22, 2023

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
Copy link
Contributor

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P4 This is either out of scope or we don't have bandwidth 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

7 participants