Implicit outputs feature is underdocumented

[dslomov]

(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.

[davido]

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?

[pauldraper]

outputs is deprecated (EDIT: #7977)

[alandonovan]

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.

[pauldraper]

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

[brandjon]

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.