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

windows-gnu: Adhere to MinGW convention for build outputs #22415

Open
wants to merge 9 commits into
base: master
Choose a base branch
from

Conversation

Doekin
Copy link

@Doekin Doekin commented Jan 5, 2025

This PR potentially addresses issues #20039, #21103, and #16811.

Background

As stated in #20039, when zig cc tries to link a library named foo, the default search paths are foo.dll, foo.lib, and libfoo.a. However, the conventional DLL name format in MinGW is libfoo.dll and libfoo.dll.a. This discrepancy causes issues for zig cc when working with build tools like CMake. This PR aims to resolve this inconsistency.

Changes

  • Updated fn fileExt(of: ObjectFormat, arch: Cpu.Arch) in std.Target.ObjectFormat to fn fileExt(of: ObjectFormat, abi: Abi, arch: Cpu.Arch).
  • Changed default output names when targeting MinGW:
    • Object files: .obj => .o
    • Static libraries: foo.lib => libfoo.a
    • Shared libraries: foo.dll => libfoo.dll, foo.lib => libfoo.dll.a
    • Debug: .pdb => (dwarf)
  • Adjusted library search paths to reflect the new output names.
  • Disabled default import library emission in zig cc mode (still enabled by default in zig build).
  • Added support for the GCC/Clang link flag -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), and foo.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

$ zig cc -target x86_64-windows-gnu main.c -lfoo -L.
error: unable to find dynamic system library 'foo' using strategy 'paths_first'. searched paths:
  .\foo.dll
  .\foo.lib
  .\libfoo.a

After

$ zig cc -target x86_64-windows-gnu main.c -lfoo -L.
error: unable to find dynamic system library 'foo' using strategy 'paths_first'. searched paths:
  .\libfoo.dll
  .\libfoo.dll.a
  .\foo.dll
  .\foo.lib
  .\libfoo.a
  .\foo.lib

(Note: foo.lib is searched twice, which might be an area for improvement.)

@alexrp
Copy link
Member

alexrp commented Jan 5, 2025

Disabled default import library emission in zig cc mode (still enabled by default in zig build).

This is the only part of this PR for which the rationale is not immediately obvious to me. Can you elaborate on this point?

@Doekin
Copy link
Author

Doekin commented Jan 5, 2025

Thank you for your feedback.

When using zig cc as a replacement for LLVM-MinGW, I noticed that main.lib was generated alongside HelloQt.exe. Since GCC and Clang do not emit import libraries by default, I felt it would be more consistent to align zig cc with this behavior.

@alexrp
Copy link
Member

alexrp commented Jan 5, 2025

Ok, that makes sense. I would suggest including that rationale in the commit message.

src/main.zig Outdated Show resolved Hide resolved
@@ -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,
Copy link
Member

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?

Copy link
Collaborator

@squeek502 squeek502 Jan 7, 2025

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?

Copy link
Contributor

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.

Copy link
Member

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

Copy link
Collaborator

@squeek502 squeek502 Jan 7, 2025

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.

Copy link
Author

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.

@Doekin Doekin force-pushed the mingw branch 2 times, most recently from 96a3649 to f32108e Compare January 7, 2025 07:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants