Automated g4 rollback of commit aa7f930.

*** Reason for rollback ***

Broke --experimental_inmemory_dotd_files

RELNOTES: None
PiperOrigin-RevId: 154477949

src/main/java/com/google/devtools/build/lib/flags/InvocationPolicyEnforcer.java

@@ -36,9 +36,7 @@
 import com.google.devtools.common.options.OptionsParsingException;
 import java.util.ArrayList;
 import java.util.Arrays;
-import java.util.HashMap;
 import java.util.List;
-import java.util.Map;
 import java.util.Set;
 import java.util.logging.Level;
 import java.util.logging.Logger;
@@ -211,14 +209,7 @@ public static List<FlagPolicy> getEffectivePolicy(
       expandedPolicies.addAll(policies);
     }
 
-    // Only keep that last policy for each flag.
-    Map<String, FlagPolicy> effectivePolicy = new HashMap<>();
-    for (FlagPolicy expandedPolicy : expandedPolicies) {
-      String flagName = expandedPolicy.getFlagName();
-      effectivePolicy.put(flagName, expandedPolicy);
-    }
-
-    return new ArrayList<>(effectivePolicy.values());
+    return expandedPolicies;
   }
 
   /**

src/main/protobuf/invocation_policy.proto

@@ -20,10 +20,18 @@ option java_package = "com.google.devtools.build.lib.runtime.proto";
 // The --invocation_policy flag takes a base64-encoded binary-serialized or text
 // formatted InvocationPolicy message.
 message InvocationPolicy {
-  // Order matters.
-  // After expanding policies on expansion flags or flags with implicit
-  // requirements, only the final policy on a specific flag will be enforced
-  // onto the user's command line.
+  // Policies will be applied in order. Later policies will override
+  // previous policies if they conflict, which is important for flags
+  // that interact with each other. For example, if there is a flag "--foo"
+  // which is an expansion flag that expands into "--bar=x --baz=y", and the
+  // policy list first has a SetValue policy for "set --bar to z", and then has
+  // a SetDefault policy to set "--foo" to its default value, both --bar and
+  // --baz will get reset to their default values, overriding the SetValue
+  // operation. The UseDefault should come before the SetValue.
+  //
+  // Note that currently, if user value is lost, either cleared by UseDefault
+  // or by being written over by a SetValue, a later white-listing of the user's
+  // inputted value will not restore it.
   repeated FlagPolicy flag_policies = 1;
 }
 
@@ -119,10 +127,8 @@ message UseDefault {
   // so that when the value is requested and no flag is found, the flag parser
   // returns the default. This is mostly relevant for expansion flags: it will
   // erase user values in *all* flags that the expansion flag expands to. Only
-  // use this on expansion flags if this is acceptable behavior. Since the last
-  // policy wins, later policies on this same flag will still remove the
-  // expanded UseDefault, so there is a way around, but it's really best not to
-  // use this on expansion flags at all.
+  // use this on expansion flags if this is acceptable behavior. Otherwise, make
+  // an appropriate policy for the expanded flags that you care about.
 }
 
 message DisallowValues {

src/test/java/com/google/devtools/build/lib/runtime/InvocationPolicyUseDefaultTest.java

@@ -101,40 +101,6 @@ public void testUseDefaultWithExpansionFlags() throws Exception {
     assertThat(testOptions.expandedD).isEqualTo(TestOptions.EXPANDED_D_DEFAULT);
   }
 
-  @Test
-  public void testUseDefaultWithExpansionFlagAndLaterOverride() throws Exception {
-    InvocationPolicy.Builder invocationPolicyBuilder = InvocationPolicy.newBuilder();
-    invocationPolicyBuilder
-        .addFlagPoliciesBuilder()
-        .setFlagName("test_expansion")
-        .getUseDefaultBuilder();
-    invocationPolicyBuilder
-        .addFlagPoliciesBuilder()
-        .setFlagName("expanded_b")
-        .getAllowValuesBuilder()
-        .addAllowedValues("false");
-
-    InvocationPolicyEnforcer enforcer = createOptionsPolicyEnforcer(invocationPolicyBuilder);
-    parser.parse("--test_expansion");
-
-    TestOptions testOptions = getTestOptions();
-    assertThat(testOptions.expandedA).isEqualTo(TestOptions.EXPANDED_A_TEST_EXPANSION);
-    assertThat(testOptions.expandedB).isEqualTo(TestOptions.EXPANDED_B_TEST_EXPANSION);
-    assertThat(testOptions.expandedC).isEqualTo(TestOptions.EXPANDED_C_TEST_EXPANSION);
-    assertThat(testOptions.expandedD).isEqualTo(TestOptions.EXPANDED_D_TEST_EXPANSION);
-
-    // If the UseDefault is run, then the value of --expanded_b is back to it's default true, which
-    // isn't allowed. However, the allowValues in the later policy should wipe the expansion's
-    // policy on --expanded_b, so that the enforcement does not fail.
-    enforcer.enforce(parser, BUILD_COMMAND);
-
-    testOptions = getTestOptions();
-    assertThat(testOptions.expandedA).isEqualTo(TestOptions.EXPANDED_A_DEFAULT);
-    assertThat(testOptions.expandedB).isEqualTo(TestOptions.EXPANDED_B_TEST_EXPANSION);
-    assertThat(testOptions.expandedC).isEqualTo(TestOptions.EXPANDED_C_DEFAULT);
-    assertThat(testOptions.expandedD).isEqualTo(TestOptions.EXPANDED_D_DEFAULT);
-  }
-
   @Test
   public void testUseDefaultWithRecursiveExpansionFlags() throws Exception {
     InvocationPolicy.Builder invocationPolicyBuilder = InvocationPolicy.newBuilder();