Automated rollback of commit a8ca83fc01e6312fe0e145e8d63fad8b84713e09.
*** Reason for rollback ***
Appears to be cause of NPEs showing up in internal integration tests.
Mostly clean rollback --BuildDriverFunction changes, which later changes have
come to rely on.
PiperOrigin-RevId: 436472087
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/CollectTargetsInPackageFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/CollectTargetsInPackageFunction.java
index 324a172..e7f9a4e 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/CollectTargetsInPackageFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/CollectTargetsInPackageFunction.java
@@ -20,7 +20,6 @@
import com.google.devtools.build.lib.packages.Package;
import com.google.devtools.build.lib.packages.Target;
import com.google.devtools.build.lib.pkgcache.TargetPatternResolverUtil;
-import com.google.devtools.build.skyframe.GraphTraversingHelper;
import com.google.devtools.build.skyframe.SkyFunction;
import com.google.devtools.build.skyframe.SkyFunctionException;
import com.google.devtools.build.skyframe.SkyKey;
@@ -51,13 +50,14 @@
Event.error(
"package contains errors: " + packageId.getPackageFragment().getPathString()));
}
- return GraphTraversingHelper.declareDependenciesAndCheckIfValuesMissing(
- env,
- Iterables.transform(
- TargetPatternResolverUtil.resolvePackageTargets(pkg, argument.getFilteringPolicy()),
- TO_TRANSITIVE_TRAVERSAL_KEY))
- ? null
- : CollectTargetsInPackageValue.INSTANCE;
+ env.getValues(
+ Iterables.transform(
+ TargetPatternResolverUtil.resolvePackageTargets(pkg, argument.getFilteringPolicy()),
+ TO_TRANSITIVE_TRAVERSAL_KEY));
+ if (env.valuesMissing()) {
+ return null;
+ }
+ return CollectTargetsInPackageValue.INSTANCE;
}
private static final Function<Target, SkyKey> TO_TRANSITIVE_TRAVERSAL_KEY =
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/CollectTestSuitesInPackageFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/CollectTestSuitesInPackageFunction.java
index 41a78ad..85671af 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/CollectTestSuitesInPackageFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/CollectTestSuitesInPackageFunction.java
@@ -21,7 +21,6 @@
import com.google.devtools.build.lib.packages.Package;
import com.google.devtools.build.lib.packages.Rule;
import com.google.devtools.build.lib.packages.TargetUtils;
-import com.google.devtools.build.skyframe.GraphTraversingHelper;
import com.google.devtools.build.skyframe.SkyFunction;
import com.google.devtools.build.skyframe.SkyFunctionException;
import com.google.devtools.build.skyframe.SkyKey;
@@ -68,9 +67,11 @@
CollectTestSuitesInPackageValue.key(label.getPackageIdentifier()));
}
collectTestSuiteInPkgDeps.remove(skyKey);
- return GraphTraversingHelper.declareDependenciesAndCheckIfValuesMissing(
- env, collectTestSuiteInPkgDeps)
- ? null
- : CollectTestSuitesInPackageValue.INSTANCE;
+ env.getValues(collectTestSuiteInPkgDeps);
+
+ if (env.valuesMissing()) {
+ return null;
+ }
+ return CollectTestSuitesInPackageValue.INSTANCE;
}
}
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetAndData.java b/src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetAndData.java
index 7171920..00fef49 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetAndData.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetAndData.java
@@ -24,7 +24,8 @@
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.SkyValue;
+import java.util.Map;
import javax.annotation.Nullable;
/**
@@ -92,15 +93,16 @@
} else {
packageAndMaybeConfiguration = ImmutableSet.of(packageKey, configurationKeyMaybe);
}
- SkyframeIterableResult packageAndMaybeConfigurationValues =
- env.getOrderedValuesAndExceptions(packageAndMaybeConfiguration);
+ Map<SkyKey, SkyValue> packageAndMaybeConfigurationValues =
+ env.getValues(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;
}
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetFunction.java
index ac56de2..a6fc2141 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetFunction.java
@@ -479,15 +479,12 @@
} else {
packageAndMaybeConfiguration = ImmutableSet.of(packageKey, configurationKeyMaybe);
}
- SkyframeLookupResult packageAndMaybeConfigurationValues =
- env.getValuesAndExceptions(packageAndMaybeConfiguration);
+ Map<SkyKey, SkyValue> packageAndMaybeConfigurationValues =
+ env.getValues(packageAndMaybeConfiguration);
if (env.valuesMissing()) {
return null;
}
PackageValue packageValue = (PackageValue) packageAndMaybeConfigurationValues.get(packageKey);
- if (packageValue == null) {
- return null;
- }
Package pkg = packageValue.getPackage();
if (configurationKeyMaybe != null) {
configuration =
@@ -994,7 +991,7 @@
Map<SkyKey, ConfiguredTargetAndData> result = Maps.newHashMapWithExpectedSize(deps.size());
Set<SkyKey> aliasPackagesToFetch = new HashSet<>();
List<Dependency> aliasDepsToRedo = new ArrayList<>();
- SkyframeLookupResult aliasPackageValues = null;
+ Map<SkyKey, SkyValue> aliasPackageValues = null;
Collection<Dependency> depsToProcess = deps;
for (int i = 0; i < 2; i++) {
for (Dependency dep : depsToProcess) {
@@ -1076,7 +1073,7 @@
if (aliasDepsToRedo.isEmpty()) {
break;
}
- aliasPackageValues = env.getValuesAndExceptions(aliasPackagesToFetch);
+ aliasPackageValues = env.getValues(aliasPackagesToFetch);
depsToProcess = aliasDepsToRedo;
}
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/GlobFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/GlobFunction.java
index 01bcb9f..2885689 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/GlobFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/GlobFunction.java
@@ -35,9 +35,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 java.util.Map;
-import java.util.Set;
import java.util.concurrent.ConcurrentHashMap;
import java.util.regex.Pattern;
import javax.annotation.Nullable;
@@ -170,15 +168,17 @@
globSubdir,
patternTail,
globberOperation);
- SkyframeIterableResult listingAndRecursiveGlobResult =
- env.getOrderedValuesAndExceptions(
+ Map<SkyKey, SkyValue> listingAndRecursiveGlobMap =
+ env.getValues(
ImmutableList.of(keyForRecursiveGlobInCurrentDirectory, directoryListingKey));
if (env.valuesMissing()) {
return null;
}
- GlobValue globValue = (GlobValue) listingAndRecursiveGlobResult.next();
+ GlobValue globValue =
+ (GlobValue) listingAndRecursiveGlobMap.get(keyForRecursiveGlobInCurrentDirectory);
matches.addTransitive(globValue.getMatches());
- listingValue = (DirectoryListingValue) listingAndRecursiveGlobResult.next();
+ listingValue =
+ (DirectoryListingValue) listingAndRecursiveGlobMap.get(directoryListingKey);
}
}
@@ -233,9 +233,8 @@
}
}
- Set<SkyKey> subdirAndSymlinksKeys = Sets.union(subdirMap.keySet(), symlinkFileMap.keySet());
- SkyframeIterableResult subdirAndSymlinksResult =
- env.getOrderedValuesAndExceptions(subdirAndSymlinksKeys);
+ Map<SkyKey, SkyValue> subdirAndSymlinksResult =
+ env.getValues(Sets.union(subdirMap.keySet(), symlinkFileMap.keySet()));
if (env.valuesMissing()) {
return null;
}
@@ -243,17 +242,14 @@
// Second pass: process the symlinks and subdirectories from the first pass, and maybe
// collect further SkyKeys if fully resolved symlink targets are themselves directories.
// Also process any known directories.
- for (SkyKey subdirAndSymlinksKey : subdirAndSymlinksKeys) {
- if (symlinkFileMap.containsKey(subdirAndSymlinksKey)) {
- FileValue symlinkFileValue = (FileValue) subdirAndSymlinksResult.next();
- if (symlinkFileValue == null) {
- return null;
- }
+ for (Map.Entry<SkyKey, SkyValue> lookedUpKeyAndValue : subdirAndSymlinksResult.entrySet()) {
+ if (symlinkFileMap.containsKey(lookedUpKeyAndValue.getKey())) {
+ FileValue symlinkFileValue = (FileValue) lookedUpKeyAndValue.getValue();
if (!symlinkFileValue.isSymlink()) {
throw new GlobFunctionException(
new InconsistentFilesystemException(
"readdir and stat disagree about whether "
- + ((RootedPath) subdirAndSymlinksKey.argument()).asPath()
+ + ((RootedPath) lookedUpKeyAndValue.getKey().argument()).asPath()
+ " is a symlink."),
Transience.TRANSIENT);
}
@@ -282,7 +278,7 @@
throw new GlobFunctionException(symlinkException, Transience.PERSISTENT);
}
- Dirent dirent = symlinkFileMap.get(subdirAndSymlinksKey);
+ Dirent dirent = symlinkFileMap.get(lookedUpKeyAndValue.getKey());
String fileName = dirent.getName();
if (symlinkFileValue.isDirectory()) {
SkyKey keyToRequest = getSkyKeyForSubdir(fileName, glob, subdirPattern);
@@ -293,28 +289,18 @@
sortedResultMap.put(dirent, glob.getSubdir().getRelative(fileName));
}
} else {
- SkyValue value = subdirAndSymlinksResult.next();
- if (value == null) {
- return null;
- }
- processSubdir(Map.entry(subdirAndSymlinksKey, value), subdirMap, glob, sortedResultMap);
+ processSubdir(lookedUpKeyAndValue, subdirMap, glob, sortedResultMap);
}
}
- Set<SkyKey> symlinkSubdirKeys = symlinkSubdirMap.keySet();
- SkyframeIterableResult symlinkSubdirResult =
- env.getOrderedValuesAndExceptions(symlinkSubdirKeys);
+ Map<SkyKey, SkyValue> symlinkSubdirResult = env.getValues(symlinkSubdirMap.keySet());
if (env.valuesMissing()) {
return null;
}
// Third pass: do needed subdirectories of symlinked directories discovered during the second
// pass.
- for (SkyKey symlinkSubdirKey : symlinkSubdirKeys) {
- processSubdir(
- Map.entry(symlinkSubdirKey, symlinkSubdirResult.next()),
- symlinkSubdirMap,
- glob,
- sortedResultMap);
+ for (Map.Entry<SkyKey, SkyValue> lookedUpKeyAndValue : symlinkSubdirResult.entrySet()) {
+ processSubdir(lookedUpKeyAndValue, symlinkSubdirMap, glob, sortedResultMap);
}
for (Map.Entry<Dirent, Object> fileMatches : sortedResultMap.entrySet()) {
addToMatches(fileMatches.getValue(), matches);
