diff options
Diffstat (limited to 'src/main/java')
39 files changed, 224 insertions, 355 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 4378934289..4b7adbc406 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 @@ -139,6 +139,7 @@ public class Artifact throw new ComparisonException("Cannot compare artifact with " + EvalUtils.getDataTypeName(o)); } + /** An object that can expand middleman artifacts. */ public interface ArtifactExpander { @@ -210,6 +211,7 @@ public class Artifact this.hashCode = path.hashCode(); this.path = path; this.root = root; + this.execPath = execPath; // These two lines establish the invariant that // execPath == rootRelativePath <=> execPath.equals(rootRelativePath) // This is important for isSourceArtifact. @@ -219,7 +221,6 @@ public class Artifact + rootRel + " at " + path + " with root " + root); } this.rootRelativePath = rootRel.equals(execPath) ? execPath : rootRel; - this.execPath = externalfy(execPath); this.owner = Preconditions.checkNotNull(owner, path); } @@ -368,12 +369,7 @@ public class Artifact doc = "Returns true if this is a source file, i.e. it is not generated." ) public final boolean isSourceArtifact() { - // All source roots should, you know, point to sources. However, for embedded binaries, they - // are actually created as derived artifacts, so we have to special-case isSourceArtifact to - // treat derived roots in the main repo where execPath==rootRelPath as source roots. - // Source artifacts have reference-identical execPaths and rootRelativePaths, so use - // of == instead of .equals() is intentional here. - return execPath == rootRelativePath || root.isSourceRoot(); + return execPath == rootRelativePath; } /** @@ -544,9 +540,15 @@ 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 externalfy(rootRelativePath); + 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( @@ -580,23 +582,7 @@ public class Artifact + "runfiles of a binary." ) public final String getExecPathString() { - return getExecPath().toString(); - } - - private PathFragment externalfy(PathFragment relativePath) { - if (root.isMainRepo()) { - return relativePath; - } - - PathFragment prefix; - if (root.isSourceRoot()) { - prefix = new PathFragment(Label.EXTERNAL_PATH_PREFIX) - .getRelative(root.getPath().getBaseName()); - } else { - prefix = new PathFragment(Label.EXTERNAL_PATH_PREFIX) - .getRelative(root.getExecRoot().getBaseName()); - } - return prefix.getRelative(relativePath); + return getExecPath().getPathString(); } /* 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 7a30ab70e7..7d187f9900 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 @@ -337,7 +337,7 @@ public class ArtifactFactory implements ArtifactResolver, ArtifactSerializer, Ar dir = repo.getSecond(); } - while (packageRoots != null && dir != null && !dir.equals(baseExecPath)) { + while (dir != null && !dir.equals(baseExecPath)) { Root sourceRoot = packageRoots.get(PackageIdentifier.create(repositoryName, dir)); if (sourceRoot != null) { return sourceRoot; 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 aad6e6e252..59010e9968 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 @@ -15,15 +15,16 @@ package com.google.devtools.build.lib.actions; import com.google.common.annotations.VisibleForTesting; -import com.google.devtools.build.lib.cmdline.Label; import com.google.devtools.build.lib.skylarkinterface.SkylarkCallable; import com.google.devtools.build.lib.skylarkinterface.SkylarkModule; import com.google.devtools.build.lib.skylarkinterface.SkylarkModuleCategory; 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.io.Serializable; import java.util.Objects; + import javax.annotation.Nullable; /** @@ -147,11 +148,7 @@ public final class Root implements Comparable<Root>, Serializable { * the empty fragment. */ public PathFragment getExecPath() { - if (isMainRepo || isSourceRoot()) { - return execPath; - } - return new PathFragment(Label.EXTERNAL_PATH_PREFIX) - .getRelative(execRoot.getBaseName()).getRelative(execPath); + return execPath; } @SkylarkCallable(name = "path", structField = true, diff --git a/src/main/java/com/google/devtools/build/lib/analysis/AnalysisUtils.java b/src/main/java/com/google/devtools/build/lib/analysis/AnalysisUtils.java index ed01807412..1abc8934e4 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/AnalysisUtils.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/AnalysisUtils.java @@ -142,7 +142,7 @@ public final class AnalysisUtils { * <p>For example "//pkg:target" -> "pkg/<fragment>/target. */ public static PathFragment getUniqueDirectory(Label label, PathFragment fragment) { - return label.getPackageIdentifier().getPackageFragment().getRelative(fragment) + return label.getPackageIdentifier().getSourceRoot().getRelative(fragment) .getRelative(label.getName()); } diff --git a/src/main/java/com/google/devtools/build/lib/analysis/CompilationHelper.java b/src/main/java/com/google/devtools/build/lib/analysis/CompilationHelper.java index 7e93108a92..8a1fddc6ff 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/CompilationHelper.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/CompilationHelper.java @@ -21,6 +21,7 @@ import com.google.devtools.build.lib.actions.MiddlemanFactory; import com.google.devtools.build.lib.collect.nestedset.NestedSet; import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder; import com.google.devtools.build.lib.collect.nestedset.Order; + import java.util.List; /** @@ -53,8 +54,8 @@ public final class CompilationHelper { } MiddlemanFactory factory = env.getMiddlemanFactory(); return ImmutableList.of(factory.createMiddlemanAllowMultiple( - env, actionOwner, ruleContext.getRule().getLabel().getPackageIdentifier().getSourceRoot(), - purpose, filesToBuild, ruleContext.getConfiguration().getMiddlemanDirectory( + env, actionOwner, ruleContext.getPackageDirectory(), purpose, filesToBuild, + ruleContext.getConfiguration().getMiddlemanDirectory( ruleContext.getRule().getRepository()))); } @@ -86,8 +87,7 @@ public final class CompilationHelper { MiddlemanFactory factory = env.getMiddlemanFactory(); Iterable<Artifact> artifacts = dep.getProvider(FileProvider.class).getFilesToBuild(); return ImmutableList.of( - factory.createMiddlemanAllowMultiple(env, actionOwner, - ruleContext.getRule().getLabel().getPackageIdentifier().getSourceRoot(), + factory.createMiddlemanAllowMultiple(env, actionOwner, ruleContext.getPackageDirectory(), purpose, artifacts, ruleContext.getConfiguration().getMiddlemanDirectory( ruleContext.getRule().getRepository()))); } 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 8cf2d37c15..e2fbee956e 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 @@ -149,7 +149,9 @@ public final class ConfiguredTargetFactory { : configuration.getGenfilesDirectory(rule.getRepository()); ArtifactOwner owner = new ConfiguredTargetKey(rule.getLabel(), configuration.getArtifactOwnerConfiguration()); - PathFragment rootRelativePath = outputFile.getLabel().toPathFragment(); + PathFragment rootRelativePath = + outputFile.getLabel().getPackageIdentifier().getSourceRoot().getRelative( + outputFile.getLabel().getName()); Artifact result = isFileset ? artifactFactory.getFilesetArtifact(rootRelativePath, root, owner) : artifactFactory.getDerivedArtifact(rootRelativePath, root, owner); @@ -200,7 +202,7 @@ public final class ConfiguredTargetFactory { } else if (target instanceof InputFile) { InputFile inputFile = (InputFile) target; Artifact artifact = artifactFactory.getSourceArtifact( - inputFile.getLabel().toPathFragment(), + inputFile.getExecPath(), Root.asSourceRoot(inputFile.getPackage().getSourceRoot(), inputFile.getPackage().getPackageIdentifier().getRepository().isMain()), new ConfiguredTargetKey(target.getLabel(), config)); diff --git a/src/main/java/com/google/devtools/build/lib/analysis/LocationExpander.java b/src/main/java/com/google/devtools/build/lib/analysis/LocationExpander.java index fc30d6a04d..225b3e14b6 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/LocationExpander.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/LocationExpander.java @@ -332,7 +332,7 @@ public class LocationExpander { for (Artifact artifact : artifacts) { PathFragment execPath = - takeExecPath ? artifact.getExecPath() : artifact.getRunfilesPath(); + takeExecPath ? artifact.getExecPath() : artifact.getRootRelativePath(); if (execPath != null) { // omit middlemen etc paths.add(execPath.getCallablePathString()); } diff --git a/src/main/java/com/google/devtools/build/lib/analysis/RuleContext.java b/src/main/java/com/google/devtools/build/lib/analysis/RuleContext.java index 4efcf035ac..656ca2595f 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/RuleContext.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/RuleContext.java @@ -87,7 +87,6 @@ import com.google.devtools.build.lib.util.OrderedSetMultimap; import com.google.devtools.build.lib.util.Preconditions; import com.google.devtools.build.lib.util.StringUtil; 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.util.ArrayList; import java.util.Collection; @@ -593,19 +592,6 @@ public final class RuleContext extends TargetContext * thus guaranteeing that it never clashes with artifacts created by rules in other packages. */ public Artifact getPackageRelativeArtifact(PathFragment relative, Root root) { - // TODO: other root types, is this never a main repo? - if (relative.startsWith(new PathFragment(Label.EXTERNAL_PATH_PREFIX))) { - // This is an external path, use a different root. - if (root.isSourceRoot()) { - root = Root.asSourceRoot(root.getPath().getRelative(relative.subFragment(0, 2))); - } else { - boolean isMainRepo = false; - Path newExecRoot = root.getExecRoot().getRelative(relative.subFragment(0, 2)); - root = Root.asDerivedRoot( - newExecRoot, newExecRoot.getRelative(root.getExecPath()), isMainRepo); - } - relative = relative.subFragment(2, relative.segmentCount()); - } return getDerivedArtifact(getPackageDirectory().getRelative(relative), root); } @@ -624,7 +610,7 @@ public final class RuleContext extends TargetContext * {@link #getUniqueDirectoryArtifact(String, PathFragment, Root)}) ensures that this is the case. */ public PathFragment getPackageDirectory() { - return getLabel().getPackageIdentifier().getPackageFragment(); + return getLabel().getPackageIdentifier().getSourceRoot(); } /** 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 2e62eaec58..8f171a3a76 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 @@ -427,19 +427,6 @@ public final class Runfiles { return newManifest; } - private static PathFragment getRootRelativePath(Artifact artifact) { - Preconditions.checkArgument(artifact != null); - if (artifact.getRoot().isMainRepo()) { - return artifact.getRootRelativePath(); - } - - return new PathFragment(Label.EXTERNAL_PATH_PREFIX) - .getRelative(artifact.isSourceArtifact() - ? artifact.getRoot().getPath().getBaseName() - : artifact.getRoot().getExecRoot().getBaseName()) - .getRelative(artifact.getRootRelativePath()); - } - /** * Returns the symlinks as a map from PathFragment to Artifact. * @@ -456,7 +443,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, Runfiles.getRootRelativePath(artifact), artifact); + checker.put(manifest, artifact.getRootRelativePath(), artifact); } // Add conditional artifacts (only included if they appear in a pruning manifest). @@ -513,7 +500,7 @@ public final class Runfiles { // workspace. private boolean sawWorkspaceName; - ManifestBuilder( + public ManifestBuilder( PathFragment workspaceName, boolean legacyExternalRunfiles) { this.manifest = new HashMap<>(); this.workspaceName = workspaceName; @@ -524,27 +511,19 @@ 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(entry.getValue())) { - // For empty files (e.g., ../repo/__init__.py), we don't have an artifact so - // isUnderWorkspace returns true. - checker.put(manifest, workspaceName.getRelative(path).normalize(), entry.getValue()); + if (isUnderWorkspace(path)) { sawWorkspaceName = true; + checker.put(manifest, workspaceName.getRelative(path), entry.getValue()); } else { - PathFragment root = entry.getValue().getRoot().getPath().asFragment(); if (legacyExternalRunfiles) { - // external/repo - PathFragment repoDir = root.subFragment(root.segmentCount() - 2, root.segmentCount()); - // Turn ../repo/foo info wsname/external/repo/foo. - checker.put( - manifest, workspaceName.getRelative(repoDir).getRelative(path).normalize(), - entry.getValue()); + checker.put(manifest, workspaceName.getRelative(path), entry.getValue()); } - checker.put( - manifest, workspaceName.getRelative(entry.getKey()).normalize(), entry.getValue()); + // Always add the non-legacy .runfiles/repo/whatever path. + checker.put(manifest, getExternalPath(path), entry.getValue()); } } } @@ -572,20 +551,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(@Nullable Artifact artifact) { - if (artifact == null) { - return true; - } - PathFragment root = artifact.getRoot().getPath().asFragment(); - return root.segmentCount() < 2 - || !root.getSegment(root.segmentCount() - 2).equals( - Label.EXTERNAL_PACKAGE_NAME.getPathString()); + private static boolean isUnderWorkspace(PathFragment path) { + return !path.startsWith(Label.EXTERNAL_PACKAGE_NAME); } } 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 93e9e8d612..b16a4a6bd4 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 @@ -1001,7 +1001,7 @@ public final class BuildConfiguration { /** * Directories in the output tree. * - * <p>The computation of the output directories should be a non-injective mapping from + * <p>The computation of the output directory should be a non-injective mapping from * BuildConfiguration instances to strings. The result should identify the aspects of the * configuration that should be reflected in the output file names. Furthermore the * returned string must not contain shell metacharacters. @@ -1067,15 +1067,15 @@ public final class BuildConfiguration { Root getRoot( RepositoryName repositoryName, String outputDirName, BlazeDirectories directories) { - // e.g., execroot/repo1/../repo2 -> execroot/repo2 - Path execRoot = directories.getExecRoot().getRelative(repositoryName.getPathUnderExecRoot()); - // e.g., execroot/repo2/bazel-out/config + // e.g., execroot/repo1 + Path execRoot = directories.getExecRoot(); + // e.g., execroot/repo1/bazel-out/config/bin Path outputDir = execRoot.getRelative(directories.getRelativeOutputPath()) .getRelative(outputDirName); if (middleman) { return INTERNER.intern(Root.middlemanRoot(execRoot, outputDir, repositoryName.isMain())); } - // e.g., [[execroot/repo2]/bazel-out/config/bin] + // e.g., [[execroot/repo1]/bazel-out/config/bin] return INTERNER.intern( Root.asDerivedRoot(execRoot, outputDir.getRelative(name), repositoryName.isMain())); } diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/python/BazelPythonSemantics.java b/src/main/java/com/google/devtools/build/lib/bazel/rules/python/BazelPythonSemantics.java index f2179c6de1..06fe867fa0 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/rules/python/BazelPythonSemantics.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/python/BazelPythonSemantics.java @@ -83,8 +83,7 @@ public class BazelPythonSemantics implements PythonSemantics { @Override public List<PathFragment> getImports(RuleContext ruleContext) { List<PathFragment> result = new ArrayList<>(); - PathFragment packageFragment = ruleContext.getLabel().getPackageIdentifier() - .getPathUnderExecRoot(); + PathFragment packageFragment = ruleContext.getLabel().getPackageIdentifier().getRunfilesPath(); // Python scripts start with x.runfiles/ as the module space, so everything must be manually // adjusted to be relative to the workspace name. packageFragment = new PathFragment(ruleContext.getWorkspaceName()) 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 686dcd9f97..48bc11c8d9 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 @@ -498,14 +498,8 @@ public class ExecutionTool { // Plant the symlink forest. try { - SymlinkForest.builder() - .setLegacyExternalRunfiles( - request.getOptions(BuildConfiguration.Options.class).legacyExternalRunfiles) - .setPackageRoots(packageRoots) - .setWorkspace(getExecRoot()) - .setProductName(runtime.getProductName()) - .setWorkspaceName(workspaceName) - .build() + new SymlinkForest( + packageRoots, getExecRoot(), runtime.getProductName(), workspaceName) .plantSymlinkForest(); } 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 index 56c40bba20..44cf5f196e 100644 --- a/src/main/java/com/google/devtools/build/lib/buildtool/SymlinkForest.java +++ b/src/main/java/com/google/devtools/build/lib/buildtool/SymlinkForest.java @@ -15,70 +15,54 @@ package com.google.devtools.build.lib.buildtool; import com.google.common.annotations.VisibleForTesting; -import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; -import com.google.common.collect.Iterables; import com.google.common.collect.Maps; import com.google.common.collect.Sets; 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; import com.google.devtools.build.lib.util.Preconditions; 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.Symlinks; + import java.io.IOException; import java.util.Map; import java.util.Set; import java.util.logging.Level; import java.util.logging.Logger; -import javax.annotation.Nullable; -import javax.annotation.concurrent.Immutable; /** * Creates a symlink forest based on a package path map. */ -@Immutable -final class SymlinkForest { +class SymlinkForest { - private static final Logger log = Logger.getLogger(SymlinkForest.class.getName()); - private static final boolean LOG_FINER = log.isLoggable(Level.FINER); + private static final Logger LOG = Logger.getLogger(SymlinkForest.class.getName()); + private static final boolean LOG_FINER = LOG.isLoggable(Level.FINER); private final ImmutableMap<PackageIdentifier, Path> packageRoots; private final Path workspace; private final String workspaceName; private final String productName; private final String[] prefixes; - private final ImmutableSet<RepositoryName> repositories; - private final boolean legacyExternalRunfiles; - private SymlinkForest( - boolean legacyExternalRunfiles, - ImmutableMap<PackageIdentifier, Path> packageRoots, - Path workspace, - String productName, - String[] prefixes, + SymlinkForest( + ImmutableMap<PackageIdentifier, Path> packageRoots, Path workspace, String productName, String workspaceName) { - this.legacyExternalRunfiles = legacyExternalRunfiles; this.packageRoots = packageRoots; this.workspace = workspace; this.workspaceName = workspaceName; this.productName = productName; - this.prefixes = prefixes; - ImmutableSet.Builder<RepositoryName> repositoryNameBuilder = ImmutableSet.builder(); - for (PackageIdentifier pkgId : packageRoots.keySet()) { - repositoryNameBuilder.add(pkgId.getRepository()); - } - this.repositories = repositoryNameBuilder.build(); + this.prefixes = new String[] { ".", "_", productName + "-"}; } /** * 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. + * @param path + * @param prefixes */ @VisibleForTesting static PackageIdentifier longestPathPrefix( @@ -93,43 +77,26 @@ final class SymlinkForest { } /** - * Delete all dir trees under each repository root. For the main repository, don't delete trees - * that start with one of a set of given 'prefixes'. Does not follow any symbolic links. + * Delete all dir trees under a given 'dir' that don't start with one of a set + * of given 'prefixes'. Does not follow any symbolic links. */ @VisibleForTesting @ThreadSafety.ThreadSafe - void deleteTreesBelowNotPrefixed() throws IOException { - for (RepositoryName repositoryName : Iterables.concat( - ImmutableList.of(RepositoryName.MAIN), repositories)) { - Path repositoryExecRoot = workspace.getRelative(repositoryName.getPathUnderExecRoot()); - FileSystemUtils.createDirectoryAndParents(repositoryExecRoot); - dirloop: - for (Path p : repositoryExecRoot.getDirectoryEntries()) { - String name = p.getBaseName(); - for (String prefix : prefixes) { - if (name.startsWith(prefix)) { - continue dirloop; - } + static void deleteTreesBelowNotPrefixed(Path dir, String[] prefixes) throws IOException { + dirloop: + for (Path p : dir.getDirectoryEntries()) { + String name = p.getBaseName(); + for (String prefix : prefixes) { + if (name.startsWith(prefix)) { + continue dirloop; } - FileSystemUtils.deleteTree(p); } + FileSystemUtils.deleteTree(p); } } - private boolean isPackage(PackageIdentifier pkgId) { - return packageRoots.containsKey(pkgId); - } - - /** - * Finds the nearest ancestor package. - */ - @Nullable - private PackageIdentifier findParentPackage(PackageIdentifier pkgId) { - return longestPathPrefix(pkgId, packageRoots.keySet()); - } - void plantSymlinkForest() throws IOException { - deleteTreesBelowNotPrefixed(); + deleteTreesBelowNotPrefixed(workspace, prefixes); // 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. @@ -155,24 +122,23 @@ final class SymlinkForest { // 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 dir = entry.getKey(); - if (!isPackage(dir)) { - PackageIdentifier parentPackage = findParentPackage(dir); - if (parentPackage != null) { - entry.getValue().add(packageRoots.get(parentPackage)); + if (!packageRoots.containsKey(dir)) { + PackageIdentifier pkgId = longestPathPrefix(dir, packageRoots.keySet()); + if (pkgId != null) { + entry.getValue().add(packageRoots.get(pkgId)); } } } // 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()) { PackageIdentifier dir = entry.getKey(); - // Handle creating top level directories for external repositories here, too. if (!dir.getRepository().isMain()) { FileSystemUtils.createDirectoryAndParents( workspace.getRelative(dir.getRepository().getPathUnderExecRoot())); } if (entry.getValue().size() > 1) { if (LOG_FINER) { - log.finer("mkdir " + workspace.getRelative(dir.getPathUnderExecRoot())); + LOG.finer("mkdir " + workspace.getRelative(dir.getPathUnderExecRoot())); } FileSystemUtils.createDirectoryAndParents( workspace.getRelative(dir.getPathUnderExecRoot())); @@ -192,20 +158,43 @@ final class SymlinkForest { // 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.getPackageFragment()) + " " + LOG.finer("ln -s " + root.getRelative(dir.getSourceRoot()) + " " + workspace.getRelative(dir.getPathUnderExecRoot())); } workspace.getRelative(dir.getPathUnderExecRoot()) - .createSymbolicLink(root.getRelative(dir.getPackageFragment())); + .createSymbolicLink(root.getRelative(dir.getSourceRoot())); } } // Make links for dirs within packages, skip parent-only dirs. for (Map.Entry<PackageIdentifier, Set<Path>> entry : dirRootsMap.entrySet()) { - PackageIdentifier child = entry.getKey(); + PackageIdentifier dir = entry.getKey(); if (entry.getValue().size() > 1) { // If this dir is at or below a package dir, link in its contents. - PackageIdentifier parent = longestPathPrefix(child, packageRoots.keySet()); - linkDirectoryEntries(parent, child, dirRootsMap); + PackageIdentifier pkgId = longestPathPrefix(dir, packageRoots.keySet()); + if (pkgId != null) { + Path root = packageRoots.get(pkgId); + try { + Path absdir = root.getRelative(dir.getSourceRoot()); + if (absdir.isDirectory()) { + if (LOG_FINER) { + LOG.finer("ln -s " + absdir + "/* " + + workspace.getRelative(dir.getSourceRoot()) + "/"); + } + for (Path target : absdir.getDirectoryEntries()) { + PathFragment p = target.relativeTo(root); + if (!dirRootsMap.containsKey(createInRepo(pkgId, p))) { + //LOG.finest("ln -s " + target + " " + linkRoot.getRelative(p)); + workspace.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. + } } } @@ -221,56 +210,20 @@ final class SymlinkForest { } // For the top-level directory, generate symlinks to everything in the directory instead of // the directory itself. - for (Path target : entry.getValue().getDirectoryEntries()) { + Path sourceDirectory = entry.getValue().getRelative(pkgId.getSourceRoot()); + for (Path target : sourceDirectory.getDirectoryEntries()) { String baseName = target.getBaseName(); Path execPath = execrootDirectory.getRelative(baseName); // Create any links that don't exist yet and don't start with bazel-. - if (!baseName.startsWith(productName + "-") && !execPath.exists(Symlinks.NOFOLLOW)) { + if (!baseName.startsWith(productName + "-") && !execPath.exists()) { execPath.createSymbolicLink(target); } } } - // Create the external/workspace directory. - if (legacyExternalRunfiles) { - workspace.getRelative(Label.EXTERNAL_PACKAGE_NAME).createSymbolicLink( - workspace.getRelative(Label.EXTERNAL_PATH_PREFIX)); - } symlinkCorrectWorkspaceName(); } - private void linkDirectoryEntries( - PackageIdentifier parent, PackageIdentifier child, - Map<PackageIdentifier, Set<Path>> dirRootsMap) { - if (parent == null) { - // No parent package in packageRoots. - return; - } - Path root = packageRoots.get(parent); - try { - Path absdir = root.getRelative(child.getPackageFragment()); - if (absdir.isDirectory()) { - if (LOG_FINER) { - log.finer("ln -s " + absdir + "/* " - + workspace.getRelative(child.getPathUnderExecRoot()) + "/"); - } - for (Path target : absdir.getDirectoryEntries()) { - PathFragment p = child.getPackageFragment().getRelative(target.getBaseName()); - if (!dirRootsMap.containsKey(createInRepo(parent, p))) { - PathFragment execFragment = child.getPathUnderExecRoot() - .getRelative(target.getBaseName()); - workspace.getRelative(execFragment).createSymbolicLink(target); - } - } - } else { - log.fine("Symlink planting skipping dir '" + absdir + "'"); - } - } catch (IOException e) { - e.printStackTrace(); - } - // Otherwise its just an otherwise empty common parent dir. - } - /** * Right now, the execution root is under the basename of the source directory, not the name * defined in the WORKSPACE file. Thus, this adds a symlink with the WORKSPACE's workspace name @@ -297,55 +250,4 @@ final class SymlinkForest { PackageIdentifier repo, PathFragment packageFragment) { return PackageIdentifier.create(repo.getRepository(), packageFragment); } - - public static Builder builder() { - return new Builder(); - } - - public static class Builder { - private boolean legacyExternalRunfiles = false; - private ImmutableMap<PackageIdentifier, Path> packageRoots = ImmutableMap.of(); - private Path workspace; - private String productName; - private String[] prefixes; - private String workspaceName; - - Builder setLegacyExternalRunfiles(boolean legacyExternalRunfiles) { - this.legacyExternalRunfiles = legacyExternalRunfiles; - return this; - } - - Builder setPackageRoots(ImmutableMap<PackageIdentifier, Path> packageRoots) { - this.packageRoots = packageRoots; - return this; - } - - Builder setWorkspace(Path workspace) { - this.workspace = workspace; - return this; - } - - Builder setProductName(String productName) { - this.productName = productName; - this.prefixes = new String[] { ".", "_", productName + "-"}; - return this; - } - - Builder setPrefixes(String[] prefixes) { - this.prefixes = prefixes; - return this; - } - - Builder setWorkspaceName(String workspaceName) { - this.workspaceName = workspaceName; - return this; - } - - public SymlinkForest build() { - Preconditions.checkState(workspace != null); - Preconditions.checkState(productName != null); - return new SymlinkForest( - legacyExternalRunfiles, packageRoots, workspace, productName, prefixes, workspaceName); - } - } } 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 7e3649102c..0fb7a08b10 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 @@ -72,7 +72,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 = BlazeInterners.newWeakInterner(); @@ -336,12 +336,12 @@ public final class Label implements Comparable<Label>, Serializable, SkylarkPrin * it will returns an empty string. */ @SkylarkCallable(name = "workspace_root", structField = true, - doc = "Returns the execution root for the workspace of this label, relative to the " - + "execroot. For instance:<br>" + 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().getPathUnderExecRoot().toString(); + return packageIdentifier.getRepository().getSourceRoot().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 f8dda81582..1e2e3d0385 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 @@ -20,8 +20,10 @@ import com.google.devtools.build.lib.concurrent.BlazeInterners; import com.google.devtools.build.lib.util.Preconditions; import com.google.devtools.build.lib.vfs.Canonicalizer; import com.google.devtools.build.lib.vfs.PathFragment; + import java.io.Serializable; import java.util.Objects; + import javax.annotation.concurrent.Immutable; /** @@ -125,6 +127,14 @@ public final class PackageIdentifier implements Comparable<PackageIdentifier>, S return repository.getPathUnderExecRoot().getRelative(pkgName); } + /** + * Returns the runfiles/execRoot path for this repository (relative to the x.runfiles/main-repo/ + * directory). + */ + public PathFragment getRunfilesPath() { + return repository.getRunfilesPath().getRelative(pkgName); + } + public PackageIdentifier makeAbsolute() { if (!repository.isDefault()) { return this; 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 5f1421f9c6..ba0f6b407b 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 @@ -22,6 +22,7 @@ import com.google.devtools.build.lib.util.Pair; import com.google.devtools.build.lib.util.StringCanonicalizer; import com.google.devtools.build.lib.util.StringUtilities; import com.google.devtools.build.lib.vfs.PathFragment; + import java.io.IOException; import java.io.ObjectInputStream; import java.io.ObjectOutputStream; @@ -213,8 +214,8 @@ public final class RepositoryName implements Serializable { } /** - * Returns the runfiles/execRoot path for this repository: ../reponame. If we don't know the name - * of this repo (i.e., it is in the main repository), return an empty path fragment. + * Returns the runfiles/execRoot path for this repository. If we don't know the name of this repo + * (i.e., it is in the main repository), return an empty path fragment. */ public PathFragment getPathUnderExecRoot() { return isDefault() || isMain() @@ -223,6 +224,15 @@ public final class RepositoryName implements Serializable { } /** + * Returns the runfiles path relative to the x.runfiles/main-repo directory. + */ + // TODO(kchodorow): remove once execroot is reorg-ed. + public PathFragment getRunfilesPath() { + return isDefault() || isMain() + ? PathFragment.EMPTY_FRAGMENT : new PathFragment("..").getRelative(strippedName()); + } + + /** * Returns the repository name, with leading "{@literal @}" (or "" for the default repository). */ @Override diff --git a/src/main/java/com/google/devtools/build/lib/exec/SymlinkTreeHelper.java b/src/main/java/com/google/devtools/build/lib/exec/SymlinkTreeHelper.java index 758b9dd34c..c5a70c61e8 100644 --- a/src/main/java/com/google/devtools/build/lib/exec/SymlinkTreeHelper.java +++ b/src/main/java/com/google/devtools/build/lib/exec/SymlinkTreeHelper.java @@ -152,8 +152,8 @@ public final class SymlinkTreeHelper { args.add("--use_metadata"); } - args.add(inputManifest.getPathString()); - args.add(symlinkTreeRoot.getPathString()); + args.add(inputManifest.relativeTo(execRoot).getPathString()); + args.add(symlinkTreeRoot.relativeTo(execRoot).getPathString()); return args; } diff --git a/src/main/java/com/google/devtools/build/lib/ideinfo/AndroidStudioInfoAspect.java b/src/main/java/com/google/devtools/build/lib/ideinfo/AndroidStudioInfoAspect.java index e3b351d9a0..e1f31fa84f 100644 --- a/src/main/java/com/google/devtools/build/lib/ideinfo/AndroidStudioInfoAspect.java +++ b/src/main/java/com/google/devtools/build/lib/ideinfo/AndroidStudioInfoAspect.java @@ -654,7 +654,7 @@ public class AndroidStudioInfoAspect extends NativeAspectClass implements Config /** Returns whether this {@link Label} refers to an external repository. */ private static boolean isExternal(Label label) { - return label.getWorkspaceRoot().startsWith(Label.EXTERNAL_PATH_PREFIX); + return label.getWorkspaceRoot().startsWith("external"); } private JavaIdeInfo makeJavaIdeInfo( 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 609a5c3230..45fa5cab50 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 @@ -290,10 +290,9 @@ public class Package { this.filename = builder.getFilename(); this.packageDirectory = filename.getParentDirectory(); - this.sourceRoot = getSourceRoot(filename, packageIdentifier.getPackageFragment()); + this.sourceRoot = getSourceRoot(filename, packageIdentifier.getSourceRoot()); if ((sourceRoot == null - || !sourceRoot.getRelative(packageIdentifier.getPackageFragment()) - .equals(packageDirectory)) + || !sourceRoot.getRelative(packageIdentifier.getSourceRoot()).equals(packageDirectory)) && !filename.getBaseName().equals("WORKSPACE")) { throw new IllegalArgumentException( "Invalid BUILD file name for package '" + packageIdentifier + "': " + filename); diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/LocalResourceContainer.java b/src/main/java/com/google/devtools/build/lib/rules/android/LocalResourceContainer.java index b5aff73920..d4a15834e8 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/android/LocalResourceContainer.java +++ b/src/main/java/com/google/devtools/build/lib/rules/android/LocalResourceContainer.java @@ -27,6 +27,7 @@ import com.google.devtools.build.lib.packages.AttributeMap; import com.google.devtools.build.lib.packages.RuleClass.ConfiguredTargetFactory.RuleErrorException; import com.google.devtools.build.lib.rules.android.ResourceContainer.ResourceType; import com.google.devtools.build.lib.vfs.PathFragment; + import java.util.Collection; import java.util.LinkedHashSet; @@ -162,7 +163,8 @@ public final class LocalResourceContainer { PathFragment assetsDir, Iterable<? extends TransitiveInfoCollection> targets) { for (TransitiveInfoCollection target : targets) { for (Artifact file : target.getProvider(FileProvider.class).getFilesToBuild()) { - PathFragment packageFragment = file.getArtifactOwner().getLabel().getPackageFragment(); + PathFragment packageFragment = file.getArtifactOwner().getLabel() + .getPackageIdentifier().getSourceRoot(); PathFragment packageRelativePath = file.getRootRelativePath().relativeTo(packageFragment); if (packageRelativePath.startsWith(assetsDir)) { @@ -190,7 +192,8 @@ public final class LocalResourceContainer { Artifact lastFile = null; for (FileProvider target : targets) { for (Artifact file : target.getFilesToBuild()) { - PathFragment packageFragment = file.getArtifactOwner().getLabel().getPackageFragment(); + PathFragment packageFragment = file.getArtifactOwner().getLabel() + .getPackageIdentifier().getSourceRoot(); PathFragment packageRelativePath = file.getRootRelativePath().relativeTo(packageFragment); PathFragment resourceDir = findResourceDir(file); 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 e9ef99984f..597e5dd1a8 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 @@ -414,7 +414,7 @@ public final class CcCommon { List<PathFragment> getSystemIncludeDirs() { List<PathFragment> result = new ArrayList<>(); PackageIdentifier packageIdentifier = ruleContext.getLabel().getPackageIdentifier(); - PathFragment pathUnderExecRoot = packageIdentifier.getPathUnderExecRoot(); + PathFragment packageFragment = packageIdentifier.getPathUnderExecRoot(); for (String includesAttr : ruleContext.attributes().get("includes", Type.STRING_LIST)) { includesAttr = ruleContext.expandMakeVariables("includes", includesAttr); if (includesAttr.startsWith("/")) { @@ -422,14 +422,10 @@ public final class CcCommon { "ignoring invalid absolute path '" + includesAttr + "'"); continue; } - PathFragment includesPath = pathUnderExecRoot.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().getPathUnderExecRoot()))) { + PathFragment includesPath = packageFragment.getRelative(includesAttr).normalize(); + 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( @@ -439,7 +435,7 @@ public final class CcCommon { + "' resolves to the workspace root, which would allow this rule and all of its " + "transitive dependents to include any file in your workspace. Please include only" + " what you need"); - } else if (!includesPath.startsWith(pathUnderExecRoot)) { + } else if (!includesPath.startsWith(packageFragment)) { ruleContext.attributeWarning( "includes", "'" @@ -447,14 +443,11 @@ public final class CcCommon { + "' resolves to '" + includesPath + "' not below the relative path of its package '" - + pathUnderExecRoot + + packageFragment + "'. This will be an error in the future"); } result.add(includesPath); - result.add(packageIdentifier.getRepository().getPathUnderExecRoot() - .getRelative(ruleContext.getConfiguration().getGenfilesFragment()) - .getRelative(packageIdentifier.getPackageFragment()) - .getRelative(includesAttr)); + result.add(ruleContext.getConfiguration().getGenfilesFragment().getRelative(includesPath)); } return result; } diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcLibraryHelper.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcLibraryHelper.java index faed073638..11fcd4777a 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcLibraryHelper.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcLibraryHelper.java @@ -1264,7 +1264,7 @@ public final class CcLibraryHelper { ruleContext.getLabel().getPackageIdentifier().getRepository().getPathUnderExecRoot(); contextBuilder.addQuoteIncludeDir(repositoryPath); contextBuilder.addQuoteIncludeDir( - repositoryPath.getRelative(ruleContext.getConfiguration().getGenfilesFragment())); + ruleContext.getConfiguration().getGenfilesFragment().getRelative(repositoryPath)); for (PathFragment systemIncludeDir : systemIncludeDirs) { contextBuilder.addSystemIncludeDir(systemIncludeDir); diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileActionBuilder.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileActionBuilder.java index 7679a99715..cc83edd40f 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileActionBuilder.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileActionBuilder.java @@ -475,14 +475,13 @@ public class CppCompileActionBuilder { continue; } // One starting ../ is okay for getting to a sibling repository. - PathFragment originalInclude = include; if (include.startsWith(new PathFragment(Label.EXTERNAL_PATH_PREFIX))) { include = include.relativeTo(Label.EXTERNAL_PATH_PREFIX); } - if (include.isAbsolute() || !include.normalize().isNormalized()) { + if (include.isAbsolute() + || !PathFragment.EMPTY_FRAGMENT.getRelative(include).normalize().isNormalized()) { ruleContext.ruleError( - "The include path '" + originalInclude - + "' references a path outside of the execution root."); + "The include path '" + include + "' references a path outside of the execution root."); } } } diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppConfiguration.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppConfiguration.java index 1db3f3af5e..f5291b8311 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppConfiguration.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppConfiguration.java @@ -519,6 +519,10 @@ public class CppConfiguration extends BuildConfiguration.Fragment { PathFragment defaultSysroot = toolchain.getBuiltinSysroot().length() == 0 ? null : new PathFragment(toolchain.getBuiltinSysroot()); + if ((defaultSysroot != null) && !defaultSysroot.isNormalized()) { + throw new IllegalArgumentException("The built-in sysroot '" + defaultSysroot + + "' is not normalized."); + } if ((cppOptions.libcTop != null) && (defaultSysroot == null)) { throw new InvalidConfigurationException("The selected toolchain " + toolchainIdentifier @@ -1061,7 +1065,7 @@ public class CppConfiguration extends BuildConfiguration.Fragment { if (packageEndIndex != -1 && s.startsWith(PACKAGE_START)) { String packageString = s.substring(PACKAGE_START.length(), packageEndIndex); try { - pathPrefix = PackageIdentifier.parse(packageString).getPathUnderExecRoot(); + pathPrefix = PackageIdentifier.parse(packageString).getSourceRoot(); } catch (LabelSyntaxException e) { throw new InvalidConfigurationException("The package '" + packageString + "' is not valid"); } diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppHelper.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppHelper.java index 6b3604dd2e..8541778e59 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppHelper.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppHelper.java @@ -491,7 +491,7 @@ public class CppHelper { Preconditions.checkState(Link.SHARED_LIBRARY_FILETYPES.matches(artifact.getFilename())); symlinkedArtifacts.add(isCppRuntime ? SolibSymlinkAction.getCppRuntimeSymlink( - ruleContext, artifact, solibDirOverride, configuration) + ruleContext, artifact, solibDirOverride, configuration) : SolibSymlinkAction.getDynamicLibrarySymlink( ruleContext, artifact, false, true, configuration)); } @@ -500,8 +500,7 @@ public class CppHelper { } return ImmutableList.of( factory.createMiddlemanAllowMultiple(ruleContext.getAnalysisEnvironment(), actionOwner, - ruleContext.getLabel().getPackageIdentifier().getPathUnderExecRoot(), purpose, - artifacts, + ruleContext.getPackageDirectory(), purpose, artifacts, configuration.getMiddlemanDirectory(ruleContext.getRule().getRepository()))); } diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppLinkActionBuilder.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppLinkActionBuilder.java index 3e0b716b58..8fd5728a56 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppLinkActionBuilder.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppLinkActionBuilder.java @@ -50,7 +50,6 @@ import com.google.devtools.build.lib.rules.cpp.Link.Staticness; import com.google.devtools.build.lib.rules.cpp.LinkerInputs.LibraryToLink; import com.google.devtools.build.lib.util.Preconditions; 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.util.ArrayList; import java.util.Collection; @@ -1541,9 +1540,8 @@ public class CppLinkActionBuilder { for (LinkerInput input : linkerInputs) { if (input.getArtifactCategory() == ArtifactCategory.DYNAMIC_LIBRARY) { PathFragment libDir = input.getArtifact().getExecPath().getParentDirectory(); - Path rootPath = input.getArtifact().getRoot().getExecRoot(); Preconditions.checkState( - rootPath.getRelative(libDir).startsWith(rootPath.getRelative(solibDir)), + libDir.startsWith(solibDir), "Artifact '%s' is not under directory '%s'.", input.getArtifact(), solibDir); diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/HeaderDiscovery.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/HeaderDiscovery.java index 847c1f5d4b..5b59a0601f 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/HeaderDiscovery.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/HeaderDiscovery.java @@ -20,7 +20,6 @@ import com.google.devtools.build.lib.actions.Action; import com.google.devtools.build.lib.actions.ActionExecutionException; import com.google.devtools.build.lib.actions.Artifact; import com.google.devtools.build.lib.actions.ArtifactResolver; -import com.google.devtools.build.lib.cmdline.LabelSyntaxException; import com.google.devtools.build.lib.cmdline.RepositoryName; import com.google.devtools.build.lib.collect.nestedset.NestedSet; import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder; @@ -100,10 +99,10 @@ public class HeaderDiscovery { return inputs.build(); } List<Path> systemIncludePrefixes = permittedSystemIncludePrefixes; + // Check inclusions. IncludeProblems problems = new IncludeProblems(); for (Path execPath : depSet.getDependencies()) { - RepositoryName repositoryName = RepositoryName.MAIN; PathFragment execPathFragment = execPath.asFragment(); if (execPathFragment.isAbsolute()) { // Absolute includes from system paths are ignored. @@ -115,24 +114,15 @@ public class HeaderDiscovery { // 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 = execPath.relativeTo(execRoot.getParentDirectory()); - String workspace = execPathFragment.getSegment(0); - execPathFragment = execPathFragment.relativeTo(workspace); - try { - repositoryName = RepositoryName.create("@" + workspace); - } catch (LabelSyntaxException e) { - throw new IllegalStateException(workspace + " is not a valid repository name"); - } + execPathFragment = execPath.relativeTo(execRoot); // funky but tolerable path + } else { + problems.add(execPathFragment.getPathString()); + continue; } } - Artifact artifact = allowedDerivedInputsMap.get( - repositoryName.getPathUnderExecRoot().getRelative(execPathFragment)); + Artifact artifact = allowedDerivedInputsMap.get(execPathFragment); if (artifact == null) { - artifact = artifactResolver.resolveSourceArtifact(execPathFragment, repositoryName); + artifact = artifactResolver.resolveSourceArtifact(execPathFragment, RepositoryName.MAIN); } if (artifact != null) { inputs.add(artifact); diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/proto/CcProtoAspect.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/proto/CcProtoAspect.java index fd77e0cda9..8abcaef9d8 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/proto/CcProtoAspect.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/proto/CcProtoAspect.java @@ -280,7 +280,18 @@ public class CcProtoAspect extends NativeAspectClass implements ConfiguredAspect } private void createProtoCompileAction(SupportData supportData, Collection<Artifact> outputs) { - String genfilesPath = ruleContext.getBinOrGenfilesDirectory().getExecPathString(); + String genfilesPath = + ruleContext + .getConfiguration() + .getGenfilesFragment() + .getRelative( + ruleContext + .getLabel() + .getPackageIdentifier() + .getRepository() + .getPathUnderExecRoot()) + .getPathString(); + ImmutableList.Builder<ToolchainInvocation> invocations = ImmutableList.builder(); invocations.add( new ToolchainInvocation("C++", checkNotNull(getProtoToolchainProvider()), genfilesPath)); diff --git a/src/main/java/com/google/devtools/build/lib/rules/genrule/GenRuleBase.java b/src/main/java/com/google/devtools/build/lib/rules/genrule/GenRuleBase.java index e4cf0da886..a81a38ff1b 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/genrule/GenRuleBase.java +++ b/src/main/java/com/google/devtools/build/lib/rules/genrule/GenRuleBase.java @@ -34,7 +34,6 @@ import com.google.devtools.build.lib.analysis.Runfiles; import com.google.devtools.build.lib.analysis.RunfilesProvider; import com.google.devtools.build.lib.analysis.TransitiveInfoCollection; import com.google.devtools.build.lib.cmdline.Label; -import com.google.devtools.build.lib.cmdline.PackageIdentifier; import com.google.devtools.build.lib.collect.nestedset.NestedSet; import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder; import com.google.devtools.build.lib.collect.nestedset.Order; @@ -337,9 +336,9 @@ public abstract class GenRuleBase implements RuleConfiguredTargetFactory { } else { dir = ruleContext.getConfiguration().getGenfilesFragment(); } - PackageIdentifier pkgId = ruleContext.getRule().getLabel().getPackageIdentifier(); - return pkgId.getRepository().getPathUnderExecRoot().getRelative(dir) - .getRelative(pkgId.getPackageFragment()).getPathString(); + PathFragment relPath = + ruleContext.getRule().getLabel().getPackageIdentifier().getSourceRoot(); + return dir.getRelative(relPath).getPathString(); } } else if (JDK_MAKE_VARIABLE.matcher("$(" + name + ")").find()) { return new ConfigurationMakeVariableContext( diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/JavaCommon.java b/src/main/java/com/google/devtools/build/lib/rules/java/JavaCommon.java index 06f4defac6..344793423e 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/java/JavaCommon.java +++ b/src/main/java/com/google/devtools/build/lib/rules/java/JavaCommon.java @@ -504,7 +504,7 @@ public class JavaCommon { if (launcher != null) { javaExecutable = launcher.getRootRelativePath(); } else { - javaExecutable = ruleContext.getFragment(Jvm.class).getJavaExecutable(); + javaExecutable = ruleContext.getFragment(Jvm.class).getRunfilesJavaExecutable(); } if (!javaExecutable.isAbsolute()) { diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/Jvm.java b/src/main/java/com/google/devtools/build/lib/rules/java/Jvm.java index d4bbad60a2..919538077f 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/java/Jvm.java +++ b/src/main/java/com/google/devtools/build/lib/rules/java/Jvm.java @@ -69,12 +69,7 @@ public final class Jvm extends BuildConfiguration.Fragment { @SkylarkCallable(name = "java_executable", structField = true, doc = "The java executable, i.e. bin/java relative to the Java home.") public PathFragment getJavaExecutable() { - if (jvmLabel == null || jvmLabel.getPackageIdentifier().getRepository().isMain()) { - return java; - } else { - return jvmLabel.getPackageIdentifier().getRepository().getPathUnderExecRoot() - .getRelative(BIN_JAVA); - } + return java; } /** @@ -88,6 +83,17 @@ public final class Jvm extends BuildConfiguration.Fragment { return jvmLabel; } + /** + * If possible, resolves java relative to the jvmLabel's repository. Otherwise, returns the + * same thing as getJavaExecutable(). + */ + public PathFragment getRunfilesJavaExecutable() { + if (jvmLabel == null || jvmLabel.getPackageIdentifier().getRepository().isMain()) { + return getJavaExecutable(); + } + return jvmLabel.getPackageIdentifier().getRepository().getRunfilesPath().getRelative(BIN_JAVA); + } + @Override public void addGlobalMakeVariables(Builder<String, String> globalMakeEnvBuilder) { globalMakeEnvBuilder.put("JAVABASE", getJavaHome().getPathString()); diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/JvmConfigurationLoader.java b/src/main/java/com/google/devtools/build/lib/rules/java/JvmConfigurationLoader.java index e6ae20c98d..6501b31aac 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/java/JvmConfigurationLoader.java +++ b/src/main/java/com/google/devtools/build/lib/rules/java/JvmConfigurationLoader.java @@ -149,7 +149,7 @@ public final class JvmConfigurationLoader implements ConfigurationFragmentFactor if (javaBase.getPackageIdentifier().getRepository().isDefault()) { return javaBase.getPackageFragment(); } - return javaBase.getPackageIdentifier().getPathUnderExecRoot(); + return javaBase.getPackageIdentifier().getSourceRoot(); } private static Jvm createLegacy(String javaHome) throws InvalidConfigurationException { diff --git a/src/main/java/com/google/devtools/build/lib/rules/proto/ProtoCompileActionBuilder.java b/src/main/java/com/google/devtools/build/lib/rules/proto/ProtoCompileActionBuilder.java index 64594ad1e7..fd1f95f02e 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/proto/ProtoCompileActionBuilder.java +++ b/src/main/java/com/google/devtools/build/lib/rules/proto/ProtoCompileActionBuilder.java @@ -340,7 +340,18 @@ public class ProtoCompileActionBuilder { public Iterable<String> argv() { ImmutableList.Builder<String> builder = ImmutableList.builder(); for (Artifact artifact : transitiveImports) { - builder.add("-I" + artifact.getRootRelativePath() + "=" + artifact.getExecPathString()); + builder.add( + "-I" + + artifact + .getRootRelativePath() + .relativeTo( + artifact + .getOwnerLabel() + .getPackageIdentifier() + .getRepository() + .getPathUnderExecRoot()) + + "=" + + artifact.getExecPathString()); } if (protosInDirectDependencies != null) { ArrayList<String> rootRelativePaths = new ArrayList<>(); diff --git a/src/main/java/com/google/devtools/build/lib/sandbox/DarwinSandboxedStrategy.java b/src/main/java/com/google/devtools/build/lib/sandbox/DarwinSandboxedStrategy.java index 13f9cf3877..44a05f055c 100644 --- a/src/main/java/com/google/devtools/build/lib/sandbox/DarwinSandboxedStrategy.java +++ b/src/main/java/com/google/devtools/build/lib/sandbox/DarwinSandboxedStrategy.java @@ -182,12 +182,13 @@ public class DarwinSandboxedStrategy extends SandboxStrategy { ImmutableMap<String, String> spawnEnvironment = StandaloneSpawnStrategy.locallyDeterminedEnv(execRoot, productName, spawn.getEnvironment()); + Set<Path> writableDirs = getWritableDirs(sandboxExecRoot, spawn.getEnvironment()); + Path runUnderPath = getRunUnderPath(spawn); HardlinkedExecRoot hardlinkedExecRoot = new HardlinkedExecRoot(execRoot, sandboxPath, sandboxExecRoot, errWriter); ImmutableSet<PathFragment> outputs = SandboxHelpers.getOutputFiles(spawn); - Set<Path> writableDirs = getWritableDirs(sandboxExecRoot, spawn.getEnvironment(), outputs); try { hardlinkedExecRoot.createFileSystem( getMounts(spawn, actionExecutionContext), outputs, writableDirs); @@ -204,7 +205,7 @@ public class DarwinSandboxedStrategy extends SandboxStrategy { new DarwinSandboxRunner( sandboxPath, sandboxExecRoot, - getWritableDirs(sandboxExecRoot, spawnEnvironment, outputs), + getWritableDirs(sandboxExecRoot, spawnEnvironment), getInaccessiblePaths(), runUnderPath, verboseFailures); @@ -235,12 +236,11 @@ public class DarwinSandboxedStrategy extends SandboxStrategy { } @Override - protected ImmutableSet<Path> getWritableDirs(Path sandboxExecRoot, Map<String, String> env, - ImmutableSet<PathFragment> outputs) { + protected ImmutableSet<Path> getWritableDirs(Path sandboxExecRoot, Map<String, String> env) { FileSystem fs = sandboxExecRoot.getFileSystem(); ImmutableSet.Builder<Path> writableDirs = ImmutableSet.builder(); - writableDirs.addAll(super.getWritableDirs(sandboxExecRoot, env, outputs)); + writableDirs.addAll(super.getWritableDirs(sandboxExecRoot, env)); writableDirs.add(fs.getPath("/dev")); String sysTmpDir = System.getenv("TMPDIR"); diff --git a/src/main/java/com/google/devtools/build/lib/sandbox/LinuxSandboxedStrategy.java b/src/main/java/com/google/devtools/build/lib/sandbox/LinuxSandboxedStrategy.java index 3a7f055b23..e2a81c8af5 100644 --- a/src/main/java/com/google/devtools/build/lib/sandbox/LinuxSandboxedStrategy.java +++ b/src/main/java/com/google/devtools/build/lib/sandbox/LinuxSandboxedStrategy.java @@ -112,9 +112,10 @@ public class LinuxSandboxedStrategy extends SandboxStrategy { Path sandboxExecRoot = sandboxPath.getRelative("execroot").getRelative(execRoot.getBaseName()); Path sandboxTempDir = sandboxPath.getRelative("tmp"); + Set<Path> writableDirs = getWritableDirs(sandboxExecRoot, spawn.getEnvironment()); + SymlinkedExecRoot symlinkedExecRoot = new SymlinkedExecRoot(sandboxExecRoot); ImmutableSet<PathFragment> outputs = SandboxHelpers.getOutputFiles(spawn); - Set<Path> writableDirs = getWritableDirs(sandboxExecRoot, spawn.getEnvironment(), outputs); try { symlinkedExecRoot.createFileSystem( getMounts(spawn, actionExecutionContext), outputs, writableDirs); @@ -123,8 +124,7 @@ public class LinuxSandboxedStrategy extends SandboxStrategy { throw new UserExecException("I/O error during sandboxed execution", e); } - SandboxRunner runner = getSandboxRunner( - spawn, sandboxPath, sandboxExecRoot, sandboxTempDir, outputs); + SandboxRunner runner = getSandboxRunner(spawn, sandboxPath, sandboxExecRoot, sandboxTempDir); try { runSpawn( spawn, @@ -152,8 +152,7 @@ public class LinuxSandboxedStrategy extends SandboxStrategy { } private SandboxRunner getSandboxRunner( - Spawn spawn, Path sandboxPath, Path sandboxExecRoot, Path sandboxTempDir, - ImmutableSet<PathFragment> outputs) + Spawn spawn, Path sandboxPath, Path sandboxExecRoot, Path sandboxTempDir) throws UserExecException { if (fullySupported) { return new LinuxSandboxRunner( @@ -161,7 +160,7 @@ public class LinuxSandboxedStrategy extends SandboxStrategy { sandboxPath, sandboxExecRoot, sandboxTempDir, - getWritableDirs(sandboxExecRoot, spawn.getEnvironment(), outputs), + getWritableDirs(sandboxExecRoot, spawn.getEnvironment()), getInaccessiblePaths(), getTmpfsPaths(), getReadOnlyBindMounts(blazeDirs, sandboxExecRoot), diff --git a/src/main/java/com/google/devtools/build/lib/sandbox/SandboxStrategy.java b/src/main/java/com/google/devtools/build/lib/sandbox/SandboxStrategy.java index 4af3c5d243..ce917f3b78 100644 --- a/src/main/java/com/google/devtools/build/lib/sandbox/SandboxStrategy.java +++ b/src/main/java/com/google/devtools/build/lib/sandbox/SandboxStrategy.java @@ -115,16 +115,12 @@ abstract class SandboxStrategy implements SandboxedSpawnActionContext { } /** Gets the list of directories that the spawn will assume to be writable. */ - protected ImmutableSet<Path> getWritableDirs(Path sandboxExecRoot, Map<String, String> env, - ImmutableSet<PathFragment> outputs) { + protected ImmutableSet<Path> getWritableDirs(Path sandboxExecRoot, Map<String, String> env) { Builder<Path> writableDirs = ImmutableSet.builder(); // We have to make the TEST_TMPDIR directory writable if it is specified. if (env.containsKey("TEST_TMPDIR")) { writableDirs.add(sandboxExecRoot.getRelative(env.get("TEST_TMPDIR"))); } - for (PathFragment output : outputs) { - writableDirs.add(sandboxExecRoot.getRelative(output).getParentDirectory()); - } return writableDirs.build(); } diff --git a/src/main/java/com/google/devtools/build/lib/sandbox/SymlinkedExecRoot.java b/src/main/java/com/google/devtools/build/lib/sandbox/SymlinkedExecRoot.java index 78e050ac42..bc60eae523 100644 --- a/src/main/java/com/google/devtools/build/lib/sandbox/SymlinkedExecRoot.java +++ b/src/main/java/com/google/devtools/build/lib/sandbox/SymlinkedExecRoot.java @@ -92,7 +92,7 @@ public final class SymlinkedExecRoot implements SandboxExecRoot { throws IOException { for (PathFragment inputPath : inputs) { Path dir = sandboxExecRoot.getRelative(inputPath).getParentDirectory(); - Preconditions.checkArgument(dir.startsWith(sandboxExecRoot.getParentDirectory())); + Preconditions.checkArgument(dir.startsWith(sandboxExecRoot)); FileSystemUtils.createDirectoryAndParentsWithCache(createdDirs, dir); } } 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 1a04d15b8c..a8acf8805a 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 @@ -1442,7 +1442,7 @@ public abstract class SkyframeExecutor implements WalkableGraphFactory { EventHandler eventHandler, Label label, BuildConfiguration configuration) { if (memoizingEvaluator.getExistingValueForTesting( PrecomputedValue.WORKSPACE_STATUS_KEY.getKeyForTesting()) == null) { - injectWorkspaceStatusData(label.getPackageIdentifier().getRepository().strippedName()); + injectWorkspaceStatusData(label.getWorkspaceRoot()); } Dependency dep; 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 57f19cfc4c..ceda17193d 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 @@ -75,7 +75,7 @@ class SkyframePackageLoaderWithValueEnvironment implements PackageProviderForCon public void addDependency(Package pkg, String fileName) throws LabelSyntaxException, IOException, InterruptedException { RootedPath fileRootedPath = RootedPath.toRootedPath(pkg.getSourceRoot(), - pkg.getPackageIdentifier().getPackageFragment().getRelative(fileName)); + pkg.getPackageIdentifier().getSourceRoot().getRelative(fileName)); FileValue result = (FileValue) env.getValue(FileValue.key(fileRootedPath)); if (result != null && !result.exists()) { throw new IOException(); |