C++ header validation fails for MSVC remote compile #19733
Comments
|
With Bzlmod, the workspace name will always be the fixed (and relatively short) string |
|
@jayconrod Can you check if this is still a problem with Bzlmod turned on? |
|
Bzlmod does indeed help. The build succeeded after:
That's not really a satisfying solution though: many of our customers aren't using bzlmod yet, and Bazel shouldn't be sensitive to the execroot directory path anyway. I don't think there's a way for Bazel to easily discover the execroot path from a remote action, nor is there a way for the remote server to discover the workspace name. I was thinking about a solution where we "solve" for the execroot path from the compiler output? Basically, take the strings whose suffixes are the allowed include files, trim the common prefix? |
Sorry, I don't quite understand this, can you explain this by an example? Thanks! |
|
Sure. Let's say the compiler gives you output like this (omitting lines that match system include prefixes): Allowed header paths including workspace names might be: Dropping the workspace name from the path: We can match those suffixes against the compiler output to determine that I'm unsure about a lot of details here, particular whether this would work for external repositories and bzlmod. WDYT? |
How will this work exactly? What if there are multiple matches? For example, the compiler output is and allowed header path is
I think Bzlmod should only make it easier. For headers in the main repo, the path should be So if |
|
I spent some time sketching out a function in Python-like pseudocode that would do what I mean. Given a set of absolute include paths, a predicate that checks whether an absolute path is a system include, and a predicate that checks whether an execroot-relative path is a valid source file, it should infer the execroot prefix. Assuming that |
|
@jayconrod Thanks for writing up the code! But sorry I'll have to check this later, this week I'll focus on flipping Bzlmod by default before Bazel 7 cut. |
|
Fair enough, that's important. I'll ping this issue after BazelCon. Again, I'd be happy to write this up as a real fix, but I want to make sure this approach sounds reasonable before I spend time on that. I've prepared a workaround today: instead of invoking |
|
@jayconrod Thanks!! |
A single file is too simple. Also, fix the toolchain to work around bazelbuild/bazel#19733.
The Windows MSVC toolchain now invokes a wrapper binary when compiling
instead of cl.exe. The wrapper binary invokes cl.exe with all arguments
but filters /showincludes output, removing EngFlow execroot paths.
For example, the line:
Note: including file: C:\worker\work\2\exec\cpp\hello.h
is replaced with:
Note: including file: cpp\hello.h
This is a workaround for bazelbuild/bazel#19733. Bazel invokes the compiler
with /showincludes to dump all included headers in order to validate them.
It strips anything that looks like a local execroot directory followed by
the workspace name before performing that validation. EngFlow execroot
directories don't look like local execroot directories though. We also don't
know Bazel's workspace name, so we can't work around this issue on the
server side. Using a wrapper like this instead makes the CppCompile action
slightly more hermetic by filtering out those absolute paths.
For linear/CUS-81
We'll open source this wrapper tool in a follow-up.
Also: change line endings in cpp/main.cc to be LF instead of CRLF.
#244) The Windows MSVC toolchain now invokes a wrapper binary when compiling instead of cl.exe. The wrapper binary invokes cl.exe with all arguments but filters /showincludes output, removing EngFlow execroot paths. For example, the line: Note: including file: C:\worker\work\2\exec\cpp\hello.h is replaced with: Note: including file: cpp\hello.h This is a workaround for bazelbuild/bazel#19733. Bazel invokes the compiler with /showincludes to dump all included headers in order to validate them. It strips anything that looks like a local execroot directory followed by the workspace name before performing that validation. EngFlow execroot directories don't look like local execroot directories though. We also don't know Bazel's workspace name, so we can't work around this issue on the server side. Using a wrapper like this instead makes the CppCompile action slightly more hermetic by filtering out those absolute paths. For linear/CUS-81 We'll open source this wrapper tool in a follow-up. Also: change line endings in cpp/main.cc to be LF instead of CRLF.
This is a wrapper around the cl.exe compiler for Windows MSVC toolchains. It works around bazelbuild/bazel#19733 by filtering EngFlow execroot directories out of MSVC's /showincludes output so Bazel does not see them. The package documentation explains this is more detail. For linear/PE-1127, linear/CUS-81, linear/CUS-64, linear/PE-1434
This is a wrapper around the cl.exe compiler for Windows MSVC toolchains. It works around bazelbuild/bazel#19733 by filtering EngFlow execroot directories out of MSVC's /showincludes output so Bazel does not see them. The package documentation explains this is more detail. For linear/PE-1127, linear/CUS-81, linear/CUS-64, linear/PE-1434
This is a wrapper around the cl.exe compiler for Windows MSVC toolchains. It works around bazelbuild/bazel#19733 by filtering EngFlow execroot directories out of MSVC's /showincludes output so Bazel does not see them. The package documentation explains this is more detail. For linear/PE-1127, linear/CUS-81, linear/CUS-64, linear/PE-1434
comius
added
P3
|
I stumbled across that issue while experimenting with BRE from linux to windows and using MSVC compiler. I basically use https://github.com/EngFlow/example as basis to compile a simple hello_world which only incudes FAILURE: Only pass the include directory - this SHOULD work IMHO SUCCESS: Passing the explicit path to the header Is this related? |
|
Today I took some time to debug about regarding what bazel is actually doing. I would then expect which again should return However, when debugging, I can see that |
|
@TimotheusBachinger If you host machine is linux, indeed, Bazel won't be able to handle the paths correctly for Windows.. In general, cross platform builds between Windows and unix don't work well. |
|
@meteorcloudy hum... so I am misunderstanding the whole concept? I assumed the path inclusion validation is executed on my (windows) remote executor? |
Description of the bug:
When building a
cc_librarythat includes header files on Windows using an MSVC toolchain with remote execution, I get this error:Which category does this issue belong to?
C++/Objective-C Rules
What's the simplest, easiest way to reproduce this bug? Please provide a minimal example if possible.
You will need a remote execution service with Windows that can execute actions in containers. I think that makes this difficult to reproduce, though maybe Google has something internally that might work? I'm working on this at EngFlow.
Assuming you have that:
bazel build --config=remote_windows_x64 //cpp:cpp_lib(add--remote_executorand any other necessary flags).Which operating system are you running Bazel on?
Windows Server 2022
What is the output of
bazel info release?release 6.3.2
If
bazel info releasereturnsdevelopment versionor(@non-git), tell us how you built Bazel.n/a
What's the output of
git remote get-url origin; git rev-parse master; git rev-parse HEAD?Is this a regression? If yes, please try to identify the Bazel commit where the bug was introduced.
No.
Have you found anything relevant by searching the web?
Issue #9172 - Windows: Bazel cannot share cpp cache between two projects with different workspace name
PR #9188 - Windows: Ignore workspace name differences while doing header checking
PR #13339 - Remote: Use execRoot as input root and do NOT set working directory by default.
Any other information, logs, or outputs that you want to share?
I spent the morning poking through the source code to figure out how this works. It looks like Bazel's
CppCompileaction is trying to validate that 1) headers included fromsrcsare in eithersrcsor direct dependencies'hdrs, and 2) headers are not included with absolute paths.For MSVC, Bazel accomplishes that by enabling the
/showincludesflag, which tells MSVC to print the path of each included file. The paths are absolute. The full output from one of these actions looks like this:This line is the one that matters:
This is not hermetic. The path of the execroot could be anything. Here's it's
C:\worker\work\2\execroot\ws. Normally it would beC:\worker\work\2\exec, but I've hacked it in this build to try and work around the issue (no luck).In ShowIncludeFilter, Bazel uses a regular expression to replace everything up to
.*execroot\\with../, so that turns into../ws/cpp/hello.h. Later, that's joined with the local execroot within the workspace, soC:/_b/gjo2whp5/execroot/ws/cpp/hello.h, which is the path we see in the error message.Bazel expects the path to be
C:/_b/gjo2whp5/execroot/example/cpp/hello.h(exampleis the name of the workspace). Because it does not match, Bazel reports an error.Tagging @meteorcloudy and @coeuvre who I think might have ideas on how to fix this. I'd be happy to work on this, but I'll need some guidance on what the solution should be and whether there's a good way to test it.
The text was updated successfully, but these errors were encountered: