facebook / buck2

Build system, successor to Buck
https://buck2.build/
Apache License 2.0
3.33k stars 194 forks source link

`actions.write_json` does not propagate dependencies #634

Closed cbarrete closed 2 weeks ago

cbarrete commented 2 weeks ago

Consider the following simplified but somewhat realistic rule implementation:

def _my_rule_impl(ctx: AnalysisContext) -> list[Provider]:
    output = ctx.actions.declare_output("some_output.foo")
    some_dir = ctx.actions.declare_output("some_dir", dir = True)
    builder_parameters = {
        "output_path": output,
        "some_dir": some_dir,
    }
    builder_parameters_file = ctx.actions.write_json("parameters.json", builder_parameters)

    ctx.actions.run(
        cmd_args([
            ctx.attrs.my_script[RunInfo],
            builder_parameters_file,
        ]).hidden(output.as_output(), some_dir.as_output()),
        category = "whatever",
    )

    return [
        DefaultInfo(default_output = output),
    ]

_package_rule = rule(
    impl = _package_rule_impl,
    attrs = {
        "my_script": attrs.default_only(attrs.exec_dep(providers = [RunInfo], default = "//path/to:my_script")),
    },
)

I am essentially calling some script that is responsible for producing my output. Because it needs a few parameters (in this case just a directory to do work within, but in practice also more structured data), I pass the parameters around via a JSON file, the path of which is passed as the unique command line argument to the builder.

My issue is that I need to add a call to hidden, and seemingly manually list the all dependencies (passing builder_parameters.values() does not typecheck). Not only is it in theory redundant, it's also error prone and brittle: any changes that don't properly update the hidden call would still pass on systems that already have the required dependencies already materialized (quite likely if those are e.g. commonly built targets), but the build would be incorrect and fail after a buck2 clean.

Ideally I'd like actions.write_json to propagate its contents as dependencies, which I believe makes sense, but any XY-problem style solutions to my problem would be welcome as well :)

zjturner commented 2 weeks ago

Did you try this?

    builder_parameters = {
        "output_path": output.as_output(),
        "some_dir": some_dir.as_output(),
    }
cbarrete commented 2 weeks ago

I did, no difference. I also tried to call builder_parameters.as_output(), but it's just a dict, no magic there!

benbrittain commented 2 weeks ago

The right thing to do in this scenario is pass with_inputs = True.

https://buck2.build/docs/api/build/actions/#actionswrite_json

If you pass with_inputs = True, you'll get back a cmd_args that expands to the JSON file but carries all the underlying inputs as dependencies (so you don't have to use, for example, hidden for them to be added to an action that already receives the JSON file)

cbarrete commented 2 weeks ago

Indeed, I missed that, thanks Ben :)