Rollback of commit 4b73e972d909bcd533f2f9940f95a00b9b73bdde.
*** Reason for rollback ***
Broke tests on CI: http://ci.bazel.io/job/bazel-tests/570/
*** Original change description ***
Roll forward execroot change
RELNOTES[INC]: Previously, an external repository would be symlinked into the
execution root at execroot/local_repo/external/remote_repo. This changes it to
be at execroot/remote_repo. This may break genrules/Skylark actions that
hardcode execution root paths. If this causes breakages for you, ensure that
genrules are using $(location :target) to access files and Skylark rules are
using http://bazel.io/docs/skylark/lib/File.html's path, dirname, etc.
functions. Cust...
--
PiperOrigin-RevId: 147833177
MOS_MIGRATED_REVID=147833177
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 4378934..4b7adbc 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 @@
throw new ComparisonException("Cannot compare artifact with " + EvalUtils.getDataTypeName(o));
}
+
/** An object that can expand middleman artifacts. */
public interface ArtifactExpander {
@@ -210,6 +211,7 @@
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 @@
+ 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 @@
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 @@
* 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 @@
+ "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 7a30ab7..7d187f9 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 @@
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 aad6e6e..59010e9 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 @@
* 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 ed01807..1abc893 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 @@
* <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 7e93108..8a1fddc 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.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 @@
}
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 @@
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 8cf2d37..e2fbee9 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 @@
: 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 @@
} 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 fc30d6a..225b3e1 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 @@
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 4efcf03..656ca25 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.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 @@
* 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 @@
* {@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 2e62eae..8f171a3 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 @@
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 @@
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 @@
// workspace.
private boolean sawWorkspaceName;
- ManifestBuilder(
+ public ManifestBuilder(
PathFragment workspaceName, boolean legacyExternalRunfiles) {
this.manifest = new HashMap<>();
this.workspaceName = workspaceName;
@@ -524,27 +511,19 @@
/**
* 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 @@
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 93e9e8d..b16a4a6 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 @@
/**
* 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 @@
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 f2179c6..06fe867 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 @@
@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 686dcd9..48bc11c 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 @@
// 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 56c40bb..44cf5f1 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 @@
}
/**
- * 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 @@
// 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 @@
// 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 @@
}
// 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 @@
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 7e36491..0fb7a08 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 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 @@
* 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 f8dda81..1e2e3d0 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.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 @@
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 5f1421f..ba0f6b4 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.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 @@
}
/**
- * 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 @@
}
/**
+ * 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 758b9dd..c5a70c6 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 @@
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 e3b351d..e1f31fa 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 @@
/** 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 609a5c3..45fa5ca 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 @@
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 b5aff73..d4a1583 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.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 @@
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 @@
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 e9ef999..597e5dd 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 @@
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 @@
"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 @@
+ "' 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 @@
+ "' 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 faed073..11fcd47 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 @@
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 7679a99..cc83edd 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 @@
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 1db3f3a..f5291b8 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 @@
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 @@
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 6b3604d..8541778 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 @@
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 @@
}
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 3e0b716..8fd5728 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.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 @@
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 847c1f5..5b59a06 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.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 @@
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 @@
// 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 fd77e0c..8abcaef 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 @@
}
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 e4cf0da..a81a38f 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.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 @@
} 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 06f4def..3447934 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 @@
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 d4bbad6..9195380 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 @@
@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 @@
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 e6ae20c..6501b31 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 @@
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 64594ad..fd1f95f 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 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 13f9cf3..44a05f0 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 @@
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 @@
new DarwinSandboxRunner(
sandboxPath,
sandboxExecRoot,
- getWritableDirs(sandboxExecRoot, spawnEnvironment, outputs),
+ getWritableDirs(sandboxExecRoot, spawnEnvironment),
getInaccessiblePaths(),
runUnderPath,
verboseFailures);
@@ -235,12 +236,11 @@
}
@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 3a7f055..e2a81c8 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 @@
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 @@
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 @@
}
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 @@
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 4af3c5d..ce917f3 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 @@
}
/** 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 78e050a..bc60eae 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 @@
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 1a04d15..a8acf88 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 @@
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 57f19cf..ceda171 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 @@
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();
