bazel icon indicating copy to clipboard operation
bazel copied to clipboard

Implicit outputs feature is underdocumented

Open dslomov opened this issue 8 years ago • 6 comments
trafficstars

(originally reported by @alandonovan) The documentation at https://bazel.build/versions/master/docs/skylark/lib/globals.html#parameters-22 is vague. It should answer the following questions:

  • What are valid attribute types in %{attr}?
  • How is each type of attribute converted to a string by %{attr}?
  • What happens if the attribute is a sequence, such as %{deps}?
  • What happens if multiple sequence attributes appear in the same template?
  • What's the typical significance of the dict key?

Peeking at the implementation, the answers appear to be:

  • Only strings and lists of strings.
  • strings are unchanged; lists of strings are expanded elementwise
  • "%{x}-%{y}" produces the cross product of lists x and y
  • if the cross product is greater than 1, it's an error. if it's empty, there are no implicit outputs. otherwise the sole member of the cross product is used.

All this begs the question: what's the point of the cross-product logic if the resulting set has at most one element?

Also: why is it that the 'implicit outputs' function has access to whichever attributes it wants through its parameters AND returns a dict of templates "%{name}-%{srcs}.x" that are then subject to further attribute expansion? It seems redundant.

There are not one but three ad-hoc conversion processes here:

  1. existing_rule
  2. the mechanism that converts rule attributes into arguments to the rule.outputs function
  3. the attributesValue function that converts a rule and a %{foo} pattern to a list of strings.

Ideally there would be one (clearly documented) conversion from attributes to Skylark values to replace #1 and #2. #3 would be replaced by an extremely simple conversion such as "%s" recursively applied to each Skylark value. Anything else---dirname, basename; removal of files extensions, etc) should be done by writing Skylark logic.

Users do not have to write output functions very often. It is better to make them explicit than concise, and doing so lets you simplify the implementation and the spec.

dslomov avatar Apr 25 '17 11:04 dslomov

It's not obvious, how to define implicit output function in Skylark rule? It's mentioned here, but not documented how to achieve it in Skylark rule: [1].

Say, we have this output definition: https://github.com/GerritCodeReview/gerrit/blob/master/tools/bzl/js.bzl#L365-L368

outputs = {
        "html": "%{name}.html",
        "js": "%{name}.js",
    },

that use two steps: vulcanize (vulcanized.html) following by cripser, that split the vulcanized.html in html and js to provide CSP compliance.

What if we would like to optionally skip the cripser step and just return vulcanized output instead:

 outputs = {
        "vulcanized.html": "%{name}.vulcanized.html",
    },

Now, this should not be produced, if i just say:

vulcanize(name = 'foo')

would produce per default the former variant, but:

vulcanize(name = 'foo', skip_cripser = True)

would produce the latter?

davido avatar Nov 28 '17 07:11 davido

outputs is deprecated (EDIT: #7977)

pauldraper avatar Sep 29 '20 01:09 pauldraper

The word "deprecated" gets used a lot in this code base. All it means is "we wish we hadn't added this feature". It does not mean "this feature will be delete, although on some occasions that does happen. It is not a reason not to document a feature; indeed, deprecated functions usually warrant more documentation, to explain the acknowledged mistake in their design and to help users understand their alternatives.

I suspect many of our deprecated functions would never have been shipped in the first place if they had been properly documented.

alandonovan avatar Sep 29 '20 12:09 alandonovan

It doesn't work with --incompatible_no_rule_outputs_param .

AFAIK no one creates such a flag unless they intend to remove the feature in a future (breaking) release.

https://docs.bazel.build/versions/master/backward-compatibility.html

pauldraper avatar Sep 30 '20 23:09 pauldraper

While I agree with everything Alan said two comments above, I can't see us prioritizing proper documentation for all the quirks of implicit outputs anytime soon. We know we'd like to remove them, and I myself need more perspective on what exactly would replace them, but have no bandwidth for that right now.

brandjon avatar Feb 15 '21 21:02 brandjon

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[bot] avatar Oct 20 '24 01:10 github-actions[bot]

This issue has been automatically closed due to inactivity. If you're still interested in pursuing this, please post @bazelbuild/triage in a comment here and we'll take a look. Thanks!

github-actions[bot] avatar Jan 19 '25 01:01 github-actions[bot]