Add support for expand_if_available for env_entry

PiperOrigin-RevId: 548694860 Change-Id: Iea82f7eed2727d08057bae98d597cbeaae8c3f28
bazelbuild · Jul 17, 2023 · cc335fd · cc335fd
1 parent 4714cd7
commit cc335fd
Show file tree

Hide file tree

Showing 5 changed files with 91 additions and 3 deletions.
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcModule.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcModule.java
@@ -1527,8 +1527,13 @@ static EnvEntry envEntryFromStarlark(StarlarkInfo envEntryStruct) throws EvalExc
     if (value == null || value.isEmpty()) {
       throw infoError(envEntryStruct, "'value' parameter of env_entry must be a nonempty string.");
     }
+    String expandIfAvailable =
+        getOptionalFieldFromStarlarkProvider(envEntryStruct, "expand_if_available", String.class);
     StringValueParser parser = new StringValueParser(value);
-    return new EnvEntry(key, parser.getChunks());
+    return new EnvEntry(
+        key,
+        parser.getChunks(),
+        expandIfAvailable == null ? ImmutableSet.of() : ImmutableSet.of(expandIfAvailable));
   }
 
   /** Creates a {@link WithFeatureSet} from a {@link StarlarkInfo}. */

diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainFeatures.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainFeatures.java
@@ -203,16 +203,22 @@ public int hashCode() {
   public static class EnvEntry {
     private final String key;
     private final ImmutableList<StringChunk> valueChunks;
+    private final ImmutableSet<String> expandIfAllAvailable;
 
     private EnvEntry(CToolchain.EnvEntry envEntry) throws EvalException {
       this.key = envEntry.getKey();
       StringValueParser parser = new StringValueParser(envEntry.getValue());
       this.valueChunks = parser.getChunks();
+      this.expandIfAllAvailable = ImmutableSet.copyOf(envEntry.getExpandIfAllAvailableList());
     }
 
-    EnvEntry(String key, ImmutableList<StringChunk> valueChunks) {
+    EnvEntry(
+        String key,
+        ImmutableList<StringChunk> valueChunks,
+        ImmutableSet<String> expandIfAllAvailable) {
       this.key = key;
       this.valueChunks = valueChunks;
+      this.expandIfAllAvailable = expandIfAllAvailable;
     }
 
     String getKey() {
@@ -227,13 +233,29 @@ String getValue() {
                   .collect(ImmutableList.toImmutableList()));
     }
 
+    ImmutableSet<String> getExpandIfAllAvailable() {
+      return expandIfAllAvailable;
+    }
+
+    private boolean canBeExpanded(CcToolchainVariables variables) {
+      for (String variable : expandIfAllAvailable) {
+        if (!variables.isAvailable(variable)) {
+          return false;
+        }
+      }
+      return true;
+    }
+
     /**
      * Adds the key/value pair this object represents to the given map of environment variables. The
      * value of the entry is expanded with the given {@code variables}.
      */
     public void addEnvEntry(
         CcToolchainVariables variables, ImmutableMap.Builder<String, String> envBuilder)
         throws ExpansionException {
+      if (!canBeExpanded(variables)) {
+        return;
+      }
       StringBuilder value = new StringBuilder();
       for (StringChunk chunk : valueChunks) {
         value.append(chunk.expand(variables));

diff --git a/src/main/protobuf/crosstool_config.proto b/src/main/protobuf/crosstool_config.proto
@@ -114,6 +114,7 @@ message CToolchain {
   message EnvEntry {
     required string key = 1;
     required string value = 2;
+    repeated string expand_if_all_available = 3;
   }
 
   // A set of features; used to support logical 'and' when specifying feature

diff --git a/src/test/java/com/google/devtools/build/lib/rules/cpp/CcToolchainFeaturesTest.java b/src/test/java/com/google/devtools/build/lib/rules/cpp/CcToolchainFeaturesTest.java
@@ -300,6 +300,66 @@ public void testEnvVars() throws Exception {
     assertThat(env).doesNotContainEntry("doNotInclude", "doNotIncludePlease");
   }
 
+  @Test
+  public void testEnvVarsWithMissingVariableIsNotExpanded() throws Exception {
+    FeatureConfiguration configuration =
+        CcToolchainTestHelper.buildFeatures(
+                "feature {",
+                "  name: 'a'",
+                "  env_set {",
+                "     action: 'c++-compile'",
+                "     env_entry { key: 'foo', value: 'bar', expand_if_all_available: 'v' }",
+                "  }",
+                "}")
+            .getFeatureConfiguration(ImmutableSet.of("a"));
+
+    ImmutableMap<String, String> env =
+        configuration.getEnvironmentVariables(CppActionNames.CPP_COMPILE, createVariables());
+
+    assertThat(env).doesNotContainEntry("foo", "bar");
+  }
+
+  @Test
+  public void testEnvVarsWithAllVariablesPresentAreExpanded() throws Exception {
+    FeatureConfiguration configuration =
+        CcToolchainTestHelper.buildFeatures(
+                "feature {",
+                "  name: 'a'",
+                "  env_set {",
+                "     action: 'c++-compile'",
+                "     env_entry { key: 'foo', value: 'bar', expand_if_all_available: 'v' }",
+                "  }",
+                "}")
+            .getFeatureConfiguration(ImmutableSet.of("a"));
+
+    ImmutableMap<String, String> env =
+        configuration.getEnvironmentVariables(
+            CppActionNames.CPP_COMPILE, createVariables("v", "1"));
+
+    assertThat(env).containsExactly("foo", "bar").inOrder();
+  }
+
+  @Test
+  public void testEnvVarsWithAllVariablesPresentAreExpandedWithVariableExpansion()
+      throws Exception {
+    FeatureConfiguration configuration =
+        CcToolchainTestHelper.buildFeatures(
+                "feature {",
+                "  name: 'a'",
+                "  env_set {",
+                "     action: 'c++-compile'",
+                "     env_entry { key: 'foo', value: '%{v}', expand_if_all_available: 'v' }",
+                "  }",
+                "}")
+            .getFeatureConfiguration(ImmutableSet.of("a"));
+
+    ImmutableMap<String, String> env =
+        configuration.getEnvironmentVariables(
+            CppActionNames.CPP_COMPILE, createVariables("v", "1"));
+
+    assertThat(env).containsExactly("foo", "1").inOrder();
+  }
+
   private static String getExpansionOfFlag(String value) throws Exception {
     return getExpansionOfFlag(value, createVariables());
   }

diff --git a/src/test/java/com/google/devtools/build/lib/rules/cpp/StarlarkCcCommonTest.java b/src/test/java/com/google/devtools/build/lib/rules/cpp/StarlarkCcCommonTest.java
@@ -1968,7 +1968,7 @@ public void testEnvEntry_string_string() throws Exception {
     EnvEntry entry = CcModule.envEntryFromStarlark(entryProvider);
     assertThat(entry).isNotNull();
     StringValueParser parser = new StringValueParser("def");
-    assertThat(entry).isEqualTo(new EnvEntry("abc", parser.getChunks()));
+    assertThat(entry).isEqualTo(new EnvEntry("abc", parser.getChunks(), ImmutableSet.of()));
   }
 
   @Test