Implement ignoring directories based on wildcards.

This is accomplished by a new directive in REPO.bazel,
"ignore_directories()". It takes a single argument, a list of
directories to ignore and it allows the same wildcards as glob().

This is done separately from .bazelignore to provide a migration path
off of that weird single-purpose configuration file.

Implementing this requires splitting RepoFileFunction into two: a part
that parses the repository file and one that creates a PackageArgs
instance. This was necessary to avoid a Skyframe dependency cycle: when
a WORKSPACE file is present and it loads a .bzl file from a repository
with a REPO.bazel file, the repo mapping for the main repository depends
on the WORKSPACE file, which depends on the .bzl file, which depends on
the IgnoredPackagePrefixesValue of its repository, which then depends on
the repo mapping of the main repository and the one the .bzl file is in,
which then depend on the WORKSPACE file.

Fixes #7093.

RELNOTES[NEW]: REPO.bazel now allows another directive,
"ignore_directories()". It takes a list of directories to ignore just
like .bazelignore does, but with glob semantics.

src/main/java/com/google/devtools/build/lib/cmdline/BUILD

@@ -51,6 +51,7 @@ java_library(
         "//src/main/java/com/google/devtools/build/lib/util:detailed_exit_code",
         "//src/main/java/com/google/devtools/build/lib/util:hash_codes",
         "//src/main/java/com/google/devtools/build/lib/util:string",
+        "//src/main/java/com/google/devtools/build/lib/vfs",
         "//src/main/java/com/google/devtools/build/lib/vfs:ospathpolicy",
         "//src/main/java/com/google/devtools/build/lib/vfs:pathfragment",
         "//src/main/java/com/google/devtools/build/skyframe:skyframe-objects",

src/main/java/com/google/devtools/build/lib/cmdline/IgnoredSubdirectories.java

@@ -14,12 +14,24 @@
 
 package com.google.devtools.build.lib.cmdline;
 
+import static com.google.common.collect.ImmutableList.toImmutableList;
 import static com.google.common.collect.ImmutableSet.toImmutableSet;
 
 import com.google.common.base.MoreObjects;
 import com.google.common.base.Preconditions;
+import com.google.common.base.Splitter;
+import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableSet;
+import com.google.common.collect.Iterables;
+import com.google.devtools.build.lib.skyframe.serialization.DeserializationContext;
+import com.google.devtools.build.lib.skyframe.serialization.ObjectCodec;
+import com.google.devtools.build.lib.skyframe.serialization.SerializationContext;
+import com.google.devtools.build.lib.skyframe.serialization.SerializationException;
 import com.google.devtools.build.lib.vfs.PathFragment;
+import com.google.devtools.build.lib.vfs.UnixGlob;
+import com.google.protobuf.CodedInputStream;
+import com.google.protobuf.CodedOutputStream;
+import java.io.IOException;
 import java.util.Objects;
 import javax.annotation.Nullable;
 
@@ -29,42 +41,98 @@
  * <p>This is currently just a prefix, but will eventually support glob-style wildcards.
  */
 public final class IgnoredSubdirectories {
-  public static final IgnoredSubdirectories EMPTY = new IgnoredSubdirectories(ImmutableSet.of());
+  public static final IgnoredSubdirectories EMPTY =
+      new IgnoredSubdirectories(ImmutableSet.of(), ImmutableList.of());
+
+  private static final Splitter SLASH_SPLITTER = Splitter.on("/");
 
   private final ImmutableSet<PathFragment> prefixes;
 
-  private IgnoredSubdirectories(ImmutableSet<PathFragment> prefixes) {
-    for (PathFragment prefix : prefixes) {
-      Preconditions.checkArgument(!prefix.isAbsolute());
+  // String[] is mutable; we keep the split version because that's faster to match and the non-split
+  // one because that allows for simpler equality checking and then matchingEntry() doesn't need to
+  // allocate new objects.
+  private final ImmutableList<String> patterns;
+  private final ImmutableList<String[]> splitPatterns;
+
+  private static class Codec implements ObjectCodec<IgnoredSubdirectories> {
+    private static final Codec INSTANCE = new Codec();
+
+    @Override
+    public Class<? extends IgnoredSubdirectories> getEncodedClass() {
+      return IgnoredSubdirectories.class;
+    }
+
+    @Override
+    public void serialize(
+        SerializationContext context, IgnoredSubdirectories obj, CodedOutputStream codedOut)
+        throws SerializationException, IOException {
+      context.serialize(obj.prefixes, codedOut);
+      context.serialize(obj.patterns, codedOut);
     }
+
+    @Override
+    public IgnoredSubdirectories deserialize(
+        DeserializationContext context, CodedInputStream codedIn)
+        throws SerializationException, IOException {
+      ImmutableSet<PathFragment> prefixes = context.deserialize(codedIn);
+      ImmutableList<String> patterns = context.deserialize(codedIn);
+
+      return new IgnoredSubdirectories(prefixes, patterns);
+    }
+  }
+
+  private IgnoredSubdirectories(
+      ImmutableSet<PathFragment> prefixes, ImmutableList<String> patterns) {
     this.prefixes = prefixes;
+    this.patterns = patterns;
+    this.splitPatterns =
+        patterns.stream()
+            .map(p -> Iterables.toArray(SLASH_SPLITTER.split(p), String.class))
+            .collect(toImmutableList());
   }
 
   public static IgnoredSubdirectories of(ImmutableSet<PathFragment> prefixes) {
-    if (prefixes.isEmpty()) {
+    return of(prefixes, ImmutableList.of());
+  }
+
+  public static IgnoredSubdirectories of(
+      ImmutableSet<PathFragment> prefixes, ImmutableList<String> patterns) {
+    if (prefixes.isEmpty() && patterns.isEmpty()) {
       return EMPTY;
-    } else {
-      return new IgnoredSubdirectories(prefixes);
     }
+
+    for (PathFragment prefix : prefixes) {
+      Preconditions.checkArgument(!prefix.isAbsolute());
+    }
+
+    return new IgnoredSubdirectories(prefixes, patterns);
   }
 
   public IgnoredSubdirectories withPrefix(PathFragment prefix) {
-    ImmutableSet<PathFragment> prefixed =
+    Preconditions.checkArgument(!prefix.isAbsolute());
+
+    ImmutableSet<PathFragment> prefixedPrefixes =
         prefixes.stream().map(prefix::getRelative).collect(toImmutableSet());
-    return new IgnoredSubdirectories(prefixed);
+
+    ImmutableList<String> prefixedPatterns =
+        patterns.stream().map(p -> prefix + "/" + p).collect(toImmutableList());
+
+    return new IgnoredSubdirectories(prefixedPrefixes, prefixedPatterns);
   }
 
   public IgnoredSubdirectories union(IgnoredSubdirectories other) {
     return new IgnoredSubdirectories(
-        ImmutableSet.<PathFragment>builder().addAll(prefixes).addAll(other.prefixes).build());
+        ImmutableSet.<PathFragment>builder().addAll(prefixes).addAll(other.prefixes).build(),
+        ImmutableList.<String>builder().addAll(patterns).addAll(other.patterns).build());
   }
 
   /** Filters out entries that cannot match anything under {@code directory}. */
   public IgnoredSubdirectories filterForDirectory(PathFragment directory) {
     ImmutableSet<PathFragment> filteredPrefixes =
         prefixes.stream().filter(p -> p.startsWith(directory)).collect(toImmutableSet());
+    ImmutableList.Builder<String> filteredPatterns = ImmutableList.builder();
 
-    return new IgnoredSubdirectories(filteredPrefixes);
+    return new IgnoredSubdirectories(filteredPrefixes, filteredPatterns.build());
   }
 
   public ImmutableSet<PathFragment> prefixes() {
@@ -95,10 +163,17 @@ public boolean allPathsAreUnder(PathFragment directory) {
 
   /** Returns the entry that matches a given directory or {@code null} if none. */
   @Nullable
-  public PathFragment matchingEntry(PathFragment directory) {
+  public String matchingEntry(PathFragment directory) {
     for (PathFragment prefix : prefixes) {
       if (directory.startsWith(prefix)) {
-        return prefix;
+        return prefix.getPathString();
+      }
+    }
+
+    String[] segmentArray = Iterables.toArray(directory.segments(), String.class);
+    for (int i = 0; i < patterns.size(); i++) {
+      if (UnixGlob.matchesPrefix(splitPatterns.get(i), segmentArray)) {
+        return patterns.get(i);
       }
     }
 
@@ -111,17 +186,22 @@ public boolean equals(Object other) {
       return false;
     }
 
+    // splitPatterns is a function of patterns so it's enough to check if patterns is equal
     IgnoredSubdirectories that = (IgnoredSubdirectories) other;
-    return Objects.equals(this.prefixes, that.prefixes);
+    return Objects.equals(this.prefixes, that.prefixes)
+        && Objects.equals(this.patterns, that.patterns);
   }
 
   @Override
   public int hashCode() {
-    return prefixes.hashCode();
+    return Objects.hash(prefixes, patterns);
   }
 
   @Override
   public String toString() {
-    return MoreObjects.toStringHelper("IgnoredSubdirectories").add("prefixes", prefixes).toString();
+    return MoreObjects.toStringHelper("IgnoredSubdirectories")
+        .add("prefixes", prefixes)
+        .add("patterns", patterns)
+        .toString();
   }
 }

src/main/java/com/google/devtools/build/lib/cmdline/TargetPattern.java

@@ -595,8 +595,7 @@ public <T, E extends Exception & QueryExceptionMarkerInterface> void eval(
           this,
           excludedSubdirectories);
       IgnoredSubdirectories ignoredSubdirectories = ignoredSubdirectoriesSupplier.get();
-      PathFragment matchingEntry =
-          ignoredSubdirectories.matchingEntry(directory.getPackageFragment());
+      String matchingEntry = ignoredSubdirectories.matchingEntry(directory.getPackageFragment());
       if (warnIfFiltered(matchingEntry, resolver)) {
         return;
       }
@@ -632,8 +631,7 @@ ListenableFuture<Void> evalAsync(
       IgnoredSubdirectories filteredIgnoredSubdirectories;
       try {
         IgnoredSubdirectories ignoredSubdirectories = ignoredSubdirectoriesSupplier.get();
-        PathFragment matchingEntry =
-            ignoredSubdirectories.matchingEntry(directory.getPackageFragment());
+        String matchingEntry = ignoredSubdirectories.matchingEntry(directory.getPackageFragment());
         if (warnIfFiltered(matchingEntry, resolver)) {
           return immediateVoidFuture();
         }
@@ -654,7 +652,7 @@ ListenableFuture<Void> evalAsync(
           executor);
     }
 
-    private boolean warnIfFiltered(PathFragment matchingEntry, TargetPatternResolver<?> resolver) {
+    private boolean warnIfFiltered(String matchingEntry, TargetPatternResolver<?> resolver) {
       if (matchingEntry != null) {
         resolver.warn(
             "Pattern '"

src/main/java/com/google/devtools/build/lib/packages/RepoCallable.java

@@ -16,8 +16,10 @@
 
 import java.util.Map;
 import net.starlark.java.annot.Param;
+import net.starlark.java.annot.ParamType;
 import net.starlark.java.annot.StarlarkMethod;
 import net.starlark.java.eval.EvalException;
+import net.starlark.java.eval.Sequence;
 import net.starlark.java.eval.Starlark;
 import net.starlark.java.eval.StarlarkThread;
 
@@ -27,6 +29,25 @@ private RepoCallable() {}
 
   public static final RepoCallable INSTANCE = new RepoCallable();
 
+  @StarlarkMethod(
+      name = "ignore_directories",
+      useStarlarkThread = true,
+      documented = false, // TODO
+      parameters = {
+        @Param(
+            name = "dirs",
+            allowedTypes = {
+              @ParamType(type = Sequence.class, generic1 = String.class),
+            })
+      })
+  public Object ignoreDirectories(Iterable<?> dirsUnchecked, StarlarkThread thread)
+      throws EvalException {
+    Sequence<String> dirs = Sequence.cast(dirsUnchecked, String.class, "dirs");
+    RepoThreadContext context = RepoThreadContext.fromOrFail(thread, "repo()");
+    context.setIgnoredDirectories(dirs);
+    return Starlark.NONE;
+  }
+
   @StarlarkMethod(
       name = "repo",
       documented = false, // documented separately
@@ -44,16 +65,7 @@ public Object repoCallable(Map<String, Object> kwargs, StarlarkThread thread)
       throw Starlark.errorf("at least one argument must be given to the 'repo' function");
     }
 
-    PackageArgs.Builder pkgArgsBuilder = PackageArgs.builder();
-    for (Map.Entry<String, Object> kwarg : kwargs.entrySet()) {
-      PackageArgs.processParam(
-          kwarg.getKey(),
-          kwarg.getValue(),
-          "repo() argument '" + kwarg.getKey() + "'",
-          context.getLabelConverter(),
-          pkgArgsBuilder);
-    }
-    context.setPackageArgs(pkgArgsBuilder.build());
+    context.setPackageArgsMap(kwargs);
     return Starlark.NONE;
   }
 }

src/main/java/com/google/devtools/build/lib/packages/RepoThreadContext.java

@@ -14,18 +14,26 @@
 
 package com.google.devtools.build.lib.packages;
 
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableMap;
 import com.google.devtools.build.lib.cmdline.RepositoryMapping;
 import com.google.devtools.build.lib.cmdline.StarlarkThreadContext;
+import java.util.Collection;
+import java.util.Map;
 import net.starlark.java.eval.EvalException;
 import net.starlark.java.eval.Starlark;
 import net.starlark.java.eval.StarlarkThread;
 
 /** Context object for a Starlark thread evaluating the REPO.bazel file. */
 public class RepoThreadContext extends StarlarkThreadContext {
   private final LabelConverter labelConverter;
-  private PackageArgs packageArgs = PackageArgs.EMPTY;
+
+  private ImmutableMap<String, Object> packageArgsMap = ImmutableMap.of();
   private boolean repoFunctionCalled = false;
 
+  private ImmutableList<String> ignoredDirectories = ImmutableList.of();
+  private boolean ignoredDirectoriesSet = false;
+
   public static RepoThreadContext fromOrFail(StarlarkThread thread, String what)
       throws EvalException {
     StarlarkThreadContext context = thread.getThreadLocal(StarlarkThreadContext.class);
@@ -52,11 +60,24 @@ public void setRepoFunctionCalled() {
     repoFunctionCalled = true;
   }
 
-  public void setPackageArgs(PackageArgs packageArgs) {
-    this.packageArgs = packageArgs;
+  public void setPackageArgsMap(Map<String, Object> kwargs) {
+    this.packageArgsMap = ImmutableMap.copyOf(kwargs);
+  }
+
+  public ImmutableMap<String, Object> getPackageArgsMap() {
+    return packageArgsMap;
+  }
+
+  public void setIgnoredDirectories(Collection<String> ignoredDirectories) throws EvalException {
+    if (ignoredDirectoriesSet) {
+      throw new EvalException("'ignored_directories()' can only be called once");
+    }
+
+    ignoredDirectoriesSet = true;
+    this.ignoredDirectories = ImmutableList.copyOf(ignoredDirectories);
   }
 
-  public PackageArgs getPackageArgs() {
-    return packageArgs;
+  public ImmutableList<String> getIgnoredDirectories() {
+    return ignoredDirectories;
   }
 }

src/main/java/com/google/devtools/build/lib/skyframe/BUILD

@@ -150,6 +150,7 @@ java_library(
         ":map_as_package_roots",
         ":metadata_consumer_for_metrics",
         ":node_dropping_inconsistency_receiver",
+        ":package_args_function",
         ":package_error_function",
         ":package_error_message_function",
         ":package_identifier_batching_callback",
@@ -831,6 +832,8 @@ java_library(
     deps = [
         ":ignored_package_prefixes_value",
         ":precomputed_value",
+        ":repo_file_function",
+        ":repo_file_value",
         "//src/main/java/com/google/devtools/build/lib/actions:file_metadata",
         "//src/main/java/com/google/devtools/build/lib/cmdline",
         "//src/main/java/com/google/devtools/build/lib/io:inconsistent_filesystem_exception",
@@ -2310,6 +2313,25 @@ java_library(
     ],
 )
 
+java_library(
+    name = "package_args_function",
+    srcs = ["PackageArgsFunction.java"],
+    deps = [
+        ":repo_file_function",
+        ":repo_file_value",
+        ":repository_mapping_value",
+        ":sky_functions",
+        "//src/main/java/com/google/devtools/build/lib/cmdline",
+        "//src/main/java/com/google/devtools/build/lib/events",
+        "//src/main/java/com/google/devtools/build/lib/packages",
+        "//src/main/java/com/google/devtools/build/lib/vfs:pathfragment",
+        "//src/main/java/com/google/devtools/build/skyframe:skyframe-objects",
+        "//src/main/java/net/starlark/java/eval",
+        "//third_party:guava",
+        "//third_party:jsr305",
+    ],
+)
+
 java_library(
     name = "repo_file_function",
     srcs = ["RepoFileFunction.java"],
@@ -2327,6 +2349,7 @@ java_library(
         "//src/main/java/com/google/devtools/build/skyframe:skyframe-objects",
         "//src/main/java/net/starlark/java/eval",
         "//src/main/java/net/starlark/java/syntax",
+        "//third_party:guava",
         "//third_party:jsr305",
     ],
 )
@@ -2340,6 +2363,7 @@ java_library(
         "//src/main/java/com/google/devtools/build/lib/packages",
         "//src/main/java/com/google/devtools/build/skyframe:skyframe-objects",
         "//third_party:auto_value",
+        "//third_party:guava",
     ],
 )