diff options
| -rw-r--r-- | src/main/java/com/google/devtools/common/options/InvocationPolicyEnforcer.java | 23 | ||||
| -rw-r--r-- | src/test/java/com/google/devtools/common/options/InvocationPolicySetValueTest.java | 30 |
2 files changed, 46 insertions, 7 deletions
diff --git a/src/main/java/com/google/devtools/common/options/InvocationPolicyEnforcer.java b/src/main/java/com/google/devtools/common/options/InvocationPolicyEnforcer.java index 562221df20..9d8c13a71d 100644 --- a/src/main/java/com/google/devtools/common/options/InvocationPolicyEnforcer.java +++ b/src/main/java/com/google/devtools/common/options/InvocationPolicyEnforcer.java @@ -349,7 +349,7 @@ public final class InvocationPolicyEnforcer { repeatableSubflagsInSetValues.put(currentSubflag.getName(), currentSubflag); } else { FlagPolicy subflagAsPolicy = - getSubflagAsPolicy(currentSubflag, originalPolicy, isExpansion); + getSingleValueSubflagAsPolicy(currentSubflag, originalPolicy, isExpansion); // In case any of the expanded flags are themselves expansions, recurse. expandedPolicies.addAll(expandPolicy(subflagAsPolicy, parser)); } @@ -425,19 +425,28 @@ public final class InvocationPolicyEnforcer { * For an expansion flag in an invocation policy, each flag it expands to must be given a * corresponding policy. */ - private static FlagPolicy getSubflagAsPolicy( + private static FlagPolicy getSingleValueSubflagAsPolicy( OptionValueDescription currentSubflag, FlagPolicy originalPolicy, boolean isExpansion) throws OptionsParsingException { FlagPolicy subflagAsPolicy = null; switch (originalPolicy.getOperationCase()) { case SET_VALUE: - assert (!currentSubflag.getAllowMultiple()); + if (currentSubflag.getAllowMultiple()) { + throw new AssertionError( + "SetValue subflags with allowMultiple should have been dealt with separately and " + + "accumulated into a single FlagPolicy."); + } + // Accept null originalValueStrings, they are expected when the subflag is also an expansion + // flag. + List<String> subflagValue; + if (currentSubflag.getOriginalValueString() == null) { + subflagValue = ImmutableList.of(); + } else { + subflagValue = ImmutableList.of(currentSubflag.getOriginalValueString()); + } subflagAsPolicy = getSetValueSubflagAsPolicy( - currentSubflag.getName(), - ImmutableList.of(currentSubflag.getOriginalValueString()), - /*allowMultiple=*/ false, - originalPolicy); + currentSubflag.getName(), subflagValue, /*allowMultiple=*/ false, originalPolicy); break; case USE_DEFAULT: diff --git a/src/test/java/com/google/devtools/common/options/InvocationPolicySetValueTest.java b/src/test/java/com/google/devtools/common/options/InvocationPolicySetValueTest.java index 430366579a..1bb13cfea4 100644 --- a/src/test/java/com/google/devtools/common/options/InvocationPolicySetValueTest.java +++ b/src/test/java/com/google/devtools/common/options/InvocationPolicySetValueTest.java @@ -232,6 +232,36 @@ public class InvocationPolicySetValueTest extends InvocationPolicyEnforcerTestBa } @Test + public void testSetValueWithExpansionFlagOnExpansionFlag() throws Exception { + InvocationPolicy.Builder invocationPolicyBuilder = InvocationPolicy.newBuilder(); + invocationPolicyBuilder + .addFlagPoliciesBuilder() + .setFlagName("test_recursive_expansion_top_level") + .getSetValueBuilder(); + + + InvocationPolicyEnforcer enforcer = createOptionsPolicyEnforcer(invocationPolicyBuilder); + // Unrelated flag, but --test_expansion is not set + parser.parse("--test_string=throwaway value"); + + // The flags that --test_expansion expands into should still be their default values + TestOptions testOptions = getTestOptions(); + assertThat(testOptions.expandedA).isEqualTo(TestOptions.EXPANDED_A_DEFAULT); + assertThat(testOptions.expandedB).isEqualTo(TestOptions.EXPANDED_B_DEFAULT); + assertThat(testOptions.expandedC).isEqualTo(TestOptions.EXPANDED_C_DEFAULT); + assertThat(testOptions.expandedD).isEqualTo(TestOptions.EXPANDED_D_DEFAULT); + + enforcer.enforce(parser, BUILD_COMMAND); + + // After policy enforcement, the flags should be the values from the expansion flag + testOptions = getTestOptions(); + assertThat(testOptions.expandedA).isEqualTo(TestOptions.EXPANDED_A_TEST_RECURSIVE_EXPANSION); + assertThat(testOptions.expandedB).isEqualTo(TestOptions.EXPANDED_B_TEST_RECURSIVE_EXPANSION); + assertThat(testOptions.expandedC).isEqualTo(TestOptions.EXPANDED_C_TEST_RECURSIVE_EXPANSION); + assertThat(testOptions.expandedD).isEqualTo(TestOptions.EXPANDED_D_TEST_RECURSIVE_EXPANSION); + } + + @Test public void testSetValueWithExpansionFunctionFlags() throws Exception { InvocationPolicy.Builder invocationPolicyBuilder = InvocationPolicy.newBuilder(); invocationPolicyBuilder |