search

bazelbuild / bazel

a fast, scalable, multi-language and extensible build system
https://bazel.build
Apache License 2.0
23.22k stars 4.07k forks source link

Enabling external_include_paths prevents "includes" from working properly with cc_library on MSVC #22898

Open KKhanhH opened 4 months ago

KKhanhH commented 4 months ago

Description of the bug:

Enabling the feature external_include_paths changes the directories specified by the "includes" attribute of cc_library from using /I flag to using /external:I flag. This will mark the include directories as external, but since it removes the /I flag, MSVC will not be able to resolve the includes.

Solution should be to just keep /I flag along with /external:I

Which category does this issue belong to?

C++ Rules

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

Create a bazel project that requires the "includes" tag in cc_library, add a "#include" that won't resolve when the includes attribute is missing. MSVC will report that the include cannot be found when external_include_paths is enabled.

Which operating system are you running Bazel on?

Windows 10

What is the output of bazel info release?

6.4.0 (bug is also present on 7.2.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 HEAD ?

No response

If this is a regression, please try to identify the Bazel commit where the bug was introduced with bazelisk --bisect.

No response

Have you found anything relevant by searching the web?

No response

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

Sub command without external_include_paths enabled

  C:\Program Files\Microsoft Visual Studio\2022\Professional\VC\Tools\MSVC\14.40.33807\bin\HostX64\x64\cl.exe /nologo /DCOMPILER_MSVC /DNOMINMAX /D_WIN32_WINNT=0x0601 /D_CRT_SECURE_NO_DEPRECATE /D_CRT_SECURE_NO_WARNINGS /bigobj /Zm500 /EHsc /wd4351 /wd4291 /wd4250 /wd4996 /Iexternal/libuv /Ibazel-out/x64_windows-fastbuild/bin/external/libuv /Iexternal/libuv/include /Ibazel-out/x64_windows-fastbuild/bin/external/libuv/include /Iexternal/libuv/src /Ibazel-out/x64_windows-fastbuild/bin/external/libuv/src /Iexternal/libuv/src/unix /Ibazel-out/x64_windows-fastbuild/bin/external/libuv/src/unix /DBAZEL_CURRENT_REPOSITORY="libuv" /showIncludes /MD /Od /Z7 /wd4117 -D__DATE__="redacted" -D__TIMESTAMP__="redacted" -D__TIME__="redacted" -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64 -D_GNU_SOURCE -DWIN32_LEAN_AND_MEAN -D_WIN32_WINNT=0x0600 /Fobazel-out/x64_windows-fastbuild/bin/external/libuv/_objs/libuv/winsock.obj /c external/libuv/src/win/winsock.c

With external_include_paths enabled

C:\Program Files\Microsoft Visual Studio\2022\Professional\VC\Tools\MSVC\14.40.33807\bin\HostX64\x64\cl.exe /nologo /DCOMPILER_MSVC /DNOMINMAX /D_WIN32_WINNT=0x0601 /D_CRT_SECURE_NO_DEPRECATE /D_CRT_SECURE_NO_WARNINGS /bigobj /Zm500 /EHsc /wd4351 /wd4291 /wd4250 /wd4996 /Iexternal/libuv /Ibazel-out/x64_windows-fastbuild/bin/external/libuv /external:I external/libuv /external:I bazel-out/x64_windows-fastbuild/bin/external/libuv /external:I external/libuv/include /external:I bazel-out/x64_windows-fastbuild/bin/external/libuv/include /external:I external/libuv/src /external:I bazel-out/x64_windows-fastbuild/bin/external/libuv/src /external:I external/libuv/src/unix /external:I bazel-out/x64_windows-fastbuild/bin/external/libuv/src/unix /DBAZEL_CURRENT_REPOSITORY="libuv" /showIncludes /MD /Od /Z7 /wd4117 -D__DATE__="redacted" -D__TIMESTAMP__="redacted" -D__TIME__="redacted" -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64 -D_GNU_SOURCE -DWIN32_LEAN_AND_MEAN -D_WIN32_WINNT=0x0600 /Fobazel-out/x64_windows-fastbuild/bin/external/libuv/_objs/libuv/winsock.obj /c external/libuv/src/win/winsock.c
fmeum commented 4 months ago

The docs at https://learn.microsoft.com/en-us/cpp/build/reference/external-external-headers-diagnostics?view=msvc-170 do sound like the flag replaces /I. Do you have a reproducer you can share?

KKhanhH commented 4 months ago

I can't seem to get a minimum reproducer, so here's the repo I was having issues with, when bazel files were added to it. From what I could tell, the only difference in the subcommand was the change in the flag, and the build fails due to not being able to find uv.h even though its include folder was in the "includes" of libuv.

uwebsockets_bazel.zip

KKhanhH commented 4 months ago

well, I guess the issue is because /external:I depends on /external:Wn first, and since adding the feature doesn't add that, it will skip the /external:I flag and not resolve the include. So, this bug isn't actually because /external:I doesn't replace /I, but because the subcommand doesn't include /external:W.

I've tried adding --cxxopt=/external:W0 but it seems to have no effect.

KKhanhH commented 4 months ago

Realized the issue, forgot that --cxxopt doesn't apply to C libraries. After fixing some issues with the build file it compiles. However there still are warnings coming from external libraries and system libraries.