Compile Line Length Limit hit under Windows

[adam-porich-sm]

Bugs: what's the simplest, easiest way to reproduce this bug? Please provide a minimal example if possible.

The following build file does not compile under windows

cc_library(
    name = "long",
    includes = ["a" * 32768]
)
cc_binary(
    name = "long_bin",
    srcs = ["main.cpp"],
    deps = [
        ":long"
    ]
)

The reported error is
Action failed to execute: java.io.IOException: ERROR: src/main/native/windows/processes-jni.cc(161): CreateProcessWithExplicitHandles("C:\Program Files (x86)\Microsoft Visual Studio 14.0\VC\bin\amd64\cl.exe" /c main.cpp /Fobazel-out/x64_windows-fastbuild/bin/_objs/long_bin/main.o /nologo /DCOMPILER_MSVC /DNOMINMAX /D_WIN32_WINNT=0x0(...)): command is longer than CreateProcessW's limit (32768 characters)

From what I could tell when trying to make a simple repo it appears this case may be handled correctly for link steps but not for compile steps.

What operating system are you running Bazel on?

Windows 10

What's the output of bazel info release?

release 0.13.0

[RNabel]

This is a known issue. Did you check whether #4149 helps in your case?

[adam-porich-sm]

Good point, but unfortunately doesn't help in my case.

Under Windows there are 3 main relevant limitations

1. Filename paths can't be longer than MAX_PATH (260) characters
2. Command-line lengths can't be longer than 32,768 characters
3. Response-file lengths can't be longer than 131,072 characters

This example breaches both 1 and 2. But the problem in my actual code is breaching 2. We have many dependent libraries with manually set include paths (On the migration path to remove them, but we aren't there yet). This results in extremely long compiler command lines, which without de-duplication (which bazel handles nicely) will overflow the response-file limits breaching the third limitation.

There are two solutions that I see. One is that the manually adding include paths appears too add two include paths one for generated files which often don't exist. And the other is to support param files the same way they are supported for link command lines.

I think ideally in the long-run both solutions would be implemented. I had a go at the param files but due to bazel supporting things like remote execution I needed a better understanding of bazel internals than I currently have.

[laszlocsomor]

There are two solutions that I see. One is that the manually adding include paths appears too add two include paths one for generated files which often don't exist. And the other is to support param files the same way they are supported for link command lines.

I'd like to reply but I'm not sure I parse this correctly, could you please explain?

Let me /cc @tomlu who's been drastically improving the parameter file handling lately. Thanks to his work it'll be easier to create parameter files, which may interest you @adamporich.

[gimo]

The only real solution is supporting parameter file handling for C++ compiler arguments. Which is currently supported for C++ linkers.

Also, a significant mitigation for the issue is that for every "includes" entry in a "cc_library" invocation I get two directories added to the compiler command line. One for the include directory in the source folder and one mirror into the "genfiles" tree. In most cases for me a library has no "genfiles" headers and so the second entry is irrelevant.

[laszlocsomor]

I see, thanks for the clarification!

The only real solution is supporting parameter file handling for C++ compiler arguments. Which is currently supported for C++ linkers.

Do you mean VC++ linker or Clang/GCC? FYI VC++ is the default compiler on Windows. Do you know whether its compiler supports parameter files? If so, then @tomlu 's work will enable updating the C++ rules so they can create parameter files on demand. (Adding @meteorcloudy, the Windows C++ compilation expert and @mhlopko the overall expert on C++ rules.)

Also, a significant mitigation for the issue is that for every "includes" entry in a "cc_library" invocation I get two directories added to the compiler command line. One for the include directory in the source folder and one mirror into the "genfiles" tree. In most cases for me a library has no "genfiles" headers and so the second entry is irrelevant.

Right, I understand that that's an unnecessary limitation for you. I'm afraid Bazel is not (yet?) smart enough to put the genfiles directory on the inclusion path only when it needs to, but maybe @mhlopko knows about an effort to change that. In any case, is it an option for you to split the libraries so they need fewer includes and therefore shorter includes lists?

[meteorcloudy]

Yes, I think a parameter file for C++ compilation action should be the solution.
@mhlopko We already have a feature called linker_param_file, do you think we can also have a compiler_param_file?
The MSVC compiler does support parameter file. See https://msdn.microsoft.com/en-us/library/610ecb4h.aspx

[laszlocsomor]

@meteorcloudy : Thanks! Shall I categorize this bug as a feature request then? I reckon compiler_param_file doesn't exist yet.

[meteorcloudy]

@laszlocsomor Please do that~

[laszlocsomor]

And if I'm not mistaken, this isn't a Windows bug. Removing the Windows tag and assigning to @mhlopko for now. Please reassign if there's a better assignee.

[hlopko]

Late is better than never.

We changed includes handling and we will now add 3 include directories per single include (srcs, genfiles, binfiles). I don't plan to optimize this in the short term.

We have param files for C++ link actions, not yet for compile actions. Unfortunately work that @tomlu did does not apply to C++ actions since they are not standard SpawnActions but use FeatureConfiguration and CcToolchainVariables to construct command lines.

What's not clear to me is which flags can go to param files. For link actions there is a heuristic that decides which flags go where. This is even worse than it looks like, we have crosstools that rely on the change of order param files introduce. Ideally all flags would go to param file, but the internal migration won't be trivial.

We are not burdened by the migration for param files for compile actions, but I'd prefer if we first solved the linking case and use the same solution for compilation and archiving. How quickly do we need to solve this?

[adam-porich-sm]

We should be able to workaround the issues in Windows for now and the limits are more relaxed under Linux.

I must confess at being a bit confused by that heuristic though. Does it not actually work to pass the arguments straight through. It's been a while since I have tried but I thought MSVC supported all arguments being passed through param files.

[tomlu]

On Tue, May 15, 2018 at 2:07 AM, Marcel Hlopko ***@***.***> wrote: Late is better than never. We changed includes handling and we will now add 3 include directories per single include (srcs, genfiles, binfiles). I don't plan to optimize this in the short term. We have param files for C++ link actions, not yet for compile actions. Unfortunately work that @tomlu <https://github.com/tomlu> did does not apply to C++ actions since they are not standard SpawnActions but use FeatureConfiguration and CcToolchainVariables to construct command lines.

It does apply, if we rewrite the C++ actions to use CommandLines the same way SpawnAction does. It's not hard, the action just has to store CommandLines then expand them when it builds the Spawn, instead of manually constructing ParamFileWriteActions. Unfortunately I don't touch C++ actions generally because they are too complicated.

What's not clear to me is which flags can go to param files. For link actions there is a heuristic <https://source.bazel.build/bazel/+/master:src/main/java/com/google/devtools/build/lib/rules/cpp/LinkCommandLine.java;l=287?q=linkcommandline> that decides which flags go where.

This is the part of "I can't touch this because it's too complicated". It's unclear to me why we can't build two command lines instead of building one, then iterating over it and separating stuff out.

…

This is even worse than it looks like, we have crosstools that rely on the change of order param files introduce. Ideally all flags would go to param file, but the internal migration won't be trivial. We are not burdened by the migration for param files for compile actions, but I'd prefer if we first solved the linking case and use the same solution for compilation and archiving. How quickly do we need to solve this? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#5163 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABbnSvHu3LtoLhzLxGVS4rctePTcD6b7ks5tynCogaJpZM4TzjQC> .

[sesmith177]

we are working on porting a large bazel c++ project (envoyproxy/envoy#129) to Windows and are running into this issue (cl.exe command line larger than 32,768 characters) as well

[hlopko]

I wrote a doc proposing unifying C++ actions with Args (what all other actions use): https://docs.google.com/document/d/11DDjHQfCY3I1kO7w0PusjCoaqmZUQQajd42HDS0F3dI/edit#heading=h.4nax2orizccp

[sesmith177]

Is there a time frame in the road map for this to be fixed? We are working around the issue now by forking Bazel and changing the "_virtual_includes" part of the path to something shorter.

[hlopko]

We discussed this offline with @laszlocsomor, and he'll ping this issue once he's done with his investigations with his plan.

[meteorcloudy]

If the default in 5.0 switched only when encountering a command line too long

I think the current feature doesn't work like this? @oquenchil What do you think, maybe we could implement this?

It's not hard to ensure the cumulative command is correct prior to delegating it to CI.

It's not only about ensuring the command is correct, it's also about when something went wrong, it should be easy to identify the problem. After all, the CI environment could differ from the local environment.

[oquenchil]

We don't have plans to make it the default for now.

[diegohavenstein]

I'm hitting this bug while trying to upgrade to Bazel 6.0.0. We did not have that issue on Bazel 5.4.0, so it seems a regression.

The min repro mentioned here (with long and long_bin) is enough to repro it on Windows, and passing the compiler_param_file also didn’t help. Did someone find this issue and solve it during their upgrade?

[meteorcloudy]

@oquenchil Did we fix this problem in Bazel 5.x and accidentally broke it in 6.0?

[fmeum]

Ran a test in CI and compiler_param_file doesn't seem to work anymore on Windows (tested with #17066): https://storage.googleapis.com/bazel-untrusted-buildkite-artifacts/018538f9-4b2c-432e-ad81-654eb26ad659/src%5Ctest%5Cpy%5Cbazel%5Cbazel_windows_cpp_test%5Ctest.log

[oquenchil]

Thanks for the repro Fabian. There is no regression in 6.0. That repro has a single argument that is too long, producing the invalid argument error. This didn't work in 5.0 either.

An example with a command line that is over 32767 characters long but no single argument goes over that limit works at head with --features=compiler_param_file and it doesn't work without the feature.

Diego, the reason why this may not be working for you might be because you have a custom Windows toolchain without the feature. See default toolchain.

[oquenchil]

With respect to enabling this by default, if someone wants to send a PR that sets enabled=True in https://cs.opensource.google/bazel/bazel/+/master:tools/cpp/windows_cc_toolchain_config.bzl;drc=25d17f53bed60464ce74d1c1a1769787ab259cf2;l=378 and every test passes I will approve.

[diegohavenstein]

Thanks for the repro Fabian. There is no regression in 6.0. That repro has a single argument that is too long, producing the invalid argument error. This didn't work in 5.0 either.

An example with a command line that is over 32767 characters long but no single argument goes over that limit works at head with --features=compiler_param_file and it doesn't work without the feature.

Diego, the reason why this may not be working for you might be because you have a custom Windows toolchain without the feature. See default toolchain.

It was working before the upgrade, with our toolchain. We did not switch the Windows toolchain. Only attempted the upgrade from 5.4.0 to 6.0.0. The feature being ignored is linker_param_file seemingly (the linker args are passed in the CL instead of using a param file with 6.0.0). But let's keep it in the other issue to not mix up things please