Automated rollback of commit b654a974876f3fd915a1e5118ebad57b1d55984f.
*** Reason for rollback ***
Needs a more careful rollout.
PiperOrigin-RevId: 346948380
diff --git a/src/main/java/com/google/devtools/build/lib/packages/StarlarkNativeModule.java b/src/main/java/com/google/devtools/build/lib/packages/StarlarkNativeModule.java
index 0345fed..d7cfe10 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/StarlarkNativeModule.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/StarlarkNativeModule.java
@@ -352,12 +352,12 @@
if (elt == null) {
continue;
}
+
l.add(elt);
}
return Tuple.copyOf(l);
}
-
if (val instanceof Map) {
Dict.Builder<Object, Object> m = Dict.builder();
for (Map.Entry<?, ?> e : ((Map<?, ?>) val).entrySet()) {
@@ -372,7 +372,6 @@
}
return m.build(mu);
}
-
if (val.getClass().isAnonymousClass()) {
// Computed defaults. They will be represented as
// "deprecation": com.google.devtools.build.lib.analysis.BaseRuleClasses$2@6960884a,
@@ -386,22 +385,24 @@
return null;
}
- if (val instanceof BuildType.SelectorList) {
- List<Object> selectors = new ArrayList<>();
- for (BuildType.Selector<?> selector : ((BuildType.SelectorList<?>) val).getSelectors()) {
- selectors.add(
- new SelectorValue(
- ((Map<?, ?>) starlarkifyValue(mu, selector.getEntries(), pkg)),
- selector.getNoMatchError()));
- }
- try {
- return SelectorList.of(selectors);
- } catch (EvalException e) {
- throw new NotRepresentableException(e.getMessage());
- }
+ if (val instanceof StarlarkValue) {
+ return val;
}
- if (val instanceof StarlarkValue) {
+ if (val instanceof BuildType.SelectorList) {
+ // This is terrible:
+ // 1) this value is opaque, and not a BUILD value, so it cannot be used in rule arguments
+ // 2) its representation has a pointer address, so it breaks hermeticity.
+ //
+ // Even though this is clearly imperfect, we return this value because otherwise
+ // native.rules() fails if there is any rule using a select() in the BUILD file.
+ //
+ // To remedy this, we should return a SelectorList. To do so, we have to
+ // 1) recurse into the Selector contents of SelectorList, so those values are Starlarkified
+ // too
+ // 2) get the right Class<?> value. We could probably get at that by looking at
+ // ((SelectorList)val).getSelectors().first().getEntries().first().getClass().
+
return val;
}
diff --git a/src/main/java/com/google/devtools/build/lib/starlarkbuildapi/StarlarkNativeModuleApi.java b/src/main/java/com/google/devtools/build/lib/starlarkbuildapi/StarlarkNativeModuleApi.java
index 298322a..c3a2470 100644
--- a/src/main/java/com/google/devtools/build/lib/starlarkbuildapi/StarlarkNativeModuleApi.java
+++ b/src/main/java/com/google/devtools/build/lib/starlarkbuildapi/StarlarkNativeModuleApi.java
@@ -105,8 +105,7 @@
+ " package.</li>" //
+ "<li>Lists are represented as tuples, and dicts are converted to new, mutable"
+ " dicts. Their elements are recursively converted in the same fashion.</li>" //
- + "<li><code>select</code> values are returned with their contents transformed as " //
- + "described above.</li>" //
+ + "<li><code>select</code> values are returned as is." //
+ "<li>Attributes for which no value was specified during rule instantiation and"
+ " whose default value is computed are excluded from the result. (Computed defaults"
+ " cannot be computed until the analysis phase.).</li>" //
diff --git a/src/test/java/com/google/devtools/build/lib/starlark/StarlarkRuleContextTest.java b/src/test/java/com/google/devtools/build/lib/starlark/StarlarkRuleContextTest.java
index f065c8d..a19f514 100644
--- a/src/test/java/com/google/devtools/build/lib/starlark/StarlarkRuleContextTest.java
+++ b/src/test/java/com/google/devtools/build/lib/starlark/StarlarkRuleContextTest.java
@@ -561,21 +561,15 @@
"test/existing_rule.bzl",
"def macro():",
" s = select({'//foo': ['//bar']})",
- " print('Passed: ' + repr(s))",
" native.cc_library(name = 'x', srcs = s)",
- " print('Returned: ' + repr(native.existing_rule('x')['srcs']))",
- // The value returned here should round-trip fine.
- " native.cc_library(name = 'y', srcs = native.existing_rule('x')['srcs'])");
+ " print(native.existing_rule('x')['srcs'])");
scratch.file(
"test/BUILD",
"load('//test:existing_rule.bzl', 'macro')",
"macro()",
"cc_library(name = 'a', srcs = [])");
getConfiguredTarget("//test:a");
- assertContainsEvent("Passed: select({\"//foo\": [\"//bar\"]}");
- // The short labels are now in their canonical form, and the sequence is represented as
- // tuple instead of list, but the meaning is unchanged.
- assertContainsEvent("Returned: select({\"//foo:foo\": (\"//bar:bar\",)}");
+ assertContainsEvent("select({Label(\"//foo:foo\"): [Label(\"//bar:bar\")]})");
}
@Test
