download_file cannot be used twice in the same rule because it cannot specify an identifier

[thoughtpolice]

The following example doesn't seem to work, though I think it should:

defs.bzl:

def __download_license_data_impl(ctx: "context") -> ["provider"]:
    base_url = lambda name: "https://github.com/spdx/license-list-data/blob/{}/json/{}.json".format(ctx.attrs.revision, name)
    def dl_json_file(name, sha1):
        name = "{}.json".format(name)
        out = ctx.actions.declare_output(name)
        ctx.actions.download_file(out, base_url(name), sha1 = sha1)
        return out

    if len(ctx.attrs.sha1) != 2:
        fail("sha1 must be a list of two strings")

    if "licenses:" != ctx.attrs.sha1[0][:9]:
        fail("first sha1 hash must start with 'licenses:'")
    if "exceptions:" != ctx.attrs.sha1[1][:11]:
        fail("second sha1 hash must start with 'exceptions:'")

    licenses_sha1 = ctx.attrs.sha1[0][9:]
    exceptions_sha1 = ctx.attrs.sha1[1][11:]

    licenses_out = dl_json_file("licenses", ctx.attrs.sha1[0])
    exceptions_out = dl_json_file("exceptions", ctx.attrs.sha1[1])

    return [
        DefaultInfo(default_outputs = [ licenses_out, exceptions_out ])
    ]

download_license_data = rule(
    impl = __download_license_data_impl,
    attrs = {
        "revision": attrs.string(),
        "sha1": attrs.list(attrs.string()),
    },
)

and BUILD:

load(":defs.bzl", "download_license_data")

download_license_data(
    name = "spdx_license_data",
    revision = "v3.19",
    sha1 = [
        "licenses:cd5e714801727eb3dce52908ed818b5f54ddd6ba",
        "exceptions:69ca85d10f0737ddd5fd6c68d047bfb0f0fc65a7",
    ],
)

yields:

When running analysis for `root//src/larry:spdx_license_data (prelude//platform:default-7b06d4530de034dc)`

Caused by:
    Analysis produced multiple actions with category `download_file` and no identifier. Add an identifier to these actions to disambiguate them

But download_file can't take an identifier!

[ndmitchell]

Oops. That looks like a bug

[ndmitchell]

Cc @krallin

[krallin]

We should just use the output file like we do for write actions, I'll put that up