From eec7250dccba5bf19b6c923b7213138c74187c20 Mon Sep 17 00:00:00 2001 From: tomlu Date: Fri, 12 Jan 2018 09:39:24 -0800 Subject: Remove isMainRepo from Root. This is no longer used. PiperOrigin-RevId: 181754475 --- .../build/lib/actions/ArtifactFactory.java | 8 ++- .../google/devtools/build/lib/actions/Root.java | 79 ++++++---------------- .../build/lib/analysis/BlazeDirectories.java | 2 +- .../lib/analysis/ConfiguredTargetFactory.java | 4 +- .../lib/analysis/config/BuildConfiguration.java | 7 +- .../devtools/build/lib/rules/cpp/FdoSupport.java | 2 +- .../build/lib/skyframe/MapAsPackageRoots.java | 2 +- .../skyframe/PackageRootsNoSymlinkCreation.java | 2 +- .../build/lib/exec/FilesetManifestTest.java | 20 +++--- .../build/lib/exec/SpawnInputExpanderTest.java | 8 +-- 10 files changed, 49 insertions(+), 85 deletions(-) (limited to 'src') diff --git a/src/main/java/com/google/devtools/build/lib/actions/ArtifactFactory.java b/src/main/java/com/google/devtools/build/lib/actions/ArtifactFactory.java index 58eaf70e14..665f5e9a77 100644 --- a/src/main/java/com/google/devtools/build/lib/actions/ArtifactFactory.java +++ b/src/main/java/com/google/devtools/build/lib/actions/ArtifactFactory.java @@ -173,8 +173,12 @@ public class ArtifactFactory implements ArtifactResolver, ArtifactSerializer, Ar Preconditions.checkArgument(execPath.isNormalized(), execPath); // TODO(bazel-team): Check that either BinTools do not change over the life of the Blaze server, // or require that a legitimate ArtifactOwner be passed in here to allow for ownership. - return getArtifact(execRoot.getRelative(execPath), Root.execRootAsDerivedRoot(execRoot, true), - execPath, ArtifactOwner.NULL_OWNER, null); + return getArtifact( + execRoot.getRelative(execPath), + Root.execRootAsDerivedRoot(execRoot), + execPath, + ArtifactOwner.NULL_OWNER, + null); } private void validatePath(PathFragment rootRelativePath, Root root) { diff --git a/src/main/java/com/google/devtools/build/lib/actions/Root.java b/src/main/java/com/google/devtools/build/lib/actions/Root.java index 7cb16981cb..e6451c9e41 100644 --- a/src/main/java/com/google/devtools/build/lib/actions/Root.java +++ b/src/main/java/com/google/devtools/build/lib/actions/Root.java @@ -52,46 +52,25 @@ import javax.annotation.Nullable; @Immutable public final class Root implements Comparable, Serializable, SkylarkValue { - /** - * Returns the given path as a source root. The path may not be {@code null}. - */ - // TODO(kchodorow): remove once roots don't need to know if they're in the main repo. - public static Root asSourceRoot(Path path, boolean isMainRepo) { - return new Root(null, path, false, isMainRepo); - } - // This must always be consistent with Package.getSourceRoot; otherwise computing source roots // from exec paths does not work, which can break the action cache for input-discovering actions. public static Root computeSourceRoot(Path packageRoot, RepositoryName repository) { if (repository.isMain()) { - return Root.asSourceRoot(packageRoot, true); + return asSourceRoot(packageRoot); } else { Path actualRoot = packageRoot; for (int i = 0; i < repository.getSourceRoot().segmentCount(); i++) { actualRoot = actualRoot.getParentDirectory(); } - return Root.asSourceRoot(actualRoot, false); + return asSourceRoot(actualRoot); } } - /** - * testonly until {@link #asSourceRoot(Path, boolean)} is deleted. - */ + /** Returns the given path as a source root. The path may not be {@code null}. */ public static Root asSourceRoot(Path path) { - return asSourceRoot(path, true); + return new Root(null, path); } - /** - * DO NOT USE IN PRODUCTION CODE! - * - *

Returns the given path as a derived root. This method only exists as a convenience for - * tests, which don't need a proper Root object. - */ - @VisibleForTesting - public static Root asDerivedRoot(Path path) { - return new Root(path, path, true); - } - /** * Returns the given path as a derived root, relative to the given exec root. The root must be a * proper sub-directory of the exec root (i.e. not equal). Neither may be {@code null}. @@ -99,33 +78,28 @@ public final class Root implements Comparable, Serializable, SkylarkValue *

Be careful with this method - all derived roots must be registered with the artifact factory * before the analysis phase. */ - // TODO(kchodorow): remove once roots don't need to know if they're in the main repo. - public static Root asDerivedRoot(Path execRoot, Path root, boolean isMainRepo) { + public static Root asDerivedRoot(Path execRoot, Path root) { Preconditions.checkArgument(root.startsWith(execRoot)); Preconditions.checkArgument(!root.equals(execRoot)); - return new Root(execRoot, root, false, isMainRepo); + return new Root(execRoot, root); } /** - * testonly until {@link #asDerivedRoot(Path, Path, boolean)} is deleted. + * DO NOT USE IN PRODUCTION CODE! + * + *

Returns the given path as a derived root. This method only exists as a convenience for + * tests, which don't need a proper Root object. */ - public static Root asDerivedRoot(Path execRoot, Path root) { - return Root.asDerivedRoot(execRoot, root, true); + @VisibleForTesting + public static Root asDerivedRoot(Path path) { + return new Root(path, path); } - // TODO(kchodorow): remove once roots don't need to know if they're in the main repo. - public static Root middlemanRoot(Path execRoot, Path outputDir, boolean isMainRepo) { + public static Root middlemanRoot(Path execRoot, Path outputDir) { Path root = outputDir.getRelative("internal"); Preconditions.checkArgument(root.startsWith(execRoot)); Preconditions.checkArgument(!root.equals(execRoot)); - return new Root(execRoot, root, true, isMainRepo); - } - - /** - * testonly until {@link #middlemanRoot(Path, Path, boolean)} is deleted. - */ - public static Root middlemanRoot(Path execRoot, Path outputDir) { - return Root.middlemanRoot(execRoot, outputDir, true); + return new Root(execRoot, root, true); } /** @@ -133,28 +107,24 @@ public final class Root implements Comparable, Serializable, SkylarkValue * root, but this is currently allowed. Do not add any further uses besides the ones that already * exist! */ - // TODO(kchodorow): remove isMainRepo once roots don't need to know if they're in the main repo. - static Root execRootAsDerivedRoot(Path execRoot, boolean isMainRepo) { - return new Root(execRoot, execRoot, false, isMainRepo); + static Root execRootAsDerivedRoot(Path execRoot) { + return new Root(execRoot, execRoot); } @Nullable private final Path execRoot; private final Path path; private final boolean isMiddlemanRoot; - private final boolean isMainRepo; private final PathFragment execPath; - - private Root(@Nullable Path execRoot, Path path, boolean isMiddlemanRoot, boolean isMainRepo) { + private Root(@Nullable Path execRoot, Path path, boolean isMiddlemanRoot) { this.execRoot = execRoot; this.path = Preconditions.checkNotNull(path); this.isMiddlemanRoot = isMiddlemanRoot; - this.isMainRepo = isMainRepo; this.execPath = isSourceRoot() ? PathFragment.EMPTY_FRAGMENT : path.relativeTo(execRoot); } - private Root(@Nullable Path execRoot, Path path, boolean isMainRepo) { - this(execRoot, path, false, isMainRepo); + private Root(@Nullable Path execRoot, Path path) { + this(execRoot, path, false); } public Path getPath() { @@ -188,10 +158,6 @@ public final class Root implements Comparable, Serializable, SkylarkValue return isMiddlemanRoot; } - public boolean isMainRepo() { - return isMainRepo; - } - @Override public int compareTo(Root o) { return path.compareTo(o.path); @@ -199,7 +165,7 @@ public final class Root implements Comparable, Serializable, SkylarkValue @Override public int hashCode() { - return Objects.hash(execRoot, path.hashCode(), isMainRepo); + return Objects.hash(execRoot, path.hashCode()); } @Override @@ -211,8 +177,7 @@ public final class Root implements Comparable, Serializable, SkylarkValue return false; } Root r = (Root) o; - return path.equals(r.path) && Objects.equals(execRoot, r.execRoot) - && Objects.equals(isMainRepo, r.isMainRepo); + return path.equals(r.path) && Objects.equals(execRoot, r.execRoot); } @Override diff --git a/src/main/java/com/google/devtools/build/lib/analysis/BlazeDirectories.java b/src/main/java/com/google/devtools/build/lib/analysis/BlazeDirectories.java index d2d84dfade..3e7df15cb0 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/BlazeDirectories.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/BlazeDirectories.java @@ -172,7 +172,7 @@ public final class BlazeDirectories { * {@link BlazeDirectories} of this server instance. Nothing else should be placed here. */ public Root getBuildDataDirectory(String workspaceName) { - return Root.asDerivedRoot(getExecRoot(workspaceName), getOutputPath(workspaceName), true); + return Root.asDerivedRoot(getExecRoot(workspaceName), getOutputPath(workspaceName)); } /** diff --git a/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredTargetFactory.java b/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredTargetFactory.java index 913b6f1a71..3ad0240ebe 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredTargetFactory.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredTargetFactory.java @@ -276,9 +276,7 @@ public final class ConfiguredTargetFactory { Artifact artifact = artifactFactory.getSourceArtifact( inputFile.getExecPath(), - Root.asSourceRoot( - inputFile.getPackage().getSourceRoot(), - inputFile.getPackage().getPackageIdentifier().getRepository().isMain()), + Root.asSourceRoot(inputFile.getPackage().getSourceRoot()), ConfiguredTargetKey.of(target.getLabel(), config)); return new InputFileConfiguredTarget(targetContext, inputFile, artifact); diff --git a/src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfiguration.java b/src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfiguration.java index c9abccc7d1..9b6f5d17b2 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfiguration.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfiguration.java @@ -1194,13 +1194,10 @@ public class BuildConfiguration implements BuildEvent { Path outputDir = execRoot.getRelative(directories.getRelativeOutputPath()) .getRelative(outputDirName); if (middleman) { - return INTERNER.intern(Root.middlemanRoot(execRoot, outputDir, - repositoryName.equals(mainRepositoryName))); + return INTERNER.intern(Root.middlemanRoot(execRoot, outputDir)); } // e.g., [[execroot/repo1]/bazel-out/config/bin] - return INTERNER.intern( - Root.asDerivedRoot(execRoot, outputDir.getRelative(nameFragment), - repositoryName.equals(mainRepositoryName))); + return INTERNER.intern(Root.asDerivedRoot(execRoot, outputDir.getRelative(nameFragment))); } } diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/FdoSupport.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/FdoSupport.java index b58c9512be..738cc982f6 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/FdoSupport.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/FdoSupport.java @@ -284,7 +284,7 @@ public class FdoSupport { Root fdoRoot = (fdoProfile == null) ? null - : Root.asDerivedRoot(execRoot, execRoot.getRelative(productName + "-fdo"), true); + : Root.asDerivedRoot(execRoot, execRoot.getRelative(productName + "-fdo")); PathFragment fdoRootExecPath = fdoProfile == null ? null diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/MapAsPackageRoots.java b/src/main/java/com/google/devtools/build/lib/skyframe/MapAsPackageRoots.java index baca2e02a1..f969c02864 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/MapAsPackageRoots.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/MapAsPackageRoots.java @@ -46,7 +46,7 @@ public class MapAsPackageRoots implements PackageRoots { for (Map.Entry entry : packageRootsMap.entrySet()) { Root root = rootMap.get(entry.getValue()); if (root == null) { - root = Root.asSourceRoot(entry.getValue(), entry.getKey().getRepository().isMain()); + root = Root.asSourceRoot(entry.getValue()); rootMap.put(entry.getValue(), root); } realPackageRoots.put(entry.getKey(), root); diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/PackageRootsNoSymlinkCreation.java b/src/main/java/com/google/devtools/build/lib/skyframe/PackageRootsNoSymlinkCreation.java index 499e7413ae..d1ee84fb37 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/PackageRootsNoSymlinkCreation.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/PackageRootsNoSymlinkCreation.java @@ -31,7 +31,7 @@ public class PackageRootsNoSymlinkCreation implements PackageRoots { @VisibleForTesting public PackageRootsNoSymlinkCreation(Path sourcePath) { - this.sourceRoot = Root.asSourceRoot(sourcePath, true); + this.sourceRoot = Root.asSourceRoot(sourcePath); } @Override diff --git a/src/test/java/com/google/devtools/build/lib/exec/FilesetManifestTest.java b/src/test/java/com/google/devtools/build/lib/exec/FilesetManifestTest.java index 5ae18eeb48..4099d2ed0d 100644 --- a/src/test/java/com/google/devtools/build/lib/exec/FilesetManifestTest.java +++ b/src/test/java/com/google/devtools/build/lib/exec/FilesetManifestTest.java @@ -72,7 +72,7 @@ public class FilesetManifestTest { "workspace/bar /dir/file", ""); - Root outputRoot = Root.asDerivedRoot(fs.getPath("/root"), fs.getPath("/root/out"), true); + Root outputRoot = Root.asDerivedRoot(fs.getPath("/root"), fs.getPath("/root/out")); Artifact artifact = new Artifact(fs.getPath("/root/out/foo"), outputRoot); FilesetManifest manifest = FilesetManifest.parseManifestFile(artifact, execRoot, "workspace", IGNORE); @@ -90,7 +90,7 @@ public class FilesetManifestTest { "workspace/baz /dir/file", ""); - Root outputRoot = Root.asDerivedRoot(fs.getPath("/root"), fs.getPath("/root/out"), true); + Root outputRoot = Root.asDerivedRoot(fs.getPath("/root"), fs.getPath("/root/out")); Artifact artifact = new Artifact(fs.getPath("/root/out/foo"), outputRoot); FilesetManifest manifest = FilesetManifest.parseManifestFile(artifact, execRoot, "workspace", IGNORE); @@ -108,7 +108,7 @@ public class FilesetManifestTest { "workspace/bar /some", ""); - Root outputRoot = Root.asDerivedRoot(fs.getPath("/root"), fs.getPath("/root/out"), true); + Root outputRoot = Root.asDerivedRoot(fs.getPath("/root"), fs.getPath("/root/out")); Artifact artifact = new Artifact(fs.getPath("/root/out/foo"), outputRoot); FilesetManifest manifest = FilesetManifest.parseManifestFile(artifact, execRoot, "workspace", IGNORE); @@ -125,7 +125,7 @@ public class FilesetManifestTest { "workspace/bar ", // <-- Note the trailing whitespace! ""); - Root outputRoot = Root.asDerivedRoot(fs.getPath("/root"), fs.getPath("/root/out"), true); + Root outputRoot = Root.asDerivedRoot(fs.getPath("/root"), fs.getPath("/root/out")); Artifact artifact = new Artifact(fs.getPath("/root/out/foo"), outputRoot); FilesetManifest manifest = FilesetManifest.parseManifestFile(artifact, execRoot, "workspace", IGNORE); @@ -140,7 +140,7 @@ public class FilesetManifestTest { "notworkspace/bar /foo/bar", ""); - Root outputRoot = Root.asDerivedRoot(fs.getPath("/root"), fs.getPath("/root/out"), true); + Root outputRoot = Root.asDerivedRoot(fs.getPath("/root"), fs.getPath("/root/out")); Artifact artifact = new Artifact(fs.getPath("/root/out/foo"), outputRoot); try { FilesetManifest.parseManifestFile(artifact, execRoot, "workspace", IGNORE); @@ -162,7 +162,7 @@ public class FilesetManifestTest { "workspace/foo /foo/bar", ""); - Root outputRoot = Root.asDerivedRoot(fs.getPath("/root"), fs.getPath("/root/out"), true); + Root outputRoot = Root.asDerivedRoot(fs.getPath("/root"), fs.getPath("/root/out")); Artifact artifact = new Artifact(fs.getPath("/root/out/foo"), outputRoot); try { FilesetManifest.parseManifestFile(artifact, execRoot, "workspace", ERROR); @@ -182,7 +182,7 @@ public class FilesetManifestTest { "workspace/foo /foo/bar", ""); - Root outputRoot = Root.asDerivedRoot(fs.getPath("/root"), fs.getPath("/root/out"), true); + Root outputRoot = Root.asDerivedRoot(fs.getPath("/root"), fs.getPath("/root/out")); Artifact artifact = new Artifact(fs.getPath("/root/out/foo"), outputRoot); FilesetManifest manifest = FilesetManifest.parseManifestFile(artifact, execRoot, "workspace", IGNORE); @@ -200,7 +200,7 @@ public class FilesetManifestTest { "workspace/foo /foo/bar", ""); - Root outputRoot = Root.asDerivedRoot(fs.getPath("/root"), fs.getPath("/root/out"), true); + Root outputRoot = Root.asDerivedRoot(fs.getPath("/root"), fs.getPath("/root/out")); Artifact artifact = new Artifact(fs.getPath("/root/out/foo"), outputRoot); FilesetManifest manifest = FilesetManifest.parseManifestFile(artifact, execRoot, "workspace", RESOLVE); @@ -218,7 +218,7 @@ public class FilesetManifestTest { "workspace/bar foo", ""); - Root outputRoot = Root.asDerivedRoot(fs.getPath("/root"), fs.getPath("/root/out"), true); + Root outputRoot = Root.asDerivedRoot(fs.getPath("/root"), fs.getPath("/root/out")); Artifact artifact = new Artifact(fs.getPath("/root/out/foo"), outputRoot); try { FilesetManifest.parseManifestFile(artifact, execRoot, "workspace", RESOLVE); @@ -242,7 +242,7 @@ public class FilesetManifestTest { "workspace/bar /baz", ""); - Root outputRoot = Root.asDerivedRoot(fs.getPath("/root"), fs.getPath("/root/out"), true); + Root outputRoot = Root.asDerivedRoot(fs.getPath("/root"), fs.getPath("/root/out")); Artifact artifact = new Artifact(fs.getPath("/root/out/foo"), outputRoot); FilesetManifest manifest = FilesetManifest.parseManifestFile(artifact, execRoot, "workspace", IGNORE); diff --git a/src/test/java/com/google/devtools/build/lib/exec/SpawnInputExpanderTest.java b/src/test/java/com/google/devtools/build/lib/exec/SpawnInputExpanderTest.java index 819389346f..71359fd215 100644 --- a/src/test/java/com/google/devtools/build/lib/exec/SpawnInputExpanderTest.java +++ b/src/test/java/com/google/devtools/build/lib/exec/SpawnInputExpanderTest.java @@ -185,7 +185,7 @@ public class SpawnInputExpanderTest { // See AnalysisUtils for the mapping from "foo" to "_foo/MANIFEST". scratchFile("/root/out/_foo/MANIFEST"); - Root outputRoot = Root.asDerivedRoot(fs.getPath("/root"), fs.getPath("/root/out"), true); + Root outputRoot = Root.asDerivedRoot(fs.getPath("/root"), fs.getPath("/root/out")); Artifact artifact = new Artifact(fs.getPath("/root/out/foo"), outputRoot); expander.parseFilesetManifest(inputMappings, artifact, "workspace"); assertThat(inputMappings).isEmpty(); @@ -199,7 +199,7 @@ public class SpawnInputExpanderTest { "workspace/bar /dir/file", ""); - Root outputRoot = Root.asDerivedRoot(fs.getPath("/root"), fs.getPath("/root/out"), true); + Root outputRoot = Root.asDerivedRoot(fs.getPath("/root"), fs.getPath("/root/out")); Artifact artifact = new Artifact(fs.getPath("/root/out/foo"), outputRoot); expander.parseFilesetManifest(inputMappings, artifact, "workspace"); assertThat(inputMappings).hasSize(1); @@ -217,7 +217,7 @@ public class SpawnInputExpanderTest { "workspace/baz /dir/file", ""); - Root outputRoot = Root.asDerivedRoot(fs.getPath("/root"), fs.getPath("/root/out"), true); + Root outputRoot = Root.asDerivedRoot(fs.getPath("/root"), fs.getPath("/root/out")); Artifact artifact = new Artifact(fs.getPath("/root/out/foo"), outputRoot); expander.parseFilesetManifest(inputMappings, artifact, "workspace"); assertThat(inputMappings).hasSize(2); @@ -235,7 +235,7 @@ public class SpawnInputExpanderTest { "workspace/bar /some", ""); - Root outputRoot = Root.asDerivedRoot(fs.getPath("/root"), fs.getPath("/root/out"), true); + Root outputRoot = Root.asDerivedRoot(fs.getPath("/root"), fs.getPath("/root/out")); Artifact artifact = new Artifact(fs.getPath("/root/out/foo"), outputRoot); expander.parseFilesetManifest(inputMappings, artifact, "workspace"); assertThat(inputMappings).hasSize(1); -- cgit v1.2.3