When requesting all transitive traversal values recursively, request packages/targets in parallel with subdirectories.

The previous implementation was vulnerable to incomplete traversal in case of cycles: if a subdirectory had a cycle, the targets in the package would never be requested. Requesting the package first would create a different problem, where the subdirectories would never be requested if the package depended on a cycle.

Also stop uniquifying Skylark import cycles when inlining. While uniquification is nice, this was leading to us emitting an error during queries that we weren't actually emitting when not inlining. Putting the cycle into the exception error message should be enough information, and since we emit an error per exception anyway, the number of events will still be lower (although each event might be more verbose).

--
MOS_MIGRATED_REVID=135846847

1 files changed, 77 insertions, 0 deletions

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
new file mode 100644
index 0000000000..7a085c48fd
--- /dev/null
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/CollectTargetsInPackageFunction.java
@@ -0,0 +1,77 @@
+// Copyright 2016 The Bazel Authors. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//    http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+package com.google.devtools.build.lib.skyframe;
+
+import com.google.common.base.Function;
+import com.google.common.collect.Iterables;
+import com.google.devtools.build.lib.cmdline.PackageIdentifier;
+import com.google.devtools.build.lib.events.Event;
+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.SkyFunction;
+import com.google.devtools.build.skyframe.SkyFunctionException;
+import com.google.devtools.build.skyframe.SkyKey;
+import com.google.devtools.build.skyframe.SkyValue;
+import javax.annotation.Nullable;
+
+/**
+ * Declares a dependency on all targets in a package, to ensure those targets are in the graph. Does
+ * no error-checking on the package id provided, so callers should have already verified that there
+ * is a package with this id.
+ */
+class CollectTargetsInPackageFunction implements SkyFunction {
+  @Nullable
+  @Override
+  public SkyValue compute(SkyKey skyKey, Environment env)
+      throws SkyFunctionException, InterruptedException {
+    CollectTargetsInPackageValue.CollectTargetsInPackageKey argument =
+        (CollectTargetsInPackageValue.CollectTargetsInPackageKey) skyKey.argument();
+    PackageIdentifier packageId = argument.getPackageId();
+    PackageValue packageValue = (PackageValue) env.getValue(PackageValue.key(packageId));
+    if (env.valuesMissing()) {
+      return null;
+    }
+    Package pkg = packageValue.getPackage();
+    if (pkg.containsErrors()) {
+      env.getListener()
+          .handle(
+              Event.error(
+                  "package contains errors: " + packageId.getPackageFragment().getPathString()));
+    }
+    env.getValues(
+        Iterables.transform(
+            TargetPatternResolverUtil.resolvePackageTargets(pkg, argument.getFilteringPolicy())
+                .getTargets(),
+            TO_TRANSITIVE_TRAVERSAL_KEY));
+    if (env.valuesMissing()) {
+      return null;
+    }
+    return CollectTargetsInPackageValue.INSTANCE;
+  }
+
+  private static final Function<Target, SkyKey> TO_TRANSITIVE_TRAVERSAL_KEY =
+      new Function<Target, SkyKey>() {
+        @Override
+        public SkyKey apply(Target target) {
+          return TransitiveTraversalValue.key(target.getLabel());
+        }
+      };
+
+  @Nullable
+  @Override
+  public String extractTag(SkyKey skyKey) {
+    return null;
+  }
+}