Remove circular symlinks to external dependencies
Fixes #87.
--
MOS_MIGRATED_REVID=91784426
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/repository/RepositoryFunction.java b/src/main/java/com/google/devtools/build/lib/bazel/repository/RepositoryFunction.java
index 0ccf02e..e581252 100644
--- a/src/main/java/com/google/devtools/build/lib/bazel/repository/RepositoryFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/bazel/repository/RepositoryFunction.java
@@ -27,10 +27,8 @@
import com.google.devtools.build.lib.skyframe.FileSymlinkCycleException;
import com.google.devtools.build.lib.skyframe.FileValue;
import com.google.devtools.build.lib.skyframe.InconsistentFilesystemException;
-import com.google.devtools.build.lib.skyframe.PackageFunction;
import com.google.devtools.build.lib.skyframe.PackageValue;
import com.google.devtools.build.lib.syntax.EvalException;
-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.RootedPath;
@@ -48,7 +46,6 @@
* Parent class for repository-related Skyframe functions.
*/
public abstract class RepositoryFunction implements SkyFunction {
- private static final String EXTERNAL_REPOSITORY_DIRECTORY = ".external-repository";
private BlazeDirectories directories;
@Override
@@ -73,7 +70,7 @@
}
public static Path getExternalRepositoryDirectory(BlazeDirectories directories) {
- return directories.getOutputBase().getRelative(EXTERNAL_REPOSITORY_DIRECTORY);
+ return directories.getOutputBase().getRelative(ExternalPackage.NAME);
}
/**
@@ -107,7 +104,7 @@
RepositoryName repositoryName, @Nullable String ruleClassName, Environment env)
throws RepositoryFunctionException {
SkyKey packageKey = PackageValue.key(
- PackageIdentifier.createInDefaultRepo(PackageFunction.EXTERNAL_PACKAGE_NAME));
+ PackageIdentifier.createInDefaultRepo(ExternalPackage.NAME));
PackageValue packageValue;
try {
packageValue = (PackageValue) env.getValueOrThrow(packageKey,
@@ -115,7 +112,7 @@
} catch (NoSuchPackageException e) {
throw new RepositoryFunctionException(
new BuildFileNotFoundException(
- PackageFunction.EXTERNAL_PACKAGE_NAME, "Could not load //external package"),
+ ExternalPackage.NAME, "Could not load //external package"),
Transience.PERSISTENT);
}
if (packageValue == null) {
@@ -126,7 +123,7 @@
if (rule == null) {
throw new RepositoryFunctionException(
new BuildFileContainsErrorsException(
- PackageFunction.EXTERNAL_PACKAGE_NAME,
+ ExternalPackage.NAME,
"The repository named '" + repositoryName + "' could not be resolved"),
Transience.PERSISTENT);
}
@@ -138,9 +135,6 @@
/**
* Adds the repository's directory to the graph and, if it's a symlink, resolves it to an
* actual directory.
- *
- * <p>Also creates a symlink from x/external/x to x, where x is the directory containing a
- * WORKSPACE file. This is used in the execution root.</p>
*/
@Nullable
protected static FileValue getRepositoryDirectory(Path repositoryDirectory, Environment env)
@@ -156,20 +150,6 @@
new IOException("Could not access " + repositoryDirectory + ": " + e.getMessage()),
Transience.PERSISTENT);
}
-
- String targetName = repositoryDirectory.getBaseName();
- try {
- Path backlink = repositoryDirectory.getRelative("external").getRelative(targetName);
- FileSystemUtils.createDirectoryAndParents(backlink.getParentDirectory());
- if (backlink.exists()) {
- backlink.delete();
- }
- backlink.createSymbolicLink(repositoryDirectory);
- } catch (IOException e) {
- throw new RepositoryFunctionException(new IOException(
- "Error creating execution root symlink for " + targetName + ": " + e.getMessage()),
- Transience.TRANSIENT);
- }
return value;
}
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 4565dc6..b6328a3 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
@@ -110,7 +110,7 @@
/**
* The root of the source tree in which this package was found. It is an invariant that
- * {@code sourceRoot.getRelative(name).equals(packageDirectory)}.
+ * {@code sourceRoot.getRelative(packageId.getPathFragment()).equals(packageDirectory)}.
*/
private Path sourceRoot;
@@ -312,9 +312,9 @@
defaultRestrictedTo = environments;
}
- public static Path getSourceRoot(Path buildFile, PathFragment nameFragment) {
+ private static Path getSourceRoot(Path buildFile, PathFragment packageFragment) {
Path current = buildFile.getParentDirectory();
- for (int i = 0, len = nameFragment.segmentCount(); i < len && current != null; i++) {
+ for (int i = 0, len = packageFragment.segmentCount(); i < len && current != null; i++) {
current = current.getParentDirectory();
}
return current;
@@ -341,12 +341,12 @@
this.filename = builder.filename;
this.packageDirectory = filename.getParentDirectory();
- this.sourceRoot = getSourceRoot(filename, nameFragment);
+ this.sourceRoot = getSourceRoot(filename, packageIdentifier.getPathFragment());
if ((sourceRoot == null
- || !sourceRoot.getRelative(nameFragment).equals(packageDirectory))
+ || !sourceRoot.getRelative(packageIdentifier.getPathFragment()).equals(packageDirectory))
&& !filename.getBaseName().equals("WORKSPACE")) {
throw new IllegalArgumentException(
- "Invalid BUILD file name for package '" + name + "': " + filename);
+ "Invalid BUILD file name for package '" + packageIdentifier + "': " + filename);
}
this.makeEnv = builder.makeEnv.build();
@@ -395,7 +395,7 @@
* Returns the source root (a directory) beneath which this package's BUILD file was found.
*
* Assumes invariant:
- * {@code getSourceRoot().getRelative(getName()).equals(getPackageDirectory())}
+ * {@code getSourceRoot().getRelative(packageId.getPathFragment()).equals(getPackageDirectory())}
*/
public Path getSourceRoot() {
return sourceRoot;
diff --git a/src/test/shell/bazel/external_integration_test.sh b/src/test/shell/bazel/external_integration_test.sh
index c68a818..aaa066a 100755
--- a/src/test/shell/bazel/external_integration_test.sh
+++ b/src/test/shell/bazel/external_integration_test.sh
@@ -406,6 +406,7 @@
EOF
chmod +x zoo/female.sh
+ bazel clean --expunge
bazel run //zoo:breeding-program >& $TEST_log \
|| echo "Expected build/run to succeed"
kill_nc
diff --git a/src/test/shell/bazel/local_repository_test.sh b/src/test/shell/bazel/local_repository_test.sh
index 6c77b5b..e891487 100755
--- a/src/test/shell/bazel/local_repository_test.sh
+++ b/src/test/shell/bazel/local_repository_test.sh
@@ -366,4 +366,59 @@
expect_log "@x-repo//x to //a:a"
}
+function test_external_includes() {
+ clib=$TEST_TMPDIR/clib
+ mkdir -p $clib/include
+ cat > $clib/include/clib.h <<EOF
+int x();
+EOF
+ cat > $clib/clib.cc <<EOF
+#include "clib.h"
+int x() {
+ return 3;
+}
+EOF
+ cat > $clib/BUILD <<EOF
+cc_library(
+ name = "clib",
+ srcs = ["clib.cc"],
+ hdrs = glob(["**/*.h"]),
+ includes = ["include"],
+ visibility = ["//visibility:public"],
+)
+EOF
+
+ cat > WORKSPACE <<EOF
+local_repository(
+ name = "clib-repo",
+ path = "$clib",
+)
+
+bind(
+ name = "clib",
+ actual = "@clib-repo//:clib"
+)
+EOF
+ cat > BUILD <<EOF
+cc_binary(
+ name = "printer",
+ srcs = ["printer.cc"],
+ deps = ["//external:clib"],
+)
+EOF
+ cat > printer.cc <<EOF
+#include <stdio.h>
+
+#include "clib.h"
+
+int main() {
+ printf("My number is %d\n", x());
+ return 0;
+}
+EOF
+
+ bazel run //:printer >& $TEST_log || fail "Running //:printer failed"
+ expect_log "My number is 3"
+}
+
run_suite "local repository tests"
