bazel icon indicating copy to clipboard operation
bazel copied to clipboard

If test bin is not found, try to use @bazel_tools

Open futt-bucker opened this issue 3 years ago • 8 comments
trafficstars

Fixes https://github.com/bazelbuild/bazel/issues/15835

In short: on windows if you run bazel coverage :mytest, bazel fails because test_wrapper (tw.exe) cannot resolve path to collect_coverage.sh. It expects this script to be under runfiles directory. But runfiles on Windows are not enabled by default, thus this script will not be where it is expected to be.

We solve the problem by checking - if a test_path does not exist, then we use $(cwd)/$(test_path) - which will point to @bazel_tools under execroot/.../external/bazel_tools/...

Before this PR: bazel coverage ... fails because collect_coverage.sh was not found under runfiles dir. After this PR: bazel coverage ... fails because collect_coverage.sh is not Win32 application (it's a bash script). So the script is found, but cannot be interpreted (will be solved in next PRs)

futt-bucker avatar Jul 07 '22 17:07 futt-bucker

Bump

futt-bucker avatar Sep 15 '22 14:09 futt-bucker

@lberki Can you take a look?

futt-bucker avatar Sep 15 '22 14:09 futt-bucker

@c-mita Please also look.

futt-bucker avatar Sep 24 '22 09:09 futt-bucker

We solve the problem by checking - if a test_path does not exist, then we use $(cwd)/$(test_path) - which will point to @bazel_tools under execroot/.../external/bazel_tools/...

This sounds very brittle, can you instead use the runfiles library to look up the script, just check all the Rlocation usages in the code.

meteorcloudy avatar Dec 16 '22 15:12 meteorcloudy

We solve the problem by checking - if a test_path does not exist, then we use (cwd)/(test_path) - which will point to @bazel_tools under execroot/.../external/bazel_tools/...

This sounds very brittle, can you instead use the runfiles library to look up the script, just check all the Rlocation usages in the code.

That's the point - Rlocation returns a path which does not exist. So this additional check ensures path to test script is correct.

Warchant avatar Dec 16 '22 17:12 Warchant

If the runfiles library doesn't return the right path, then something isn't right at a higher level and we should fix that. What is the path that Rlocation returns and what would the correct path be?

fmeum avatar Dec 16 '22 19:12 fmeum

If the runfiles library doesn't return the right path, then something isn't right at a higher level and we should fix that. What is the path that Rlocation returns and what would the correct path be?

If you run bazel coverage ... -s on a bazel project with gtest on Windows, all tests fail with this error:

ERROR(tools/test/windows/tw.cc:1294) ERROR: src/main/native/windows/process.cc(202): CreateProcessW("C:\ba\execroot\bazel_playground\bazel-out\x64_windows-fastbuild\bin\cpp\test\gmock_example.exe.runfiles\bazel_playground\external\bazel_tools\tools\test\collect_coverage.sh"  cpp/test/gmock_example.exe): The system cannot find the file specified.

My .bazelrc:

startup --output_user_root=C:/b
startup --output_base=C:/ba

Pay attention that path to collect_coverage.sh is under runfiles (gmock_example.exe.runfiles) directory. By default, runfiles are disabled on windows, and runfiles dir is always empty (it will only contain MANIFEST):

$ tree /F C:\ba\execroot\bazel_playground\bazel-out\x64_windows-fastbuild\bin\cpp\test\gmock_example.exe.runfiles
Folder PATH listing for volume Acer
Volume serial number is C432-FCCE
C:\BA\EXECROOT\BAZEL_PLAYGROUND\BAZEL-OUT\X64_WINDOWS-FASTBUILD\BIN\CPP\TEST\GMOCK_EXAMPLE.EXE.RUNFILES
│   MANIFEST
│   
└───bazel_playground

bazel_playground - is an empty folder. (it's a name of my project).


If I add these 2 to my .bazelrc:

startup --windows_enable_symlinks
build --enable_runfiles

Then bazel_playground is not empty dir, it has this structure:

$ tree /F C:\ba\execroot\bazel_playground\bazel-out\x64_windows-fastbuild\bin\cpp\test\test_for_cpp_file.exe.runfiles
Folder PATH listing for volume Acer
Volume serial number is C432-FCCE
C:\BA\EXECROOT\BAZEL_PLAYGROUND\BAZEL-OUT\X64_WINDOWS-FASTBUILD\BIN\CPP\TEST\TEST_FOR_CPP_FILE.EXE.RUNFILES   
│   MANIFEST
│
└───bazel_playground
    └───cpp
        └───test
                test_for_cpp_file.exe

But collect_coverage.sh is still not there.

If you drill down to the code, the issue is that Rlocation returns a path that is missing (path to a file collect_coverage.sh).


We will omit the problem that collect_coverage.sh is a bash script which can't be run on Windows, it is a different issue: https://github.com/bazelbuild/bazel/issues/15835

Warchant avatar Dec 17 '22 11:12 Warchant

Rlocation falls back to using the runfiles directory if it can't find the file in the runfiles manifest.

The problem seems to be that collect_coverage.sh isn't added to the runfiles of the test action and the test runner action looks it up by its exec path, not the runfiles path. This differs from the real test binary, which is available as a runfile: https://github.com/bazelbuild/bazel/blob/658ba15d9f37969edfaae507d267ee3aaba8b44e/src/main/java/com/google/devtools/build/lib/analysis/test/TestStrategy.java#L230

Given that the coverage collection script is added as an input, your path logic seems correct to look something up that only lives under the execroot. But doing that in all cases seems overly broad, maybe guard it behind the COVERAGE_DIR variable?

fmeum avatar Dec 18 '22 07:12 fmeum

Closing, because there was no agreement Amon reviewers and because it’s now pretty stale.

comius avatar Nov 27 '23 11:11 comius