Crash when defining a Starlark transition involving platforms

[lberki]

I'm trying to create a Starlark split transition that changes the current platform based on a flag. Code:

touch WORKSPACE
cat >rules.bzl <<EOF
def _my_flag_impl(ctx):
    return [config_common.FeatureFlagInfo(value = ctx.build_setting_value)]

my_flag = rule(
    implementation = _my_flag_impl,
    build_setting = config.string_list(flag = True),
)

def _my_transition_impl(inputs, outputs):
    result = {}

    for platform in inputs["//:my_flag"]:
        result[platform] = {"//command_line_option:platforms": [platform]}

    return result

_my_transition = transition(
    implementation = _my_transition_impl,
    inputs = ["//:my_flag"],
    outputs = [
	"//command_line_option:platforms",
    ],
)

def _my_rule_impl(ctx):
    return []

my_rule = rule(
    implementation = _my_rule_impl,
    attrs = {
	"deps": attr.label_list(
            cfg = _my_transition,
	),
        "_whitelist_function_transition": attr.label(
            default = "@bazel_tools//tools/whitelists/function_transition_whitelist",
	),
    },
)
EOF

cat > BUILD <<EOF
load(":rules.bzl", "my_flag", "my_rule")

my_flag(
    name = "my_flag",
    build_setting_default = [],
)

cc_binary(
    name = "bin",
    srcs = ["bin.cc"],
)

my_rule(
    name = "my_rule",
    deps = [":bin"],
)
EOF

bazel build --//:my_flag=//:foo,//:bar :my_rule

The result is a big ugly crash. The relevant part of the stack trace is:

java.lang.ClassCastException: class java.lang.String cannot be cast to class com.google.devtools.build.lib.cmdline.Label (java.lang.String is in module java.base of loader 'bootstrap'; com.google.devtools.build.lib.cmdline.Label is in unnamed module of loader 'app')
	at com.google.devtools.build.lib.skyframe.PlatformMappingValue.computeMapping(PlatformMappingValue.java:218)
	at com.google.devtools.build.lib.skyframe.PlatformMappingValue.lambda$map$0(PlatformMappingValue.java:191)
	at com.google.common.cache.LocalCache$LocalManualCache$1.load(LocalCache.java:4875)
	at com.google.common.cache.LocalCache$LoadingValueReference.loadFuture(LocalCache.java:3527)
	at com.google.common.cache.LocalCache$Segment.loadSync(LocalCache.java:2276)
	at com.google.common.cache.LocalCache$Segment.lockedGetOrLoad(LocalCache.java:2154)
	at com.google.common.cache.LocalCache$Segment.get(LocalCache.java:2044)
	at com.google.common.cache.LocalCache.get(LocalCache.java:3951)
	at com.google.common.cache.LocalCache$LocalManualCache.get(LocalCache.java:4870)
	at com.google.devtools.build.lib.skyframe.PlatformMappingValue.map(PlatformMappingValue.java:191)
	at com.google.devtools.build.lib.skyframe.BuildConfigurationValue.keyWithPlatformMapping(BuildConfigurationValue.java:68)
	at com.google.devtools.build.lib.analysis.config.ConfigurationResolver.resolveConfigurations(ConfigurationResolver.java:302)
	at com.google.devtools.build.lib.skyframe.ConfiguredTargetFunction.computeDependencies(ConfiguredTargetFunction.java:584)
	at com.google.devtools.build.lib.skyframe.ConfiguredTargetFunction.compute(ConfiguredTargetFunction.java:317)
	at com.google.devtools.build.skyframe.AbstractParallelEvaluator$Evaluate.run(AbstractParallelEvaluator.java:451)

[gregestren]

I think the simplest immediate fix to get your code working is to change

result[platform] = {"//command_line_option:platforms": [platform]}

to

result[platform] = {"//command_line_option:platforms": [Label(platform)]}

That got it working for me.

This also works:

def _my_transition_impl(inputs, outputs):
    result = []
    for platform in inputs["//ltest:my_flag"]:
        result.append({"//command_line_option:platforms": [Label(platform)]})
    return result

The most working version of split transitions today is going to be [{new settings1}, {new settings 2}, etc.]. In theory {"key1": {new settings 1}, etc.} should work. But I don't think the key accessing logic has been written yet.

But obviously Bazel shouldn't crash. I'll prioritize a fix to stop that.

As for the typing, I think the core Starlark folks were hesitant to add too many types to the config module. Half the point of Starlark flags is that they can model more complex types purely in Starlark (in _my_flag_impl). But there's a caveat that I don't think transitions take the results of _my_flag_impl because that creates weird dependencies on the transitions.

[aiuto]

cc/ @bazelbuild/configurability

[gregestren]

Lots more context at

bazel/src/main/java/com/google/devtools/build/lib/analysis/skylark/FunctionTransitionUtil.java

Line 272 in ca48e9a

// TODO(b/153867317): check for crashing options types in this logic.

I recognize that bug isn't fully visible to all - ping me if you'd like to hear more details.

tl;dr; is I might have a way forward on this that doesn't just fix the crash but (maybe?) resolves the subtleties described above.

[aiuto]

This was a faux ping. I was trying to see if team assignment worked. This was a convenient issue to test on.

…

On Thu, Apr 23, 2020 at 4:22 PM Greg ***@***.***> wrote: Lots more context at https://github.com/bazelbuild/bazel/blob/ca48e9ac4ddae455e816fb0bd3895f572d149d56/src/main/java/com/google/devtools/build/lib/analysis/skylark/FunctionTransitionUtil.java#L272 I recognize that bug isn't fully visible to all - ping me if you'd like to hear more details. tl;dr; is I might have a way forward on this that doesn't just fix the crash but (maybe?) resolves the subtleties described above. — You are receiving this because you are on a team that was mentioned. Reply to this email directly, view it on GitHub <#8936 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAXHHHHZ3DVQ5GEIDAUDBV3ROCPORANCNFSM4IFFUUBA> .

[gregestren]

Still a worthy ping, especially given its priority and length of time sitting around. :)