diff --git a/src/main/java/com/google/devtools/build/lib/analysis/SourceManifestAction.java b/src/main/java/com/google/devtools/build/lib/analysis/SourceManifestAction.java index d9ae7c2bf8bbf2..944c881810f5e0 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/SourceManifestAction.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/SourceManifestAction.java @@ -224,8 +224,12 @@ public enum ManifestType implements ManifestWriter { @Override public void writeEntry(Writer manifestWriter, PathFragment rootRelativePath, Artifact symlink) throws IOException { - manifestWriter.append(rootRelativePath.getPathString()); - // This trailing whitespace is REQUIRED to process the single entry line correctly. + // Replace spaces in the link name by a special character (U+0001). + // This character is very unlikely to be used in a path, and has a single-byte UTF-8 + // encoding, so it's easily replaceable by 'sed' and other shell tools. + String p = rootRelativePath.getPathString().replace(' ', '\u0001'); + manifestWriter.append(p); + // This trailing delimiter is REQUIRED to process the single entry line correctly. manifestWriter.append(' '); if (symlink != null) { manifestWriter.append(symlink.getPath().getPathString()); diff --git a/src/main/tools/build-runfiles-windows.cc b/src/main/tools/build-runfiles-windows.cc index 297116d3106027..25ecb3f98706cd 100644 --- a/src/main/tools/build-runfiles-windows.cc +++ b/src/main/tools/build-runfiles-windows.cc @@ -19,6 +19,7 @@ #include +#include #include #include #include @@ -179,6 +180,8 @@ class RunfilesCreator { link = wline.substr(0, space_pos); target = wline.substr(space_pos + 1); } + // Spaces in the link name are replaced by a special character (\x01). + std::replace(link.begin(), link.end(), '\x01', ' '); // Removing leading and trailing spaces Trim(link); diff --git a/src/main/tools/build-runfiles.cc b/src/main/tools/build-runfiles.cc index 9a2ef97991353a..377ec0f17d385e 100644 --- a/src/main/tools/build-runfiles.cc +++ b/src/main/tools/build-runfiles.cc @@ -53,6 +53,7 @@ #include #include +#include #include #include @@ -154,11 +155,10 @@ class RunfilesCreator { const char *s = strchr(buf, ' '); if (!s) { DIE("missing field delimiter at line %d: '%s'\n", lineno, buf); - } else if (strchr(s+1, ' ')) { - DIE("link or target filename contains space on line %d: '%s'\n", - lineno, buf); } std::string link(buf, s-buf); + // Spaces in the link name are replaced by a special character (\x01). + std::replace(link.begin(), link.end(), '\x01', ' '); const char *target = s+1; if (!allow_relative && target[0] != '\0' && target[0] != '/' && target[1] != ':') { // Match Windows paths, e.g. C:\foo or C:/foo. diff --git a/src/test/shell/bazel/BUILD b/src/test/shell/bazel/BUILD index d0a8d02948c10e..cc76a03a7da734 100644 --- a/src/test/shell/bazel/BUILD +++ b/src/test/shell/bazel/BUILD @@ -788,7 +788,7 @@ sh_test( size = "medium", srcs = ["runfiles_test.sh"], data = [":test-deps"], - tags = ["no_windows"], + deps = ["@bazel_tools//tools/bash/runfiles"], ) sh_test( diff --git a/src/test/shell/bazel/runfiles_test.sh b/src/test/shell/bazel/runfiles_test.sh index 512be362e37792..3fb87b9aab9aed 100755 --- a/src/test/shell/bazel/runfiles_test.sh +++ b/src/test/shell/bazel/runfiles_test.sh @@ -17,11 +17,37 @@ # Test runfiles creation # -# Load the test setup defined in the parent directory -CURRENT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" -source "${CURRENT_DIR}/../integration_test_setup.sh" \ +# --- begin runfiles.bash initialization v2 --- +# Copy-pasted from the Bazel Bash runfiles library v2. +set -uo pipefail; f=bazel_tools/tools/bash/runfiles/runfiles.bash +source "${RUNFILES_DIR:-/dev/null}/$f" 2>/dev/null || \ + source "$(grep -sm1 "^$f " "${RUNFILES_MANIFEST_FILE:-/dev/null}" | cut -f2- -d' ')" 2>/dev/null || \ + source "$0.runfiles/$f" 2>/dev/null || \ + source "$(grep -sm1 "^$f " "$0.runfiles_manifest" | cut -f2- -d' ')" 2>/dev/null || \ + source "$(grep -sm1 "^$f " "$0.exe.runfiles_manifest" | cut -f2- -d' ')" 2>/dev/null || \ + { echo>&2 "ERROR: cannot find $f"; exit 1; }; f=; set -e +# --- end runfiles.bash initialization v2 --- + +source "$(rlocation "io_bazel/src/test/shell/integration_test_setup.sh")" \ || { echo "integration_test_setup.sh not found!" >&2; exit 1; } +case "$(uname -s | tr [:upper:] [:lower:])" in +msys*|mingw*|cygwin*) + declare -r is_windows=true + ;; +*) + declare -r is_windows=false + ;; +esac + +if "$is_windows"; then + export MSYS_NO_PATHCONV=1 + export MSYS2_ARG_CONV_EXCL="*" + export EXT=".exe" +else + export EXT="" +fi + # Make sure runfiles are created under a custom-named subdirectory when # workspace() is specified in the WORKSPACE file. function test_runfiles() { @@ -48,10 +74,10 @@ public class Noise { } EOF - bazel build //foo:foo >& $TEST_log || fail "Build failed" - [[ -d bazel-bin/foo/foo.runfiles/$name ]] || fail "$name runfiles directory not created" - [[ -d bazel-bin/foo/foo.runfiles/$name/foo ]] || fail "No foo subdirectory under $name" - [[ -x bazel-bin/foo/foo.runfiles/$name/foo/foo ]] || fail "No foo executable under $name" + bazel build --enable_runfiles=yes //foo:foo >& $TEST_log || fail "Build failed" + [[ -d "bazel-bin/foo/foo${EXT}.runfiles/$name" ]] || fail "$name runfiles directory not created" + [[ -d "bazel-bin/foo/foo${EXT}.runfiles/$name/foo" ]] || fail "No foo subdirectory under $name" + [[ -x "bazel-bin/foo/foo${EXT}.runfiles/$name/foo/foo" ]] || fail "No foo executable under $name" } function test_legacy_runfiles_change() { @@ -77,19 +103,19 @@ EOF cat > thing.cc < $TEST_log \ + bazel build --enable_runfiles=yes --legacy_external_runfiles //:thing &> $TEST_log \ || fail "Build failed" - [[ -d bazel-bin/thing.runfiles/foo/external/bar ]] \ + [[ -d bazel-bin/thing${EXT}.runfiles/foo/external/bar ]] \ || fail "bar not found" - bazel build --nolegacy_external_runfiles //:thing &> $TEST_log \ + bazel build --enable_runfiles=yes --nolegacy_external_runfiles //:thing &> $TEST_log \ || fail "Build failed" - [[ ! -d bazel-bin/thing.runfiles/foo/external/bar ]] \ + [[ ! -d bazel-bin/thing${EXT}.runfiles/foo/external/bar ]] \ || fail "Old bar still found" - bazel build --legacy_external_runfiles //:thing &> $TEST_log \ + bazel build --enable_runfiles=yes --legacy_external_runfiles //:thing &> $TEST_log \ || fail "Build failed" - [[ -d bazel-bin/thing.runfiles/foo/external/bar ]] \ + [[ -d bazel-bin/thing${EXT}.runfiles/foo/external/bar ]] \ || fail "bar not recreated" } @@ -122,19 +148,85 @@ sh_test( ) EOF - bazel build --spawn_strategy=local --nobuild_runfile_links //:test \ + bazel build --spawn_strategy=local --enable_runfiles=yes --nobuild_runfile_links //:test \ || fail "Building //:test failed" - [[ ! -f bazel-bin/test.runfiles/foo/data/hello ]] || fail "expected no runfile data/hello" - [[ ! -f bazel-bin/test.runfiles/foo/data/world ]] || fail "expected no runfile data/world" - [[ ! -f bazel-bin/test.runfiles/MANIFEST ]] || fail "expected output manifest to not exist" + [[ ! -f "bazel-bin/test${EXT}.runfiles/foo/data/hello" ]] || fail "expected no runfile data/hello" + [[ ! -f "bazel-bin/test${EXT}.runfiles/foo/data/world" ]] || fail "expected no runfile data/world" + [[ ! -f "bazel-bin/test${EXT}.runfiles/MANIFEST" ]] || fail "expected output manifest to not exist" - bazel test --spawn_strategy=local --nobuild_runfile_links //:test \ + bazel test --spawn_strategy=local --enable_runfiles=yes --nobuild_runfile_links --test_output=errors //:test \ || fail "Testing //:foo failed" - [[ -f bazel-bin/test.runfiles/foo/data/hello ]] || fail "expected runfile data/hello to exist" - [[ -f bazel-bin/test.runfiles/foo/data/world ]] || fail "expected runfile data/world to exist" - [[ -f bazel-bin/test.runfiles/MANIFEST ]] || fail "expected output manifest to exist" + [[ -f "bazel-bin/test${EXT}.runfiles/foo/data/hello" ]] || fail "expected runfile data/hello to exist" + [[ -f "bazel-bin/test${EXT}.runfiles/foo/data/world" ]] || fail "expected runfile data/world to exist" + [[ -f "bazel-bin/test${EXT}.runfiles/MANIFEST" ]] || fail "expected output manifest to exist" +} + +function test_space_in_runfile_path() { + mkdir -p 'foo/a a' # one space + mkdir -p 'repo/b b/c c' # more than one space + touch 'foo/x.txt' # no space + touch 'foo/a a/y.txt' # space in runfile link and target path + touch 'repo/b b/z.txt' # space in target path only + touch 'repo/b b/c c/w.txt' # space in runfile link and target path in ext.repo + touch 'repo/b b/WORKSPACE' + cat >'repo/b b/BUILD' <<'eof' +filegroup( + name = "files", + srcs = glob(["**"]), + visibility = ["//visibility:public"], +) +eof + cat >WORKSPACE <<'eof' +workspace(name = "foo_ws") +local_repository( + name = "space_ws", + path = "repo/b b", +) +eof + cat >BUILD <<'eof' +sh_binary( + name = "x", + srcs = ["x.sh"], + data = ["@space_ws//:files"] + glob(["foo/**"]), + deps = ["@bazel_tools//tools/bash/runfiles"], +) +eof + cat >x.sh <<'eof' +#!/bin/bash +# --- begin runfiles.bash initialization v2 --- +# Copy-pasted from the Bazel Bash runfiles library v2. +set -uo pipefail; f=bazel_tools/tools/bash/runfiles/runfiles.bash +source "${RUNFILES_DIR:-/dev/null}/$f" 2>/dev/null || \ + source "$(grep -sm1 "^$f " "${RUNFILES_MANIFEST_FILE:-/dev/null}" | cut -f2- -d' ')" 2>/dev/null || \ + source "$0.runfiles/$f" 2>/dev/null || \ + source "$(grep -sm1 "^$f " "$0.runfiles_manifest" | cut -f2- -d' ')" 2>/dev/null || \ + source "$(grep -sm1 "^$f " "$0.exe.runfiles_manifest" | cut -f2- -d' ')" 2>/dev/null || \ + { echo>&2 "ERROR: cannot find $f"; exit 1; }; f=; set -e +# --- end runfiles.bash initialization v2 --- +echo "Hello from x.sh" +echo "x=($(rlocation "foo_ws/foo/x.txt"))" +echo "y=($(rlocation "foo_ws/foo/a a/y.txt"))" +echo "z=($(rlocation "space_ws/z.txt"))" +echo "w=($(rlocation "space_ws/c c/w.txt"))" +eof + chmod +x x.sh + + # Look up runfiles using the runfiles manifest + bazel run --enable_runfiles=yes --nobuild_runfile_links //:x >&"$TEST_log" + expect_log "Hello from x.sh" + expect_log "^x=(.*foo/x.txt)" + expect_log "^y=(.*foo/a a/y.txt)" + expect_log "^z=(.*space_ws/z.txt)" + expect_log "^w=(.*space_ws/c c/w.txt)" + + # See if runfiles links are generated + bazel build --enable_runfiles=yes --build_runfile_links //:x >&"$TEST_log" + [[ -e "bazel-bin/x${EXT}.runfiles/foo_ws/foo/x.txt" ]] || fail "Cannot find x.txt" + [[ -e "bazel-bin/x${EXT}.runfiles/foo_ws/foo/a a/y.txt" ]] || fail "Cannot find y.txt" + [[ -e "bazel-bin/x${EXT}.runfiles/space_ws/z.txt" ]] || fail "Cannot find z.txt" + [[ -e "bazel-bin/x${EXT}.runfiles/space_ws/c c/w.txt" ]] || fail "Cannot find w.txt" } run_suite "runfiles tests" diff --git a/tools/bash/runfiles/runfiles.bash b/tools/bash/runfiles/runfiles.bash index d3aa0c3d238d14..9c1fdbaa129eb2 100644 --- a/tools/bash/runfiles/runfiles.bash +++ b/tools/bash/runfiles/runfiles.bash @@ -68,7 +68,7 @@ # cat "$(rlocation my_workspace/path/to/my/data.txt)" # -if [[ ! -d "${RUNFILES_DIR:-/dev/null}" && ! -f "${RUNFILES_MANIFEST_FILE:-/dev/null}" ]]; then +if [[ ! -d "${RUNFILES_DIR:-/dev/null/x}" && ! -f "${RUNFILES_MANIFEST_FILE:-/dev/null/x}" ]]; then if [[ -f "$0.runfiles_manifest" ]]; then export RUNFILES_MANIFEST_FILE="$0.runfiles_manifest" elif [[ -f "$0.runfiles/MANIFEST" ]]; then @@ -112,17 +112,18 @@ function rlocation() { fi return 1 else - if [[ -e "${RUNFILES_DIR:-/dev/null}/$1" ]]; then + if [[ -e "${RUNFILES_DIR:-/dev/null/x}/$1" ]]; then if [[ "${RUNFILES_LIB_DEBUG:-}" == 1 ]]; then echo >&2 "INFO[runfiles.bash]: rlocation($1): found under RUNFILES_DIR ($RUNFILES_DIR), return" fi echo "${RUNFILES_DIR}/$1" - elif [[ -f "${RUNFILES_MANIFEST_FILE:-/dev/null}" ]]; then + elif [[ -f "${RUNFILES_MANIFEST_FILE:-/dev/null/x}" ]]; then if [[ "${RUNFILES_LIB_DEBUG:-}" == 1 ]]; then echo >&2 "INFO[runfiles.bash]: rlocation($1): looking in RUNFILES_MANIFEST_FILE ($RUNFILES_MANIFEST_FILE)" fi - local -r result=$(grep -m1 "^$1 " "${RUNFILES_MANIFEST_FILE}" | cut -d ' ' -f 2-) - if [[ -e "${result:-/dev/null}" ]]; then + # Spaces in the link name are replaced by a special character (U+0001). + local -r result=$(grep -m1 "^$(echo "$1" | tr ' ' '\001') " "${RUNFILES_MANIFEST_FILE}" | cut -d ' ' -f 2-) + if [[ -e "${result:-/dev/null/x}" ]]; then if [[ "${RUNFILES_LIB_DEBUG:-}" == 1 ]]; then echo >&2 "INFO[runfiles.bash]: rlocation($1): found in manifest as ($result)" fi @@ -151,12 +152,12 @@ export -f rlocation # libraries under @bazel_tools//tools//runfiles, then that binary needs # these envvars in order to initialize its own runfiles library. function runfiles_export_envvars() { - if [[ ! -f "${RUNFILES_MANIFEST_FILE:-/dev/null}" \ - && ! -d "${RUNFILES_DIR:-/dev/null}" ]]; then + if [[ ! -f "${RUNFILES_MANIFEST_FILE:-/dev/null/x}" \ + && ! -d "${RUNFILES_DIR:-/dev/null/x}" ]]; then return 1 fi - if [[ ! -f "${RUNFILES_MANIFEST_FILE:-/dev/null}" ]]; then + if [[ ! -f "${RUNFILES_MANIFEST_FILE:-/dev/null/x}" ]]; then if [[ -f "$RUNFILES_DIR/MANIFEST" ]]; then export RUNFILES_MANIFEST_FILE="$RUNFILES_DIR/MANIFEST" elif [[ -f "${RUNFILES_DIR}_manifest" ]]; then @@ -164,7 +165,7 @@ function runfiles_export_envvars() { else export RUNFILES_MANIFEST_FILE= fi - elif [[ ! -d "${RUNFILES_DIR:-/dev/null}" ]]; then + elif [[ ! -d "${RUNFILES_DIR:-/dev/null/x}" ]]; then if [[ "$RUNFILES_MANIFEST_FILE" == */MANIFEST \ && -d "${RUNFILES_MANIFEST_FILE%/MANIFEST}" ]]; then export RUNFILES_DIR="${RUNFILES_MANIFEST_FILE%/MANIFEST}"