diff options
author | ccalvarin <ccalvarin@google.com> | 2017-09-26 14:42:31 -0400 |
---|---|---|
committer | John Cater <jcater@google.com> | 2017-09-27 10:01:09 -0400 |
commit | 659feca7dee332f75801010b018505b7ba5a185e (patch) | |
tree | 183cbd7b3a1ad10939c3a23d05033ce176c311d2 /src/main/java/com/google | |
parent | 5b98fb43ecaa6a9343ba4aa649f74193f0cf0956 (diff) |
Move the canonicalization of an option value to the option value itself.
Ideally, the canonical form we output from OptionUtils would be the same as for the command canonicalize-flags, but that must wait for dependencies to be cleaned up. Still, in the meantime, keep the --foo=1 normalization of --foo, and apply it to all other boolean flag values (t,true,yes,y, and false equivalents), so that the canoncalize-flags output is more canonical, even if it isn't using the --[no]foo form yet.
RELNOTES: Boolean flag values will now get normalized to 1 or 0 in canonicalize-flags output.
PiperOrigin-RevId: 170084599
Diffstat (limited to 'src/main/java/com/google')
3 files changed, 44 insertions, 41 deletions
diff --git a/src/main/java/com/google/devtools/build/lib/util/OptionsUtils.java b/src/main/java/com/google/devtools/build/lib/util/OptionsUtils.java index bae41d869f..c94863ed86 100644 --- a/src/main/java/com/google/devtools/build/lib/util/OptionsUtils.java +++ b/src/main/java/com/google/devtools/build/lib/util/OptionsUtils.java @@ -17,8 +17,6 @@ package com.google.devtools.build.lib.util; import com.google.common.collect.ImmutableList; import com.google.devtools.build.lib.vfs.PathFragment; import com.google.devtools.common.options.Converter; -import com.google.devtools.common.options.Converters; -import com.google.devtools.common.options.OptionsParsingException; import com.google.devtools.common.options.OptionsProvider; import com.google.devtools.common.options.ParsedOptionDescription; import java.util.ArrayList; @@ -43,23 +41,7 @@ public final class OptionsUtils { if (result.length() != 0) { result.append(' '); } - String value = option.getUnconvertedValue(); - if (option.isBooleanOption()) { - boolean isEnabled = false; - try { - isEnabled = new Converters.BooleanConverter().convert(value); - } catch (OptionsParsingException e) { - throw new RuntimeException("Unexpected parsing exception", e); - } - result - .append(isEnabled ? "--" : "--no") - .append(option.getOptionDefinition().getOptionName()); - } else { - result.append("--").append(option.getOptionDefinition().getOptionName()); - if (value != null) { // Can be null for Void options. - result.append("=").append(ShellEscaper.escapeString(value)); - } - } + result.append(option.getCanonicalFormWithValueEscaper(ShellEscaper::escapeString)); } return result.toString(); } @@ -82,22 +64,7 @@ public final class OptionsUtils { if (option.isHidden()) { continue; } - String value = option.getUnconvertedValue(); - if (option.isBooleanOption()) { - boolean isEnabled = false; - try { - isEnabled = new Converters.BooleanConverter().convert(value); - } catch (OptionsParsingException e) { - throw new RuntimeException("Unexpected parsing exception", e); - } - builder.add((isEnabled ? "--" : "--no") + option.getOptionDefinition().getOptionName()); - } else { - String optionString = "--" + option.getOptionDefinition().getOptionName(); - if (value != null) { // Can be null for Void options. - optionString += "=" + value; - } - builder.add(optionString); - } + builder.add(option.getCanonicalForm()); } return builder.build(); } 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 176d51e66a..221dcf030f 100644 --- a/src/main/java/com/google/devtools/common/options/OptionsParserImpl.java +++ b/src/main/java/com/google/devtools/common/options/OptionsParserImpl.java @@ -158,12 +158,7 @@ class OptionsParserImpl { }) // Ignore expansion options. .filter(value -> !value.getOptionDefinition().isExpansionOption()) - .map( - value -> - "--" - + value.getOptionDefinition().getOptionName() - + "=" - + value.getUnconvertedValue()) + .map(ParsedOptionDescription::getDeprecatedCanonicalForm) .collect(toCollection(ArrayList::new)); } diff --git a/src/main/java/com/google/devtools/common/options/ParsedOptionDescription.java b/src/main/java/com/google/devtools/common/options/ParsedOptionDescription.java index 0910579541..1f43172fb2 100644 --- a/src/main/java/com/google/devtools/common/options/ParsedOptionDescription.java +++ b/src/main/java/com/google/devtools/common/options/ParsedOptionDescription.java @@ -15,6 +15,7 @@ package com.google.devtools.common.options; import com.google.common.collect.ImmutableList; +import java.util.function.Function; import javax.annotation.Nullable; /** @@ -49,6 +50,46 @@ public final class ParsedOptionDescription { return commandLineForm; } + public String getCanonicalForm() { + return getCanonicalFormWithValueEscaper(s -> s); + } + + public String getCanonicalFormWithValueEscaper(Function<String, String> escapingFunction) { + // For boolean flags (note that here we do not check for TriState flags, only flags with actual + // boolean values, so that we know the return type of getConvertedValue), use the --[no]flag + // form for the canonical value. + if (optionDefinition.getType().equals(boolean.class)) { + try { + return ((boolean) getConvertedValue() ? "--" : "--no") + optionDefinition.getOptionName(); + } catch (OptionsParsingException e) { + throw new RuntimeException("Unexpected parsing exception", e); + } + } else { + String optionString = "--" + optionDefinition.getOptionName(); + if (unconvertedValue != null) { // Can be null for Void options. + optionString += "=" + escapingFunction.apply(unconvertedValue); + } + return optionString; + } + } + + @Deprecated + // TODO(b/65646296) Once external dependencies are cleaned up, use getCanonicalForm() + String getDeprecatedCanonicalForm() { + String value = unconvertedValue; + // For boolean flags (note that here we do not check for TriState flags, only flags with actual + // boolean values, so that we know the return type of getConvertedValue), set them all to 1 or + // 0, instead of keeping the wide variety of values we accept in their original form. + if (optionDefinition.getType().equals(boolean.class)) { + try { + value = (boolean) getConvertedValue() ? "1" : "0"; + } catch (OptionsParsingException e) { + throw new RuntimeException("Unexpected parsing exception", e); + } + } + return String.format("--%s=%s", optionDefinition.getOptionName(), value); + } + public boolean isBooleanOption() { return optionDefinition.getType().equals(boolean.class); } |