Keep order of dict attributes the same as it was in the BUILD file.

This is a change from behavior introduced in unknown commit, which was added to preserve determinism even though the ordering of dicts in Python is non-deterministic. Now that we don't call into Python anymore, this is not necessary.

RELNOTES[INC]: The order of dict-valued attributes is now the order in the BUILD file (or in the Skylark dict they were created from) and not lexicographically sorted.

PiperOrigin-RevId: 187003317

2 files changed, 35 insertions, 0 deletions

diff --git a/src/test/java/com/google/devtools/build/lib/packages/BuildTypeTest.java b/src/test/java/com/google/devtools/build/lib/packages/BuildTypeTest.java
index a3604c31ce..0321498994 100644
--- a/src/test/java/com/google/devtools/build/lib/packages/BuildTypeTest.java
+++ b/src/test/java/com/google/devtools/build/lib/packages/BuildTypeTest.java
@@ -54,6 +54,21 @@ public class BuildTypeTest {
   }
 
   @Test
+  public void testKeepsDictOrdering() throws Exception {
+    Map<Object, String> input = new ImmutableMap.Builder<Object, String>()
+        .put("c", "//c")
+        .put("b", "//b")
+        .put("a", "//a")
+        .put("f", "//f")
+        .put("e", "//e")
+        .put("d", "//d")
+        .build();
+
+    assertThat(BuildType.LABEL_DICT_UNARY.convert(input, null, null).keySet())
+        .containsExactly("c", "b", "a", "f", "e", "d").inOrder();
+  }
+
+  @Test
   public void testLabelKeyedStringDictConvertsToMapFromLabelToString() throws Exception {
     Map<Object, String> input = new ImmutableMap.Builder<Object, String>()
         .put("//absolute:label", "absolute value")
diff --git a/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleContextTest.java b/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleContextTest.java
index 2a86f2ff2f..cb5a2b5ea8 100644
--- a/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleContextTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleContextTest.java
@@ -27,6 +27,7 @@ import com.google.devtools.build.lib.actions.ActionAnalysisMetadata;
 import com.google.devtools.build.lib.actions.Artifact;
 import com.google.devtools.build.lib.analysis.ActionsProvider;
 import com.google.devtools.build.lib.analysis.ConfiguredRuleClassProvider;
+import com.google.devtools.build.lib.analysis.ConfiguredTarget;
 import com.google.devtools.build.lib.analysis.TransitiveInfoCollection;
 import com.google.devtools.build.lib.analysis.actions.FileWriteAction;
 import com.google.devtools.build.lib.analysis.actions.SpawnAction;
@@ -35,6 +36,7 @@ import com.google.devtools.build.lib.analysis.skylark.SkylarkRuleContext;
 import com.google.devtools.build.lib.analysis.util.MockRule;
 import com.google.devtools.build.lib.cmdline.Label;
 import com.google.devtools.build.lib.packages.Info;
+import com.google.devtools.build.lib.packages.SkylarkProvider.SkylarkKey;
 import com.google.devtools.build.lib.packages.SkylarkProviderIdentifier;
 import com.google.devtools.build.lib.rules.java.JavaInfo;
 import com.google.devtools.build.lib.rules.java.JavaSourceJarsProvider;
@@ -2138,4 +2140,22 @@ public class SkylarkRuleContextTest extends SkylarkTestCase {
       }
     }
   }
+
+  @Test
+  public void testMapAttributeOrdering() throws Exception {
+    scratch.file("a/a.bzl",
+        "key_provider = provider(fields=['keys'])",
+        "def _impl(ctx):",
+        "  return [key_provider(keys=ctx.attr.value.keys())]",
+        "a = rule(implementation=_impl, attrs={'value': attr.string_dict()})");
+    scratch.file("a/BUILD",
+        "load(':a.bzl', 'a')",
+        "a(name='a', value={'c': 'c', 'b': 'b', 'a': 'a', 'f': 'f', 'e': 'e', 'd': 'd'})");
+
+    ConfiguredTarget a = getConfiguredTarget("//a");
+    SkylarkKey key = new SkylarkKey(Label.parseAbsolute("//a:a.bzl"), "key_provider");
+    @SuppressWarnings("unchecked")
+    SkylarkList<String> keys = (SkylarkList<String>) a.get(key).getValue("keys");
+    assertThat(keys).containsExactly("c", "b", "a", "f", "e", "d").inOrder();
+  }
 }