Android platforms breaks incremental dexing and desugaring about rules_android HOT 4 CLOSED

Well, replacing this:

with this workaround / hack gets it working again:

    deps_list = []
    for attr in _aspect_attrs():
        if attr == "deps" and hasattr(ctx, "split_attr"):
            deps = _utils.dedupe_split_attr(ctx.split_attr.deps)
        else:
            deps = getattr(ctx.attr, attr, [])

        if str(type(deps)) == "list":
            deps_list += deps
        elif str(type(deps)) == "Target":
            deps_list.append(deps)

    return deps_list

but it would be good to figure out why the ordering is different and maybe address that. That'll take some more time.

from rules_android.

Yes it looks like one part of the code is looking through jars from one configuration, and trying to look up those same jars in a dict formed from jars in a another configuration.

In this code specifically:

with the above repro the jar is bazel-out/k8-fastbuild-android-ST-a9ac79439181/bin/java/com/basicapp/libbasic_lib.jar and dex_archives_dict contains

bazel-out/k8-fastbuild-android-ST-9b738726d3a8/bin/java/com/basicapp/libbasic_lib.jar -> bazel-out/k8-fastbuild-android-ST-9b738726d3a8/bin/java/com/basicapp/_dx/basic_lib/libbasic_lib.jar.dex.zip
bazel-out/k8-fastbuild-android-ST-9b738726d3a8/bin/java/com/basicapp/_migrated/basic_lib_resources.jar -> bazel-out/k8-fastbuild-android-ST-9b738726d3a8/bin/java/com/basicapp/_dx/basic_lib/basic_lib_resources.jar.dex.zip

even though libbasic_lib.jar is in the dict, it's from a different configuration (k8-fastbuild-android-ST-a9ac79439181, k8-fastbuild-android-ST-9b738726d3a8). Interestingly this does not appear to happen internally (i.e. with multiple values to --android_platforms)....

from rules_android.

I'm pretty sure this comes down to differences in how the split transition is handled in different parts of the code.

In

the value for dex_archives_dict (eventually) comes from ctx.attr.deps:

and the value for classpath (eventually) comes from ctx.split_attr.deps:

dedupe_split_attr sorts the branches of the split_attr by key:

while ctx.attr.deps has a bunch of legacy behavior:

https://github.com/bazelbuild/bazel/blob/e751c5d7b3793824fc264e3c0110e677dab3bb7d/src/main/java/com/google/devtools/build/lib/analysis/PrerequisitesCollection.java#L127-L138

I suppose internally these just so happen to come out in the same order.

from rules_android.

I think I've gotten to the bottom of all this. This comes down to differences in which split of a split transition gets returned when the attribute that has the split transition is accessed via ctx.attr.

Way deep in bazel there is this little sort call:
https://github.com/bazelbuild/bazel/blob/94dcfc5739bca155922811703c95793a39b21a3a/src/main/java/com/google/devtools/build/lib/analysis/producers/PrerequisitesProducer.java#L322

Which split gets returns comes down to the mnemonic of the configuration for each split:
https://github.com/bazelbuild/bazel/blob/94dcfc5739bca155922811703c95793a39b21a3a/src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetAndData.java#L315

In the repro, the mnemonics look like
k8-fastbuild-android-ST-9b738726d3a8
k8-fastbuild-android-ST-a9ac79439181

The cpu is k8 in both, so the order is ending up being based on the hash -- almost arbitrary. (Had the hashes come out a little different, we might never have noticed this problem...)

Internally, they look like
arm64-v8a-fastbuild-android-ST-66eefe4ed9be
armeabi-v7a-fastbuild-android-ST-9d65607305a3

with the correct android cpu at the beginning, so that when these are sorted, they come out the same as when sorting the keys from ctx.split_attr.deps. (This might be more coincidental than purposeful, but it works.)

The mnemonic is determined by this code over here:
https://github.com/bazelbuild/bazel/blob/94dcfc5739bca155922811703c95793a39b21a3a/src/main/java/com/google/devtools/build/lib/analysis/config/OutputPathMnemonicComputer.java#L178

And the cpu value is determined here:
https://github.com/bazelbuild/bazel/blob/94dcfc5739bca155922811703c95793a39b21a3a/src/main/java/com/google/devtools/build/lib/analysis/config/OutputPathMnemonicComputer.java#L243

And indeed in bazel coreOptions.cpu / --cpu is k8 in both cases -- and so setting coreOptions.platformInOutputDir via --experimental_platform_in_output_dir should get the correct cpu from the platform.

However, --experimental_platform_in_output_dir is not actually set internally -- so how is the correct cpu being set... Indeed internally coreOptions.cpu / --cpu is arm64-v8a and armeabi-v7a for the relevant splits.... After a little more digging, the final piece is platform mappings: we have a platform mapping that sets --cpu to the correct values.

So the expedient solution is to just set --experimental_platform_in_output_dir. Presumably eventually that will become the default and it will all just work, but we should probably just access the deps via split_attr to ensure the correct split is used.

from rules_android.