Bazel 7.0 - can't compile with /DUNICODE /D_UNICODE

[carpenterjc]

Description of the bug:

We have defaults set to build using the Unicode version of the Microsoft APIs, using /DUNICODE /D_UNICODE to use the unicode version of the APIs by default. This has broken building external/bazel_tools/src/main/cpp/util/path_windows.cc Which should be using the APIs post fixed with "A" where the char type is always narrow.

external/bazel_tools/src/main/cpp/util/path_windows.cc(99): error C2664: 'DWORD ExpandEnvironmentStringsW(LPCWSTR,LPWSTR,DWORD)': cannot convert argument 1 from 'const _Elem *' to 'LPCWSTR'
        with
        [
            _Elem=char
        ]
external/bazel_tools/src/main/cpp/util/path_windows.cc(99): note: Types pointed to are unrelated; conversion requires reinterpret_cast, C-style cast or function-style cast
C:\Program Files (x86)\Windows Kits\10\include\10.0.22621.0\um\processenv.h(198): note: see declaration of 'ExpandEnvironmentStringsW'
external/bazel_tools/src/main/cpp/util/path_windows.cc(108): error C2664: 'DWORD ExpandEnvironmentStringsW(LPCWSTR,LPWSTR,DWORD)': cannot convert argument 1 from 'const _Elem *' to 'LPCWSTR'
        with
        [
            _Elem=char
        ]
external/bazel_tools/src/main/cpp/util/path_windows.cc(108): note: Types pointed to are unrelated; conversion requires reinterpret_cast, C-style cast or function-style cast
C:\Program Files (x86)\Windows Kits\10\include\10.0.22621.0\um\processenv.h(198): note: see declaration of 'ExpandEnvironmentStringsW'

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.

Create an empty python binary and build that target

py_binary(
    name = "foo",
    srcs = [":foo.py"],
    python_version = "PY3",
    visibility = ["//visibility:public"],
)

With an empty windows project run this command.
bazel build --copt=/D_UNICODE --copt=/DUNICODE //:foo

Which operating system are you running Bazel on?

windows

What is the output of bazel info release?

release 7.0.0rc1

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

Is this a regression? If yes, please try to identify the Bazel commit where the bug was introduced.

Yes, moving to bazel 7.0.0rc1

Have you found anything relevant by searching the web?

No response

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

external/bazel_tools/src/main/cpp/util/path_windows.cc(99): error C2664: 'DWORD ExpandEnvironmentStringsW(LPCWSTR,LPWSTR,DWORD)': cannot convert argument 1 from 'const _Elem *' to 'LPCWSTR'
        with
        [
            _Elem=char
        ]
external/bazel_tools/src/main/cpp/util/path_windows.cc(99): note: Types pointed to are unrelated; conversion requires reinterpret_cast, C-style cast or function-style cast
C:\Program Files (x86)\Windows Kits\10\include\10.0.22621.0\um\processenv.h(198): note: see declaration of 'ExpandEnvironmentStringsW'
external/bazel_tools/src/main/cpp/util/path_windows.cc(108): error C2664: 'DWORD ExpandEnvironmentStringsW(LPCWSTR,LPWSTR,DWORD)': cannot convert argument 1 from 'const _Elem *' to 'LPCWSTR'
        with
        [
            _Elem=char
        ]
external/bazel_tools/src/main/cpp/util/path_windows.cc(108): note: Types pointed to are unrelated; conversion requires reinterpret_cast, C-style cast or function-style cast
C:\Program Files (x86)\Windows Kits\10\include\10.0.22621.0\um\processenv.h(198): note: see declaration of 'ExpandEnvironmentStringsW'

[fmeum]

CC @meteorcloudy

[meteorcloudy]

@bazel-io fork 7.0.0

[meteorcloudy]

If the problem is specific to the most recent release or the Bazel@HEAD version and it doesn't occur in earlier versions, you can use the bazelisk --bisect=.. command to identify the specific version or commit where the issue was introduced. For more information, visit https://github.com/bazelbuild/bazelisk#--bisect.

@carpenterjc Thanks for reporting the bug! Did you try to use bazelisk --bisect to identify the culprit commit?

[carpenterjc]

I ran bazelisk bisect.
first bad commit is 1895585

I had a bit more of a look at the dependency graph and it seems it a dependency on @bazel_tools//tools/launcher:launcher_maker from py_binary

[meteorcloudy]

/cc @rickeylev

[meteorcloudy]

Hmm, I'm actually not able to reproduce the issue:

pcloudy@bazel-windows-playground:~/workdir/test
$ export USE_BAZEL_VERSION=last_green
pcloudy@bazel-windows-playground:~/workdir/test
$ bazel build --copt=/D_UNICODE --copt=/DUNICODE //:foo
2023/11/02 14:28:24 Using unreleased version at commit 3e05519a820e01f6377e866fed9988203dd69c07
2023/11/02 14:28:24 Downloading https://storage.googleapis.com/bazel-builds/artifacts/windows/3e05519a820e01f6377e866fed9988203dd69c07/bazel...
Extracting Bazel installation...
Starting local Bazel server and connecting to it...
INFO: Invocation ID: 1e781848-497a-4c80-9f00-1caaa2e4214a
INFO: Reading 'startup' options from c:\users\pcloudy\.bazelrc: --output_user_root=C:/src/tmp
INFO: Options provided by the client:
  Inherited 'common' options: --isatty=1 --terminal_columns=194
INFO: Options provided by the client:
  'build' options: --python_path=C:/python3/python.exe
INFO: Reading rc options for 'build' from c:\users\pcloudy\.bazelrc:
  'build' options: --curses=yes --color=yes --verbose_failures --announce_rc --disk_cache=C:/src/tmp/bazel_disk_cache
WARNING: --enable_bzlmod is set, but no MODULE.bazel file was found at the workspace root. Bazel will create an empty MODULE.bazel file. Please consider migrating your external dependencies from
 WORKSPACE to MODULE.bazel. For more details, please refer to https://github.com/bazelbuild/bazel/issues/18958.
INFO: Analyzed target //:foo (72 packages loaded, 625 targets configured).
INFO: Found 1 target...
Target //:foo up-to-date:
  bazel-bin/foo.zip
  bazel-bin/foo.exe
INFO: Elapsed time: 24.890s, Critical Path: 2.51s
INFO: 33 processes: 13 internal, 20 local.
INFO: Build completed successfully, 33 total actions

[carpenterjc]

Sorry, I have checked in my .bazelrc in the reproduction I have these set:

build --copt=/D_UNICODE
build --copt=/DUNICODE 
build --host_copt=/D_UNICODE
build --host_copt=/DUNICODE 

I bet its because I have also set this via the --host_copt as well.

[meteorcloudy]

Thanks! Now I can confirm this is reproducible at 1895585 but not at its parent commit 8290f94

[meteorcloudy]

@rickeylev Looks like since 1895585, py_binary will always build the Windows native launcher from source instead of using the prebuilt binary embedded in bazel_tools?

With 1895585

$ bazel cquery 'somepath(//:foo, @bazel_tools//src/main/cpp/util:filesystem)'
...
INFO: Found 2 targets...
//:foo (3ee5b1f)
@bazel_tools//tools/launcher:launcher_maker (03ba560)
@bazel_tools//src/tools/launcher:launcher_maker (03ba560)
@bazel_tools//src/main/cpp/util:filesystem (03ba560)

With 8290f94

$ bazel cquery 'somepath(//:foo, @bazel_tools//src/main/cpp/util:filesystem)'
...
INFO: Found 2 targets...
INFO: Empty query results
INFO: Elapsed time: 17.892s, Critical Path: 0.00s
INFO: 0 processes.
INFO: Build completed successfully, 0 total actions

[rickeylev]

It wasn't entirely clear to me how this particular launcher/launcher_maker thing is supposed to work. From what I could ascertain, the "launcher maker" takes "launcher" as an input, and then copies it and appends a bit of data to it.

In Bazel 6, this was implemented in Java as a special action: https://github.com/bazelbuild/bazel/blob/release-6.4.0/src/main/java/com/google/devtools/build/lib/analysis/actions/LauncherFileWriteAction.java

In Bazel 7, for Starlarkification, I copied what I saw the java rules doing. That code is here: https://github.com/bazelbuild/bazel/blob/master/src/main/starlark/builtins_bzl/common/python/py_executable_bazel.bzl#L336
Looking into what these targets are in bazel 7, launcher_maker is a cc_binary and it does the equiv of what LauncherFileWriteAction did. But, I don't see any prebuilt binary to use, so it has to be built from source.

The only solution I see is for Bazel to prebuild launcher_maker, or to move the launcher-maker logic back into Java. Unless there is a way to force launcher_maker to ignore --copt?

[fmeum]

Even with --copt ignored, a hard requirement on a C++ toolchain for launching Java or Python executables on Windows wouldn't be ideal. Having prebuilt binaries would help here.

[meteorcloudy]

Looking into what these targets are in bazel 7, launcher_maker is a cc_binary and it does the equiv of what LauncherFileWriteAction did. But, I don't see any prebuilt binary to use, so it has to be built from source.

I see. I can see two options:

1. We prebuilt the launcher_maker and embedded it in @bazel_tools the same way as @bazel_tools//tools/launcher:launcher. But we need to be careful to make sure it matches the exec platform and rebuild it from source when necessary.
2. Implement the same functionality in a python script so that avoid another case of requiring cc toolchain to build py_binary on Windows.

[meteorcloudy]

I also wonder if it's possible to implement LauncherFileWriteAction purely in Starlark with the ctx.actions.write API?

[rickeylev]

implement LauncherFileWriteAction purely in Starlark with ctx.actions.write

Almost, but not quite. write() writes the file in its entirety with data that comes from the analysis phase. We can't read the file during analysis, so we can't push its data into the write() call. There aren't any mechanisms to expand a File reference to its content (there is expand_template(), but that is unlikely to work well on a binary file, plus we'd need to embed some sort of marker in the pre-built file to do such expansion).

It might be possible to replace it with run_shell(), though, and do something like: cp A B && echo -n $lenBInLittleEndian >> B && echo -n C >> B. I don't look forward to computing the little-endian long byte value using shell :). I also don't like using run_shell in rules in general because it tends to be a platform compatibility headache. The aforementioned cp && echo command would probably have to be a Windows batch file, for example, I think? Or have both a Windows and non-windows shell implementation. This then starts to sound more complicated than just having a Java helper.

I can imagine adding a more advanced write() function to PyBuiltins, but if we touch PyBuiltins.java, we could just as easily have it expose LauncherFileWriteAction. My point being, either option requires some Java code.

Implement the same functionality in a python script

Because this is part of building a py_binary, trying to use a py_binary as part of the process introduces bootstrapping issues.

[rickeylev]

notes meteorcloudy sent me:

• This file controls what binaries are prebuilt into bazel: https://github.com/bazelbuild/bazel/blob/master/src/create_embedded_tools.py#L50
• This file controls what @bazel_tools shows: https://github.com/bazelbuild/bazel/blob/master/tools/launcher/BUILD.tools

So using a prebuilt binary should be a simple matter of adding an extra line to the create_embedded_tool.py script, then added a select for launcher_maker in the BUILD.tools file. This is what the plain launcher.exe prebuilt binary does.

[iancha1992]

A fix for this issue has been included in Bazel 7.0.0 RC5. Please test out the release candidate and report any issues as soon as possible. Thanks!