C++: Change Skylark API whitelisting to be part of flag.
This uses SkylarkSemantics now instead of the C++ configuration. The flag is:
--experimental_cc_skylark_api_enabled_packages
RELNOTES:none
PiperOrigin-RevId: 207235431
diff --git a/src/test/java/com/google/devtools/build/lib/packages/SkylarkSemanticsConsistencyTest.java b/src/test/java/com/google/devtools/build/lib/packages/SkylarkSemanticsConsistencyTest.java
index ed22d1e..21328c8 100644
--- a/src/test/java/com/google/devtools/build/lib/packages/SkylarkSemanticsConsistencyTest.java
+++ b/src/test/java/com/google/devtools/build/lib/packages/SkylarkSemanticsConsistencyTest.java
@@ -16,6 +16,7 @@
import static com.google.common.truth.Truth.assertThat;
+import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.devtools.build.lib.skyframe.serialization.DeserializationContext;
import com.google.devtools.build.lib.skyframe.serialization.DynamicCodec;
@@ -36,26 +37,22 @@
*
* <p>When adding a new option, it is trivial to make a transposition error or a copy/paste error.
* These tests guard against such errors. The following possible bugs are considered:
+ *
* <ul>
* <li>If a new option is added to {@code SkylarkSemantics} but not to {@code
* SkylarkSemanticsOptions}, or vice versa, then the programmer will either be unable to
* implement its behavior, or unable to test it from the command line and add user
* documentation. We hope that the programmer notices this on their own.
- *
- * <li>If {@link SkylarkSemanticsOptions#toSkylarkSemantics} or {@link
- * SkylarkSemanticsCodec#deserialize} is not updated to set all fields of {@code
- * SkylarkSemantics}, then it will fail immediately because all fields of {@link
+ * <li>If {@link SkylarkSemanticsOptions#toSkylarkSemantics} is not updated to set all fields of
+ * {@code SkylarkSemantics}, then it will fail immediately because all fields of {@link
* SkylarkSemantics.Builder} are mandatory.
- *
* <li>To catch a copy/paste error where the wrong field's data is threaded through {@code
* toSkylarkSemantics()} or {@code deserialize(...)}, we repeatedly generate matching random
* instances of the input and expected output objects.
- *
* <li>The {@link #checkDefaultsMatch} test ensures that there is no divergence between the
* default values of the two classes.
- *
- * <li>There is no test coverage for failing to update the non-generated webpage documentation.
- * So don't forget that!
+ * <li>There is no test coverage for failing to update the non-generated webpage documentation. So
+ * don't forget that!
* </ul>
*/
@RunWith(JUnit4.class)
@@ -121,6 +118,10 @@
private static SkylarkSemanticsOptions buildRandomOptions(Random rand) throws Exception {
return parseOptions(
// <== Add new options here in alphabetic order ==>
+ "--experimental_cc_skylark_api_enabled_packages="
+ + rand.nextDouble()
+ + ","
+ + rand.nextDouble(),
"--experimental_enable_repo_mapping=" + rand.nextBoolean(),
"--incompatible_bzl_disallow_load_after_statement=" + rand.nextBoolean(),
"--incompatible_depset_is_not_iterable=" + rand.nextBoolean(),
@@ -151,6 +152,8 @@
private static SkylarkSemantics buildRandomSemantics(Random rand) {
return SkylarkSemantics.builder()
// <== Add new options here in alphabetic order ==>
+ .experimentalCcSkylarkApiEnabledPackages(
+ ImmutableList.of(String.valueOf(rand.nextDouble()), String.valueOf(rand.nextDouble())))
.experimentalEnableRepoMapping(rand.nextBoolean())
.incompatibleBzlDisallowLoadAfterStatement(rand.nextBoolean())
.incompatibleDepsetIsNotIterable(rand.nextBoolean())
diff --git a/src/test/java/com/google/devtools/build/lib/rules/cpp/BUILD b/src/test/java/com/google/devtools/build/lib/rules/cpp/BUILD
index e15d458..71cfffb 100644
--- a/src/test/java/com/google/devtools/build/lib/rules/cpp/BUILD
+++ b/src/test/java/com/google/devtools/build/lib/rules/cpp/BUILD
@@ -96,6 +96,7 @@
name = "CcImportBaseConfiguredTargetTest",
srcs = ["CcImportBaseConfiguredTargetTest.java"],
deps = [
+ ":SkylarkCcCommonTestHelper",
"//src/main/java/com/google/devtools/build/lib:build-base",
"//src/main/java/com/google/devtools/build/lib/actions",
"//src/main/java/com/google/devtools/build/lib/rules/cpp",
diff --git a/src/test/java/com/google/devtools/build/lib/rules/cpp/CcImportBaseConfiguredTargetTest.java b/src/test/java/com/google/devtools/build/lib/rules/cpp/CcImportBaseConfiguredTargetTest.java
index 548bf8e..41bdb56 100644
--- a/src/test/java/com/google/devtools/build/lib/rules/cpp/CcImportBaseConfiguredTargetTest.java
+++ b/src/test/java/com/google/devtools/build/lib/rules/cpp/CcImportBaseConfiguredTargetTest.java
@@ -34,7 +34,9 @@
protected String skylarkImplementationLoadStatement = "";
@Before
- public void setSkylarkImplementationLoadStatement() {
+ public void setSkylarkImplementationLoadStatement() throws Exception {
+ setSkylarkSemanticsOptions(SkylarkCcCommonTestHelper.CC_SKYLARK_WHITELIST_FLAG);
+ invalidatePackages();
setIsSkylarkImplementation();
}
diff --git a/src/test/java/com/google/devtools/build/lib/rules/cpp/SkylarkCcCommonTest.java b/src/test/java/com/google/devtools/build/lib/rules/cpp/SkylarkCcCommonTest.java
index 61f2058..6bd4f03 100644
--- a/src/test/java/com/google/devtools/build/lib/rules/cpp/SkylarkCcCommonTest.java
+++ b/src/test/java/com/google/devtools/build/lib/rules/cpp/SkylarkCcCommonTest.java
@@ -45,8 +45,9 @@
public class SkylarkCcCommonTest extends BuildViewTestCase {
@Before
- public void setConfiguration() throws Exception {
- useConfiguration("--experimental_enable_cc_skylark_api");
+ public void setSkylarkSemanticsOptions() throws Exception {
+ setSkylarkSemanticsOptions(SkylarkCcCommonTestHelper.CC_SKYLARK_WHITELIST_FLAG);
+ invalidatePackages();
}
@Test
@@ -134,7 +135,7 @@
AnalysisMock.get()
.ccSupport()
.setupCrosstool(mockToolsConfig, "feature { name: 'foo_feature' }");
- useConfiguration("--features=foo_feature", "--experimental_enable_cc_skylark_api");
+ useConfiguration("--features=foo_feature");
scratch.file(
"a/BUILD",
"load(':rule.bzl', 'crule')",
@@ -733,7 +734,7 @@
@Test
public void testIsLinkingDynamicLibraryLinkVariables() throws Exception {
- useConfiguration("--linkopt=-pie", "--experimental_enable_cc_skylark_api");
+ useConfiguration("--linkopt=-pie");
assertThat(
commandLineForVariables(
CppActionNames.CPP_LINK_EXECUTABLE,
@@ -758,9 +759,7 @@
@Test
public void testIsUsingLinkerLinkVariables() throws Exception {
- useConfiguration(
- "--linkopt=-i_dont_want_to_see_this_on_archiver_command_line",
- "--experimental_enable_cc_skylark_api");
+ useConfiguration("--linkopt=-i_dont_want_to_see_this_on_archiver_command_line");
assertThat(
commandLineForVariables(
CppActionNames.CPP_LINK_EXECUTABLE,
@@ -1064,15 +1063,15 @@
}
@Test
- public void testCcLinkingProviderParamsWithoutFlag() throws Exception {
- useConfiguration("--noexperimental_enable_cc_skylark_api");
- setUpCcLinkingProviderParamsTest();
+ public void testFlagWhitelist() throws Exception {
+ setSkylarkSemanticsOptions("--experimental_cc_skylark_api_enabled_packages=\"\"");
+ SkylarkCcCommonTestHelper.createFiles(scratch, "foo/bar");
reporter.removeHandler(failFastHandler);
- getConfiguredTarget("//a:r");
+ getConfiguredTarget("//foo:bin");
assertContainsEvent(
- "Pass --experimental_enable_cc_skylark_api in order to "
- + "use the C++ API. Beware that we will be making breaking changes to this API "
- + "without prior warning.");
+ "You can try it out by passing "
+ + "--experimental_cc_skylark_api_enabled_packages=<list of packages>. Beware that we "
+ + "will be making breaking changes to this API without prior warning.");
}
@Test
@@ -1082,7 +1081,7 @@
.setupCrosstool(
mockToolsConfig,
"supports_interface_shared_objects: false");
- useConfiguration("--experimental_enable_cc_skylark_api");
+ useConfiguration();
setUpCcLinkingProviderParamsTest();
ConfiguredTarget r = getConfiguredTarget("//a:r");
@@ -1251,14 +1250,6 @@
}
@Test
- public void testWhitelist() throws Exception {
- SkylarkCcCommonTestHelper.createFiles(scratch, "foo/bar");
- reporter.removeHandler(failFastHandler);
- getConfiguredTarget("//foo:bin");
- assertContainsEvent("C++ Skylark API is for the time being");
- }
-
- @Test
public void testCopts() throws Exception {
SkylarkCcCommonTestHelper.createFilesForTestingCompilation(
scratch, "tools/build_defs/foo", "copts=depset(['-COMPILATION_OPTION'])");
diff --git a/src/test/java/com/google/devtools/build/lib/rules/cpp/SkylarkCcCommonTestHelper.java b/src/test/java/com/google/devtools/build/lib/rules/cpp/SkylarkCcCommonTestHelper.java
index 1aafab1..0a4dc6b 100644
--- a/src/test/java/com/google/devtools/build/lib/rules/cpp/SkylarkCcCommonTestHelper.java
+++ b/src/test/java/com/google/devtools/build/lib/rules/cpp/SkylarkCcCommonTestHelper.java
@@ -19,6 +19,9 @@
/** Methods useful for tests testing the C++ Skylark API. */
public final class SkylarkCcCommonTestHelper {
+ public static final String CC_SKYLARK_WHITELIST_FLAG =
+ "--experimental_cc_skylark_api_enabled_packages=tools/build_defs,experimental";
+
public static void createFilesForTestingCompilation(
Scratch scratch, String bzlFilePath, String compileProviderLines) throws Exception {
createFiles(scratch, bzlFilePath, compileProviderLines, "");
