thought-machine / please

High-performance extensible build system for reproducible multi-language builds.
https://please.build
Apache License 2.0
2.47k stars 205 forks source link

sh_cmd does not inject provenance into generated shell script #3009

Open alunduil-tm opened 10 months ago

alunduil-tm commented 10 months ago

Using sh_cmd does not indicate what rule (possibly derivative) generated the shell script. This can make finding the origin of such a shell script difficult in a code base with layers of build definitions.

It would be ideal to have comments at the top of the shell script providing a stack (even partial) indicating which build rules to find to edit this command in this shell script.

Example

# Generated by ${RULE_NAME} of type ${BUILD_DEFINITION_FUNCTION_NAME}
...
# Generated by ${BUILD_DEFINITION_FUNCTION_NAME} in ${FILE_FOR_BUIlD_DEFINITION}

...

Specifically

# Generated by my_shell_consumer of type derived_rule
# Generated by hidden_derived_rule in //.../hidden.build_def

...

The invocation by sh_cmd itself is not as insightful but might be useful if the definition of that rule needs to be changed as well (much rarer).

This could have the side-effect of clearly indicating these files as generated as well since they don't seem to be checked for hashing in plz-out and are simply assumed correct if the timestamp is more recent than the sources (which is incorrect if making local edits to these files).

Please let me know what other information would be useful, but I'll try to put together a pull request when I get time to show my proposed solution as well.

Tatskaari commented 10 months ago

As discussed offline, there is a convention that child rules should follow that can make following this trail easier, where the target name would be _{parent_rule_name}#{tag}. Unfortunately, the please parser doesn't really have any other information to hand to give any more context, as it stands.

Another requested feature is to query all targets of a given kind e.g. go_library, which is similarly not possible. We have talked about adding more information to the parse scope. For example, if we have a call like so:

go_library(
    name = "foo",
    ...
)

It would be good to record that we're currently evaluating go_library(), and that the expected target is :foo. If we had this information in the interpreter scope, we could feasibly give you the providence of any other targets generated while interpreting this rule.

We'd need some new syntax or something though. It's only by convention that we know the above call will produce :foo, which isn't universally true.

izissise commented 10 months ago

This is actually the thinking behind the stacktrace pull request https://github.com/thought-machine/please/pull/2896 to have more information in the parse scope.

alunduil-tm commented 10 months ago

@izissise if you have an issue or something and would like to merge this request into #2896 feel free as I think having stracktraces for rule abstractions would solve this handily.

izissise commented 7 months ago

2896 was closed as stall :/