Skip to content

Commit

Permalink
Use RepositoryMapping instead of RepoContext
Browse files Browse the repository at this point in the history
  • Loading branch information
@fmeum
fmeum committed Aug 25, 2023
1 parent 287b7e5 commit c9d7cb0
Show file tree
Hide file tree
Showing 4 changed files with 26 additions and 35 deletions.
13 changes: 3 additions & 10 deletions src/main/java/com/google/devtools/build/lib/analysis/config/CoreOptionConverters.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -232,19 +232,12 @@ private static Label convertOptionsLabel(String input, @Nullable Object conversi
return Label.parseCanonical(input);
}
Preconditions.checkArgument(
conversionContext instanceof RepositoryMapping
|| conversionContext instanceof Label.RepoContext,
conversionContext instanceof RepositoryMapping,
"bad conversion context type: %s",
conversionContext.getClass().getName());
Label.RepoContext repoContext;
if (conversionContext instanceof Label.RepoContext) {
repoContext = (Label.RepoContext) conversionContext;
} else {
repoContext =
Label.RepoContext.of(RepositoryName.MAIN, (RepositoryMapping) conversionContext);
}
// This can happen in the second round of option parsing.
return Label.parseWithRepoContext(input, repoContext);
return Label.parseWithRepoContext(
input, Label.RepoContext.of(RepositoryName.MAIN, (RepositoryMapping) conversionContext));
} catch (LabelSyntaxException e) {
throw new OptionsParsingException(e.getMessage());
}
Expand Down
4 changes: 2 additions & 2 deletions src/main/java/com/google/devtools/build/lib/skyframe/PlatformMappingFunction.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ public PlatformMappingValue compute(SkyKey skyKey, Environment env)
// If no flag was passed and the default mapping file does not exist treat this as if the
// mapping file was empty rather than an error.
return new PlatformMappingValue(
ImmutableMap.of(), ImmutableMap.of(), ImmutableSet.of(), mainRepoContext);
ImmutableMap.of(), ImmutableMap.of(), ImmutableSet.of(), mainRepoContext.repoMapping());
}
throw new PlatformMappingException(
new MissingInputFileException(
Expand Down Expand Up @@ -391,7 +391,7 @@ static final class Mappings {
PlatformMappingValue toPlatformMappingValue(
ImmutableSet<Class<? extends FragmentOptions>> optionsClasses) {
return new PlatformMappingValue(
platformsToFlags, flagsToPlatforms, optionsClasses, mainRepoContext);
platformsToFlags, flagsToPlatforms, optionsClasses, mainRepoContext.repoMapping());
}
}

Expand Down
16 changes: 8 additions & 8 deletions src/main/java/com/google/devtools/build/lib/skyframe/PlatformMappingValue.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@
import com.google.devtools.build.lib.analysis.config.BuildOptions;
import com.google.devtools.build.lib.analysis.config.FragmentOptions;
import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.cmdline.Label.RepoContext;
import com.google.devtools.build.lib.cmdline.RepositoryMapping;
import com.google.devtools.build.lib.concurrent.ThreadSafety;
import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec;
import com.google.devtools.build.lib.vfs.PathFragment;
Expand Down Expand Up @@ -146,7 +146,7 @@ public SkyKeyInterner<Key> getSkyKeyInterner() {
private final ImmutableSet<Class<? extends FragmentOptions>> optionsClasses;
private final LoadingCache<NativeAndStarlarkFlags, OptionsParsingResult> parserCache;
private final LoadingCache<BuildConfigurationKey, BuildConfigurationKey> mappingCache;
private final RepoContext mainRepoContext;
private final RepositoryMapping mainRepositoryMapping;

/**
* Creates a new mapping value which will match on the given platforms (if a target platform is
Expand All @@ -157,21 +157,21 @@ public SkyKeyInterner<Key> getSkyKeyInterner() {
* @param flagsToPlatforms mapping from a set of command line style flags to a target platform
* that should be set if the flags match the mapped options
* @param optionsClasses default options classes that should be used for options parsing
* @param mainRepoContext the main repo context used to parse label-valued options
* @param mainRepositoryMapping the main repo mapping used to parse label-valued options
*/
PlatformMappingValue(
ImmutableMap<Label, NativeAndStarlarkFlags> platformsToFlags,
ImmutableMap<ImmutableSet<String>, Label> flagsToPlatforms,
ImmutableSet<Class<? extends FragmentOptions>> optionsClasses,
RepoContext mainRepoContext) {
RepositoryMapping mainRepositoryMapping) {
this.platformsToFlags = checkNotNull(platformsToFlags);
this.flagsToPlatforms = checkNotNull(flagsToPlatforms);
this.optionsClasses = checkNotNull(optionsClasses);
this.mainRepoContext = checkNotNull(mainRepoContext);
this.mainRepositoryMapping = checkNotNull(mainRepositoryMapping);
this.parserCache =
Caffeine.newBuilder()
.initialCapacity(platformsToFlags.size() + flagsToPlatforms.size())
.build(flags -> parse(flags, this.mainRepoContext));
.build(flags -> parse(flags, this.mainRepositoryMapping));
this.mappingCache = Caffeine.newBuilder().weakKeys().build(this::computeMapping);
}

Expand Down Expand Up @@ -266,14 +266,14 @@ private OptionsParsingResult parseWithCache(NativeAndStarlarkFlags args)
}
}

private OptionsParsingResult parse(NativeAndStarlarkFlags args, RepoContext mainRepoContext)
private OptionsParsingResult parse(NativeAndStarlarkFlags args, RepositoryMapping mainRepoMapping)
throws OptionsParsingException {
OptionsParser parser =
OptionsParser.builder()
.optionsClasses(optionsClasses)
// We need the ability to re-map internal options in the mappings file.
.ignoreInternalOptions(false)
.withConversionContext(mainRepoContext)
.withConversionContext(mainRepoMapping)
.build();
parser.parse(args.nativeFlags().asList());
parser.setStarlarkOptions(args.starlarkFlags());
Expand Down
28 changes: 13 additions & 15 deletions src/test/java/com/google/devtools/build/lib/skyframe/PlatformMappingValueTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,13 +43,11 @@ public final class PlatformMappingValueTest {

private static final Label PLATFORM1 = Label.parseCanonicalUnchecked("//platforms:one");
private static final Label PLATFORM2 = Label.parseCanonicalUnchecked("@dep~1.0//platforms:two");
private static final Label.RepoContext REPO_CONTEXT =
Label.RepoContext.of(
RepositoryName.MAIN,
RepositoryMapping.create(
ImmutableMap.of(
"", RepositoryName.MAIN, "dep", RepositoryName.createUnvalidated("dep~1.0")),
RepositoryName.MAIN));
private static final RepositoryMapping REPO_MAPPING =
RepositoryMapping.create(
ImmutableMap.of(
"", RepositoryName.MAIN, "dep", RepositoryName.createUnvalidated("dep~1.0")),
RepositoryName.MAIN);

private static final BuildOptions DEFAULT_BUILD_CONFIG_PLATFORM_OPTIONS =
BuildOptions.getDefaultBuildOptionsForFragments(BUILD_CONFIG_PLATFORM_OPTIONS);
Expand All @@ -62,7 +60,7 @@ public void testMapNoMappings() throws OptionsParsingException {
ImmutableMap<ImmutableSet<String>, Label> flagsToPlatforms = ImmutableMap.of();
PlatformMappingValue mappingValue =
new PlatformMappingValue(
platformsToFlags, flagsToPlatforms, BUILD_CONFIG_PLATFORM_OPTIONS, REPO_CONTEXT);
platformsToFlags, flagsToPlatforms, BUILD_CONFIG_PLATFORM_OPTIONS, REPO_MAPPING);

BuildConfigurationKey key =
BuildConfigurationKey.withoutPlatformMapping(DEFAULT_BUILD_CONFIG_PLATFORM_OPTIONS);
Expand All @@ -84,7 +82,7 @@ public void testMapPlatformToFlags() throws Exception {
ImmutableMap<ImmutableSet<String>, Label> flagsToPlatforms = ImmutableMap.of();
PlatformMappingValue mappingValue =
new PlatformMappingValue(
platformsToFlags, flagsToPlatforms, BUILD_CONFIG_PLATFORM_OPTIONS, REPO_CONTEXT);
platformsToFlags, flagsToPlatforms, BUILD_CONFIG_PLATFORM_OPTIONS, REPO_MAPPING);

BuildOptions modifiedOptions = DEFAULT_BUILD_CONFIG_PLATFORM_OPTIONS.clone();
modifiedOptions.get(PlatformOptions.class).platforms = ImmutableList.of(PLATFORM1);
Expand All @@ -102,7 +100,7 @@ public void testMapFlagsToPlatform() throws Exception {
ImmutableMap<Label, NativeAndStarlarkFlags> platformsToFlags = ImmutableMap.of();
PlatformMappingValue mappingValue =
new PlatformMappingValue(
platformsToFlags, flagsToPlatforms, BUILD_CONFIG_PLATFORM_OPTIONS, REPO_CONTEXT);
platformsToFlags, flagsToPlatforms, BUILD_CONFIG_PLATFORM_OPTIONS, REPO_MAPPING);

BuildOptions modifiedOptions = DEFAULT_BUILD_CONFIG_PLATFORM_OPTIONS.clone();
modifiedOptions.get(CoreOptions.class).cpu = "one";
Expand All @@ -123,7 +121,7 @@ public void testMapFlagsToPlatformPriority() throws Exception {
ImmutableMap<Label, NativeAndStarlarkFlags> platformsToFlags = ImmutableMap.of();
PlatformMappingValue mappingValue =
new PlatformMappingValue(
platformsToFlags, flagsToPlatforms, BUILD_CONFIG_PLATFORM_OPTIONS, REPO_CONTEXT);
platformsToFlags, flagsToPlatforms, BUILD_CONFIG_PLATFORM_OPTIONS, REPO_MAPPING);

BuildOptions modifiedOptions = DEFAULT_BUILD_CONFIG_PLATFORM_OPTIONS.clone();
modifiedOptions.get(CoreOptions.class).cpu = "foo";
Expand All @@ -141,7 +139,7 @@ public void testMapFlagsToPlatformNoneMatching() throws Exception {
ImmutableMap<Label, NativeAndStarlarkFlags> platformsToFlags = ImmutableMap.of();
PlatformMappingValue mappingValue =
new PlatformMappingValue(
platformsToFlags, flagsToPlatforms, BUILD_CONFIG_PLATFORM_OPTIONS, REPO_CONTEXT);
platformsToFlags, flagsToPlatforms, BUILD_CONFIG_PLATFORM_OPTIONS, REPO_MAPPING);

BuildOptions modifiedOptions = DEFAULT_BUILD_CONFIG_PLATFORM_OPTIONS.clone();
modifiedOptions.get(CoreOptions.class).cpu = "bar";
Expand All @@ -160,7 +158,7 @@ public void testMapNoPlatformOptions() throws Exception {
ImmutableMap<Label, NativeAndStarlarkFlags> platformsToFlags = ImmutableMap.of();
PlatformMappingValue mappingValue =
new PlatformMappingValue(
platformsToFlags, flagsToPlatforms, BUILD_CONFIG_PLATFORM_OPTIONS, REPO_CONTEXT);
platformsToFlags, flagsToPlatforms, BUILD_CONFIG_PLATFORM_OPTIONS, REPO_MAPPING);

BuildOptions options = BuildOptions.of(ImmutableList.of());

Expand All @@ -186,7 +184,7 @@ public void testMapNoMappingIfPlatformIsSetButNotMatching() throws Exception {
platformsToFlags,
flagsToPlatforms,
BUILD_CONFIG_PLATFORM_OPTIONS,
Label.RepoContext.of(RepositoryName.MAIN, RepositoryMapping.ALWAYS_FALLBACK));
RepositoryMapping.ALWAYS_FALLBACK);

BuildConfigurationKey mapped = mappingValue.map(keyForOptions(modifiedOptions));

Expand All @@ -205,7 +203,7 @@ public void testMapNoMappingIfPlatformIsSetAndNoPlatformMapping() throws Excepti
ImmutableMap<Label, NativeAndStarlarkFlags> platformsToFlags = ImmutableMap.of();
PlatformMappingValue mappingValue =
new PlatformMappingValue(
platformsToFlags, flagsToPlatforms, BUILD_CONFIG_PLATFORM_OPTIONS, REPO_CONTEXT);
platformsToFlags, flagsToPlatforms, BUILD_CONFIG_PLATFORM_OPTIONS, REPO_MAPPING);

BuildConfigurationKey mapped = mappingValue.map(keyForOptions(modifiedOptions));

Expand Down

0 comments on commit c9d7cb0

Please sign in to comment.