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

Please do not use uppercase header. use dbghelp.h instead. Cross compiling always needs lowercase header. #2542

Closed
trcrsired opened this issue Dec 10, 2024 · 10 comments

Comments

@trcrsired
Copy link
Contributor

/home/cqwrteur/toolchains_build/ninja/src/minidump-win32.cc:18:10: fatal error: 'DbgHelp.h' file not found
   18 | #include <DbgHelp.h>
      |          ^~~~~~~~~~~
1 error generated.

https://github.com/trcrsired/windows-msvc-sysroot/tree/main/include

@trcrsired
Copy link
Contributor Author

Also llvm does not support manifestinput

$ ./aarch64-windows-msvc.sh 
[2/2] Linking CXX executable ninja.exe
FAILED: ninja.exe 
: && /home/cqwrteur/toolchains/llvm/x86_64-generic-linux-gnu/llvm/bin/clang++ --target=aarch64-windows-msvc -nostartfiles -nostdlib -fuse-ld=lld -Wno-unused-command-line-argument --sysroot=/home/cqwrteur/toolchains/windows-msvc-sysroot -D_DLL=1 -lmsvcrt -fuse-ld=lld -O3 -DNDEBUG -D_DLL -D_MT -Xclang --dependent-lib=msvcrt -flto=thin -Xlinker /subsystem:console  -fuse-ld=lld-link CMakeFiles/libninja.dir/src/build_log.cc.obj CMakeFiles/libninja.dir/src/build.cc.obj CMakeFiles/libninja.dir/src/clean.cc.obj CMakeFiles/libninja.dir/src/clparser.cc.obj CMakeFiles/libninja.dir/src/dyndep.cc.obj CMakeFiles/libninja.dir/src/dyndep_parser.cc.obj CMakeFiles/libninja.dir/src/debug_flags.cc.obj CMakeFiles/libninja.dir/src/deps_log.cc.obj CMakeFiles/libninja.dir/src/disk_interface.cc.obj CMakeFiles/libninja.dir/src/edit_distance.cc.obj CMakeFiles/libninja.dir/src/elide_middle.cc.obj CMakeFiles/libninja.dir/src/eval_env.cc.obj CMakeFiles/libninja.dir/src/graph.cc.obj CMakeFiles/libninja.dir/src/graphviz.cc.obj CMakeFiles/libninja.dir/src/json.cc.obj CMakeFiles/libninja.dir/src/line_printer.cc.obj CMakeFiles/libninja.dir/src/manifest_parser.cc.obj CMakeFiles/libninja.dir/src/metrics.cc.obj CMakeFiles/libninja.dir/src/missing_deps.cc.obj CMakeFiles/libninja.dir/src/parser.cc.obj CMakeFiles/libninja.dir/src/real_command_runner.cc.obj CMakeFiles/libninja.dir/src/state.cc.obj CMakeFiles/libninja.dir/src/status_printer.cc.obj CMakeFiles/libninja.dir/src/string_piece_util.cc.obj CMakeFiles/libninja.dir/src/util.cc.obj CMakeFiles/libninja.dir/src/version.cc.obj CMakeFiles/libninja.dir/src/subprocess-win32.cc.obj CMakeFiles/libninja.dir/src/includes_normalize-win32.cc.obj CMakeFiles/libninja.dir/src/msvc_helper-win32.cc.obj CMakeFiles/libninja.dir/src/msvc_helper_main-win32.cc.obj CMakeFiles/libninja.dir/src/getopt.c.obj CMakeFiles/libninja.dir/src/minidump-win32.cc.obj CMakeFiles/libninja-re2c.dir/src/depfile_parser.cc.obj CMakeFiles/libninja-re2c.dir/src/lexer.cc.obj CMakeFiles/ninja.dir/src/ninja.cc.obj -o ninja.exe -Xlinker /MANIFEST:EMBED -Xlinker /implib:ninja.lib -Xlinker /pdb:ninja.pdb -Xlinker /version:0.0   -lkernel32 -luser32 -lgdi32 -lwinspool -lshell32 -lole32 -loleaut32 -luuid -lcomdlg32 -ladvapi32 -loldnames -Xlinker /MANIFESTINPUT:/home/cqwrteur/toolchains_build/ninja/windows/ninja.manifest && :
lld-link: error: unable to find mt.exe in PATH: No such file or directory
clang++: error: linker command failed with exit code 1 (use -v to see invocation)

@jhasse
Copy link
Collaborator

jhasse commented Dec 10, 2024

PR welcome :)

Regarding mt.exe: I'm not sure how to fix that.

@zufuliu
Copy link
Contributor

zufuliu commented Dec 15, 2024

DbgHelp.h is the Windows SDK header name, lower case dbghelp.h is the mingw-w64 name.
image

minidump-win32.cc is only enabled for MSVC target, maybe you can try GNU target (aarch64-w64-windows-gnu) instead?

#ifdef _MSC_VER
#include <windows.h>
#include <DbgHelp.h>
#include "util.h"

@trcrsired
Copy link
Contributor Author

DbgHelp.h is the Windows SDK header name, lower case dbghelp.h is the mingw-w64 name. image

minidump-win32.cc is only enabled for MSVC target, maybe you can try GNU target (aarch64-w64-windows-gnu) instead?

#ifdef _MSC_VER
#include <windows.h>
#include <DbgHelp.h>
#include "util.h"

That is the problem. Cross-compiling Windows MSVC target is impossible unless you make the header lowercase. For, Windows case does not matter but it matters for linux.

I am using msvc's target for the same behavior as gcc or it just fails to compile

@trcrsired
Copy link
Contributor Author

trcrsired commented Dec 16, 2024

https://github.com/trcrsired/windows-msvc-sysroot/blob/main/include/dbghelp.h
@zufuliu

All headers should be in lowercase to maintain consistency, as Microsoft's approach is inconsistent. They have mixed headers and libs, while LLVM uses lowercase.

I use Windows .exe as the portable binary for Linux and Android. The case must be lowercase so I could do my job.

As the maintainer of this windows-msvc-sysroot, I believe using lowercase headers is the correct approach. On Windows, filenames are case insensitive, so this will not cause any issues. I will submit a PR to address this.

@trcrsired
Copy link
Contributor Author

Regarding the mt.exe issue, it was caused by llvm-mt requiring libxml2 as a dependency, which I hadn't built. This oversight was entirely my own, as confirmed by the LLVM team.

@trcrsired
Copy link
Contributor Author

trcrsired commented Dec 16, 2024

@zufuliu Also ninja uses windows.h, based on your logic, it should use Windows.h instead.

Just use the lowercase header, and everything is consistent and fine. Plus $ARCH-windows-gnu targets can also use this minidump. I will submit a PR

@trcrsired
Copy link
Contributor Author

I have submitted a PR #2545

@trcrsired
Copy link
Contributor Author

@zufuliu
Copy link
Contributor

zufuliu commented Dec 18, 2024

based on your logic, it should use Windows.h instead.

tested clang with -Wnonportable-include-path, it does not warn on change file case, so PR #2545 is OK to me.

@jhasse jhasse added this to the 1.13.0 milestone Dec 19, 2024
@jhasse jhasse closed this as completed Dec 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants