Windows, VFS: stat treats junctions as symlinks
FileStatus.isSpecialFile() no longer returns true
for junctions.
This allows FileSystem.direntFromStat(FileStatus)
to return SYMLINK instead of UNKNOWN for
junctions, allowing recursive glob() to traverse
junctions.
Fixes https://github.com/bazelbuild/bazel/issues/9176
Closes #9269.
Change-Id: I533f353c1915dcec232943fc2799b99f6ef9de05
PiperOrigin-RevId: 270669100
diff --git a/src/main/java/com/google/devtools/build/lib/windows/WindowsFileSystem.java b/src/main/java/com/google/devtools/build/lib/windows/WindowsFileSystem.java
index 5b3669b..77b78fd 100644
--- a/src/main/java/com/google/devtools/build/lib/windows/WindowsFileSystem.java
+++ b/src/main/java/com/google/devtools/build/lib/windows/WindowsFileSystem.java
@@ -136,17 +136,20 @@
new FileStatus() {
@Override
public boolean isFile() {
- return attributes.isRegularFile() || (isSpecialFile() && !isDirectory());
+ return !isSymbolicLink && (attributes.isRegularFile() || isSpecialFile());
}
@Override
public boolean isSpecialFile() {
- return attributes.isOther();
+ // attributes.isOther() returns false for symlinks but returns true for junctions.
+ // Bazel treats junctions like symlinks. So let's return false here for junctions.
+ // This fixes https://github.com/bazelbuild/bazel/issues/9176
+ return !isSymbolicLink && attributes.isOther();
}
@Override
public boolean isDirectory() {
- return attributes.isDirectory();
+ return !isSymbolicLink && attributes.isDirectory();
}
@Override
diff --git a/src/test/shell/bazel/new_local_repo_test.sh b/src/test/shell/bazel/new_local_repo_test.sh
index c41f65b..92b62f7 100755
--- a/src/test/shell/bazel/new_local_repo_test.sh
+++ b/src/test/shell/bazel/new_local_repo_test.sh
@@ -96,18 +96,54 @@
echo "dummy" >$pkg/B/a.txt
# Build 1: g.txt should contain external/B/a.txt
- cd $pkg/A
- bazel build //:G || fail "build failed"
- cat bazel-genfiles/g.txt >$TEST_log
+ ( cd $pkg/A
+ bazel build //:G
+ cat bazel-genfiles/g.txt >$TEST_log
+ )
expect_log "external/B/a.txt"
expect_not_log "external/B/b.txt"
# Build 2: add B/b.txt and see if the glob picks it up.
- echo "dummy" > ../B/b.txt
- bazel build //:G || fail "build failed"
- cat bazel-genfiles/g.txt >$TEST_log
+ # Shut down the server afterwards so the test cleanup can remove $pkg/A.
+ ( cd $pkg/A
+ echo "dummy" > ../B/b.txt
+ bazel build //:G || fail "build failed"
+ cat bazel-genfiles/g.txt >$TEST_log
+ bazel shutdown >& /dev/null
+ )
expect_log "external/B/a.txt"
expect_log "external/B/b.txt"
}
+# Regression test for https://github.com/bazelbuild/bazel/issues/9176
+function test_recursive_glob_in_new_local_repository() {
+ local -r pkg="${FUNCNAME[0]}"
+ mkdir -p "$pkg/A" "$pkg/B/subdir/inner"
+ touch "$pkg/B/root.txt"
+ touch "$pkg/B/subdir/outer.txt"
+ touch "$pkg/B/subdir/inner/inner.txt"
+ cat >"$pkg/A/WORKSPACE" <<eof
+new_local_repository(
+ name = "myext",
+ path = "../B",
+ build_file = "BUILD.myext",
+)
+eof
+ cat >"$pkg/A/BUILD.myext" <<eof
+filegroup(name = "all_files", srcs = glob(["**"]))
+eof
+
+ # Shut down the server afterwards so the test cleanup can remove $pkg/A.
+ ( cd "$pkg/A"
+ bazel query 'deps(@myext//:all_files)' >& "$TEST_log"
+ bazel shutdown >& /dev/null
+ )
+ expect_log '@myext//:all_files'
+ expect_log '@myext//:subdir/outer.txt'
+ expect_log '@myext//:subdir/inner/inner.txt'
+ expect_log '@myext//:root.txt'
+ expect_log '@myext//:WORKSPACE'
+ expect_log '@myext//:BUILD.bazel'
+}
+
run_suite "new_local_repository correctness tests"
diff --git a/src/test/shell/integration/loading_phase_test.sh b/src/test/shell/integration/loading_phase_test.sh
index 7c738cc..9c37b01 100755
--- a/src/test/shell/integration/loading_phase_test.sh
+++ b/src/test/shell/integration/loading_phase_test.sh
@@ -424,4 +424,52 @@
expect_not_log "IllegalArgumentException"
}
+# Regression test for https://github.com/bazelbuild/bazel/issues/9176
+function test_windows_only__glob_with_junction() {
+ if ! $is_windows; then
+ echo "Skipping $FUNCNAME because execution platform is not Windows"
+ return
+ fi
+
+ mkdir -p foo/bar foo2
+ touch foo/bar/x.txt
+ touch foo/a.txt
+ touch foo2/b.txt
+ cat >BUILD <<eof
+filegroup(name = 'x', srcs = glob(["foo/**"]))
+filegroup(name = 'y', srcs = glob(["foo2/**"]))
+eof
+ # Create junction foo2/bar2 -> foo/bar
+ cmd.exe /C mklink /J foo2\\bar2 foo\\bar >NUL
+
+ bazel query 'deps(//:x)' >& "$TEST_log"
+ expect_log "//:x"
+ expect_log "//:foo/a.txt"
+ expect_log "//:foo/bar/x.txt"
+
+ bazel query 'deps(//:y)' >& "$TEST_log"
+ expect_log "//:y"
+ expect_log "//:foo2/b.txt"
+ expect_log "//:foo2/bar2/x.txt"
+}
+
+# Regression test for https://github.com/bazelbuild/bazel/pull/9269#issuecomment-531221290
+# Verify that bazel-bin and the other bazel-* symlinks are not treated as
+# packages when expanding the "//..." pattern.
+function test_bazel_bin_is_not_a_package() {
+ local -r pkg="${FUNCNAME[0]}"
+ mkdir "$pkg" || fail "Could not mkdir $pkg"
+ echo "filegroup(name = '$pkg')" > "$pkg/BUILD"
+
+ # Ensure bazel-<pkg> is created.
+ bazel build --symlink_prefix="foo_prefix-" "//$pkg" || fail "build failed"
+ [[ -d "foo_prefix-bin" ]] || fail "bazel-bin was not created"
+
+ # Assert that "//..." does not expand to //foo_prefix-*
+ bazel query //... >& "$TEST_log"
+ expect_log_once "//$pkg:$pkg"
+ expect_log_once "//.*:$pkg"
+ expect_not_log "//foo_prefix"
+}
+
run_suite "Integration tests of ${PRODUCT_NAME} using loading/analysis phases."
