diff options
Diffstat (limited to 'src/main/java/com/google/devtools/build/lib')
22 files changed, 225 insertions, 353 deletions
diff --git a/src/main/java/com/google/devtools/build/lib/actions/Artifact.java b/src/main/java/com/google/devtools/build/lib/actions/Artifact.java index 4c4a5e1546..b3b86823da 100644 --- a/src/main/java/com/google/devtools/build/lib/actions/Artifact.java +++ b/src/main/java/com/google/devtools/build/lib/actions/Artifact.java @@ -46,13 +46,12 @@ import javax.annotation.Nullable; /** * An Artifact represents a file used by the build system, whether it's a source * file or a derived (output) file. Not all Artifacts have a corresponding - * FileTarget object in the <code>build.lib.packages</code> API: for example, + * FileTarget object in the <code>build.packages</code> API: for example, * low-level intermediaries internal to a given rule, such as a Java class files * or C++ object files. However all FileTargets have a corresponding Artifact. * - * <p>In any given call to - * {@link com.google.devtools.build.lib.skyframe.SkyframeExecutor#buildArtifacts}, no two Artifacts - * in the action graph may refer to the same path. + * <p>In any given call to Builder#buildArtifacts(), no two Artifacts in the + * action graph may refer to the same path. * * <p>Artifacts generally fall into two classifications, source and derived, but * there exist a few other cases that are fuzzy and difficult to classify. The @@ -155,64 +154,6 @@ public class Artifact private final PathFragment rootRelativePath; private final ArtifactOwner owner; - private static boolean pathEndsWith(PathFragment path, PathFragment suffix) { - if (suffix.isNormalized()) { - return path.endsWith(suffix); - } - - for (int suffixIndex = suffix.segmentCount() - 1, pathIndex = path.segmentCount() - 1; - suffixIndex >= 0; suffixIndex--) { - if (suffix.getSegment(suffixIndex).equals("..")) { - if (suffixIndex > 0) { - // The path foo/bar/../baz matches the suffix foo/baz. - suffixIndex--; - continue; - } else { - // The path repo/foo matches the suffix ../repo/foo. - return true; - } - } - if (pathIndex < 0 || !path.getSegment(pathIndex).equals(suffix.getSegment(suffixIndex))) { - return false; - } - pathIndex--; - } - - return true; - } - - // Like startsWith, but allows prefix to match the path.getParentDirectory() instead of just - // path (so that bazel-out/config/repo/foo "starts with" bazel-out/config/bin). - private static boolean pathMostlyStartsWith(Path path, Path prefix) { - return path.startsWith(prefix) || path.startsWith(prefix.getParentDirectory()); - } - - private static PathFragment getRelativePath(PathFragment path, PathFragment ancestorDirectory) { - if (path.startsWith(ancestorDirectory)) { - // Local path. - return path.relativeTo(ancestorDirectory); - } - - // External repository. - int ancestorLength = ancestorDirectory.segmentCount(); - - PathFragment builder = PathFragment.EMPTY_FRAGMENT; - int diffIndex = -1; - for (int i = 0; i < ancestorLength; i++) { - if (diffIndex == -1 && i < path.segmentCount() - && ancestorDirectory.getSegment(i).equals(path.getSegment(i))) { - continue; - } - diffIndex = i; - builder = builder.getRelative(".."); - } - int tailIndex = diffIndex == -1 ? ancestorLength : diffIndex; - for (int i = tailIndex; i < path.segmentCount(); i++) { - builder = builder.getRelative(path.getSegment(i)); - } - return builder; - } - /** * Constructs an artifact for the specified path, root and execPath. The root must be an ancestor * of path, and execPath must be a non-absolute tail of path. Outside of testing, this method @@ -225,18 +166,18 @@ public class Artifact * </pre> * * <p>In a derived Artifact, the execPath will overlap with part of the root, which in turn will - * be below the execRoot. + * be below of the execRoot. * <pre> * [path] == [/root][pathTail] == [/execRoot][execPath] == [/execRoot][rootPrefix][pathTail] * <pre> */ @VisibleForTesting public Artifact(Path path, Root root, PathFragment execPath, ArtifactOwner owner) { - if (root == null || !pathMostlyStartsWith(path, root.getPath())) { + if (root == null || !path.startsWith(root.getPath())) { throw new IllegalArgumentException(root + ": illegal root for " + path + " (execPath: " + execPath + ")"); } - if (execPath == null || execPath.isAbsolute() || !pathEndsWith(path.asFragment(), execPath)) { + if (execPath == null || execPath.isAbsolute() || !path.asFragment().endsWith(execPath)) { throw new IllegalArgumentException(execPath + ": illegal execPath for " + path + " (root: " + root + ")"); } @@ -247,14 +188,12 @@ public class Artifact // These two lines establish the invariant that // execPath == rootRelativePath <=> execPath.equals(rootRelativePath) // This is important for isSourceArtifact. - PathFragment rootRel = getRelativePath(path.asFragment(), root.getPath().asFragment()); - if (!pathEndsWith(execPath, rootRel)) { + PathFragment rootRel = path.relativeTo(root.getPath()); + if (!execPath.endsWith(rootRel)) { throw new IllegalArgumentException(execPath + ": illegal execPath doesn't end with " + rootRel + " at " + path + " with root " + root); } - - this.rootRelativePath = root.getPath().getRelative(rootRel).asFragment().normalize().equals( - root.getPath().getRelative(execPath).asFragment().normalize()) ? execPath : rootRel; + this.rootRelativePath = rootRel.equals(execPath) ? execPath : rootRel; this.owner = Preconditions.checkNotNull(owner, path); } @@ -559,10 +498,16 @@ public class Artifact /** * For targets in external repositories, this returns the path the artifact live at in the * runfiles tree. For local targets, it returns the rootRelativePath. - * TODO(kchodorow): remove. */ public final PathFragment getRunfilesPath() { - return getRootRelativePath(); + PathFragment relativePath = rootRelativePath; + if (relativePath.segmentCount() > 1 + && relativePath.getSegment(0).equals(Label.EXTERNAL_PATH_PREFIX)) { + // Turn external/repo/foo into ../repo/foo. + relativePath = relativePath.relativeTo(Label.EXTERNAL_PATH_PREFIX); + relativePath = new PathFragment("..").getRelative(relativePath); + } + return relativePath; } @SkylarkCallable( 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 48960c5cf6..2642687713 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 @@ -19,7 +19,6 @@ import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSortedSet; import com.google.devtools.build.lib.actions.Artifact.SpecialArtifactType; -import com.google.devtools.build.lib.cmdline.Label; import com.google.devtools.build.lib.cmdline.PackageIdentifier; import com.google.devtools.build.lib.cmdline.RepositoryName; import com.google.devtools.build.lib.concurrent.ThreadSafety.ThreadSafe; @@ -201,8 +200,7 @@ public class ArtifactFactory implements ArtifactResolver, ArtifactSerializer, Ar private void validatePath(PathFragment rootRelativePath, Root root) { Preconditions.checkArgument(!rootRelativePath.isAbsolute(), rootRelativePath); - Preconditions.checkArgument( - root.getWorkspaceDirectory().getRelative(rootRelativePath).normalize().isNormalized()); + Preconditions.checkArgument(rootRelativePath.isNormalized(), rootRelativePath); Preconditions.checkArgument(root.getPath().startsWith(execRoot), "%s %s", root, execRoot); Preconditions.checkArgument(!root.getPath().equals(execRoot), "%s %s", root, execRoot); // TODO(bazel-team): this should only accept roots from derivedRoots. @@ -316,9 +314,7 @@ public class ArtifactFactory implements ArtifactResolver, ArtifactSerializer, Ar PathFragment execPath = baseExecPath == null ? relativePath : baseExecPath.getRelative(relativePath); execPath = execPath.normalize(); - if (execPath.containsUplevelReferences() - && (execPath.segmentCount() >= 2 - && !execPath.startsWith(new PathFragment(Label.EXTERNAL_PATH_PREFIX)))) { + if (execPath.containsUplevelReferences()) { // Source exec paths cannot escape the source root. return null; } @@ -379,9 +375,8 @@ public class ArtifactFactory implements ArtifactResolver, ArtifactSerializer, Ar for (PathFragment execPath : execPaths) { PathFragment execPathNormalized = execPath.normalize(); - if (execPathNormalized.subFragment(1, execPathNormalized.segmentCount()) - .containsUplevelReferences()) { - // Source exec paths cannot escape the execution root. + if (execPathNormalized.containsUplevelReferences()) { + // Source exec paths cannot escape the source root. result.put(execPath, null); continue; } 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 f92e4bda3c..b09dfa35c8 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 @@ -122,10 +122,6 @@ public final class Root implements Comparable<Root>, Serializable { return execPath; } - public PathFragment getWorkspaceDirectory() { - return new PathFragment(isSourceRoot() ? path.getBaseName() : execRoot.getBaseName()); - } - @SkylarkCallable(name = "path", structField = true, doc = "Returns the relative path from the exec root to the actual root.") public String getExecPathString() { diff --git a/src/main/java/com/google/devtools/build/lib/analysis/Runfiles.java b/src/main/java/com/google/devtools/build/lib/analysis/Runfiles.java index 273506f284..d1f1434a39 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/Runfiles.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/Runfiles.java @@ -428,7 +428,7 @@ public final class Runfiles { Map<PathFragment, Artifact> manifest = getSymlinksAsMap(checker); // Add unconditional artifacts (committed to inclusion on construction of runfiles). for (Artifact artifact : getUnconditionalArtifactsWithoutMiddlemen()) { - checker.put(manifest, artifact.getRootRelativePath().normalize(), artifact); + checker.put(manifest, artifact.getRootRelativePath(), artifact); } // Add conditional artifacts (only included if they appear in a pruning manifest). @@ -485,7 +485,7 @@ public final class Runfiles { // workspace. private boolean sawWorkspaceName; - ManifestBuilder( + public ManifestBuilder( PathFragment workspaceName, boolean legacyExternalRunfiles) { this.manifest = new HashMap<>(); this.workspaceName = workspaceName; @@ -496,18 +496,20 @@ public final class Runfiles { /** * Adds a map under the workspaceName. */ - void addUnderWorkspace( + public void addUnderWorkspace( Map<PathFragment, Artifact> inputManifest, ConflictChecker checker) { for (Map.Entry<PathFragment, Artifact> entry : inputManifest.entrySet()) { PathFragment path = entry.getKey(); if (isUnderWorkspace(path)) { sawWorkspaceName = true; - } else if (legacyExternalRunfiles) { - // Turn ../repo/foo info wsname/external/repo/foo. - checker.put(manifest, workspaceName.getRelative(Label.EXTERNAL_PACKAGE_NAME) - .getRelative(workspaceName).getRelative(path).normalize(), entry.getValue()); + checker.put(manifest, workspaceName.getRelative(path), entry.getValue()); + } else { + if (legacyExternalRunfiles) { + checker.put(manifest, workspaceName.getRelative(path), entry.getValue()); + } + // Always add the non-legacy .runfiles/repo/whatever path. + checker.put(manifest, getExternalPath(path), entry.getValue()); } - checker.put(manifest, workspaceName.getRelative(path).normalize(), entry.getValue()); } } @@ -534,14 +536,17 @@ public final class Runfiles { return manifest; } + private PathFragment getExternalPath(PathFragment path) { + return checkForWorkspace(path.relativeTo(Label.EXTERNAL_PACKAGE_NAME)); + } + private PathFragment checkForWorkspace(PathFragment path) { - sawWorkspaceName = sawWorkspaceName - || path.getSegment(0).equals(workspaceName.getPathString()); + sawWorkspaceName = sawWorkspaceName || path.getSegment(0).equals(workspaceName); return path; } private static boolean isUnderWorkspace(PathFragment path) { - return !path.startsWith(new PathFragment(Label.EXTERNAL_PATH_PREFIX)); + return !path.startsWith(Label.EXTERNAL_PACKAGE_NAME); } } diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/genrule/GenRule.java b/src/main/java/com/google/devtools/build/lib/bazel/rules/genrule/GenRule.java index 816865149b..e906c11e17 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/rules/genrule/GenRule.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/genrule/GenRule.java @@ -205,7 +205,7 @@ public class GenRule implements RuleConfiguredTargetFactory { } PathFragment relPath = ruleContext.getRule().getLabel().getPackageIdentifier().getPathFragment(); - return dir.getRelative(relPath).normalize().getPathString(); + return dir.getRelative(relPath).getPathString(); } } else { return super.lookupMakeVariable(name); diff --git a/src/main/java/com/google/devtools/build/lib/buildtool/BuildTool.java b/src/main/java/com/google/devtools/build/lib/buildtool/BuildTool.java index 034651422b..a11cd65461 100644 --- a/src/main/java/com/google/devtools/build/lib/buildtool/BuildTool.java +++ b/src/main/java/com/google/devtools/build/lib/buildtool/BuildTool.java @@ -18,6 +18,7 @@ import com.google.common.base.Joiner; import com.google.common.base.Stopwatch; import com.google.common.base.Throwables; import com.google.common.base.Verify; +import com.google.common.collect.ImmutableMap; import com.google.devtools.build.lib.actions.BuildFailedException; import com.google.devtools.build.lib.actions.TestExecException; import com.google.devtools.build.lib.analysis.AnalysisPhaseCompleteEvent; @@ -72,8 +73,11 @@ import com.google.devtools.build.lib.syntax.Type; import com.google.devtools.build.lib.util.AbruptExitException; import com.google.devtools.build.lib.util.ExitCode; import com.google.devtools.build.lib.util.Preconditions; +import com.google.devtools.build.lib.vfs.Path; +import com.google.devtools.build.lib.vfs.PathFragment; import java.util.Collection; +import java.util.Map; import java.util.Set; import java.util.concurrent.TimeUnit; import java.util.logging.Logger; @@ -200,7 +204,7 @@ public final class BuildTool { if (needsExecutionPhase(request.getBuildOptions())) { env.getSkyframeExecutor().injectTopLevelContext(request.getTopLevelArtifactContext()); executionTool.executeBuild(request.getId(), analysisResult, result, - configurations, analysisResult.getPackageRoots()); + configurations, transformPackageRoots(analysisResult.getPackageRoots())); } String delayedErrorMsg = analysisResult.getError(); @@ -295,6 +299,15 @@ public final class BuildTool { } } + private ImmutableMap<PathFragment, Path> transformPackageRoots( + ImmutableMap<PackageIdentifier, Path> packageRoots) { + ImmutableMap.Builder<PathFragment, Path> builder = ImmutableMap.builder(); + for (Map.Entry<PackageIdentifier, Path> entry : packageRoots.entrySet()) { + builder.put(entry.getKey().getPathFragment(), entry.getValue()); + } + return builder.build(); + } + private void reportExceptionError(Exception e) { if (e.getMessage() != null) { getReporter().handle(Event.error(e.getMessage())); diff --git a/src/main/java/com/google/devtools/build/lib/buildtool/ExecutionTool.java b/src/main/java/com/google/devtools/build/lib/buildtool/ExecutionTool.java index c809e64436..a07fb9de73 100644 --- a/src/main/java/com/google/devtools/build/lib/buildtool/ExecutionTool.java +++ b/src/main/java/com/google/devtools/build/lib/buildtool/ExecutionTool.java @@ -61,7 +61,6 @@ import com.google.devtools.build.lib.analysis.config.BuildConfiguration; import com.google.devtools.build.lib.analysis.config.BuildConfigurationCollection; import com.google.devtools.build.lib.buildtool.buildevent.ExecutionPhaseCompleteEvent; import com.google.devtools.build.lib.buildtool.buildevent.ExecutionStartingEvent; -import com.google.devtools.build.lib.cmdline.PackageIdentifier; import com.google.devtools.build.lib.events.Event; import com.google.devtools.build.lib.events.EventHandler; import com.google.devtools.build.lib.events.EventKind; @@ -122,7 +121,6 @@ import java.util.logging.Logger; * @see BuildView */ public class ExecutionTool { - private static class StrategyConverter { private Table<Class<? extends ActionContext>, String, ActionContext> classMap = HashBasedTable.create(); @@ -335,9 +333,9 @@ public class ExecutionTool { * creation */ void executeBuild(UUID buildId, AnalysisResult analysisResult, - BuildResult buildResult, - BuildConfigurationCollection configurations, - Map<PackageIdentifier, Path> packageRoots) + BuildResult buildResult, + BuildConfigurationCollection configurations, + ImmutableMap<PathFragment, Path> packageRoots) throws BuildFailedException, InterruptedException, TestExecException, AbruptExitException { Stopwatch timer = Stopwatch.createStarted(); prepare(packageRoots); @@ -497,7 +495,8 @@ public class ExecutionTool { } } - private void prepare(Map<PackageIdentifier, Path> packageRoots) throws ExecutorInitException { + private void prepare(ImmutableMap<PathFragment, Path> packageRoots) + throws ExecutorInitException { // Prepare for build. Profiler.instance().markPhase(ProfilePhase.PREPARE); @@ -516,12 +515,12 @@ public class ExecutionTool { } } - private void plantSymlinkForest(Map<PackageIdentifier, Path> packageRoots) + private void plantSymlinkForest(ImmutableMap<PathFragment, Path> packageRoots) throws ExecutorInitException { try { - SymlinkForest forest = new SymlinkForest( - packageRoots, getExecRoot(), runtime.getProductName()); - forest.plantLinkForest(); + FileSystemUtils.deleteTreesBelowNotPrefixed(getExecRoot(), + new String[] { ".", "_", runtime.getProductName() + "-"}); + FileSystemUtils.plantLinkForest(packageRoots, getExecRoot(), runtime.getProductName()); } catch (IOException e) { throw new ExecutorInitException("Source forest creation failed", e); } diff --git a/src/main/java/com/google/devtools/build/lib/buildtool/SymlinkForest.java b/src/main/java/com/google/devtools/build/lib/buildtool/SymlinkForest.java deleted file mode 100644 index 4cf27ebf38..0000000000 --- a/src/main/java/com/google/devtools/build/lib/buildtool/SymlinkForest.java +++ /dev/null @@ -1,206 +0,0 @@ -// Copyright 2016 The Bazel Authors. All rights reserved. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. -package com.google.devtools.build.lib.buildtool; - -import com.google.common.collect.Maps; -import com.google.common.collect.Sets; -import com.google.devtools.build.lib.cmdline.PackageIdentifier; -import com.google.devtools.build.lib.cmdline.RepositoryName; -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 java.io.IOException; -import java.util.Map; -import java.util.Set; -import java.util.logging.Level; -import java.util.logging.Logger; - -/** - * Creates a symlink forest based on a package path map. - */ -class SymlinkForest { - - private static final Logger LOG = Logger.getLogger(SymlinkForest.class.getName()); - private static final boolean LOG_FINER = LOG.isLoggable(Level.FINER); - - private final Map<PackageIdentifier, Path> packageRoots; - private final Path workspace; - private final String productName; - - SymlinkForest( - Map<PackageIdentifier, Path> packageRoots, Path workspace, String productName) { - this.packageRoots = packageRoots; - this.workspace = workspace; - this.productName = productName; - } - - /** - * Takes a map of directory fragments to root paths, and creates a symlink - * forest under an existing linkRoot to the corresponding source dirs or - * files. Symlink are made at the highest dir possible, linking files directly - * only when needed with nested packages. - */ - void plantLinkForest() throws IOException { - deleteExisting(); - - // Create a sorted map of all dirs (packages and their ancestors) to sets of their roots. - // Packages come from exactly one root, but their shared ancestors may come from more. - // The map is maintained sorted lexicographically, so parents are before their children. - Map<PackageIdentifier, Set<Path>> dirRootsMap = Maps.newTreeMap(); - for (Map.Entry<PackageIdentifier, Path> entry : packageRoots.entrySet()) { - PackageIdentifier packageIdentifier = entry.getKey(); - PathFragment pkgDir = packageIdentifier.getPackageFragment(); - Path pkgRoot = entry.getValue(); - for (int i = 0; i <= pkgDir.segmentCount(); i++) { - PackageIdentifier dir = PackageIdentifier.create( - packageIdentifier.getRepository(), pkgDir.subFragment(0, i)); - Set<Path> roots = dirRootsMap.get(dir); - if (roots == null) { - roots = Sets.newHashSet(); - dirRootsMap.put(dir, roots); - } - roots.add(pkgRoot); - } - } - // Now add in roots for all non-pkg dirs that are in between two packages, and missed above. - for (Map.Entry<PackageIdentifier, Set<Path>> entry : dirRootsMap.entrySet()) { - PackageIdentifier packageIdentifier = entry.getKey(); - if (!packageRoots.containsKey(packageIdentifier)) { - PackageIdentifier pkgDir = longestPathPrefix(packageIdentifier, packageRoots.keySet()); - if (pkgDir != null) { - entry.getValue().add(packageRoots.get(pkgDir)); - } - } - } - // Create output dirs for all dirs that have more than one root and need to be split. - for (Map.Entry<PackageIdentifier, Set<Path>> entry : dirRootsMap.entrySet()) { - PathFragment dir = entry.getKey().getPathFragment(); - if (entry.getValue().size() > 1) { - if (LOG_FINER) { - LOG.finer("mkdir " + workspace.getRelative(dir)); - } - FileSystemUtils.createDirectoryAndParents(workspace.getRelative(dir)); - } - } - // Make dir links for single rooted dirs. - for (Map.Entry<PackageIdentifier, Set<Path>> entry : dirRootsMap.entrySet()) { - PackageIdentifier pkgId = entry.getKey(); - Path linkRoot = workspace.getRelative(pkgId.getRepository().getPathFragment()); - PathFragment dir = entry.getKey().getPackageFragment(); - Set<Path> roots = entry.getValue(); - // Simple case of one root for this dir. - if (roots.size() == 1) { - // Special case: the main repository is not deleted (because it contains symlinks to - // bazel-out et al) so don't attempt to symlink it in. - if (pkgId.equals(PackageIdentifier.EMPTY_PACKAGE_IDENTIFIER)) { - symlinkEmptyPackage(roots.iterator().next()); - continue; - } - if (dir.segmentCount() > 0) { - PackageIdentifier parent = PackageIdentifier.create( - pkgId.getRepository(), dir.getParentDirectory()); - if (dir.segmentCount() > 0 && dirRootsMap.get(parent).size() == 1) { - continue; // skip--an ancestor will link this one in from above - } - } - - // This is the top-most dir that can be linked to a single root. Make it so. - Path root = roots.iterator().next(); // lone root in set - if (LOG_FINER) { - LOG.finer("ln -s " + root.getRelative(dir) + " " + linkRoot.getRelative(dir)); - } - linkRoot.getRelative(dir).createSymbolicLink(root.getRelative(dir)); - } - } - // Make links for dirs within packages, skip parent-only dirs. - for (Map.Entry<PackageIdentifier, Set<Path>> entry : dirRootsMap.entrySet()) { - Path linkRoot = workspace.getRelative(entry.getKey().getRepository().getPathFragment()); - PathFragment dir = entry.getKey().getPackageFragment(); - if (entry.getValue().size() > 1) { - // If this dir is at or below a package dir, link in its contents. - PackageIdentifier pkgDir = longestPathPrefix(entry.getKey(), packageRoots.keySet()); - if (pkgDir != null) { - Path root = packageRoots.get(pkgDir); - try { - Path absdir = root.getRelative(dir); - if (absdir.isDirectory()) { - if (LOG_FINER) { - LOG.finer("ln -s " + absdir + "/* " + linkRoot.getRelative(dir) + "/"); - } - for (Path target : absdir.getDirectoryEntries()) { - PackageIdentifier dirent = PackageIdentifier.create( - pkgDir.getRepository(), target.relativeTo(root)); - if (!dirRootsMap.containsKey(dirent)) { - linkRoot.getRelative(dirent.getPackageFragment()).createSymbolicLink(target); - } - } - } else { - LOG.fine("Symlink planting skipping dir '" + absdir + "'"); - } - } catch (IOException e) { - e.printStackTrace(); - } - // Otherwise its just an otherwise empty common parent dir. - } - } - } - } - - private void deleteExisting() throws IOException { - FileSystemUtils.createDirectoryAndParents(workspace); - FileSystemUtils.deleteTreesBelowNotPrefixed(workspace, - new String[] { ".", "_", productName + "-"}); - for (Map.Entry<PackageIdentifier, Path> entry : packageRoots.entrySet()) { - RepositoryName repo = entry.getKey().getRepository(); - Path repoPath = workspace.getRelative(repo.getPathFragment()); - if (!repo.isMain() && repoPath.exists()) { - FileSystemUtils.deleteTree(repoPath); - } - } - } - - /** - * For the top-level directory, generate symlinks to everything in the directory instead of the - * directory itself. - */ - private void symlinkEmptyPackage(Path emptyPackagePath) throws IOException { - for (Path target : emptyPackagePath.getDirectoryEntries()) { - String baseName = target.getBaseName(); - // Create any links that don't exist yet and don't start with bazel-. - if (!baseName.startsWith(productName + "-") - && !workspace.getRelative(baseName).exists()) { - workspace.getRelative(baseName).createSymbolicLink(target); - } - } - } - - /** - * Returns the longest prefix from a given set of 'prefixes' that are - * contained in 'path'. I.e the closest ancestor directory containing path. - * Returns null if none found. - */ - static PackageIdentifier longestPathPrefix( - PackageIdentifier packageIdentifier, Set<PackageIdentifier> prefixes) { - PathFragment pkg = packageIdentifier.getPackageFragment(); - for (int i = pkg.segmentCount(); i >= 0; i--) { - PackageIdentifier prefix = PackageIdentifier.create( - packageIdentifier.getRepository(), pkg.subFragment(0, i)); - if (prefixes.contains(prefix)) { - return prefix; - } - } - return null; - } -} diff --git a/src/main/java/com/google/devtools/build/lib/cmdline/Label.java b/src/main/java/com/google/devtools/build/lib/cmdline/Label.java index 75b7b2955f..73c7e5ecdb 100644 --- a/src/main/java/com/google/devtools/build/lib/cmdline/Label.java +++ b/src/main/java/com/google/devtools/build/lib/cmdline/Label.java @@ -65,7 +65,7 @@ public final class Label implements Comparable<Label>, Serializable, SkylarkPrin public static final PackageIdentifier EXTERNAL_PACKAGE_IDENTIFIER = PackageIdentifier.createInMainRepo(EXTERNAL_PACKAGE_NAME); - public static final String EXTERNAL_PATH_PREFIX = ".."; + public static final String EXTERNAL_PATH_PREFIX = "external"; private static final Interner<Label> LABEL_INTERNER = Interners.newWeakInterner(); @@ -310,7 +310,7 @@ public final class Label implements Comparable<Label>, Serializable, SkylarkPrin doc = "Returns the execution root for the workspace of this label, relative to the execroot. " + "For instance:<br>" + "<pre class=language-python>Label(\"@repo//pkg/foo:abc\").workspace_root ==" - + " \"../repo\"</pre>") + + " \"external/repo\"</pre>") public String getWorkspaceRoot() { return packageIdentifier.getRepository().getPathFragment().toString(); } diff --git a/src/main/java/com/google/devtools/build/lib/cmdline/PackageIdentifier.java b/src/main/java/com/google/devtools/build/lib/cmdline/PackageIdentifier.java index 4f0652a981..74cd6fa8d7 100644 --- a/src/main/java/com/google/devtools/build/lib/cmdline/PackageIdentifier.java +++ b/src/main/java/com/google/devtools/build/lib/cmdline/PackageIdentifier.java @@ -48,17 +48,14 @@ public final class PackageIdentifier implements Comparable<PackageIdentifier>, S return INTERNER.intern(new PackageIdentifier(repository, pkgName)); } - static final String DEFAULT_REPOSITORY = ""; - public static final RepositoryName MAIN_REPOSITORY_NAME; + public static final String DEFAULT_REPOSITORY = ""; public static final RepositoryName DEFAULT_REPOSITORY_NAME; - public static final PackageIdentifier EMPTY_PACKAGE_IDENTIFIER; + public static final RepositoryName MAIN_REPOSITORY_NAME; static { try { DEFAULT_REPOSITORY_NAME = RepositoryName.create(DEFAULT_REPOSITORY); MAIN_REPOSITORY_NAME = RepositoryName.create("@"); - EMPTY_PACKAGE_IDENTIFIER = - PackageIdentifier.create(MAIN_REPOSITORY_NAME, PathFragment.EMPTY_FRAGMENT); } catch (LabelSyntaxException e) { throw new IllegalStateException(e); } diff --git a/src/main/java/com/google/devtools/build/lib/cmdline/RepositoryName.java b/src/main/java/com/google/devtools/build/lib/cmdline/RepositoryName.java index eab5320371..6b89a6ee7a 100644 --- a/src/main/java/com/google/devtools/build/lib/cmdline/RepositoryName.java +++ b/src/main/java/com/google/devtools/build/lib/cmdline/RepositoryName.java @@ -205,10 +205,10 @@ public final class RepositoryName implements Serializable { * Returns the runfiles path for this repository (relative to the x.runfiles/main-repo/ * directory). If we don't know the name of this repo (i.e., it is in the main repository), * return an empty path fragment. - * TODO(kchodorow): remove this. */ public PathFragment getRunfilesPath() { - return getPathFragment(); + return isDefault() || isMain() + ? PathFragment.EMPTY_FRAGMENT : new PathFragment("..").getRelative(strippedName()); } /** diff --git a/src/main/java/com/google/devtools/build/lib/packages/Package.java b/src/main/java/com/google/devtools/build/lib/packages/Package.java index ae0915e3cd..44d9e308a3 100644 --- a/src/main/java/com/google/devtools/build/lib/packages/Package.java +++ b/src/main/java/com/google/devtools/build/lib/packages/Package.java @@ -294,7 +294,7 @@ public class Package { this.filename = builder.getFilename(); this.packageDirectory = filename.getParentDirectory(); - this.sourceRoot = getSourceRoot(filename, packageIdentifier.getPackageFragment()); + this.sourceRoot = getSourceRoot(filename, packageIdentifier.getPathFragment()); if ((sourceRoot == null || !sourceRoot.getRelative(packageIdentifier.getPathFragment()).equals(packageDirectory)) && !filename.getBaseName().equals("WORKSPACE")) { diff --git a/src/main/java/com/google/devtools/build/lib/pkgcache/PathPackageLocator.java b/src/main/java/com/google/devtools/build/lib/pkgcache/PathPackageLocator.java index 687ed98fc0..f00606daf4 100644 --- a/src/main/java/com/google/devtools/build/lib/pkgcache/PathPackageLocator.java +++ b/src/main/java/com/google/devtools/build/lib/pkgcache/PathPackageLocator.java @@ -18,7 +18,6 @@ import com.google.common.base.Preconditions; import com.google.common.base.Verify; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableSet; -import com.google.devtools.build.lib.cmdline.Label; import com.google.devtools.build.lib.cmdline.PackageIdentifier; import com.google.devtools.build.lib.events.Event; import com.google.devtools.build.lib.events.EventHandler; @@ -111,11 +110,8 @@ public class PathPackageLocator implements Serializable { // $OUTPUT_BASE/external, which is created by the appropriate RepositoryDirectoryValue. This // is true for the invocation in GlobCache, but not for the locator.getBuildFileForPackage() // invocation in Parser#include(). - Path buildFile = outputBase - .getRelative(Label.EXTERNAL_PACKAGE_NAME) - .getRelative(packageIdentifier.getRepository().strippedName()) - .getRelative(packageIdentifier.getPackageFragment()) - .getRelative("BUILD"); + Path buildFile = outputBase.getRelative( + packageIdentifier.getPathFragment()).getRelative("BUILD"); FileStatus stat = cache.get().statNullable(buildFile, Symlinks.FOLLOW); if (stat != null && stat.isFile()) { return buildFile; diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/JackCompilationHelper.java b/src/main/java/com/google/devtools/build/lib/rules/android/JackCompilationHelper.java index 130f7a87ba..c067d366aa 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/android/JackCompilationHelper.java +++ b/src/main/java/com/google/devtools/build/lib/rules/android/JackCompilationHelper.java @@ -432,11 +432,11 @@ public final class JackCompilationHelper { */ private Artifact postprocessPartialJackAndAddResources( Artifact partialJackLibrary, Artifact resources) { - PathFragment partialPath = new PathFragment( - partialJackLibrary.getRootRelativePath().getPathString() - .replace(PARTIAL_JACK_DIRECTORY, JACK_DIRECTORY)); - Artifact result = ruleContext.getDerivedArtifact( - partialPath, ruleContext.getBinOrGenfilesDirectory()); + Artifact result = ruleContext.getUniqueDirectoryArtifact( + JACK_DIRECTORY, + partialJackLibrary.getRootRelativePath().relativeTo( + ruleContext.getUniqueDirectory(PARTIAL_JACK_DIRECTORY)), + ruleContext.getBinOrGenfilesDirectory()); CustomCommandLine.Builder builder = CustomCommandLine.builder() // Have jack double-check its behavior and crash rather than producing invalid output diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCommon.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCommon.java index fb1ca8c96d..8ccd6e1dea 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCommon.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCommon.java @@ -397,12 +397,9 @@ public final class CcCommon { continue; } PathFragment includesPath = packageFragment.getRelative(includesAttr).normalize(); - // It's okay for the includes path to start with ../workspace-name for external repos. - if ((packageIdentifier.getRepository().isMain() && !includesPath.isNormalized()) - || (!packageIdentifier.getRepository().isMain() - && !includesPath.startsWith(packageIdentifier.getRepository().getPathFragment()))) { + if (!includesPath.isNormalized()) { ruleContext.attributeError("includes", - includesAttr + " references a path above the execution root (" + includesPath + ")."); + "Path references a path above the execution root."); } if (includesPath.segmentCount() == 0) { ruleContext.attributeError( diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileAction.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileAction.java index bb2149e004..5d273fe816 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileAction.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileAction.java @@ -947,12 +947,7 @@ public class CppCompileAction extends AbstractAction // are, it's probably due to a non-hermetic #include, & we should stop // the build with an error. if (execPath.startsWith(execRoot)) { - // funky but tolerable path - execPathFragment = execPath.relativeTo(execRoot); - } else if (execPath.startsWith(execRoot.getParentDirectory())) { - // External repository. - execPathFragment = new PathFragment("..") - .getRelative(execPath.relativeTo(execRoot.getParentDirectory())); + execPathFragment = execPath.relativeTo(execRoot); // funky but tolerable path } else { problems.add(execPathFragment.getPathString()); continue; diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/JavaCompilationHelper.java b/src/main/java/com/google/devtools/build/lib/rules/java/JavaCompilationHelper.java index 9cf27c994d..d0da0c420a 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/java/JavaCompilationHelper.java +++ b/src/main/java/com/google/devtools/build/lib/rules/java/JavaCompilationHelper.java @@ -468,8 +468,7 @@ public final class JavaCompilationHelper extends BaseJavaCompilationHelper { String basename = FileSystemUtils.removeExtension(outputJar.getExecPath().getBaseName()); return getConfiguration().getBinDirectory().getExecPath() .getRelative(ruleContext.getUniqueDirectory("_javac")) - .getRelative(basename + suffix) - .normalize(); + .getRelative(basename + suffix); } /** diff --git a/src/main/java/com/google/devtools/build/lib/rules/python/PyCommon.java b/src/main/java/com/google/devtools/build/lib/rules/python/PyCommon.java index 05e479d980..ae43116519 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/python/PyCommon.java +++ b/src/main/java/com/google/devtools/build/lib/rules/python/PyCommon.java @@ -437,7 +437,7 @@ public final class PyCommon { } PathFragment workspaceName = new PathFragment(ruleContext.getRule().getWorkspaceName()); - return workspaceName.getRelative(mainArtifact.getRunfilesPath()).normalize().getPathString(); + return workspaceName.getRelative(mainArtifact.getRunfilesPath()).getPathString(); } public Artifact getExecutable() { diff --git a/src/main/java/com/google/devtools/build/lib/rules/repository/RepositoryFunction.java b/src/main/java/com/google/devtools/build/lib/rules/repository/RepositoryFunction.java index 7f28eb52f2..043bd0f0df 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/repository/RepositoryFunction.java +++ b/src/main/java/com/google/devtools/build/lib/rules/repository/RepositoryFunction.java @@ -262,7 +262,7 @@ public abstract class RepositoryFunction { /** * Uses a remote repository name to fetch the corresponding Rule describing how to get it. - * + * * This should be the unique entry point for resolving a remote repository function. */ @Nullable @@ -352,7 +352,9 @@ public abstract class RepositoryFunction { } public static Path getExternalRepositoryDirectory(BlazeDirectories directories) { - return directories.getOutputBase().getRelative(Label.EXTERNAL_PACKAGE_NAME); + return directories + .getOutputBase() + .getRelative(Label.EXTERNAL_PATH_PREFIX); } /** diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ExternalFilesHelper.java b/src/main/java/com/google/devtools/build/lib/skyframe/ExternalFilesHelper.java index 9a35fba289..792cc5fa13 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/ExternalFilesHelper.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/ExternalFilesHelper.java @@ -138,7 +138,7 @@ public class ExternalFilesHelper { return FileType.EXTERNAL_MUTABLE; } if (rootedPath.asPath().startsWith(outputBase)) { - Path externalRepoDir = outputBase.getRelative(Label.EXTERNAL_PACKAGE_NAME); + Path externalRepoDir = outputBase.getRelative(Label.EXTERNAL_PATH_PREFIX); if (rootedPath.asPath().startsWith(externalRepoDir)) { anyNonOutputExternalFilesSeen = true; return FileType.EXTERNAL_REPO; diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframePackageLoaderWithValueEnvironment.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframePackageLoaderWithValueEnvironment.java index f8aea5a1b5..604ef07b0c 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframePackageLoaderWithValueEnvironment.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframePackageLoaderWithValueEnvironment.java @@ -74,7 +74,7 @@ class SkyframePackageLoaderWithValueEnvironment implements PackageProviderForCon @Override public void addDependency(Package pkg, String fileName) throws LabelSyntaxException, IOException { RootedPath fileRootedPath = RootedPath.toRootedPath(pkg.getSourceRoot(), - pkg.getPackageIdentifier().getPackageFragment().getRelative(fileName)); + pkg.getPackageIdentifier().getPathFragment().getRelative(fileName)); FileValue result = (FileValue) env.getValue(FileValue.key(fileRootedPath)); if (result != null && !result.exists()) { throw new IOException(); diff --git a/src/main/java/com/google/devtools/build/lib/vfs/FileSystemUtils.java b/src/main/java/com/google/devtools/build/lib/vfs/FileSystemUtils.java index aaac85007c..0facfefde1 100644 --- a/src/main/java/com/google/devtools/build/lib/vfs/FileSystemUtils.java +++ b/src/main/java/com/google/devtools/build/lib/vfs/FileSystemUtils.java @@ -16,6 +16,9 @@ package com.google.devtools.build.lib.vfs; import static java.nio.charset.StandardCharsets.ISO_8859_1; import com.google.common.base.Predicate; +import com.google.common.collect.ImmutableMap; +import com.google.common.collect.Maps; +import com.google.common.collect.Sets; import com.google.common.io.ByteSink; import com.google.common.io.ByteSource; import com.google.common.io.ByteStreams; @@ -32,6 +35,10 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; import java.util.List; +import java.util.Map; +import java.util.Set; +import java.util.logging.Level; +import java.util.logging.Logger; /** * Helper functions that implement often-used complex operations on file @@ -40,6 +47,9 @@ import java.util.List; @ConditionallyThreadSafe // ThreadSafe except for deleteTree. public class FileSystemUtils { + static final Logger LOG = Logger.getLogger(FileSystemUtils.class.getName()); + static final boolean LOG_FINER = LOG.isLoggable(Level.FINER); + private FileSystemUtils() {} /**************************************************************************** @@ -127,6 +137,21 @@ public class FileSystemUtils { } /** + * Returns the longest prefix from a given set of 'prefixes' that are + * contained in 'path'. I.e the closest ancestor directory containing path. + * Returns null if none found. + */ + static PathFragment longestPathPrefix(PathFragment path, Set<PathFragment> prefixes) { + for (int i = path.segmentCount(); i >= 0; i--) { + PathFragment prefix = path.subFragment(0, i); + if (prefixes.contains(prefix)) { + return prefix; + } + } + return null; + } + + /** * Removes the shortest suffix beginning with '.' from the basename of the * filename string. If the basename contains no '.', the filename is returned * unchanged. @@ -651,6 +676,120 @@ public class FileSystemUtils { return true; } + /** + * Takes a map of directory fragments to root paths, and creates a symlink + * forest under an existing linkRoot to the corresponding source dirs or + * files. Symlink are made at the highest dir possible, linking files directly + * only when needed with nested packages. + */ + public static void plantLinkForest(ImmutableMap<PathFragment, Path> packageRootMap, + Path linkRoot, String productName) + throws IOException { + Path emptyPackagePath = null; + + // Create a sorted map of all dirs (packages and their ancestors) to sets of their roots. + // Packages come from exactly one root, but their shared ancestors may come from more. + // The map is maintained sorted lexicographically, so parents are before their children. + Map<PathFragment, Set<Path>> dirRootsMap = Maps.newTreeMap(); + for (Map.Entry<PathFragment, Path> entry : packageRootMap.entrySet()) { + PathFragment pkgDir = entry.getKey(); + Path pkgRoot = entry.getValue(); + if (pkgDir.segmentCount() == 0) { + emptyPackagePath = entry.getValue(); + } + for (int i = 1; i <= pkgDir.segmentCount(); i++) { + PathFragment dir = pkgDir.subFragment(0, i); + Set<Path> roots = dirRootsMap.get(dir); + if (roots == null) { + roots = Sets.newHashSet(); + dirRootsMap.put(dir, roots); + } + roots.add(pkgRoot); + } + } + // Now add in roots for all non-pkg dirs that are in between two packages, and missed above. + for (Map.Entry<PathFragment, Set<Path>> entry : dirRootsMap.entrySet()) { + PathFragment dir = entry.getKey(); + if (!packageRootMap.containsKey(dir)) { + PathFragment pkgDir = longestPathPrefix(dir, packageRootMap.keySet()); + if (pkgDir != null) { + entry.getValue().add(packageRootMap.get(pkgDir)); + } + } + } + // Create output dirs for all dirs that have more than one root and need to be split. + for (Map.Entry<PathFragment, Set<Path>> entry : dirRootsMap.entrySet()) { + PathFragment dir = entry.getKey(); + if (entry.getValue().size() > 1) { + if (LOG_FINER) { + LOG.finer("mkdir " + linkRoot.getRelative(dir)); + } + createDirectoryAndParents(linkRoot.getRelative(dir)); + } + } + // Make dir links for single rooted dirs. + for (Map.Entry<PathFragment, Set<Path>> entry : dirRootsMap.entrySet()) { + PathFragment dir = entry.getKey(); + Set<Path> roots = entry.getValue(); + // Simple case of one root for this dir. + if (roots.size() == 1) { + if (dir.segmentCount() > 1 && dirRootsMap.get(dir.getParentDirectory()).size() == 1) { + continue; // skip--an ancestor will link this one in from above + } + // This is the top-most dir that can be linked to a single root. Make it so. + Path root = roots.iterator().next(); // lone root in set + if (LOG_FINER) { + LOG.finer("ln -s " + root.getRelative(dir) + " " + linkRoot.getRelative(dir)); + } + linkRoot.getRelative(dir).createSymbolicLink(root.getRelative(dir)); + } + } + // Make links for dirs within packages, skip parent-only dirs. + for (Map.Entry<PathFragment, Set<Path>> entry : dirRootsMap.entrySet()) { + PathFragment dir = entry.getKey(); + if (entry.getValue().size() > 1) { + // If this dir is at or below a package dir, link in its contents. + PathFragment pkgDir = longestPathPrefix(dir, packageRootMap.keySet()); + if (pkgDir != null) { + Path root = packageRootMap.get(pkgDir); + try { + Path absdir = root.getRelative(dir); + if (absdir.isDirectory()) { + if (LOG_FINER) { + LOG.finer("ln -s " + absdir + "/* " + linkRoot.getRelative(dir) + "/"); + } + for (Path target : absdir.getDirectoryEntries()) { + PathFragment p = target.relativeTo(root); + if (!dirRootsMap.containsKey(p)) { + //LOG.finest("ln -s " + target + " " + linkRoot.getRelative(p)); + linkRoot.getRelative(p).createSymbolicLink(target); + } + } + } else { + LOG.fine("Symlink planting skipping dir '" + absdir + "'"); + } + } catch (IOException e) { + e.printStackTrace(); + } + // Otherwise its just an otherwise empty common parent dir. + } + } + } + + if (emptyPackagePath != null) { + // For the top-level directory, generate symlinks to everything in the directory instead of + // the directory itself. + for (Path target : emptyPackagePath.getDirectoryEntries()) { + String baseName = target.getBaseName(); + // Create any links that don't exist yet and don't start with bazel-. + if (!baseName.startsWith(productName + "-") + && !linkRoot.getRelative(baseName).exists()) { + linkRoot.getRelative(baseName).createSymbolicLink(target); + } + } + } + } + /**************************************************************************** * Whole-file I/O utilities for characters and bytes. These convenience * methods are not efficient and should not be used for large amounts of data! |