Add a check to not print deprecated flag warnings from flags set by the invocation policy.
Invocation policies are not always entirely synced with the current non-deprecated set of blaze flags. Therefore, sometimes they set flags that are deprecated or no longer exist. For the latter case, we log and ignore the invocation policy trying to set the flag. For the former case, we'll now do a more similar thing and not print a spammy warning when the invocation policy tries to set a deprecated flag.
PiperOrigin-RevId: 210403153
diff --git a/src/main/java/com/google/devtools/common/options/OptionsParserImpl.java b/src/main/java/com/google/devtools/common/options/OptionsParserImpl.java
index 2c15430..9d0db90 100644
--- a/src/main/java/com/google/devtools/common/options/OptionsParserImpl.java
+++ b/src/main/java/com/google/devtools/common/options/OptionsParserImpl.java
@@ -14,6 +14,7 @@
package com.google.devtools.common.options;
+import static com.google.devtools.common.options.OptionPriority.PriorityCategory.INVOCATION_POLICY;
import static java.util.Comparator.comparing;
import static java.util.stream.Collectors.toCollection;
@@ -164,7 +165,12 @@
return result;
}
- private void maybeAddDeprecationWarning(OptionDefinition optionDefinition) {
+ private void maybeAddDeprecationWarning(
+ OptionDefinition optionDefinition, PriorityCategory priority) {
+ // Don't add a warning for deprecated flag set by the invocation policy.
+ if (priority.equals(INVOCATION_POLICY)) {
+ return;
+ }
// Continue to support the old behavior for @Deprecated options.
String warning = optionDefinition.getDeprecationWarning();
if (!warning.isEmpty() || (optionDefinition.getField().isAnnotationPresent(Deprecated.class))) {
@@ -393,7 +399,7 @@
throws OptionsParsingException {
OptionDefinition optionDefinition = parsedOption.getOptionDefinition();
// All options can be deprecated; check and warn before doing any option-type specific work.
- maybeAddDeprecationWarning(optionDefinition);
+ maybeAddDeprecationWarning(optionDefinition, parsedOption.getPriority().getPriorityCategory());
// Track the value, before any remaining option-type specific work that is done outside of
// the OptionValueDescription.
OptionValueDescription entry =
diff --git a/src/test/java/com/google/devtools/common/options/InvocationPolicyMiscTest.java b/src/test/java/com/google/devtools/common/options/InvocationPolicyMiscTest.java
new file mode 100644
index 0000000..316b528
--- /dev/null
+++ b/src/test/java/com/google/devtools/common/options/InvocationPolicyMiscTest.java
@@ -0,0 +1,108 @@
+// Copyright 2018 The Bazel Authors. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+package com.google.devtools.common.options;
+
+import static com.google.common.truth.Truth.assertThat;
+
+import com.google.devtools.build.lib.runtime.proto.InvocationPolicyOuterClass.InvocationPolicy;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.JUnit4;
+
+/** Miscellaneous tests for {@link InvocationPolicy} */
+@RunWith(JUnit4.class)
+public class InvocationPolicyMiscTest extends InvocationPolicyEnforcerTestBase {
+
+ private static final String BUILD_COMMAND = "build";
+ private static final String TEST_DEPRECATED_USER_VALUE = "user value";
+ private static final String TEST_DEPRECATED_POLICY_VALUE = "policy value";
+
+ /**
+ * Test that deprecated flags set via setValue in the invocation policy don't elicit an extra
+ * deprecation warning on top of the one elicted by the user setting the flag.
+ */
+ @Test
+ public void testDoPrintDeprecationWarning_setValue() throws Exception {
+ parser.parse("--test_deprecated=" + TEST_DEPRECATED_USER_VALUE);
+ InvocationPolicy.Builder invocationPolicyBuilder = InvocationPolicy.newBuilder();
+ invocationPolicyBuilder
+ .addFlagPoliciesBuilder()
+ .setFlagName("test_deprecated")
+ .getUseDefaultBuilder();
+ InvocationPolicyEnforcer enforcer = createOptionsPolicyEnforcer(invocationPolicyBuilder);
+
+ enforcer.enforce(parser, BUILD_COMMAND);
+
+ assertThat(parser.getWarnings())
+ .containsExactly(
+ "Option 'test_deprecated' is deprecated: Flag for testing deprecation behavior.");
+ }
+
+ /**
+ * Test that deprecated flags set via UseDefault in the invocation policy don't elicit an extra
+ * deprecation warning on top of the one elicted by the user setting the flag.
+ */
+ @Test
+ public void testDoPrintDeprecationWarning_useDefault() throws Exception {
+ parser.parse("--test_deprecated=" + TEST_DEPRECATED_USER_VALUE);
+ InvocationPolicy.Builder invocationPolicyBuilder = InvocationPolicy.newBuilder();
+ invocationPolicyBuilder
+ .addFlagPoliciesBuilder()
+ .setFlagName("test_deprecated")
+ .getSetValueBuilder()
+ .addFlagValue(TEST_DEPRECATED_POLICY_VALUE);
+ InvocationPolicyEnforcer enforcer = createOptionsPolicyEnforcer(invocationPolicyBuilder);
+
+ enforcer.enforce(parser, BUILD_COMMAND);
+
+ assertThat(parser.getWarnings())
+ .containsExactly(
+ "Option 'test_deprecated' is deprecated: Flag for testing deprecation behavior.");
+ }
+
+ /**
+ * Test that deprecated flags touched via UseDefault in the invocation policy don't elicit a
+ * deprecation warning.
+ */
+ @Test
+ public void testDontPrintDeprecationWarning_useDefault() throws Exception {
+ InvocationPolicy.Builder invocationPolicyBuilder = InvocationPolicy.newBuilder();
+ invocationPolicyBuilder
+ .addFlagPoliciesBuilder()
+ .setFlagName("test_deprecated")
+ .getUseDefaultBuilder();
+ InvocationPolicyEnforcer enforcer = createOptionsPolicyEnforcer(invocationPolicyBuilder);
+
+ enforcer.enforce(parser, BUILD_COMMAND);
+
+ assertThat(parser.getWarnings()).isEmpty();
+ }
+
+ /* Test that deprecated flags set via SetValue in the invocation policy don't elicit a
+ deprecation warning. */
+ @Test
+ public void testDontPrintDeprecatioNWarning_setValue() throws Exception {
+ InvocationPolicy.Builder invocationPolicyBuilder = InvocationPolicy.newBuilder();
+ invocationPolicyBuilder
+ .addFlagPoliciesBuilder()
+ .setFlagName("test_deprecated")
+ .getSetValueBuilder()
+ .addFlagValue(TEST_DEPRECATED_POLICY_VALUE);
+ InvocationPolicyEnforcer enforcer = createOptionsPolicyEnforcer(invocationPolicyBuilder);
+
+ enforcer.enforce(parser, BUILD_COMMAND);
+
+ assertThat(parser.getWarnings()).isEmpty();
+ }
+}
diff --git a/src/test/java/com/google/devtools/common/options/TestOptions.java b/src/test/java/com/google/devtools/common/options/TestOptions.java
index c38d975..d006240 100644
--- a/src/test/java/com/google/devtools/common/options/TestOptions.java
+++ b/src/test/java/com/google/devtools/common/options/TestOptions.java
@@ -315,4 +315,12 @@
defaultValue = "false"
)
public boolean specialExpBar;
+
+ @Option(
+ name = "test_deprecated",
+ defaultValue = "default",
+ deprecationWarning = "Flag for testing deprecation behavior.",
+ documentationCategory = OptionDocumentationCategory.UNCATEGORIZED,
+ effectTags = {OptionEffectTag.NO_OP})
+ public String testDeprecated;
}
