Skip to content

Commit

Permalink
Set Locale to English when uppercasing strings to match Enums
Browse files Browse the repository at this point in the history 
    Fixes bazelbuild/bazel#5157

    If a user's default system locale is not `en`, `en_US` or `en_UK`, there may be a chance that `String#toUpperCase` will result in a string that does not exist in the Enum declaration. This is the case in #5157.

    To fix this, it's either

    1) setting the Locale in the individual `toUpperCase` calls or
    2) set Locale to English by default from `Bazel.java`.

    I chose the first because it seemed less intrusive, but I'm open to suggestions.

    Closes #5184.

    PiperOrigin-RevId: 196261078
  • Loading branch information
Luca Di Grazia committed Sep 4, 2022
1 parent 6e95eb6 commit f240da7
Show file tree
Hide file tree
Showing 6 changed files with 159 additions and 59 deletions.
19 changes: 9 additions & 10 deletions ...java_tools/junitrunner/java/com/google/testing/junit/runner/sharding/ShardingFilters.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,13 +15,11 @@
package com.google.testing.junit.runner.sharding;

import com.google.testing.junit.runner.sharding.api.ShardingFilterFactory;

import org.junit.runner.Description;
import org.junit.runner.manipulation.Filter;

import java.util.Collection;

import java.util.Locale;
import javax.inject.Inject;
import org.junit.runner.Description;
import org.junit.runner.manipulation.Filter;

/**
* A factory for test sharding filters.
Expand Down Expand Up @@ -97,13 +95,14 @@ private ShardingFilterFactory getShardingFilterFactory() {
}
ShardingFilterFactory shardingFilterFactory;
try {
shardingFilterFactory = ShardingStrategy.valueOf(strategy.toUpperCase());
shardingFilterFactory = ShardingStrategy.valueOf(strategy.toUpperCase(Locale.ENGLISH));
} catch (IllegalArgumentException e) {
try {
Class<?> strategyClass = Thread.currentThread().getContextClassLoader().loadClass(strategy);
shardingFilterFactory = (ShardingFilterFactory) strategyClass.newInstance();
} catch (ClassNotFoundException | InstantiationException |
IllegalAccessException | IllegalArgumentException e2) {
ClassLoader classLoader = Thread.currentThread().getContextClassLoader();
Class<? extends ShardingFilterFactory> strategyClass =
classLoader.loadClass(strategy).asSubclass(ShardingFilterFactory.class);
shardingFilterFactory = strategyClass.getConstructor().newInstance();
} catch (ReflectiveOperationException | IllegalArgumentException e2) {
throw new RuntimeException(
"Could not create custom sharding strategy class " + strategy, e2);
}
Expand Down
49 changes: 36 additions & 13 deletions ...a/bazelbuild.bazel/src/main/java/com/google/devtools/build/lib/packages/FilesetEntry.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,47 +22,70 @@
import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable;
import com.google.devtools.build.lib.concurrent.ThreadSafety.ThreadSafe;
import com.google.devtools.build.lib.starlarkbuildapi.FilesetEntryApi;
import com.google.devtools.build.lib.skylarkinterface.SkylarkModule;
import com.google.devtools.build.lib.skylarkinterface.SkylarkPrinter;
import com.google.devtools.build.lib.skylarkinterface.SkylarkValue;
import com.google.devtools.build.lib.vfs.PathFragment;
import java.util.Collection;
import java.util.Collections;
import java.util.LinkedHashSet;
import java.util.List;
import java.util.Locale;
import java.util.Set;
import javax.annotation.Nullable;
import net.starlark.java.eval.Printer;
import net.starlark.java.eval.StarlarkValue;

/**
* FilesetEntry is a value object used to represent a "FilesetEntry" inside a "Fileset" BUILD rule.
*/
@SkylarkModule(
name = "FilesetEntry",
doc = "",
documented = false)
@Immutable
@ThreadSafe
public final class FilesetEntry implements StarlarkValue, FilesetEntryApi {
public final class FilesetEntry implements SkylarkValue {

private static final SymlinkBehavior DEFAULT_SYMLINK_BEHAVIOR = SymlinkBehavior.COPY;
public static final SymlinkBehavior DEFAULT_SYMLINK_BEHAVIOR = SymlinkBehavior.COPY;
public static final String DEFAULT_STRIP_PREFIX = ".";
public static final String STRIP_PREFIX_WORKSPACE = "%workspace%";

@Override
public boolean isImmutable() {
return true;
// TODO(laszlocsomor): set this to true. I think we could do this right now, but am not sure.
// Maybe we have to verify that Skylark recognizes every member's type to be recursively
// immutable; as of 15/01/2016 this is not true for enum types in general, to name an example.
return false;
}

public static List<String> makeStringList(List<Label> labels) {
if (labels == null) {
return Collections.emptyList();
}
List<String> strings = Lists.newArrayListWithCapacity(labels.size());
for (Label label : labels) {
strings.add(label.toString());
}
return strings;
}

public static List<?> makeList(Collection<?> list) {
return list == null ? Lists.newArrayList() : Lists.newArrayList(list);
}

@Override
public void repr(Printer printer) {
public void repr(SkylarkPrinter printer) {
printer.append("FilesetEntry(srcdir = ");
printer.repr(srcLabel.toString());
printer.repr(getSrcLabel().toString());
printer.append(", files = ");
printer.repr(files == null ? ImmutableList.of() : Lists.transform(files, Label::toString));
printer.repr(makeStringList(getFiles()));
printer.append(", excludes = ");
printer.repr(excludes == null ? ImmutableList.of() : excludes.asList());
printer.repr(makeList(getExcludes()));
printer.append(", destdir = ");
printer.repr(destDir.getPathString());
printer.repr(getDestDir().getPathString());
printer.append(", strip_prefix = ");
printer.repr(stripPrefix);
printer.repr(getStripPrefix());
printer.append(", symlinks = ");
printer.repr(symlinkBehavior.toString());
printer.repr(getSymlinkBehavior().toString());
printer.append(")");
}

Expand Down
4 changes: 1 addition & 3 deletions ...b_Java/bazelbuild.bazel/src/main/java/com/google/devtools/build/lib/packages/License.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@

import com.google.common.annotations.VisibleForTesting;
import com.google.common.collect.HashBasedTable;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.ImmutableTable;
import com.google.common.collect.Sets;
Expand Down Expand Up @@ -211,8 +210,7 @@ public static License parseLicense(List<String> licStrings) throws LicenseParsin
for (String str : licStrings) {
if (str.startsWith("exception=")) {
try {
Label label =
Label.parseAbsolute(str.substring("exception=".length()), ImmutableMap.of());
Label label = Label.parseAbsolute(str.substring("exception=".length()));
exceptions.add(label);
} catch (LabelSyntaxException e) {
throw new LicenseParsingException(e.getMessage());
Expand Down
5 changes: 3 additions & 2 deletions ...va/bazelbuild.bazel/src/main/java/com/google/devtools/build/lib/packages/TestTimeout.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import com.google.common.collect.Maps;
import com.google.common.collect.Range;
import com.google.common.collect.RangeMap;
import com.google.devtools.build.lib.syntax.Type;
import com.google.devtools.common.options.Converter;
import com.google.devtools.common.options.OptionsParsingException;
import java.time.Duration;
Expand Down Expand Up @@ -110,7 +111,7 @@ public enum TestTimeout {

private final int timeout;

TestTimeout(int timeout) {
private TestTimeout(int timeout) {
this.timeout = timeout;
}

Expand Down Expand Up @@ -202,7 +203,7 @@ public Map<TestTimeout, Duration> convert(String input) throws OptionsParsingExc
// so we can't fully emulate String.split(String, 0).
if (!token.isEmpty() || values.size() > 1) {
try {
values.add(Duration.ofSeconds(Integer.parseInt(token)));
values.add(Duration.ofSeconds(Integer.valueOf(token)));
} catch (NumberFormatException e) {
throw new OptionsParsingException("'" + input + "' is not an int");
}
Expand Down
32 changes: 18 additions & 14 deletions ...elbuild.bazel/src/main/java/com/google/devtools/build/lib/rules/cpp/CppToolchainInfo.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ public final class CppToolchainInfo {

private final String targetLibc;
private final String hostSystemName;
private final ImmutableList<String> dynamicLibraryLinkFlags;
private final FlagList dynamicLibraryLinkFlags;
private final ImmutableList<String> legacyLinkOptions;
private final ImmutableListMultimap<LinkingMode, String> legacyLinkOptionsFromLinkingMode;
private final ImmutableListMultimap<LipoMode, String> legacyLinkOptionsFromLipoMode;
Expand All @@ -103,7 +103,7 @@ public final class CppToolchainInfo {
private final ImmutableListMultimap<LipoMode, String> lipoCFlags;
private final ImmutableListMultimap<LipoMode, String> lipoCxxFlags;

private final ImmutableList<String> unfilteredCompilerFlags;
private final FlagList unfilteredCompilerFlags;

private final boolean supportsFission;
private final boolean supportsStartEndLib;
Expand Down Expand Up @@ -182,7 +182,9 @@ public static CppToolchainInfo create(
defaultSysroot,
toolchain.getTargetLibc(),
toolchain.getHostSystemName(),
ImmutableList.copyOf(toolchain.getDynamicLibraryLinkerFlagList()),
new FlagList(
ImmutableList.copyOf(toolchain.getDynamicLibraryLinkerFlagList()),
ImmutableList.of()),
ImmutableList.copyOf(toolchain.getLinkerFlagList()),
linkOptionsFromLinkingModeBuilder.build(),
computeLinkOptionsFromLipoMode(toolchain),
Expand All @@ -209,7 +211,8 @@ public static CppToolchainInfo create(
cxxFlagsBuilder.build(),
lipoCFlagsBuilder.build(),
lipoCxxFlagsBuilder.build(),
ImmutableList.copyOf(toolchain.getUnfilteredCxxFlagList()),
new FlagList(
ImmutableList.copyOf(toolchain.getUnfilteredCxxFlagList()), ImmutableList.of()),
toolchain.getSupportsFission(),
toolchain.getSupportsStartEndLib(),
toolchain.getSupportsEmbeddedRuntimes(),
Expand Down Expand Up @@ -239,7 +242,7 @@ public static CppToolchainInfo create(
PathFragment runtimeSysroot,
String targetLibc,
String hostSystemName,
ImmutableList<String> dynamicLibraryLinkFlags,
FlagList dynamicLibraryLinkFlags,
ImmutableList<String> legacyLinkOptions,
ImmutableListMultimap<LinkingMode, String> legacyLinkOptionsFromLinkingMode,
ImmutableListMultimap<LipoMode, String> legacyLinkOptionsFromLipoMode,
Expand All @@ -260,7 +263,7 @@ public static CppToolchainInfo create(
ImmutableListMultimap<CompilationMode, String> cxxFlagsByCompilationMode,
ImmutableListMultimap<LipoMode, String> lipoCFlags,
ImmutableListMultimap<LipoMode, String> lipoCxxFlags,
ImmutableList<String> unfilteredCompilerFlags,
FlagList unfilteredCompilerFlags,
boolean supportsFission,
boolean supportsStartEndLib,
boolean supportsEmbeddedRuntimes,
Expand Down Expand Up @@ -342,13 +345,11 @@ private static CToolchain addLegacyFeatures(
}

for (ArtifactCategory category : ArtifactCategory.values()) {
if (!definedCategories.contains(category) && category.getDefaultPrefix() != null
&& category.getDefaultExtension() != null) {
if (!definedCategories.contains(category) && category.getDefaultPattern() != null) {
toolchainBuilder.addArtifactNamePattern(
ArtifactNamePattern.newBuilder()
.setCategoryName(category.toString().toLowerCase())
.setPrefix(category.getDefaultPrefix())
.setExtension(category.getDefaultExtension())
.setPattern(category.getDefaultPattern())
.build());
}
}
Expand Down Expand Up @@ -639,8 +640,11 @@ public PathFragment getRuntimeSysroot() {
* Returns link options for the specified flag list, combined with universal options for all
* shared libraries (regardless of link staticness).
*/
ImmutableList<String> getSharedLibraryLinkOptions(ImmutableList<String> flags) {
return ImmutableList.<String>builder().addAll(flags).addAll(dynamicLibraryLinkFlags).build();
ImmutableList<String> getSharedLibraryLinkOptions(FlagList flags) {
return ImmutableList.<String>builder()
.addAll(flags.evaluate())
.addAll(dynamicLibraryLinkFlags.evaluate())
.build();
}

/**
Expand Down Expand Up @@ -749,11 +753,11 @@ public ImmutableListMultimap<LipoMode, String> getLipoCxxFlags() {
/** Returns unfiltered compiler options for C++ from this toolchain. */
public ImmutableList<String> getUnfilteredCompilerOptions(@Nullable PathFragment sysroot) {
if (sysroot == null) {
return unfilteredCompilerFlags;
return unfilteredCompilerFlags.evaluate();
}
return ImmutableList.<String>builder()
.add("--sysroot=" + sysroot)
.addAll(unfilteredCompilerFlags)
.addAll(unfilteredCompilerFlags.evaluate())
.build();
}

Expand Down
Loading

0 comments on commit f240da7

Please sign in to comment.