Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bazel 6.0.0 upgrade not possible due to "command is longer than CreateProcessW's limit (32767 characters)" #17068

Closed
diegohavenstein opened this issue Dec 22, 2022 · 13 comments
Assignees
Labels
P1 I'll work on this now. (Assignee required) team-Rules-CPP Issues for C++ rules type: bug

Comments

@diegohavenstein
Copy link

diegohavenstein commented Dec 22, 2022

Description of the bug:

While attempting to upgrade from Bazel 5.4.0 to Bazel 6.0.0, we found that we hit an error during linking of our "main" library. This library takes a long parameter list, more than 41k characters long. Until now, that was fine, because we used linker_param_file feature.

When building with -s and Bazel 5.4.0, the name of the file is shown

external\clang_windows\bin\llvm-lib.exe @bazel-out/x64_windows-dbg/bin/ourProject/ourMainLib.lib-2.params

With Bazel 6.0.0 and -s, our parameters are shown, and the error from the title

ERROR: D:/tc/work/953952133bf40cea/ourProject/BUILD.bazel:7:17: Linking ourProject/ourMainLib.lib failed: (Exit -1): llvm-lib.exe failed: error executing command
...
: command is longer than CreateProcessW's limit (32767 characters)

so it seems Bazel 6 is ignoring the link_input_params feature, given the linker call we see. This is reproducible also with the toy example in #5163

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

Use the repro from #5163 on Windows. With link_input_params enabled, it should work with Bazel 5.4.0. Bazel 6 should fail with the error I posted above

Which operating system are you running Bazel on?

Windows

What is the output of bazel info release?

release 6.0.0

If bazel info release returns development version or (@non-git), tell us how you built Bazel.

No response

What's the output of git remote get-url origin; git rev-parse master; git rev-parse HEAD ?

No response

Have you found anything relevant by searching the web?

No response

Any other information, logs, or outputs that you want to share?

There's an ongoing Slack thread: https://bazelbuild.slack.com/archives/CA31HN1T3/p1671645326680889

@konste
Copy link

konste commented Dec 22, 2022

There is no built-in link_input_params feature. Do you mean linker_param_file feature?

@diegohavenstein
Copy link
Author

diegohavenstein commented Dec 22, 2022

yeah, sorry, that's a typo. Fixed it

@oquenchil oquenchil added P1 I'll work on this now. (Assignee required) and removed untriaged labels Jan 5, 2023
@oquenchil
Copy link
Contributor

Could you please provide a minimal repro that reproduces it? That will make it easier to track down.

@diegohavenstein
Copy link
Author

@oquenchil This is reproducible with the toy example in #5163

@oquenchil
Copy link
Contributor

See here: #5163 (comment)

You probably don't have the feature in your toolchain. I cannot reproduce it.

@diegohavenstein
Copy link
Author

diegohavenstein commented Jan 18, 2023

we have the feature enabled. What seems to be the problem is that the value of supports_param_files (arg to cc_toolchain) is ignored on Windows. We have a select there, but I hardcoded it to True to double check.

On Windows, when I set supports_param_files = True as described above, the linker_param_file feature is not available. I verified this by using the following flag groups:

    return feature(
        name = "linker_param_file",
        flag_sets = [
            flag_set(
                actions = all_link_actions + [ACTION_NAMES.cpp_link_static_library],
                flag_groups = [
                    flag_group(
                        flags = ["@%{linker_param_file}"],
                        expand_if_available = "linker_param_file",
                    ),
                    flag_group(
                        flags = ["I DO NOT EXPECT THIS TO BE THE CASE"],
                        expand_if_not_available = "linker_param_file",
                    ),
                ],
            ),
        ],
    )

and I see the expand_if_not_available. On 5.4.0, the first flag group is used, as should be. It seems like the supports_param_files arg is not correctly interpreted on Windows after the upgrade, or at least it does not lead to linker_param_file being available anymore

@gregoryT5
Copy link

gregoryT5 commented Jan 20, 2023

I found that enabling the new archive_param_file feature (build with --features=archive_param_file) addressed a similar issue for me.

That new option was intentionally disabled on macOS because of a performance hit (generates an archive per library), but I think it may have also been accidentally disabled on Windows as part of the relevant change (bff9730). If that is the issue, it would probably be good to have that feature enabled by default for most (or all?) Windows toolchains, since link command line length is a common problem on Windows.

@diegohavenstein
Copy link
Author

Thanks @gregoryT5! After passing in archive_param_file in our toolchain, we're able to build now

Will not close the issue yet though, since I agree that this should be default behaviour on Windows

@oquenchil
Copy link
Contributor

That indeed seems to be the culprit. @googlewalt archive_param_file should set enabled = True just like for the unix toolchain. I think this was unintentional. Could you please send a fix?

Thank you @gregoryT5 !

@googlewalt
Copy link
Contributor

Sorry about the bug. Should be fixed now.

@fmeum
Copy link
Collaborator

fmeum commented Jan 24, 2023

@bazel-io flag

@bazel-io bazel-io added the potential release blocker Flagged by community members using "@bazel-io flag". Should be added to a release blocker milestone label Jan 24, 2023
@ShreeM01
Copy link
Contributor

@bazel-io fork 6.1.0

@bazel-io bazel-io removed the potential release blocker Flagged by community members using "@bazel-io flag". Should be added to a release blocker milestone label Jan 26, 2023
ShreeM01 added a commit that referenced this issue Jan 26, 2023
Closes #17068.

PiperOrigin-RevId: 504229788
Change-Id: I7b6f7f4f2c8ccb802f5a76bd54eaf0a8358793b1

Co-authored-by: Googler <[email protected]>
hvadehra pushed a commit that referenced this issue Feb 14, 2023
Closes #17068.

PiperOrigin-RevId: 504229788
Change-Id: I7b6f7f4f2c8ccb802f5a76bd54eaf0a8358793b1
github-actions bot pushed a commit to chachako/android-studio that referenced this issue Feb 18, 2023
This is a new feature added to the bazel toolchain.
Without it there will not be a link param file on
Windows. For the Skia Parser we encountered this
bug with bazel 6.0.0:
- bazelbuild/bazel#17068

Bug: 267540183
Test: N/A
Change-Id: I31a53dddfa1d570565ffdb95216ca71196f01379

Former-commit-id: 82d995e32ad6fcbc6e019b0a16e8c7fdc862cfcc
@Bobarshad
Copy link

can you please have look at the same issue here

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P1 I'll work on this now. (Assignee required) team-Rules-CPP Issues for C++ rules type: bug
Projects
None yet
Development

No branches or pull requests

9 participants