Stale transition value

[nikhilkalige]

I have following code that I am using to create a transition. It transitions on //hal/mach:mach build setting as defined below

string_flag(
    name = "mach",
    build_setting_default = "",
    values = ["x86", "arm"],
    visibility = ["//visibility:public"],
)

def _mach_transition_impl(settings, attr):
    print(attr.name, attr.mach)
    return {"//hal/mach:mach": attr.mach}

_mach_transition = transition(
    implementation = _mach_transition_impl,
    inputs = [],
    outputs = ["//hal/mach:mach"],
)

def _create_image_impl(ctx):
    inputs = []
    content_map = {}
    for app in ctx.attr.applications:
        if DefaultInfo in app:
            inputs.extend(app[DefaultInfo].files.to_list())
        for f in app.files.to_list():
            inputs.append(f)

    manifest_file = ctx.actions.declare_file(ctx.label.name + ".manifest")
    inputs.append(manifest_file)
    output = [file.path for file in inputs]
    ctx.actions.write(
        manifest_file,
        "\n".join(output),
    )
    return DefaultInfo(files = depset(inputs))

_create_image = rule(
    implementation = _create_image_impl,
    cfg = _mach_transition,
    attrs = {
        "applications": attr.label_list(mandatory = True), # , cfg = _mach_transition),
        "mach": attr.string(mandatory = True),
        "_allowlist_function_transition": attr.label(
            default = "@bazel_tools//tools/allowlists/function_transition_allowlist",
        ),
    },
)

I create the following using the above

image.create_image(
    name = "pc",
    mach = "x86",
    applications = [ ... ]
)

image.create_image(
    name = "arm",
    mach = "arm",
    applications = [ ... ]
)

When I change the mach to a invalid value, say armX, I get an error, but if I change it again to armY, I still get an error saying the armX is invalid.

With armX
Error in fail: Error setting //hal/mach:mach: invalid value 'armX'. Allowed values are ["x86", "arm"]

With armY
Error in fail: Error setting //hal/mach:mach: invalid value 'armX'. Allowed values are ["x86", "arm"]

I don't understand why I am still getting stale values.

I have a minimal example here https://github.com/nikhilkalige/bazel-select-impl-test , but this does not seem to suffer the same problem, where as my larger codebase fails to pick up the changes to files.

I am only seeing this error with newer version of bazel, I tried it out 5 post release (1a7d524) and one more slightly older version too.

[gregestren]

Interesting report. I'll give it a look as soon as I can...

[gregestren]

Thanks. I confirmed this.

It's cache key imprecision when trying to cache transition execution:

bazel/src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkRuleTransitionProvider.java

Lines 77 to 81 in 0428828

	CacheKey(StarlarkDefinedConfigTransition starlarkDefinedConfigTransition, Rule rule) {
	this.starlarkDefinedConfigTransition = starlarkDefinedConfigTransition;
	this.ruleLabel = rule.getLabel();
	this.hashCode = Objects.hash(starlarkDefinedConfigTransition, ruleLabel);
	}

I think this is as simple as storing the full rule instead of rule.getLabel() - key point being that also takes into account the target's attributes, so when you change mach = "armX" to mach = "armY" that change invalidates the cache.

The point of the cache was to avoid redundant execution of expensive transitions. I'd want to do a few quick performance tests to check that this fix doesn't slow anything down (I don't think it will).

Thanks for catching this!

[gregestren]

FYI I have a pending fix for this at https://bazel-review.googlesource.com/c/bazel/+/178498. I just want to check it doesn't introduce performance regressions before submitting.

[illicitonion]

@bazel-io fork 5.2

[brentleyjones]

@bazel-io flag

[ckolli5]

@bazel-io fork 5.2.0