Skip to content

Commit

Permalink
Make the --platform_mappings flag immutable.
Browse files Browse the repository at this point in the history
If it cannot be changed in transitions, all errors will be caught in the first usage (in `SkyframeExecutor.createBuildConfigurationKey`), and so no further errors need to be considered.

Work towards platform-based flags: #19409.

PiperOrigin-RevId: 596918285
Change-Id: I68a1808d836716893491fd212eb2b5bc9352f541
  • Loading branch information
katre authored and copybara-github committed Jan 9, 2024
1 parent 576e61a commit 1d4ab4d
Show file tree
Hide file tree
Showing 4 changed files with 25 additions and 11 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,9 @@ public static boolean platformIsDefault(Label platform) {
OptionEffectTag.CHANGES_INPUTS,
OptionEffectTag.LOADING_AND_ANALYSIS
},
metadataTags = {
OptionMetadataTag.IMMUTABLE,
},
help =
"The location of a mapping file that describes which platform to use if none is set or "
+ "which flags to set when a platform already exists. Must be relative to the main "
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,8 @@ public StateMachine step(Tasks tasks) {
Collection<String> transitionKeys = entry.getValue();
tasks.lookUp(
PlatformMappingValue.Key.create(entry.getKey().orElse(null)),
// No need to consider error handling: this flag cannot be changed and so errors were
// detected and handled in SkyframeExecutor.createBuildConfigurationKey.
rawValue -> {
var value = (PlatformMappingValue) rawValue;
// Maps the value from all transition keys with the same platform mappings path.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1852,6 +1852,7 @@ private BuildConfigurationKey createBuildConfigurationKey(
PlatformMappingValue.Key.create(platformMappingPath);
EvaluationResult<SkyValue> evaluationResult =
evaluateSkyKeys(eventHandler, ImmutableSet.of(platformMappingKey));
// Handle all possible errors with the platform mapping by reporting them to the user.
if (evaluationResult.hasError()) {
throw new InvalidConfigurationException(
Code.PLATFORM_MAPPING_EVALUATION_FAILURE, evaluationResult.getError().getException());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -115,15 +115,14 @@ public PlatformMappingValue compute(SkyKey skyKey, Environment env)
+ " '%s' but that path refers to a directory, not a file",
workspaceRelativeMappingPath, root),
Code.PLATFORM_MAPPINGS_FILE_IS_DIRECTORY),
Location.BUILTIN),
SkyFunctionException.Transience.PERSISTENT);
Location.BUILTIN));
}

List<String> lines;
try {
lines = FileSystemUtils.readLines(fileValue.realRootedPath().asPath(), UTF_8);
} catch (IOException e) {
throw new PlatformMappingException(e, SkyFunctionException.Transience.TRANSIENT);
throw new PlatformMappingException(e);
}

Mappings parsed = parse(env, lines, mainRepoContext);
Expand All @@ -147,8 +146,7 @@ public PlatformMappingValue compute(SkyKey skyKey, Environment env)
+ "package path roots, '%s'",
workspaceRelativeMappingPath, pathEntries),
Code.PLATFORM_MAPPINGS_FILE_NOT_FOUND),
Location.BUILTIN),
SkyFunctionException.Transience.PERSISTENT);
Location.BUILTIN));
}

private static FailureDetail createFailureDetail(String message, Code detailedCode) {
Expand All @@ -161,8 +159,20 @@ private static FailureDetail createFailureDetail(String message, Code detailedCo
@VisibleForTesting
static final class PlatformMappingException extends SkyFunctionException {

PlatformMappingException(Exception cause, Transience transience) {
super(cause, transience);
PlatformMappingException(OptionsParsingException cause) {
super(cause, Transience.PERSISTENT);
}

PlatformMappingException(MissingInputFileException cause) {
super(cause, Transience.PERSISTENT);
}

PlatformMappingException(IOException cause) {
super(cause, Transience.TRANSIENT);
}

PlatformMappingException(PlatformMappingParsingException cause) {
super(cause, Transience.PERSISTENT);
}
}

Expand Down Expand Up @@ -280,7 +290,7 @@ private static NativeAndStarlarkFlags parseStarlarkFlags(
return NativeAndStarlarkFlags.create(
nativeFlags.build(), fakeNativeParser.getStarlarkOptions());
} catch (OptionsParsingException e) {
throw new PlatformMappingException(e, Transience.PERSISTENT);
throw new PlatformMappingException(e);
}
}

Expand Down Expand Up @@ -368,9 +378,7 @@ private static PlatformMappingException parsingException(String message) {
}

private static PlatformMappingException parsingException(String message, Exception cause) {
return new PlatformMappingException(
new PlatformMappingParsingException(message, cause),
SkyFunctionException.Transience.PERSISTENT);
return new PlatformMappingException(new PlatformMappingParsingException(message, cause));
}

/**
Expand Down

0 comments on commit 1d4ab4d

Please sign in to comment.