-
-
Notifications
You must be signed in to change notification settings - Fork 2.6k
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
windows-gnu
: Adhere to MinGW convention for build outputs
#22415
base: master
Are you sure you want to change the base?
Conversation
This is the only part of this PR for which the rationale is not immediately obvious to me. Can you elaborate on this point? |
Thank you for your feedback. When using |
Ok, that makes sense. I would suggest including that rationale in the commit message. |
@@ -444,9 +444,10 @@ pub fn resolve(options: Options) ResolveError!Config { | |||
if (options.debug_format) |x| break :b x; | |||
break :b switch (target.ofmt) { | |||
.elf, .goff, .macho, .wasm, .xcoff => .{ .dwarf = .@"32" }, | |||
.coff => .code_view, | |||
.coff => if (target.abi.isGnu()) .{ .dwarf = .@"32" } else .code_view, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This change -- DWARF by default (i.e. embedded into the binary; no PDB) for MinGW -- is probably the only controversial change in this PR. I don't personally have a problem with it since you can still opt into getting a PDB, but we should probably think carefully about this change nonetheless.
cc @squeek502 @andrewrk for thoughts, maybe others?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't feel like I know enough about why MinGW made this choice to give any useful feedback here.
I have a more general question, though (and this is also coming from ignorance, this is not necessarily intended as a criticism of these changes): is the stuff in this PR related to 'targeting the MinGW ABI' or is it more related to 'emulating the MinGW toolchain'? If it's the latter, is that something that Zig-the-compiler is/should be interested in? Would it make any sense to limit any 'emulating the MinGW toolchain' to zig cc
, specifically?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Native debuggers on Windows all expect PDBs, I think this change would hurt debuggability there - it would mean users would have to always specify a non-default option to use the default platform debuggers.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
An interesting data point for this discussion: msys2/MINGW-packages#5646
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
An interesting data point for this discussion: msys2/MINGW-packages#5646
This is also mentioned in #20039:
Zig uses PDB or CodeView format for debug symbols, while Rust and GCC use DWARF
- When using Zig library in Rust or C on Windows, the mix of debug symbol formats makes either language un-debuggable
- GDB has no support of PDB files (which use CodeView debug format) and LLDB's support is problematic ([1], [2]), which makes open-sourced VS Code debuggers like CodeLLDB unusable for zig debugging purposes
I'm not sure there's a default that's "good" here, and gnu
being the default ABI for Windows in Zig makes the default matter.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is the stuff in this PR related to 'targeting the MinGW ABI' or is it more related to 'emulating the MinGW toolchain'?
It's more about the latter. The goal of this PR is to improve compatibility.
This change aligns `zig cc` with GCC and Clang, which do not emit import libraries by default.
Added the `objFileExt()` function to Target, streamlining access to target-specific data in a manner consistent with other convenience functions.
…for enhanced flexibility
96a3649
to
f32108e
Compare
This PR potentially addresses issues #20039, #21103, and #16811.
Background
As stated in #20039, when
zig cc
tries to link a library namedfoo
, the default search paths arefoo.dll
,foo.lib
, andlibfoo.a
. However, the conventional DLL name format in MinGW islibfoo.dll
andlibfoo.dll.a
. This discrepancy causes issues forzig cc
when working with build tools like CMake. This PR aims to resolve this inconsistency.Changes
fn fileExt(of: ObjectFormat, arch: Cpu.Arch)
instd.Target.ObjectFormat
tofn fileExt(of: ObjectFormat, abi: Abi, arch: Cpu.Arch)
..obj
=>.o
foo.lib
=>libfoo.a
foo.dll
=>libfoo.dll
,foo.lib
=>libfoo.dll.a
.pdb
=> (dwarf)zig cc
mode (still enabled by default inzig build
).-mwindows
(complements zig cc: support --subsystem linker flag #11396).Example
GCC/Clang produce a 66KiB file when creating a simple shared library. In contrast, zig 0.13 would produce three files:
a.exe
(144.5 KiB),a.pdb
(872.0 KiB), andfoo.lib
(1.2 KiB). After these changes,zig cc
produces a single file:a.exe
(874.0 KiB).When attempting to link to a non-existent library, the error message has changed as follows:
Before
After
(Note:
foo.lib
is searched twice, which might be an area for improvement.)