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

sh_cmd does not inject provenance into generated shell script #3009

Open
alunduil-tm opened this issue Jan 2, 2024 · 4 comments
Open

sh_cmd does not inject provenance into generated shell script #3009

alunduil-tm opened this issue Jan 2, 2024 · 4 comments

Comments

@alunduil-tm
Copy link

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

Tatskaari commented Jan 2, 2024

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

izissise commented Jan 3, 2024

This is actually the thinking behind the stacktrace pull request #2896
to have more information in the parse scope.

@alunduil-tm
Copy link
Author

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

#2896 was closed as stall :/

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants