From 00da3665c4f1910141b320b750c0270a7919ef6b Mon Sep 17 00:00:00 2001 From: lberki Date: Tue, 5 Nov 2024 14:02:50 +0100 Subject: [PATCH] Implement ignoring directories based on wildcards. (#24203) 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. Closes #24032. PiperOrigin-RevId: 693227896 Change-Id: Ia3e02a2bfe9caf999fc641f75261b528b19c1d03 --- .../starlark/StarlarkGlobalsImpl.java | 4 +- .../google/devtools/build/lib/cmdline/BUILD | 1 + .../lib/cmdline/IgnoredSubdirectories.java | 123 ++++++++++++--- .../build/lib/cmdline/TargetPattern.java | 8 +- .../build/lib/packages/RepoCallable.java | 59 ------- .../build/lib/packages/RepoFileGlobals.java | 81 ++++++++++ .../build/lib/packages/RepoThreadContext.java | 40 +++-- .../google/devtools/build/lib/skyframe/BUILD | 27 +++- .../BazelSkyframeExecutorConstants.java | 4 - .../IgnoredPackagePrefixesFunction.java | 146 ++++++++++++------ .../skyframe/IgnoredPackagePrefixesValue.java | 8 +- .../build/lib/skyframe/PackageFunction.java | 43 ++++-- .../build/lib/skyframe/RepoFileFunction.java | 40 ++--- .../build/lib/skyframe/RepoFileValue.java | 14 +- .../lib/skyframe/RepoPackageArgsFunction.java | 142 +++++++++++++++++ .../build/lib/skyframe/SkyFunctions.java | 2 + .../build/lib/skyframe/SkyframeExecutor.java | 1 + .../packages/AbstractPackageLoader.java | 8 +- .../build/lib/skyframe/packages/BUILD | 2 +- .../devtools/build/lib/vfs/UnixGlob.java | 41 ++++- .../bzlmod/ModuleExtensionResolutionTest.java | 5 +- .../bazel/bzlmod/ModuleFileFunctionTest.java | 6 +- .../cmdline/IgnoredSubdirectoriesTest.java | 99 ++++++++++++ .../repository/RepositoryDelegatorTest.java | 6 +- .../google/devtools/build/lib/skyframe/BUILD | 2 + .../ContainingPackageLookupFunctionTest.java | 5 +- .../skyframe/FilesetEntryFunctionTest.java | 5 +- .../build/lib/skyframe/GlobTestBase.java | 8 +- .../skyframe/PackageLookupFunctionTest.java | 6 +- ...ursiveFilesystemTraversalFunctionTest.java | 5 +- .../IgnoredPackagePrefixesValueCodecTest.java | 21 ++- src/test/shell/bazel/bazelignore_test.sh | 71 +++++++++ 32 files changed, 784 insertions(+), 249 deletions(-) delete mode 100644 src/main/java/com/google/devtools/build/lib/packages/RepoCallable.java create mode 100644 src/main/java/com/google/devtools/build/lib/packages/RepoFileGlobals.java create mode 100644 src/main/java/com/google/devtools/build/lib/skyframe/RepoPackageArgsFunction.java create mode 100644 src/test/java/com/google/devtools/build/lib/cmdline/IgnoredSubdirectoriesTest.java diff --git a/src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkGlobalsImpl.java b/src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkGlobalsImpl.java index bae72e298463c5..5005259efa5446 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkGlobalsImpl.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkGlobalsImpl.java @@ -23,7 +23,7 @@ import com.google.devtools.build.lib.collect.nestedset.Depset; import com.google.devtools.build.lib.packages.BuildGlobals; import com.google.devtools.build.lib.packages.Proto; -import com.google.devtools.build.lib.packages.RepoCallable; +import com.google.devtools.build.lib.packages.RepoFileGlobals; import com.google.devtools.build.lib.packages.SelectorList; import com.google.devtools.build.lib.packages.StarlarkGlobals; import com.google.devtools.build.lib.packages.StarlarkNativeModule; @@ -133,7 +133,7 @@ public ImmutableMap getModuleToplevels() { @Override public ImmutableMap getRepoToplevels() { ImmutableMap.Builder env = ImmutableMap.builder(); - Starlark.addMethods(env, RepoCallable.INSTANCE); + Starlark.addMethods(env, RepoFileGlobals.INSTANCE); return env.buildOrThrow(); } diff --git a/src/main/java/com/google/devtools/build/lib/cmdline/BUILD b/src/main/java/com/google/devtools/build/lib/cmdline/BUILD index 00d4764187ff85..d9b981f1a72e9c 100644 --- a/src/main/java/com/google/devtools/build/lib/cmdline/BUILD +++ b/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", diff --git a/src/main/java/com/google/devtools/build/lib/cmdline/IgnoredSubdirectories.java b/src/main/java/com/google/devtools/build/lib/cmdline/IgnoredSubdirectories.java index 356251845662b2..fcd8ca98fa09b7 100644 --- a/src/main/java/com/google/devtools/build/lib/cmdline/IgnoredSubdirectories.java +++ b/src/main/java/com/google/devtools/build/lib/cmdline/IgnoredSubdirectories.java @@ -14,49 +14,115 @@ 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; /** * A set of subdirectories to ignore during target pattern matching or globbing. - * - *

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 prefixes; - private IgnoredSubdirectories(ImmutableSet 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 patterns; + private final ImmutableList splitPatterns; + + private static class Codec implements ObjectCodec { + private static final Codec INSTANCE = new Codec(); + + @Override + public Class 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 prefixes = context.deserialize(codedIn); + ImmutableList patterns = context.deserialize(codedIn); + + return new IgnoredSubdirectories(prefixes, patterns); + } + } + + private IgnoredSubdirectories( + ImmutableSet prefixes, ImmutableList 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 prefixes) { - if (prefixes.isEmpty()) { + return of(prefixes, ImmutableList.of()); + } + + public static IgnoredSubdirectories of( + ImmutableSet prefixes, ImmutableList 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 prefixed = + Preconditions.checkArgument(!prefix.isAbsolute()); + + ImmutableSet prefixedPrefixes = prefixes.stream().map(prefix::getRelative).collect(toImmutableSet()); - return new IgnoredSubdirectories(prefixed); + + ImmutableList prefixedPatterns = + patterns.stream().map(p -> prefix + "/" + p).collect(toImmutableList()); + + return new IgnoredSubdirectories(prefixedPrefixes, prefixedPatterns); } public IgnoredSubdirectories union(IgnoredSubdirectories other) { return new IgnoredSubdirectories( - ImmutableSet.builder().addAll(prefixes).addAll(other.prefixes).build()); + ImmutableSet.builder().addAll(prefixes).addAll(other.prefixes).build(), + ImmutableList.copyOf( + ImmutableSet.builder().addAll(patterns).addAll(other.patterns).build())); } /** Filters out entries that cannot match anything under {@code directory}. */ @@ -64,7 +130,16 @@ public IgnoredSubdirectories filterForDirectory(PathFragment directory) { ImmutableSet filteredPrefixes = prefixes.stream().filter(p -> p.startsWith(directory)).collect(toImmutableSet()); - return new IgnoredSubdirectories(filteredPrefixes); + String[] splitDirectory = + Iterables.toArray(SLASH_SPLITTER.split(directory.getPathString()), String.class); + ImmutableList.Builder filteredPatterns = ImmutableList.builder(); + for (int i = 0; i < patterns.size(); i++) { + if (UnixGlob.canMatchChild(splitPatterns.get(i), splitDirectory)) { + filteredPatterns.add(patterns.get(i)); + } + } + + return new IgnoredSubdirectories(filteredPrefixes, filteredPatterns.build()); } public ImmutableSet prefixes() { @@ -95,10 +170,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 +193,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(); } } diff --git a/src/main/java/com/google/devtools/build/lib/cmdline/TargetPattern.java b/src/main/java/com/google/devtools/build/lib/cmdline/TargetPattern.java index e1ff3593701309..99687ba531e401 100644 --- a/src/main/java/com/google/devtools/build/lib/cmdline/TargetPattern.java +++ b/src/main/java/com/google/devtools/build/lib/cmdline/TargetPattern.java @@ -595,8 +595,7 @@ public 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 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 evalAsync( executor); } - private boolean warnIfFiltered(PathFragment matchingEntry, TargetPatternResolver resolver) { + private boolean warnIfFiltered(String matchingEntry, TargetPatternResolver resolver) { if (matchingEntry != null) { resolver.warn( "Pattern '" diff --git a/src/main/java/com/google/devtools/build/lib/packages/RepoCallable.java b/src/main/java/com/google/devtools/build/lib/packages/RepoCallable.java deleted file mode 100644 index a78514be8f5fa5..00000000000000 --- a/src/main/java/com/google/devtools/build/lib/packages/RepoCallable.java +++ /dev/null @@ -1,59 +0,0 @@ -// Copyright 2023 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.packages; - -import java.util.Map; -import net.starlark.java.annot.Param; -import net.starlark.java.annot.StarlarkMethod; -import net.starlark.java.eval.EvalException; -import net.starlark.java.eval.Starlark; -import net.starlark.java.eval.StarlarkThread; - -/** Definition of the {@code repo()} function used in REPO.bazel files. */ -public final class RepoCallable { - private RepoCallable() {} - - public static final RepoCallable INSTANCE = new RepoCallable(); - - @StarlarkMethod( - name = "repo", - documented = false, // documented separately - extraKeywords = @Param(name = "kwargs"), - useStarlarkThread = true) - public Object repoCallable(Map kwargs, StarlarkThread thread) - throws EvalException { - RepoThreadContext context = RepoThreadContext.fromOrFail(thread, "repo()"); - if (context.isRepoFunctionCalled()) { - throw Starlark.errorf("'repo' can only be called once in the REPO.bazel file"); - } - context.setRepoFunctionCalled(); - - if (kwargs.isEmpty()) { - throw Starlark.errorf("at least one argument must be given to the 'repo' function"); - } - - PackageArgs.Builder pkgArgsBuilder = PackageArgs.builder(); - for (Map.Entry kwarg : kwargs.entrySet()) { - PackageArgs.processParam( - kwarg.getKey(), - kwarg.getValue(), - "repo() argument '" + kwarg.getKey() + "'", - context.getLabelConverter(), - pkgArgsBuilder); - } - context.setPackageArgs(pkgArgsBuilder.build()); - return Starlark.NONE; - } -} diff --git a/src/main/java/com/google/devtools/build/lib/packages/RepoFileGlobals.java b/src/main/java/com/google/devtools/build/lib/packages/RepoFileGlobals.java new file mode 100644 index 00000000000000..07631e1763e459 --- /dev/null +++ b/src/main/java/com/google/devtools/build/lib/packages/RepoFileGlobals.java @@ -0,0 +1,81 @@ +// Copyright 2023 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.packages; + +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; + +/** Definition of the {@code repo()} function used in REPO.bazel files. */ +public final class RepoFileGlobals { + private RepoFileGlobals() {} + + public static final RepoFileGlobals INSTANCE = new RepoFileGlobals(); + + @StarlarkMethod( + name = "ignore_directories", + doc = + "The list of directories to ignore in this repository.

This function takes a list" + + " of strings and a directory is ignored if any of the given strings matches its" + + " repository-relative path according to the semantics of the glob()" + + " function. This function can be used to ignore directories that are implementation" + + " details of source control systems, output files of other build systems, etc.", + useStarlarkThread = true, + parameters = { + @Param( + name = "dirs", + allowedTypes = { + @ParamType(type = Sequence.class, generic1 = String.class), + }) + }) + public void ignoreDirectories(Iterable dirsUnchecked, StarlarkThread thread) + throws EvalException { + Sequence dirs = Sequence.cast(dirsUnchecked, String.class, "dirs"); + RepoThreadContext context = RepoThreadContext.fromOrFail(thread, "repo()"); + + if (context.isIgnoredDirectoriesSet()) { + throw new EvalException("'ignored_directories()' can only be called once"); + } + + context.setIgnoredDirectories(dirs); + } + + @StarlarkMethod( + name = "repo", + documented = false, // documented separately + extraKeywords = @Param(name = "kwargs"), + useStarlarkThread = true) + public void repoCallable(Map kwargs, StarlarkThread thread) throws EvalException { + RepoThreadContext context = RepoThreadContext.fromOrFail(thread, "repo()"); + if (context.isRepoFunctionCalled()) { + throw Starlark.errorf("'repo' can only be called once in the REPO.bazel file"); + } + + if (context.isIgnoredDirectoriesSet()) { + throw Starlark.errorf("if repo() is called, it must be called before any other functions"); + } + + if (kwargs.isEmpty()) { + throw Starlark.errorf("at least one argument must be given to the 'repo' function"); + } + + context.setPackageArgsMap(kwargs); + } +} diff --git a/src/main/java/com/google/devtools/build/lib/packages/RepoThreadContext.java b/src/main/java/com/google/devtools/build/lib/packages/RepoThreadContext.java index 88c0f4fc8445e5..1cb47dca84fbac 100644 --- a/src/main/java/com/google/devtools/build/lib/packages/RepoThreadContext.java +++ b/src/main/java/com/google/devtools/build/lib/packages/RepoThreadContext.java @@ -14,18 +14,23 @@ package com.google.devtools.build.lib.packages; -import com.google.devtools.build.lib.cmdline.RepositoryMapping; +import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableMap; 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 packageArgsMap = ImmutableMap.of(); private boolean repoFunctionCalled = false; + private ImmutableList ignoredDirectories = ImmutableList.of(); + private boolean ignoredDirectoriesSet = false; + public static RepoThreadContext fromOrFail(StarlarkThread thread, String what) throws EvalException { StarlarkThreadContext context = thread.getThreadLocal(StarlarkThreadContext.class); @@ -35,28 +40,33 @@ public static RepoThreadContext fromOrFail(StarlarkThread thread, String what) throw Starlark.errorf("%s can only be called from REPO.bazel", what); } - public RepoThreadContext(LabelConverter labelConverter, RepositoryMapping mainRepoMapping) { - super(() -> mainRepoMapping); - this.labelConverter = labelConverter; - } - - public LabelConverter getLabelConverter() { - return labelConverter; + public RepoThreadContext() { + super(() -> null); } public boolean isRepoFunctionCalled() { return repoFunctionCalled; } - public void setRepoFunctionCalled() { + public void setPackageArgsMap(Map kwargs) { repoFunctionCalled = true; + this.packageArgsMap = ImmutableMap.copyOf(kwargs); + } + + public ImmutableMap getPackageArgsMap() { + return packageArgsMap; + } + + public void setIgnoredDirectories(Collection ignoredDirectories) throws EvalException { + ignoredDirectoriesSet = true; + this.ignoredDirectories = ImmutableList.copyOf(ignoredDirectories); } - public void setPackageArgs(PackageArgs packageArgs) { - this.packageArgs = packageArgs; + public boolean isIgnoredDirectoriesSet() { + return ignoredDirectoriesSet; } - public PackageArgs getPackageArgs() { - return packageArgs; + public ImmutableList getIgnoredDirectories() { + return ignoredDirectories; } } diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/BUILD b/src/main/java/com/google/devtools/build/lib/skyframe/BUILD index 3b984ccaf13323..89749fd1647253 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/BUILD +++ b/src/main/java/com/google/devtools/build/lib/skyframe/BUILD @@ -174,7 +174,7 @@ java_library( ":recursive_pkg_function", ":recursive_pkg_value", ":repo_file_function", - ":repo_file_value", + ":repo_package_args_function", ":repository_mapping_function", ":repository_mapping_value", ":rule_configured_target_value", @@ -830,6 +830,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", @@ -2309,23 +2311,40 @@ java_library( ], ) +java_library( + name = "repo_package_args_function", + srcs = ["RepoPackageArgsFunction.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:jsr305", + ], +) + java_library( name = "repo_file_function", srcs = ["RepoFileFunction.java"], deps = [ ":precomputed_value", ":repo_file_value", - ":repository_mapping_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/events", "//src/main/java/com/google/devtools/build/lib/packages", "//src/main/java/com/google/devtools/build/lib/rules:repository/repository_directory_value", "//src/main/java/com/google/devtools/build/lib/vfs", - "//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", "//src/main/java/net/starlark/java/syntax", + "//third_party:guava", "//third_party:jsr305", ], ) @@ -2336,9 +2355,9 @@ java_library( deps = [ ":sky_functions", "//src/main/java/com/google/devtools/build/lib/cmdline", - "//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", ], ) diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/BazelSkyframeExecutorConstants.java b/src/main/java/com/google/devtools/build/lib/skyframe/BazelSkyframeExecutorConstants.java index 86c80c3cf464a8..6e35c102b0afd6 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/BazelSkyframeExecutorConstants.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/BazelSkyframeExecutorConstants.java @@ -14,7 +14,6 @@ package com.google.devtools.build.lib.skyframe; import com.google.common.collect.ImmutableList; -import com.google.common.collect.ImmutableSet; import com.google.devtools.build.lib.packages.BuildFileName; import com.google.devtools.build.lib.repository.ExternalPackageHelper; import com.google.devtools.build.lib.skyframe.PackageFunction.ActionOnIOExceptionReadingBuildFile; @@ -26,9 +25,6 @@ public class BazelSkyframeExecutorConstants { private BazelSkyframeExecutorConstants() {} - public static final ImmutableSet HARDCODED_IGNORED_PACKAGE_PREFIXES = - ImmutableSet.of(); - /** * The file .bazelignore can be used to specify directories to be ignored by bazel * diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/IgnoredPackagePrefixesFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/IgnoredPackagePrefixesFunction.java index c935316d34a8c8..2ec7acffb258e2 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/IgnoredPackagePrefixesFunction.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/IgnoredPackagePrefixesFunction.java @@ -15,6 +15,7 @@ import static com.google.devtools.build.lib.rules.repository.RepositoryDelegatorFunction.VENDOR_DIRECTORY; +import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableSet; import com.google.common.io.CharStreams; import com.google.common.io.LineProcessor; @@ -23,6 +24,7 @@ import com.google.devtools.build.lib.io.InconsistentFilesystemException; import com.google.devtools.build.lib.pkgcache.PathPackageLocator; import com.google.devtools.build.lib.rules.repository.RepositoryDirectoryValue; +import com.google.devtools.build.lib.skyframe.RepoFileFunction.BadRepoFileException; import com.google.devtools.build.lib.vfs.PathFragment; import com.google.devtools.build.lib.vfs.Root; import com.google.devtools.build.lib.vfs.RootedPath; @@ -40,9 +42,19 @@ /** * A {@link SkyFunction} for {@link IgnoredPackagePrefixesValue}. * - *

It is used to implement the `.bazelignore` feature. + *

It is used to compute which directories should be ignored in a package. These either come from + * the {@code .bazelignore} file or from the {@code ignored_directories()} function in {@code + * REPO.bazel}. */ public class IgnoredPackagePrefixesFunction implements SkyFunction { + /** + * A version of {@link IgnoredPackagePrefixesFunction} that always returns the empty value. + * + *

Used for tests where the extra complications incurred by evaluating the function are + * undesired. + */ + public static final SkyFunction NOOP = (skyKey, env) -> IgnoredPackagePrefixesValue.EMPTY; + private final PathFragment ignoredPackagePrefixesFile; public IgnoredPackagePrefixesFunction(PathFragment ignoredPackagePrefixesFile) { @@ -71,64 +83,102 @@ public static void getIgnoredPackagePrefixes( } @Nullable - @Override - public SkyValue compute(SkyKey key, Environment env) + private ImmutableList computeIgnoredPatterns( + Environment env, RepositoryName repositoryName) throws IgnoredPatternsFunctionException, InterruptedException { - RepositoryName repositoryName = (RepositoryName) key.argument(); - ImmutableSet.Builder ignoredPackagePrefixesBuilder = ImmutableSet.builder(); - if (!ignoredPackagePrefixesFile.equals(PathFragment.EMPTY_FRAGMENT)) { - PathPackageLocator pkgLocator = PrecomputedValue.PATH_PACKAGE_LOCATOR.get(env); + try { + RepoFileValue repoFileValue = + (RepoFileValue) + env.getValueOrThrow( + RepoFileValue.key(repositoryName), IOException.class, BadRepoFileException.class); + if (env.valuesMissing()) { return null; } - if (repositoryName.isMain()) { - PathFragment vendorDir = null; - if (VENDOR_DIRECTORY.get(env).isPresent()) { - vendorDir = VENDOR_DIRECTORY.get(env).get().asFragment(); + return repoFileValue.ignoredDirectories(); + } catch (IOException e) { + throw new IgnoredPatternsFunctionException(e); + } catch (BadRepoFileException e) { + throw new IgnoredPatternsFunctionException(e); + } + } + + @Nullable + private ImmutableSet computeIgnoredPrefixes( + Environment env, RepositoryName repositoryName) + throws IgnoredPatternsFunctionException, InterruptedException { + ImmutableSet.Builder ignoredPackagePrefixesBuilder = ImmutableSet.builder(); + PathPackageLocator pkgLocator = PrecomputedValue.PATH_PACKAGE_LOCATOR.get(env); + if (env.valuesMissing()) { + return null; + } + + if (repositoryName.isMain()) { + PathFragment vendorDir = null; + if (VENDOR_DIRECTORY.get(env).isPresent()) { + vendorDir = VENDOR_DIRECTORY.get(env).get().asFragment(); + } + + for (Root packagePathEntry : pkgLocator.getPathEntries()) { + PathFragment workspaceRoot = packagePathEntry.asPath().asFragment(); + if (vendorDir != null && vendorDir.startsWith(workspaceRoot)) { + ignoredPackagePrefixesBuilder.add(vendorDir.relativeTo(workspaceRoot)); } - for (Root packagePathEntry : pkgLocator.getPathEntries()) { - PathFragment workspaceRoot = packagePathEntry.asPath().asFragment(); - if (vendorDir != null && vendorDir.startsWith(workspaceRoot)) { - ignoredPackagePrefixesBuilder.add(vendorDir.relativeTo(workspaceRoot)); - } - - RootedPath rootedPatternFile = - RootedPath.toRootedPath(packagePathEntry, ignoredPackagePrefixesFile); - FileValue patternFileValue = (FileValue) env.getValue(FileValue.key(rootedPatternFile)); - if (patternFileValue == null) { - return null; - } - if (patternFileValue.isFile()) { - getIgnoredPackagePrefixes(rootedPatternFile, ignoredPackagePrefixesBuilder); - break; - } + RootedPath rootedPatternFile = + RootedPath.toRootedPath(packagePathEntry, ignoredPackagePrefixesFile); + FileValue patternFileValue = (FileValue) env.getValue(FileValue.key(rootedPatternFile)); + if (patternFileValue == null) { + return null; } - } else { - // Make sure the repository is fetched. - RepositoryDirectoryValue repositoryValue = - (RepositoryDirectoryValue) env.getValue(RepositoryDirectoryValue.key(repositoryName)); - if (repositoryValue == null) { + if (patternFileValue.isFile()) { + getIgnoredPackagePrefixes(rootedPatternFile, ignoredPackagePrefixesBuilder); + break; + } + } + } else { + // Make sure the repository is fetched. + RepositoryDirectoryValue repositoryValue = + (RepositoryDirectoryValue) env.getValue(RepositoryDirectoryValue.key(repositoryName)); + if (repositoryValue == null) { + return null; + } + if (repositoryValue.repositoryExists()) { + RootedPath rootedPatternFile = + RootedPath.toRootedPath( + Root.fromPath(repositoryValue.getPath()), ignoredPackagePrefixesFile); + FileValue patternFileValue = (FileValue) env.getValue(FileValue.key(rootedPatternFile)); + if (patternFileValue == null) { return null; } - if (repositoryValue.repositoryExists()) { - RootedPath rootedPatternFile = - RootedPath.toRootedPath( - Root.fromPath(repositoryValue.getPath()), ignoredPackagePrefixesFile); - FileValue patternFileValue = (FileValue) env.getValue(FileValue.key(rootedPatternFile)); - if (patternFileValue == null) { - return null; - } - if (patternFileValue.isFile()) { - getIgnoredPackagePrefixes(rootedPatternFile, ignoredPackagePrefixesBuilder); - } + if (patternFileValue.isFile()) { + getIgnoredPackagePrefixes(rootedPatternFile, ignoredPackagePrefixesBuilder); } } } - return IgnoredPackagePrefixesValue.of(ignoredPackagePrefixesBuilder.build()); + return ignoredPackagePrefixesBuilder.build(); + } + + @Nullable + @Override + public SkyValue compute(SkyKey key, Environment env) + throws IgnoredPatternsFunctionException, InterruptedException { + RepositoryName repositoryName = (RepositoryName) key.argument(); + + ImmutableList ignoredPatterns = computeIgnoredPatterns(env, repositoryName); + if (env.valuesMissing()) { + return null; + } + + ImmutableSet ignoredPrefixes = computeIgnoredPrefixes(env, repositoryName); + if (env.valuesMissing()) { + return null; + } + + return IgnoredPackagePrefixesValue.of(ignoredPrefixes, ignoredPatterns); } private static final class PathFragmentLineProcessor @@ -172,5 +222,13 @@ public IgnoredPatternsFunctionException(InconsistentFilesystemException e) { public IgnoredPatternsFunctionException(InvalidIgnorePathException e) { super(e, Transience.PERSISTENT); } + + public IgnoredPatternsFunctionException(IOException e) { + super(e, Transience.TRANSIENT); + } + + public IgnoredPatternsFunctionException(BadRepoFileException e) { + super(e, Transience.PERSISTENT); + } } } diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/IgnoredPackagePrefixesValue.java b/src/main/java/com/google/devtools/build/lib/skyframe/IgnoredPackagePrefixesValue.java index 237bfd0a3148a8..d286920754a718 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/IgnoredPackagePrefixesValue.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/IgnoredPackagePrefixesValue.java @@ -13,6 +13,7 @@ // limitations under the License. package com.google.devtools.build.lib.skyframe; +import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableSet; import com.google.devtools.build.lib.cmdline.IgnoredSubdirectories; import com.google.devtools.build.lib.cmdline.RepositoryName; @@ -37,10 +38,11 @@ private IgnoredPackagePrefixesValue(IgnoredSubdirectories ignoredSubdirectories) this.ignoredSubdirectories = ignoredSubdirectories; } - public static IgnoredPackagePrefixesValue of(ImmutableSet patterns) { - return patterns.isEmpty() + public static IgnoredPackagePrefixesValue of( + ImmutableSet prefixes, ImmutableList patterns) { + return prefixes.isEmpty() && patterns.isEmpty() ? EMPTY - : new IgnoredPackagePrefixesValue(IgnoredSubdirectories.of(patterns)); + : new IgnoredPackagePrefixesValue(IgnoredSubdirectories.of(prefixes, patterns)); } public static IgnoredPackagePrefixesValue of(IgnoredSubdirectories ignoredSubdirectories) { diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/PackageFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/PackageFunction.java index 7d070f41fbb809..08050f14004f2c 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/PackageFunction.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/PackageFunction.java @@ -58,6 +58,7 @@ import com.google.devtools.build.lib.server.FailureDetails.PackageLoading.Code; import com.google.devtools.build.lib.skyframe.PackageFunctionWithMultipleGlobDeps.SkyframeGlobbingIOException; import com.google.devtools.build.lib.skyframe.RepoFileFunction.BadRepoFileException; +import com.google.devtools.build.lib.skyframe.RepoPackageArgsFunction.RepoPackageArgsValue; import com.google.devtools.build.lib.skyframe.StarlarkBuiltinsFunction.BuiltinsFailedException; import com.google.devtools.build.lib.util.DetailedExitCode; import com.google.devtools.build.lib.util.Pair; @@ -409,8 +410,11 @@ public SkyValue compute(SkyKey key, Environment env) (PackageLookupValue) env.getValueOrThrow( packageLookupKey, + BadRepoFileException.class, BuildFileNotFoundException.class, InconsistentFilesystemException.class); + } catch (BadRepoFileException e) { + throw badRepoFileException(e, packageId); } catch (BuildFileNotFoundException e) { throw new PackageFunctionException(e, Transience.PERSISTENT); } catch (InconsistentFilesystemException e) { @@ -909,6 +913,18 @@ private static boolean maybeAddEventAboutLabelCrossingSubpackage( } } + private static PackageFunctionException badRepoFileException( + Exception cause, PackageIdentifier packageId) { + return PackageFunctionException.builder() + .setType(PackageFunctionException.Type.BUILD_FILE_CONTAINS_ERRORS) + .setPackageIdentifier(packageId) + .setTransience(Transience.PERSISTENT) + .setException(cause) + .setMessage("bad REPO.bazel file") + .setPackageLoadingCode(PackageLoading.Code.BAD_REPO_FILE) + .build(); + } + @ForOverride protected abstract Globber makeGlobber( NonSkyframeGlobber nonSkyframeGlobber, @@ -947,31 +963,26 @@ private LoadedPackage loadPackage( IgnoredPackagePrefixesValue repositoryIgnoredPackagePrefixes = (IgnoredPackagePrefixesValue) env.getValue(IgnoredPackagePrefixesValue.key(packageId.getRepository())); - RepoFileValue repoFileValue; + RepoPackageArgsValue repoPackageArgsValue; if (shouldUseRepoDotBazel) { try { - repoFileValue = - (RepoFileValue) + repoPackageArgsValue = + (RepoPackageArgsValue) env.getValueOrThrow( - RepoFileValue.key(packageId.getRepository()), + RepoPackageArgsFunction.key(packageId.getRepository()), IOException.class, BadRepoFileException.class); } catch (IOException | BadRepoFileException e) { - throw PackageFunctionException.builder() - .setType(PackageFunctionException.Type.BUILD_FILE_CONTAINS_ERRORS) - .setPackageIdentifier(packageId) - .setTransience(Transience.PERSISTENT) - .setException(e) - .setMessage("bad REPO.bazel file") - .setPackageLoadingCode(PackageLoading.Code.BAD_REPO_FILE) - .build(); + throw badRepoFileException(e, packageId); } } else { - repoFileValue = RepoFileValue.EMPTY; + repoPackageArgsValue = RepoPackageArgsValue.EMPTY; } + if (env.valuesMissing()) { return null; } + String workspaceName = workspaceNameValue.getName(); RepositoryMapping repositoryMapping = repositoryMappingValue.getRepositoryMapping(); RepositoryMapping mainRepositoryMapping = mainRepositoryMappingValue.getRepositoryMapping(); @@ -1130,7 +1141,7 @@ private LoadedPackage loadPackage( pkgBuilder.mergePackageArgsFrom( PackageArgs.builder().setDefaultVisibility(defaultVisibility)); - pkgBuilder.mergePackageArgsFrom(repoFileValue.packageArgs()); + pkgBuilder.mergePackageArgsFrom(repoPackageArgsValue.getPackageArgs()); if (compiled.ok()) { packageFactory.executeBuildFile( @@ -1474,6 +1485,10 @@ public PackageFunctionException(NoSuchPackageException e, Transience transience) super(e, transience); } + public PackageFunctionException(BadRepoFileException e, Transience transience) { + super(e, transience); + } + static Builder builder() { return new Builder(); } diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/RepoFileFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/RepoFileFunction.java index 8cfa75e5ddbc77..9e0283faa990f0 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/RepoFileFunction.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/RepoFileFunction.java @@ -14,22 +14,20 @@ package com.google.devtools.build.lib.skyframe; +import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableMap; import com.google.devtools.build.lib.actions.FileValue; import com.google.devtools.build.lib.cmdline.LabelConstants; -import com.google.devtools.build.lib.cmdline.PackageIdentifier; import com.google.devtools.build.lib.cmdline.RepositoryMapping; import com.google.devtools.build.lib.cmdline.RepositoryName; import com.google.devtools.build.lib.events.Event; import com.google.devtools.build.lib.events.ExtendedEventHandler; import com.google.devtools.build.lib.packages.BazelStarlarkEnvironment; import com.google.devtools.build.lib.packages.DotBazelFileSyntaxChecker; -import com.google.devtools.build.lib.packages.LabelConverter; -import com.google.devtools.build.lib.packages.PackageArgs; import com.google.devtools.build.lib.packages.RepoThreadContext; import com.google.devtools.build.lib.rules.repository.RepositoryDirectoryValue; import com.google.devtools.build.lib.vfs.FileSystemUtils; import com.google.devtools.build.lib.vfs.Path; -import com.google.devtools.build.lib.vfs.PathFragment; import com.google.devtools.build.lib.vfs.Root; import com.google.devtools.build.lib.vfs.RootedPath; import com.google.devtools.build.skyframe.SkyFunction; @@ -87,29 +85,17 @@ public SkyValue compute(SkyKey skyKey, Environment env) } if (!repoFileValue.exists()) { // It's okay to not have a REPO.bazel file. - return RepoFileValue.of(PackageArgs.EMPTY); + return RepoFileValue.of(ImmutableMap.of(), ImmutableList.of()); } // Now we can actually evaluate the file. StarlarkSemantics starlarkSemantics = PrecomputedValue.STARLARK_SEMANTICS.get(env); - RepositoryMappingValue repoMapping = - (RepositoryMappingValue) env.getValue(RepositoryMappingValue.key(repoName)); - RepositoryMappingValue mainRepoMapping = - (RepositoryMappingValue) env.getValue(RepositoryMappingValue.key(RepositoryName.MAIN)); if (env.valuesMissing()) { return null; } - StarlarkFile repoFile = readAndParseRepoFile(repoFilePath.asPath(), env); - PackageArgs packageArgs = - evalRepoFile( - repoFile, - repoName, - repoMapping.getRepositoryMapping(), - mainRepoMapping.getRepositoryMapping(), - starlarkSemantics, - env.getListener()); - return RepoFileValue.of(packageArgs); + StarlarkFile repoFile = readAndParseRepoFile(repoFilePath.asPath(), env); + return evalRepoFile(repoFile, repoName, starlarkSemantics, env.getListener()); } private static StarlarkFile readAndParseRepoFile(Path path, Environment env) @@ -131,7 +117,7 @@ private static StarlarkFile readAndParseRepoFile(Path path, Environment env) return starlarkFile; } - private static String getDisplayNameForRepo( + public static String getDisplayNameForRepo( RepositoryName repoName, RepositoryMapping mainRepoMapping) { String displayName = repoName.getDisplayForm(mainRepoMapping); if (displayName.isEmpty()) { @@ -140,15 +126,13 @@ private static String getDisplayNameForRepo( return displayName; } - private PackageArgs evalRepoFile( + private RepoFileValue evalRepoFile( StarlarkFile starlarkFile, RepositoryName repoName, - RepositoryMapping repoMapping, - RepositoryMapping mainRepoMapping, StarlarkSemantics starlarkSemantics, ExtendedEventHandler handler) throws RepoFileFunctionException, InterruptedException { - String repoDisplayName = getDisplayNameForRepo(repoName, mainRepoMapping); + String repoDisplayName = getDisplayNameForRepo(repoName, null); try (Mutability mu = Mutability.create("repo file", repoName)) { new DotBazelFileSyntaxChecker("REPO.bazel files", /* canLoadBzl= */ false) .check(starlarkFile); @@ -161,14 +145,10 @@ private PackageArgs evalRepoFile( /* contextDescription= */ "", SymbolGenerator.create(repoName)); thread.setPrintHandler(Event.makeDebugPrintHandler(handler)); - RepoThreadContext context = - new RepoThreadContext( - new LabelConverter( - PackageIdentifier.create(repoName, PathFragment.EMPTY_FRAGMENT), repoMapping), - mainRepoMapping); + RepoThreadContext context = new RepoThreadContext(); context.storeInThread(thread); Starlark.execFileProgram(program, predeclared, thread); - return context.getPackageArgs(); + return RepoFileValue.of(context.getPackageArgsMap(), context.getIgnoredDirectories()); } catch (SyntaxError.Exception e) { Event.replayEventsOn(handler, e.errors()); throw new RepoFileFunctionException( diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/RepoFileValue.java b/src/main/java/com/google/devtools/build/lib/skyframe/RepoFileValue.java index 34763fee4f2c50..e2469f964fa152 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/RepoFileValue.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/RepoFileValue.java @@ -15,8 +15,9 @@ package com.google.devtools.build.lib.skyframe; import com.google.auto.value.AutoValue; +import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableMap; import com.google.devtools.build.lib.cmdline.RepositoryName; -import com.google.devtools.build.lib.packages.PackageArgs; import com.google.devtools.build.skyframe.AbstractSkyKey; import com.google.devtools.build.skyframe.SkyFunctionName; import com.google.devtools.build.skyframe.SkyValue; @@ -24,12 +25,15 @@ /** Contains information about the REPO.bazel file at the root of a repo. */ @AutoValue public abstract class RepoFileValue implements SkyValue { - public static final RepoFileValue EMPTY = of(PackageArgs.EMPTY); + public static final RepoFileValue EMPTY = of(ImmutableMap.of(), ImmutableList.of()); - public abstract PackageArgs packageArgs(); + public abstract ImmutableMap packageArgsMap(); - public static RepoFileValue of(PackageArgs packageArgs) { - return new AutoValue_RepoFileValue(packageArgs); + public abstract ImmutableList ignoredDirectories(); + + public static RepoFileValue of( + ImmutableMap packageArgsMap, ImmutableList ignoredDirectories) { + return new AutoValue_RepoFileValue(packageArgsMap, ignoredDirectories); } public static Key key(RepositoryName repoName) { diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/RepoPackageArgsFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/RepoPackageArgsFunction.java new file mode 100644 index 00000000000000..c517ee87c54153 --- /dev/null +++ b/src/main/java/com/google/devtools/build/lib/skyframe/RepoPackageArgsFunction.java @@ -0,0 +1,142 @@ +// Copyright 2024 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.devtools.build.lib.cmdline.PackageIdentifier; +import com.google.devtools.build.lib.cmdline.RepositoryName; +import com.google.devtools.build.lib.events.Event; +import com.google.devtools.build.lib.packages.LabelConverter; +import com.google.devtools.build.lib.packages.PackageArgs; +import com.google.devtools.build.lib.vfs.PathFragment; +import com.google.devtools.build.skyframe.AbstractSkyKey; +import com.google.devtools.build.skyframe.SkyFunction; +import com.google.devtools.build.skyframe.SkyFunctionException; +import com.google.devtools.build.skyframe.SkyFunctionName; +import com.google.devtools.build.skyframe.SkyKey; +import com.google.devtools.build.skyframe.SkyValue; +import java.util.Map; +import javax.annotation.Nullable; +import net.starlark.java.eval.EvalException; + +/** A {@link SkyFunction} that returns the {@link PackageArgs} for a given repository. */ +public class RepoPackageArgsFunction implements SkyFunction { + public static final RepoPackageArgsFunction INSTANCE = new RepoPackageArgsFunction(); + + /** {@link SkyValue} wrapping a PackageArgs. */ + public static final class RepoPackageArgsValue implements SkyValue { + public static final RepoPackageArgsValue EMPTY = new RepoPackageArgsValue(PackageArgs.EMPTY); + + private final PackageArgs packageArgs; + + public RepoPackageArgsValue(PackageArgs packageArgs) { + this.packageArgs = packageArgs; + } + + public PackageArgs getPackageArgs() { + return packageArgs; + } + + @Override + public int hashCode() { + return packageArgs.hashCode(); + } + + @Override + public boolean equals(Object other) { + if (other instanceof RepoPackageArgsValue that) { + return that.packageArgs.equals(packageArgs); + } else { + return false; + } + } + } + + public static Key key(RepositoryName repoName) { + return new Key(repoName); + } + + /** Thrown when there is something wrong with the arguments of the {@code repo()} function. */ + private static class BadPackageArgsException extends Exception { + public BadPackageArgsException(String message, Exception cause) { + super(message, cause); + } + } + + /** A {@link SkyFunctionException} for {@link RepoPackageArgsFunction}. */ + private static class RepoPackageArgsFunctionException extends SkyFunctionException { + private RepoPackageArgsFunctionException(BadPackageArgsException e) { + super(e, Transience.PERSISTENT); + } + } + + /** Key type for {@link RepoPackageArgsValue}. */ + public static class Key extends AbstractSkyKey { + + private Key(RepositoryName repoName) { + super(repoName); + } + + @Override + public SkyFunctionName functionName() { + return SkyFunctions.REPO_PACKAGE_ARGS; + } + } + + private RepoPackageArgsFunction() {} + + @Nullable + @Override + public SkyValue compute(SkyKey skyKey, Environment env) + throws SkyFunctionException, InterruptedException { + RepositoryName repositoryName = (RepositoryName) skyKey.argument(); + + RepoFileValue repoFileValue = (RepoFileValue) env.getValue(RepoFileValue.key(repositoryName)); + RepositoryMappingValue repositoryMappingValue = + (RepositoryMappingValue) env.getValue(RepositoryMappingValue.key(repositoryName)); + RepositoryMappingValue mainRepoMapping = + (RepositoryMappingValue) env.getValue(RepositoryMappingValue.key(RepositoryName.MAIN)); + + if (env.valuesMissing()) { + return null; + } + + String repoDisplayName = + RepoFileFunction.getDisplayNameForRepo( + repositoryName, mainRepoMapping.getRepositoryMapping()); + + PackageArgs.Builder pkgArgsBuilder = PackageArgs.builder(); + LabelConverter labelConverter = + new LabelConverter( + PackageIdentifier.create(repositoryName, PathFragment.EMPTY_FRAGMENT), + repositoryMappingValue.getRepositoryMapping()); + try { + for (Map.Entry kwarg : repoFileValue.packageArgsMap().entrySet()) { + PackageArgs.processParam( + kwarg.getKey(), + kwarg.getValue(), + "repo() argument '" + kwarg.getKey() + "'", + labelConverter, + pkgArgsBuilder); + } + } catch (EvalException e) { + env.getListener().handle(Event.error(e.getMessageWithStack())); + throw new RepoPackageArgsFunctionException( + new BadPackageArgsException( + "error evaluating REPO.bazel file for " + repoDisplayName, e)); + } + + return new RepoPackageArgsValue(pkgArgsBuilder.build()); + } +} diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkyFunctions.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkyFunctions.java index 43ff6b02eaa5b5..2558d70c6458af 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/SkyFunctions.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkyFunctions.java @@ -144,6 +144,8 @@ public final class SkyFunctions { SkyFunctionName.createHermetic("RESOLVED_FILE"); public static final SkyFunctionName MODULE_FILE = SkyFunctionName.createNonHermetic("MODULE_FILE"); + public static final SkyFunctionName REPO_PACKAGE_ARGS = + SkyFunctionName.createHermetic("REPO_PACKAGE_ARGS"); public static final SkyFunctionName REPO_FILE = SkyFunctionName.createHermetic("REPO_FILE"); public static final SkyFunctionName BUILD_DRIVER = SkyFunctionName.createNonHermetic("BUILD_DRIVER"); diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java index 050c3bad3d4713..a76b7445bedac8 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java @@ -760,6 +760,7 @@ private ImmutableMap skyFunctions() { : (k, env) -> { throw new IllegalStateException("supposed to be unused"); }); + map.put(SkyFunctions.REPO_PACKAGE_ARGS, RepoPackageArgsFunction.INSTANCE); map.put(SkyFunctions.EXTERNAL_PACKAGE, new ExternalPackageFunction(externalPackageHelper)); // Inject an empty default BAZEL_DEP_GRAPH SkyFunction for unit tests. map.put( diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/packages/AbstractPackageLoader.java b/src/main/java/com/google/devtools/build/lib/skyframe/packages/AbstractPackageLoader.java index 9d6bc9bb0606d6..e9ee7bae3b0e2b 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/packages/AbstractPackageLoader.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/packages/AbstractPackageLoader.java @@ -79,6 +79,7 @@ import com.google.devtools.build.lib.skyframe.PrecomputedFunction; import com.google.devtools.build.lib.skyframe.PrecomputedValue; import com.google.devtools.build.lib.skyframe.RepoFileFunction; +import com.google.devtools.build.lib.skyframe.RepoPackageArgsFunction; import com.google.devtools.build.lib.skyframe.RepositoryMappingFunction; import com.google.devtools.build.lib.skyframe.RepositoryMappingValue; import com.google.devtools.build.lib.skyframe.SkyFunctions; @@ -89,7 +90,6 @@ import com.google.devtools.build.lib.util.io.TimestampGranularityMonitor; import com.google.devtools.build.lib.vfs.FileStateKey; import com.google.devtools.build.lib.vfs.Path; -import com.google.devtools.build.lib.vfs.PathFragment; import com.google.devtools.build.lib.vfs.Root; import com.google.devtools.build.skyframe.Differencer; import com.google.devtools.build.skyframe.Differencer.DiffWithDelta.Delta; @@ -560,10 +560,7 @@ public String getBaseNameForLoadedPackage(PackageIdentifier packageName) { getCrossRepositoryLabelViolationStrategy(), getBuildFilesByPriority(), getExternalPackageHelper())) - .put( - SkyFunctions.IGNORED_PACKAGE_PREFIXES, - new IgnoredPackagePrefixesFunction( - /* ignoredPackagePrefixesFile= */ PathFragment.EMPTY_FRAGMENT)) + .put(SkyFunctions.IGNORED_PACKAGE_PREFIXES, IgnoredPackagePrefixesFunction.NOOP) .put(SkyFunctions.CONTAINING_PACKAGE_LOOKUP, new ContainingPackageLookupFunction()) .put( SkyFunctions.BZL_COMPILE, @@ -584,6 +581,7 @@ public String getBaseNameForLoadedPackage(PackageIdentifier packageName) { SkyFunctions.REPO_FILE, new RepoFileFunction( ruleClassProvider.getBazelStarlarkEnvironment(), directories.getWorkspace())) + .put(SkyFunctions.REPO_PACKAGE_ARGS, RepoPackageArgsFunction.INSTANCE) .put(SkyFunctions.EXTERNAL_PACKAGE, new ExternalPackageFunction(getExternalPackageHelper())) .put( BzlmodRepoRuleValue.BZLMOD_REPO_RULE, diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/packages/BUILD b/src/main/java/com/google/devtools/build/lib/skyframe/packages/BUILD index bc67b392f0b7e8..31d7444cf0ac47 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/packages/BUILD +++ b/src/main/java/com/google/devtools/build/lib/skyframe/packages/BUILD @@ -63,6 +63,7 @@ java_library( "//src/main/java/com/google/devtools/build/lib/skyframe:precomputed_function", "//src/main/java/com/google/devtools/build/lib/skyframe:precomputed_value", "//src/main/java/com/google/devtools/build/lib/skyframe:repo_file_function", + "//src/main/java/com/google/devtools/build/lib/skyframe:repo_package_args_function", "//src/main/java/com/google/devtools/build/lib/skyframe:repository_mapping_function", "//src/main/java/com/google/devtools/build/lib/skyframe:repository_mapping_value", "//src/main/java/com/google/devtools/build/lib/skyframe:sky_functions", @@ -71,7 +72,6 @@ java_library( "//src/main/java/com/google/devtools/build/lib/util:value_or_exception", "//src/main/java/com/google/devtools/build/lib/util/io", "//src/main/java/com/google/devtools/build/lib/vfs", - "//src/main/java/com/google/devtools/build/lib/vfs:pathfragment", "//src/main/java/com/google/devtools/build/skyframe", "//src/main/java/com/google/devtools/build/skyframe:skyframe-objects", "//src/main/java/net/starlark/java/eval", diff --git a/src/main/java/com/google/devtools/build/lib/vfs/UnixGlob.java b/src/main/java/com/google/devtools/build/lib/vfs/UnixGlob.java index d822a0f007ba57..13a40fcda21d2d 100644 --- a/src/main/java/com/google/devtools/build/lib/vfs/UnixGlob.java +++ b/src/main/java/com/google/devtools/build/lib/vfs/UnixGlob.java @@ -167,6 +167,11 @@ public static String checkPatternForError(String pattern) { return null; } + /** Returns whether {@code pattern} matches {@code path}. */ + public static boolean matches(String[] pattern, String[] path) { + return matchesPattern(pattern, path, 0, 0, null, MatchMode.EXACT); + } + /** Calls {@link #matches(String, String, Map) matches(pattern, str, null)} */ public static boolean matches(String pattern, String str) { return matches(pattern, str, null); @@ -901,7 +906,7 @@ public static void removeExcludes(Set paths, Collection excludes path -> { String[] segments = Iterables.toArray(Splitter.on('/').split(path), String.class); for (String[] splitPattern : splitPatterns) { - if (matchesPattern(splitPattern, segments, 0, 0, patternCache)) { + if (matchesPattern(splitPattern, segments, 0, 0, patternCache, MatchMode.EXACT)) { return true; } } @@ -909,21 +914,43 @@ public static void removeExcludes(Set paths, Collection excludes }); } + /** Returns whether any path under {@code path} can match {@code pattern}. */ + public static boolean canMatchChild(String[] pattern, String[] path) { + return matchesPattern(pattern, path, 0, 0, null, MatchMode.CAN_MATCH_CHILD); + } + + /** Returns whether {@code pattern} matches a prefix of {@code path}. */ + public static boolean matchesPrefix(String[] pattern, String[] path) { + return matchesPattern(pattern, path, 0, 0, null, MatchMode.PREFIX); + } + + /** How {@code #matchesPattern()} should work */ + private enum MatchMode { + EXACT, // The path should exactly match the pattern + PREFIX, // The pattern should match a prefix of the path + CAN_MATCH_CHILD, // Whether there can be any path under the prefix that matches the pattern + } + /** Returns true if {@code pattern} matches {@code path} starting from the given segments. */ private static boolean matchesPattern( - String[] pattern, String[] path, int i, int j, Map patternCache) { + String[] pattern, + String[] path, + int i, + int j, + Map patternCache, + MatchMode matchMode) { if (i == pattern.length) { - return j == path.length; + return matchMode == MatchMode.PREFIX || j == path.length; } if (pattern[i].equals("**")) { - return matchesPattern(pattern, path, i + 1, j, patternCache) - || (j < path.length && matchesPattern(pattern, path, i, j + 1, patternCache)); + return matchesPattern(pattern, path, i + 1, j, patternCache, matchMode) + || (j < path.length && matchesPattern(pattern, path, i, j + 1, patternCache, matchMode)); } if (j == path.length) { - return false; + return matchMode == MatchMode.CAN_MATCH_CHILD; } if (matches(pattern[i], path[j], patternCache)) { - return matchesPattern(pattern, path, i + 1, j + 1, patternCache); + return matchesPattern(pattern, path, i + 1, j + 1, patternCache, matchMode); } return false; } diff --git a/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleExtensionResolutionTest.java b/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleExtensionResolutionTest.java index 52ffff36b4586d..8b6292bee8c2e9 100644 --- a/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleExtensionResolutionTest.java +++ b/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleExtensionResolutionTest.java @@ -209,10 +209,7 @@ public void setup() throws Exception { SkyFunctions.LOCAL_REPOSITORY_LOOKUP, new LocalRepositoryLookupFunction( BazelSkyframeExecutorConstants.EXTERNAL_PACKAGE_HELPER)) - .put( - SkyFunctions.IGNORED_PACKAGE_PREFIXES, - new IgnoredPackagePrefixesFunction( - /* ignoredPackagePrefixesFile= */ PathFragment.EMPTY_FRAGMENT)) + .put(SkyFunctions.IGNORED_PACKAGE_PREFIXES, IgnoredPackagePrefixesFunction.NOOP) .put( SkyFunctions.REPOSITORY_MAPPING, new RepositoryMappingFunction(ruleClassProvider)) diff --git a/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleFileFunctionTest.java b/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleFileFunctionTest.java index 02b6118f9ea9fe..67ef65daba99d2 100644 --- a/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleFileFunctionTest.java +++ b/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleFileFunctionTest.java @@ -66,7 +66,6 @@ import com.google.devtools.build.lib.vfs.FileStateKey; import com.google.devtools.build.lib.vfs.FileSystemUtils; import com.google.devtools.build.lib.vfs.Path; -import com.google.devtools.build.lib.vfs.PathFragment; import com.google.devtools.build.lib.vfs.Root; import com.google.devtools.build.lib.vfs.RootedPath; import com.google.devtools.build.lib.vfs.SyscallCache; @@ -163,10 +162,7 @@ private void setUpWithBuiltinModules(ImmutableMap b CrossRepositoryLabelViolationStrategy.ERROR, BazelSkyframeExecutorConstants.BUILD_FILES_BY_PRIORITY, BazelSkyframeExecutorConstants.EXTERNAL_PACKAGE_HELPER)) - .put( - SkyFunctions.IGNORED_PACKAGE_PREFIXES, - new IgnoredPackagePrefixesFunction( - /* ignoredPackagePrefixesFile= */ PathFragment.EMPTY_FRAGMENT)) + .put(SkyFunctions.IGNORED_PACKAGE_PREFIXES, IgnoredPackagePrefixesFunction.NOOP) .put( SkyFunctions.LOCAL_REPOSITORY_LOOKUP, new LocalRepositoryLookupFunction( diff --git a/src/test/java/com/google/devtools/build/lib/cmdline/IgnoredSubdirectoriesTest.java b/src/test/java/com/google/devtools/build/lib/cmdline/IgnoredSubdirectoriesTest.java new file mode 100644 index 00000000000000..5909f73a525f04 --- /dev/null +++ b/src/test/java/com/google/devtools/build/lib/cmdline/IgnoredSubdirectoriesTest.java @@ -0,0 +1,99 @@ +// Copyright 2024 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.cmdline; + +import static com.google.common.collect.ImmutableSet.toImmutableSet; +import static com.google.common.truth.Truth.assertThat; + +import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableSet; +import com.google.devtools.build.lib.vfs.PathFragment; +import java.util.Arrays; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.JUnit4; + +/** Tests for {@link IgnoredSubdirectories}. */ +@RunWith(JUnit4.class) +public class IgnoredSubdirectoriesTest { + private static ImmutableSet prefixes(String... prefixes) { + return Arrays.stream(prefixes).map(PathFragment::create).collect(toImmutableSet()); + } + + private static ImmutableList patterns(String... patterns) { + return ImmutableList.copyOf(patterns); + } + + @Test + public void testSimpleUnion() throws Exception { + IgnoredSubdirectories one = IgnoredSubdirectories.of(prefixes("prefix1"), patterns("pattern1")); + IgnoredSubdirectories two = IgnoredSubdirectories.of(prefixes("prefix2"), patterns("pattern2")); + IgnoredSubdirectories union = one.union(two); + assertThat(union) + .isEqualTo( + IgnoredSubdirectories.of( + prefixes("prefix1", "prefix2"), patterns("pattern1", "pattern2"))); + } + + @Test + public void testSelfUnionNoop() throws Exception { + IgnoredSubdirectories ignored = IgnoredSubdirectories.of(prefixes("pre"), patterns("pat")); + assertThat(ignored.union(ignored)).isEqualTo(ignored); + } + + @Test + public void filterPrefixes() throws Exception { + IgnoredSubdirectories original = + IgnoredSubdirectories.of(prefixes("foo", "bar", "barbaz", "bar/qux")); + IgnoredSubdirectories filtered = original.filterForDirectory(PathFragment.create("bar")); + assertThat(filtered).isEqualTo(IgnoredSubdirectories.of(prefixes("bar", "bar/qux"))); + } + + @Test + public void filterPatterns() throws Exception { + IgnoredSubdirectories original = + IgnoredSubdirectories.of( + prefixes(), + patterns( + "**/sub", + "foo", + "bar/*/onesub", + "bar/qux/**", + "bar/ba*", + "bar/not/*/twosub", + "bar/**/barsub", + "bar/sub/subsub")); + IgnoredSubdirectories filtered = original.filterForDirectory(PathFragment.create("bar/sub")); + assertThat(filtered) + .isEqualTo( + IgnoredSubdirectories.of( + prefixes(), patterns("**/sub", "bar/*/onesub", "bar/**/barsub", "bar/sub/subsub"))); + } + + @Test + public void filterPatternsForHiddenFiles() throws Exception { + IgnoredSubdirectories original = + IgnoredSubdirectories.of( + prefixes(), + patterns("not/sub", "*dden/sub", ".hidden/**/sub", ".hi*/*/sub", "*/sub", "**/sub")); + IgnoredSubdirectories filtered = original.filterForDirectory(PathFragment.create(".hidden")); + // Glob semantics say that "*dden" is not supposed to match ".hidden". + // "**" and "*" and ".hi*" should, though. + assertThat(filtered) + .isEqualTo( + IgnoredSubdirectories.of( + prefixes(), patterns(".hidden/**/sub", ".hi*/*/sub", "*/sub", "**/sub"))); + } +} diff --git a/src/test/java/com/google/devtools/build/lib/rules/repository/RepositoryDelegatorTest.java b/src/test/java/com/google/devtools/build/lib/rules/repository/RepositoryDelegatorTest.java index 785330cc2b5aa9..d942f845bd6f9f 100644 --- a/src/test/java/com/google/devtools/build/lib/rules/repository/RepositoryDelegatorTest.java +++ b/src/test/java/com/google/devtools/build/lib/rules/repository/RepositoryDelegatorTest.java @@ -89,7 +89,6 @@ import com.google.devtools.build.lib.util.io.TimestampGranularityMonitor; import com.google.devtools.build.lib.vfs.FileStateKey; import com.google.devtools.build.lib.vfs.Path; -import com.google.devtools.build.lib.vfs.PathFragment; import com.google.devtools.build.lib.vfs.Root; import com.google.devtools.build.lib.vfs.SyscallCache; import com.google.devtools.build.skyframe.EvaluationContext; @@ -231,10 +230,7 @@ public void setupDelegator() throws Exception { SkyFunctions.STARLARK_BUILTINS, new StarlarkBuiltinsFunction(ruleClassProvider.getBazelStarlarkEnvironment())) .put(SkyFunctions.CONTAINING_PACKAGE_LOOKUP, new ContainingPackageLookupFunction()) - .put( - SkyFunctions.IGNORED_PACKAGE_PREFIXES, - new IgnoredPackagePrefixesFunction( - /* ignoredPackagePrefixesFile= */ PathFragment.EMPTY_FRAGMENT)) + .put(SkyFunctions.IGNORED_PACKAGE_PREFIXES, IgnoredPackagePrefixesFunction.NOOP) .put( SkyFunctions.REPOSITORY_MAPPING, new RepositoryMappingFunction(ruleClassProvider)) diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/BUILD b/src/test/java/com/google/devtools/build/lib/skyframe/BUILD index b5a67f69bfb8ab..023c9a871f8df3 100644 --- a/src/test/java/com/google/devtools/build/lib/skyframe/BUILD +++ b/src/test/java/com/google/devtools/build/lib/skyframe/BUILD @@ -559,6 +559,7 @@ java_library( "//src/main/java/com/google/devtools/build/lib/skyframe:invalid_glob_pattern_exception", "//src/main/java/com/google/devtools/build/lib/skyframe:package_lookup_function", "//src/main/java/com/google/devtools/build/lib/skyframe:precomputed_value", + "//src/main/java/com/google/devtools/build/lib/skyframe:repo_file_function", "//src/main/java/com/google/devtools/build/lib/skyframe:repository_mapping_value", "//src/main/java/com/google/devtools/build/lib/skyframe:sky_functions", "//src/main/java/com/google/devtools/build/lib/skyframe:skyframe_cluster", @@ -1442,6 +1443,7 @@ java_test( "//src/main/java/com/google/devtools/build/lib/skyframe:package_lookup_function", "//src/main/java/com/google/devtools/build/lib/skyframe:package_lookup_value", "//src/main/java/com/google/devtools/build/lib/skyframe:precomputed_value", + "//src/main/java/com/google/devtools/build/lib/skyframe:repo_file_function", "//src/main/java/com/google/devtools/build/lib/skyframe:repository_mapping_value", "//src/main/java/com/google/devtools/build/lib/skyframe:sky_functions", "//src/main/java/com/google/devtools/build/lib/skyframe:skyframe_cluster", diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/ContainingPackageLookupFunctionTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/ContainingPackageLookupFunctionTest.java index 50d2a6009483e2..da8f5ae7626265 100644 --- a/src/test/java/com/google/devtools/build/lib/skyframe/ContainingPackageLookupFunctionTest.java +++ b/src/test/java/com/google/devtools/build/lib/skyframe/ContainingPackageLookupFunctionTest.java @@ -111,10 +111,7 @@ public final void setUp() throws Exception { BazelSkyframeExecutorConstants.BUILD_FILES_BY_PRIORITY, BazelSkyframeExecutorConstants.EXTERNAL_PACKAGE_HELPER)); skyFunctions.put(SkyFunctions.PACKAGE, PackageFunction.newBuilder().build()); - skyFunctions.put( - SkyFunctions.IGNORED_PACKAGE_PREFIXES, - new IgnoredPackagePrefixesFunction( - /*ignoredPackagePrefixesFile=*/ PathFragment.EMPTY_FRAGMENT)); + skyFunctions.put(SkyFunctions.IGNORED_PACKAGE_PREFIXES, IgnoredPackagePrefixesFunction.NOOP); skyFunctions.put( FileStateKey.FILE_STATE, new FileStateFunction( diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/FilesetEntryFunctionTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/FilesetEntryFunctionTest.java index ab6d6545f0fb58..9a59fd46bf3dbb 100644 --- a/src/test/java/com/google/devtools/build/lib/skyframe/FilesetEntryFunctionTest.java +++ b/src/test/java/com/google/devtools/build/lib/skyframe/FilesetEntryFunctionTest.java @@ -123,10 +123,7 @@ public void setUp() throws Exception { CrossRepositoryLabelViolationStrategy.ERROR, BazelSkyframeExecutorConstants.BUILD_FILES_BY_PRIORITY, BazelSkyframeExecutorConstants.EXTERNAL_PACKAGE_HELPER)); - skyFunctions.put( - SkyFunctions.IGNORED_PACKAGE_PREFIXES, - new IgnoredPackagePrefixesFunction( - /*ignoredPackagePrefixesFile=*/ PathFragment.EMPTY_FRAGMENT)); + skyFunctions.put(SkyFunctions.IGNORED_PACKAGE_PREFIXES, IgnoredPackagePrefixesFunction.NOOP); skyFunctions.put( SkyFunctions.FILESET_ENTRY, new FilesetEntryFunction((unused) -> rootDirectory)); skyFunctions.put(SkyFunctions.WORKSPACE_NAME, new TestWorkspaceNameFunction()); diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/GlobTestBase.java b/src/test/java/com/google/devtools/build/lib/skyframe/GlobTestBase.java index 44f9aabd20cc4a..625bca91a656e0 100644 --- a/src/test/java/com/google/devtools/build/lib/skyframe/GlobTestBase.java +++ b/src/test/java/com/google/devtools/build/lib/skyframe/GlobTestBase.java @@ -145,6 +145,8 @@ private Map createFunctionMap() { ExternalFileAction.DEPEND_ON_EXTERNAL_PKG_FOR_EXTERNAL_REPO_PATHS, directories); + AnalysisMock analysisMock = AnalysisMock.get(); + RuleClassProvider ruleClassProvider = analysisMock.createRuleClassProvider(); Map skyFunctions = new HashMap<>(); createGlobSkyFunction(skyFunctions); skyFunctions.put( @@ -158,6 +160,10 @@ private Map createFunctionMap() { CrossRepositoryLabelViolationStrategy.ERROR, BazelSkyframeExecutorConstants.BUILD_FILES_BY_PRIORITY, BazelSkyframeExecutorConstants.EXTERNAL_PACKAGE_HELPER)); + skyFunctions.put( + SkyFunctions.REPO_FILE, + new RepoFileFunction( + ruleClassProvider.getBazelStarlarkEnvironment(), directories.getWorkspace())); skyFunctions.put( SkyFunctions.IGNORED_PACKAGE_PREFIXES, BazelSkyframeExecutorConstants.IGNORED_PACKAGE_PREFIXES_FUNCTION); @@ -173,8 +179,6 @@ private Map createFunctionMap() { skyFunctions.put(FileValue.FILE, new FileFunction(pkgLocator, directories)); skyFunctions.put( FileSymlinkCycleUniquenessFunction.NAME, new FileSymlinkCycleUniquenessFunction()); - AnalysisMock analysisMock = AnalysisMock.get(); - RuleClassProvider ruleClassProvider = analysisMock.createRuleClassProvider(); skyFunctions.put( WorkspaceFileValue.WORKSPACE_FILE, new WorkspaceFileFunction( diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/PackageLookupFunctionTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/PackageLookupFunctionTest.java index 50bfff05cdced9..a9d8fc9925db21 100644 --- a/src/test/java/com/google/devtools/build/lib/skyframe/PackageLookupFunctionTest.java +++ b/src/test/java/com/google/devtools/build/lib/skyframe/PackageLookupFunctionTest.java @@ -113,6 +113,7 @@ public final void setUp() throws Exception { ExternalFilesHelper externalFilesHelper = ExternalFilesHelper.createForTesting( pkgLocator, ExternalFileAction.DEPEND_ON_EXTERNAL_PKG_FOR_EXTERNAL_REPO_PATHS, directories); + RuleClassProvider ruleClassProvider = analysisMock.createRuleClassProvider(); Map skyFunctions = new HashMap<>(); skyFunctions.put( SkyFunctions.PACKAGE_LOOKUP, @@ -133,11 +134,14 @@ public final void setUp() throws Exception { skyFunctions.put( SkyFunctions.DIRECTORY_LISTING_STATE, new DirectoryListingStateFunction(externalFilesHelper, SyscallCache.NO_CACHE)); + skyFunctions.put( + SkyFunctions.REPO_FILE, + new RepoFileFunction( + ruleClassProvider.getBazelStarlarkEnvironment(), directories.getWorkspace())); skyFunctions.put( SkyFunctions.IGNORED_PACKAGE_PREFIXES, new IgnoredPackagePrefixesFunction( PathFragment.create(IGNORED_PACKAGE_PREFIXES_FILE_PATH_STRING))); - RuleClassProvider ruleClassProvider = analysisMock.createRuleClassProvider(); skyFunctions.put( WorkspaceFileValue.WORKSPACE_FILE, new WorkspaceFileFunction( diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/RecursiveFilesystemTraversalFunctionTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/RecursiveFilesystemTraversalFunctionTest.java index a08ad34786ad98..fb431770277dfb 100644 --- a/src/test/java/com/google/devtools/build/lib/skyframe/RecursiveFilesystemTraversalFunctionTest.java +++ b/src/test/java/com/google/devtools/build/lib/skyframe/RecursiveFilesystemTraversalFunctionTest.java @@ -182,10 +182,7 @@ public void setUp() { CrossRepositoryLabelViolationStrategy.ERROR, BazelSkyframeExecutorConstants.BUILD_FILES_BY_PRIORITY, BazelSkyframeExecutorConstants.EXTERNAL_PACKAGE_HELPER)); - skyFunctions.put( - SkyFunctions.IGNORED_PACKAGE_PREFIXES, - new IgnoredPackagePrefixesFunction( - /*ignoredPackagePrefixesFile=*/ PathFragment.EMPTY_FRAGMENT)); + skyFunctions.put(SkyFunctions.IGNORED_PACKAGE_PREFIXES, IgnoredPackagePrefixesFunction.NOOP); skyFunctions.put(SkyFunctions.PACKAGE, PackageFunction.newBuilder().build()); skyFunctions.put( WorkspaceFileValue.WORKSPACE_FILE, diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/serialization/IgnoredPackagePrefixesValueCodecTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/serialization/IgnoredPackagePrefixesValueCodecTest.java index dc04ba9babd67c..66decb938da0a9 100644 --- a/src/test/java/com/google/devtools/build/lib/skyframe/serialization/IgnoredPackagePrefixesValueCodecTest.java +++ b/src/test/java/com/google/devtools/build/lib/skyframe/serialization/IgnoredPackagePrefixesValueCodecTest.java @@ -13,10 +13,12 @@ // limitations under the License. package com.google.devtools.build.lib.skyframe.serialization; +import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableSet; import com.google.devtools.build.lib.skyframe.IgnoredPackagePrefixesValue; import com.google.devtools.build.lib.skyframe.serialization.testutils.SerializationTester; import com.google.devtools.build.lib.vfs.PathFragment; +import java.util.Arrays; import org.junit.Test; import org.junit.runner.RunWith; import org.junit.runners.JUnit4; @@ -25,13 +27,24 @@ @RunWith(JUnit4.class) public class IgnoredPackagePrefixesValueCodecTest { + private static ImmutableSet prefixes(String... prefixes) { + return Arrays.stream(prefixes).map(PathFragment::create).collect(ImmutableSet.toImmutableSet()); + } + + private static ImmutableList patterns(String... patterns) { + return ImmutableList.copyOf(patterns); + } + @Test public void testCodec() throws Exception { new SerializationTester( - IgnoredPackagePrefixesValue.of(ImmutableSet.of()), - IgnoredPackagePrefixesValue.of(ImmutableSet.of(PathFragment.create("foo"))), - IgnoredPackagePrefixesValue.of( - ImmutableSet.of(PathFragment.create("foo"), PathFragment.create("bar/moo")))) + IgnoredPackagePrefixesValue.of(prefixes(), patterns()), + IgnoredPackagePrefixesValue.of(prefixes("foo"), patterns()), + IgnoredPackagePrefixesValue.of(prefixes("foo", "bar/moo"), patterns()), + IgnoredPackagePrefixesValue.of(prefixes(), patterns("foo")), + IgnoredPackagePrefixesValue.of(prefixes(), patterns("foo")), + IgnoredPackagePrefixesValue.of(prefixes(), patterns("foo/**")), + IgnoredPackagePrefixesValue.of(prefixes("foo"), patterns("foo/**"))) .runTests(); } } diff --git a/src/test/shell/bazel/bazelignore_test.sh b/src/test/shell/bazel/bazelignore_test.sh index dfd3a831a5d50f..ee5abe627fdc8d 100755 --- a/src/test/shell/bazel/bazelignore_test.sh +++ b/src/test/shell/bazel/bazelignore_test.sh @@ -217,4 +217,75 @@ test_invalid_path() { expect_log "java.nio.file.InvalidPathException: Nul character not allowed" } +test_target_patterns_with_wildcards_in_repo_bazel() { + rm -rf work && mkdir work && cd work + setup_module_dot_bazel + cat >REPO.bazel <<'EOF' +ignore_directories(["**/sub", "foo/bar/.dot/*"]) +EOF + + for pkg in foo foo/sub foo/sub/subsub foo/bar/.dot/pkg foo/notsub foo/bar/.dot; do + mkdir -p "$pkg" + echo 'filegroup(name="fg")' > "$pkg/BUILD.bazel" + done + + bazel query //foo/... > "$TEST_TMPDIR/targets" + assert_not_contains "//foo/sub:fg" "$TEST_TMPDIR/targets" + assert_not_contains "//foo/sub/subsub:fg" "$TEST_TMPDIR/targets" + assert_not_contains "//foo/bar/.dot/pkg:fg" "$TEST_TMPDIR/targets" + assert_contains "//foo/notsub:fg" "$TEST_TMPDIR/targets" +} + +test_globs_with_wildcards_in_repo_bazel() { + rm -rf work && mkdir work && cd work + setup_module_dot_bazel + cat >REPO.bazel <<'EOF' +ignore_directories(["**/sub", "foo/.hidden*"]) +EOF + + mkdir -p foo foo/bar sub bar/sub foo/.hidden_excluded foo/.included + touch foo/foofile foo/bar/barfile sub/subfile bar/sub/subfile + touch foo/.hidden_excluded/file foo/.included/file + + cat > BUILD <<'EOF' +filegroup(name="fg", srcs=glob(["**"])) + +EOF + bazel query //:all-targets > "$TEST_TMPDIR/targets" + assert_contains ":foo/foofile" "$TEST_TMPDIR/targets" + assert_contains ":foo/bar/barfile" "$TEST_TMPDIR/targets" + assert_contains ":foo/.included/file" "$TEST_TMPDIR/targets" + assert_not_contains ":sub/subfile" "$TEST_TMPDIR/targets" + assert_not_contains ":bar/sub/subfile" "$TEST_TMPDIR/targets" + assert_not_contains ":foo/.hidden_excluded/file" "$TEST_TMPDIR/targets" +} + + +test_syntax_error_in_repo_bazel() { + rm -rf work && mkdir work && cd work + setup_module_dot_bazel + cat >REPO.bazel <<'EOF' +SYNTAX ERROR +EOF + + touch BUILD.bazel + bazel query //:all && fail "failure expected" + if [[ $? != 7 ]]; then + fail "expected an analysis failure" + fi +} + +test_ignore_directories_after_repo_in_repo_bazel() { + rm -rf work && mkdir work && cd work + setup_module_dot_bazel + cat >REPO.bazel <<'EOF' +ignore_directories(["**/sub", "foo/.hidden*"]) +repo(default_visibility=["//visibility:public"]) +EOF + + touch BUILD.bazel + bazel query //:all >& "$TEST_log" && fail "failure expected" + expect_log "it must be called before any other functions" +} + run_suite "Integration tests for .bazelignore"