Skip to content

Commit

Permalink
Remove remaining calls to getOrderedValuesAndExceptions and tests.
Browse files Browse the repository at this point in the history
After this, all that remains is unused implementations.

PiperOrigin-RevId: 485928424
Change-Id: Idfa158fc1dfed3d13f1b23b55d23867b5db5b956
  • Loading branch information
justinhorvitz authored and Copybara-Service committed Nov 3, 2022
1 parent c20b1d6 commit b5052b8
Show file tree
Hide file tree
Showing 25 changed files with 168 additions and 262 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@
import com.google.devtools.build.lib.packages.Target;
import com.google.devtools.build.skyframe.SkyFunction;
import com.google.devtools.build.skyframe.SkyKey;
import com.google.devtools.build.skyframe.SkyframeIterableResult;
import com.google.devtools.build.skyframe.SkyframeLookupResult;
import javax.annotation.Nullable;

/**
Expand Down Expand Up @@ -104,15 +104,16 @@ static ConfiguredTargetAndData fromConfiguredTargetInSkyframe(
} else {
packageAndMaybeConfiguration = ImmutableSet.of(packageKey, configurationKeyMaybe);
}
SkyframeIterableResult packageAndMaybeConfigurationValues =
env.getOrderedValuesAndExceptions(packageAndMaybeConfiguration);
SkyframeLookupResult packageAndMaybeConfigurationValues =
env.getValuesAndExceptions(packageAndMaybeConfiguration);
// Don't test env.valuesMissing(), because values may already be missing from the caller.
PackageValue packageValue = (PackageValue) packageAndMaybeConfigurationValues.next();
PackageValue packageValue = (PackageValue) packageAndMaybeConfigurationValues.get(packageKey);
if (packageValue == null) {
return null;
}
if (configurationKeyMaybe != null) {
configuration = (BuildConfigurationValue) packageAndMaybeConfigurationValues.next();
configuration =
(BuildConfigurationValue) packageAndMaybeConfigurationValues.get(configurationKeyMaybe);
if (configuration == null) {
return null;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
import com.google.devtools.build.lib.server.FailureDetails.Toolchain.Code;
import com.google.devtools.build.skyframe.SkyFunction.Environment;
import com.google.devtools.build.skyframe.SkyframeIterableResult;
import com.google.devtools.build.skyframe.SkyframeLookupResult;
import java.util.ArrayList;
import java.util.List;
import javax.annotation.Nullable;
Expand All @@ -36,7 +37,7 @@ public static List<ConstraintValueInfo> getConstraintValueInfo(
Iterable<ConfiguredTargetKey> constraintValueKeys, Environment env)
throws InterruptedException, InvalidConstraintValueException {

SkyframeIterableResult values = env.getOrderedValuesAndExceptions(constraintValueKeys);
SkyframeLookupResult values = env.getValuesAndExceptions(constraintValueKeys);
boolean valuesMissing = env.valuesMissing();
List<ConstraintValueInfo> constraintValues = valuesMissing ? null : new ArrayList<>();
for (ConfiguredTargetKey key : constraintValueKeys) {
Expand All @@ -59,13 +60,12 @@ public static List<ConstraintValueInfo> getConstraintValueInfo(
*/
@Nullable
private static ConstraintValueInfo findConstraintValueInfo(
ConfiguredTargetKey key, SkyframeIterableResult values)
throws InvalidConstraintValueException {

ConfiguredTargetKey key, SkyframeLookupResult values) throws InvalidConstraintValueException {
try {
ConfiguredTargetValue ctv =
(ConfiguredTargetValue)
values.nextOrThrow(
values.getOrThrow(
key,
ConfiguredValueCreationException.class,
NoSuchThingException.class,
ActionConflictException.class);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@
import com.google.devtools.build.lib.vfs.RootedPath;
import com.google.devtools.build.skyframe.SkyFunction.Environment;
import com.google.devtools.build.skyframe.SkyKey;
import com.google.devtools.build.skyframe.SkyframeIterableResult;
import com.google.devtools.build.skyframe.SkyframeLookupResult;
import java.util.ArrayList;
import java.util.List;
import java.util.Map;
Expand Down Expand Up @@ -199,23 +199,23 @@ public void streamPackagesUnderDirectory(
ImmutableSet.copyOf(
Iterables.filter(ignoredSubdirectories, path -> path.startsWith(directory)));

SkyframeIterableResult recursivePackageValues =
env.getOrderedValuesAndExceptions(
Iterables.transform(
roots,
r ->
RecursivePkgValue.key(
repository,
RootedPath.toRootedPath(r, directory),
filteredIgnoredSubdirectories)));
Iterable<RecursivePkgValue.Key> recursivePackageKeys =
Iterables.transform(
roots,
r ->
RecursivePkgValue.key(
repository,
RootedPath.toRootedPath(r, directory),
filteredIgnoredSubdirectories));
SkyframeLookupResult recursivePackageValues = env.getValuesAndExceptions(recursivePackageKeys);
NoSuchPackageException firstNspe = null;
while (recursivePackageValues.hasNext()) {
for (RecursivePkgValue.Key key : recursivePackageKeys) {
RecursivePkgValue lookup;
try {
lookup =
(RecursivePkgValue)
recursivePackageValues.nextOrThrow(
NoSuchPackageException.class, ProcessPackageDirectoryException.class);
recursivePackageValues.getOrThrow(
key, NoSuchPackageException.class, ProcessPackageDirectoryException.class);
} catch (NoSuchPackageException e) {
// NoSuchPackageException can happen during error bubbling in a no-keep-going build.
if (firstNspe == null) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,6 @@
import com.google.devtools.build.skyframe.SkyFunctionException.Transience;
import com.google.devtools.build.skyframe.SkyKey;
import com.google.devtools.build.skyframe.SkyValue;
import com.google.devtools.build.skyframe.SkyframeIterableResult;
import com.google.devtools.build.skyframe.SkyframeLookupResult;
import com.google.errorprone.annotations.CanIgnoreReturnValue;
import java.io.IOException;
Expand Down Expand Up @@ -286,10 +285,10 @@ private static void handleGlobDepsAndPropagateFilesystemExceptions(
throws InternalInconsistentFilesystemException, FileSymlinkException, InterruptedException {
checkState(Iterables.all(depKeys, SkyFunctions.isSkyFunction(SkyFunctions.GLOB)), depKeys);
FileSymlinkException arbitraryFse = null;
SkyframeIterableResult skyframeIterableResult = env.getOrderedValuesAndExceptions(depKeys);
while (skyframeIterableResult.hasNext()) {
SkyframeLookupResult result = env.getValuesAndExceptions(depKeys);
for (SkyKey key : depKeys) {
try {
skyframeIterableResult.nextOrThrow(IOException.class, BuildFileNotFoundException.class);
result.getOrThrow(key, IOException.class, BuildFileNotFoundException.class);
} catch (InconsistentFilesystemException e) {
throw new InternalInconsistentFilesystemException(packageIdentifier, e);
} catch (FileSymlinkException e) {
Expand Down Expand Up @@ -725,11 +724,12 @@ private static List<BzlLoadValue> computeBzlLoadsNoInlining(
Environment env, List<BzlLoadValue.Key> keys)
throws InterruptedException, BzlLoadFailedException {
List<BzlLoadValue> bzlLoads = Lists.newArrayListWithExpectedSize(keys.size());
SkyframeIterableResult starlarkLookupResults = env.getOrderedValuesAndExceptions(keys);
for (int i = 0; i < keys.size(); i++) {
SkyframeLookupResult starlarkLookupResults = env.getValuesAndExceptions(keys);
for (BzlLoadValue.Key key : keys) {
// TODO(adonovan): if get fails, report the source location
// in the corresponding programLoads[i] (see caller).
bzlLoads.add((BzlLoadValue) starlarkLookupResults.nextOrThrow(BzlLoadFailedException.class));
bzlLoads.add(
(BzlLoadValue) starlarkLookupResults.getOrThrow(key, BzlLoadFailedException.class));
}
return env.valuesMissing() ? null : bzlLoads;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
import com.google.devtools.build.lib.server.FailureDetails.Toolchain.Code;
import com.google.devtools.build.skyframe.SkyFunction.Environment;
import com.google.devtools.build.skyframe.SkyframeIterableResult;
import com.google.devtools.build.skyframe.SkyframeLookupResult;
import java.util.HashMap;
import java.util.Map;
import javax.annotation.Nullable;
Expand All @@ -51,7 +52,7 @@ public static Map<ConfiguredTargetKey, PlatformInfo> getPlatformInfo(
return null;
}

SkyframeIterableResult values = env.getOrderedValuesAndExceptions(platformKeys);
SkyframeLookupResult values = env.getValuesAndExceptions(platformKeys);
boolean valuesMissing = env.valuesMissing();
Map<ConfiguredTargetKey, PlatformInfo> platforms = valuesMissing ? null : new HashMap<>();
for (ConfiguredTargetKey key : platformKeys) {
Expand Down Expand Up @@ -80,12 +81,13 @@ private static void validatePlatformKeys(
.map(PackageValue::key)
.collect(toImmutableSet());

SkyframeIterableResult values = env.getOrderedValuesAndExceptions(packageKeys);
SkyframeLookupResult values = env.getValuesAndExceptions(packageKeys);
boolean valuesMissing = env.valuesMissing();
Map<PackageIdentifier, Package> packages = valuesMissing ? null : new HashMap<>();
while (values.hasNext()) {
for (PackageValue.Key packageKey : packageKeys) {
try {
PackageValue packageValue = (PackageValue) values.nextOrThrow(NoSuchPackageException.class);
PackageValue packageValue =
(PackageValue) values.getOrThrow(packageKey, NoSuchPackageException.class);
if (!valuesMissing && packageValue != null) {
packages.put(packageValue.getPackage().getPackageIdentifier(), packageValue.getPackage());
}
Expand Down Expand Up @@ -125,13 +127,13 @@ private static void validatePlatformKeys(
* InvalidPlatformException} is thrown.
*/
@Nullable
private static PlatformInfo findPlatformInfo(
ConfiguredTargetKey key, SkyframeIterableResult values) throws InvalidPlatformException {

private static PlatformInfo findPlatformInfo(ConfiguredTargetKey key, SkyframeLookupResult values)
throws InvalidPlatformException {
try {
ConfiguredTargetValue ctv =
(ConfiguredTargetValue)
values.nextOrThrow(
values.getOrThrow(
key,
ConfiguredValueCreationException.class,
NoSuchThingException.class,
ActionConflictException.class);
Expand Down Expand Up @@ -191,10 +193,6 @@ public InvalidPlatformException(Label label, ActionConflictException e) {
super(formatError(label, DEFAULT_ERROR), e);
}

InvalidPlatformException(Label label, String error) {
super(formatError(label, error));
}

@Override
protected Code getDetailedCode() {
return Code.INVALID_PLATFORM_VALUE;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@
import com.google.devtools.build.skyframe.SkyFunction;
import com.google.devtools.build.skyframe.SkyKey;
import com.google.devtools.build.skyframe.SkyValue;
import com.google.devtools.build.skyframe.SkyframeIterableResult;
import com.google.devtools.build.skyframe.SkyframeLookupResult;
import javax.annotation.Nullable;

/**
Expand Down Expand Up @@ -103,15 +103,16 @@ public SkyValue compute(SkyKey skyKey, Environment env) throws InterruptedExcept
RepositoryMapping mainRepoMapping = repositoryMappingValue.getRepositoryMapping();
ImmutableList<SkyKey> skyKeys = getSkyKeys(skyKey, eventHandler, mainRepoMapping);

SkyframeIterableResult tokensByKey = env.getOrderedValuesAndExceptions(skyKeys);
SkyframeLookupResult tokensByKey = env.getValuesAndExceptions(skyKeys);
if (env.valuesMissing()) {
return null;
}

for (SkyKey key : skyKeys) {
try {
SkyValue value =
tokensByKey.nextOrThrow(
tokensByKey.getOrThrow(
key,
TargetParsingException.class,
ProcessPackageDirectoryException.class,
InconsistentFilesystemException.class);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@
import com.google.devtools.build.skyframe.SkyFunction.Environment;
import com.google.devtools.build.skyframe.SkyKey;
import com.google.devtools.build.skyframe.SkyValue;
import com.google.devtools.build.skyframe.SkyframeIterableResult;
import com.google.devtools.build.skyframe.SkyframeLookupResult;
import com.google.errorprone.annotations.ForOverride;
import java.util.Map;
import javax.annotation.Nullable;
Expand Down Expand Up @@ -145,7 +145,7 @@ public final ReturnT visitDirectory(RecursivePkgKey recursivePkgKey, Environment
Iterable<SkyKey> childDeps = processPackageDirectoryResult.getChildDeps();
ConsumerT consumer = getInitialConsumer();

SkyframeIterableResult dependentSkyValues;
SkyframeLookupResult dependentSkyValues;
if (processPackageDirectoryResult.packageExists()) {
PathFragment rootRelativePath = recursivePkgKey.getRootedPath().getRootRelativePath();
SkyKey packageErrorMessageKey =
Expand All @@ -155,13 +155,13 @@ public final ReturnT visitDirectory(RecursivePkgKey recursivePkgKey, Environment
// NoSuchPackageException. Since we don't catch such an exception here, this SkyFunction will
// return immediately with a missing value, and the NoSuchPackageException will propagate up.
dependentSkyValues =
env.getOrderedValuesAndExceptions(
env.getValuesAndExceptions(
Iterables.concat(ImmutableList.of(packageErrorMessageKey), childDeps));
if (env.valuesMissing()) {
return null;
}
PackageErrorMessageValue pkgErrorMessageValue =
(PackageErrorMessageValue) dependentSkyValues.next();
(PackageErrorMessageValue) dependentSkyValues.get(packageErrorMessageKey);
if (pkgErrorMessageValue == null) {
return null;
}
Expand All @@ -185,15 +185,15 @@ public final ReturnT visitDirectory(RecursivePkgKey recursivePkgKey, Environment
throw new IllegalStateException(pkgErrorMessageValue.getResult().toString());
}
} else {
dependentSkyValues = env.getOrderedValuesAndExceptions(childDeps);
dependentSkyValues = env.getValuesAndExceptions(childDeps);
if (env.valuesMissing()) {
return null;
}
return null;
}
}
ImmutableMap.Builder<SkyKey, SkyValue> subdirectorySkyValuesFromDeps =
ImmutableMap.builderWithExpectedSize(Iterables.size(childDeps));
for (SkyKey skyKey : childDeps) {
SkyValue skyValue = dependentSkyValues.next();
SkyValue skyValue = dependentSkyValues.get(skyKey);
if (skyValue == null) {
return null;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@
import com.google.devtools.build.skyframe.SkyFunctionException;
import com.google.devtools.build.skyframe.SkyKey;
import com.google.devtools.build.skyframe.SkyValue;
import com.google.devtools.build.skyframe.SkyframeIterableResult;
import com.google.devtools.build.skyframe.SkyframeLookupResult;
import java.io.IOException;
import java.math.BigInteger;
import java.util.ArrayList;
Expand Down Expand Up @@ -706,11 +706,11 @@ private ImmutableList<RecursiveFilesystemTraversalValue> traverseSourceChildren(
}
}

SkyframeIterableResult result = env.getOrderedValuesAndExceptions(childKeys);
SkyframeLookupResult result = env.getValuesAndExceptions(childKeys);
ImmutableList.Builder<RecursiveFilesystemTraversalValue> childValues =
ImmutableList.builderWithExpectedSize(childKeys.size());
for (SkyKey key : childKeys) {
SkyValue value = result.next();
SkyValue value = result.get(key);
if (value == null) {
continue;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@
import com.google.devtools.build.skyframe.SkyFunctionException.Transience;
import com.google.devtools.build.skyframe.SkyKey;
import com.google.devtools.build.skyframe.SkyValue;
import com.google.devtools.build.skyframe.SkyframeIterableResult;
import com.google.devtools.build.skyframe.SkyframeLookupResult;
import java.util.List;
import javax.annotation.Nullable;
import net.starlark.java.eval.StarlarkSemantics;
Expand Down Expand Up @@ -200,13 +200,13 @@ private static ImmutableList<ConfiguredTargetKey> configureRegisteredExecutionPl
.build())
.collect(toImmutableList());

SkyframeIterableResult values = env.getOrderedValuesAndExceptions(keys);
SkyframeLookupResult values = env.getValuesAndExceptions(keys);
ImmutableList.Builder<ConfiguredTargetKey> validPlatformKeys = new ImmutableList.Builder<>();
boolean valuesMissing = false;
for (ConfiguredTargetKey platformKey : keys) {
Label platformLabel = platformKey.getLabel();
try {
SkyValue value = values.nextOrThrow(ConfiguredValueCreationException.class);
SkyValue value = values.getOrThrow(platformKey, ConfiguredValueCreationException.class);
if (value == null) {
valuesMissing = true;
continue;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@
import com.google.devtools.build.skyframe.SkyFunctionException.Transience;
import com.google.devtools.build.skyframe.SkyKey;
import com.google.devtools.build.skyframe.SkyValue;
import com.google.devtools.build.skyframe.SkyframeIterableResult;
import com.google.devtools.build.skyframe.SkyframeLookupResult;
import java.util.List;
import javax.annotation.Nullable;
import net.starlark.java.eval.StarlarkSemantics;
Expand Down Expand Up @@ -189,14 +189,14 @@ private static ImmutableList<DeclaredToolchainInfo> configureRegisteredToolchain
.build())
.collect(toImmutableList());

SkyframeIterableResult values = env.getOrderedValuesAndExceptions(keys);
SkyframeLookupResult values = env.getValuesAndExceptions(keys);
ImmutableList.Builder<DeclaredToolchainInfo> toolchains = new ImmutableList.Builder<>();
boolean valuesMissing = false;
for (SkyKey key : keys) {
ConfiguredTargetKey configuredTargetKey = (ConfiguredTargetKey) key.argument();
Label toolchainLabel = configuredTargetKey.getLabel();
try {
SkyValue value = values.nextOrThrow(ConfiguredValueCreationException.class);
SkyValue value = values.getOrThrow(key, ConfiguredValueCreationException.class);
if (value == null) {
valuesMissing = true;
continue;
Expand Down
Loading

0 comments on commit b5052b8

Please sign in to comment.