feat: copy `.pdb` files to `Editable Installation` and `Wheel` for easier debugging on windows

WSH032 commented 2 months ago

Close #2213

What I Did

I added a --with-debuginfo option to both maturin dev and maturin build. Currently, it only works on msvc. When this option is specified:

maturin dev will copy the debug file (e.g., mylib.pdb) with the same name as the artifact (e.g., mylib.dll) to the editable install directory.
maturin build will include the debug file in the wheel.
For bin bindings, *.pdb will be distributed along with *.exe in the Scripts directory.

Even if developers do not generate debug information, there will be no impact as long as they don't manually specify --with-debuginfo.

Specifically, with the following configuration:

# Cargo.toml

[lib]
name = "lib_name"

# pyproject.toml

python-source = "my_project"
module-name = "my_project._lib_name"

The resulting directory structure would look like this:

my-project
├── pyproject.toml
├── Cargo.toml
├── my_project
│   ├── __init__.py
│   ├── bar.py
│   ├── lib_name.pdb
│   └── _lib_name.cp310-win_amd64.pyd
├── README.md
└── src
    └── lib.rs

Please note that the debug file is lib_name.pdb instead of _lib_name.pdb, due to the linker flag /PDBALTPATH:%_PDB%.

Why Didn't I Implement Support for `macOS` and `Linux`?

`Linux`

On Linux, the default split-debuginfo is off, so the debug information is included in the .so file. Therefore, no additional actions are needed by default.

`macOS`

Currently, the default value for split-debuginfo on macOS is packed, but it seems recommended to set it to unpacked, see https://internals.rust-lang.org/t/help-test-faster-incremental-debug-macos-builds-on-nightly/14016.

I tried to reference python-build-standalone's approach. I downloaded these three distribution packages:

I found that only the x86_64-pc-windows-msvc-install_only package contained .pdb debug information, while the other two platforms did not have any separate debug information. So, I am unsure how to implement this feature for those platforms.

Most importantly, I do not have experience developing on macOS, nor do I have a macOS machine to perform the necessary testing. Therefore, this will need to be left for others to implement.

Next, I will work on writing tests and documentation, but I would like to get your feedback on the current implementation first.

netlify[bot] commented 2 months ago

Deploy Preview for maturin-guide ready!

Built without sensitive environment variables

Name	Link
Latest commit	fe46b4c0609c6cb729a326f7456004184b9acdb2
Latest deploy log	https://app.netlify.com/sites/maturin-guide/deploys/66e797f4a6c26f0008448d08
Deploy Preview	https://deploy-preview-2220--maturin-guide.netlify.app
Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

WSH032 commented 2 months ago

Thank you for the review!

For Linux, when --with-debuginfo is presented, we can explicitly pass -C split-debuginfo=off to rustc, similarly for macOS we pass -C split-debuginfo=unpacked.

(And it'd be better to check for existing split-debuginfo rustc flag and error out when its value is incompatible with --with-debuginfo.)

I'm concerned that there is no simple and robust way to check the value of split-debuginfo. Developers can set it not only through the RUSTFLAGS environment variable but also via .cargo/config.toml. I'm unsure if Cargo provides an existing method to retrieve this value.

Considering the difficulty of confirming existing configurations and the uncertainty of whether developers are indeed generating the corresponding debug files, I implemented --with-debuginfo in a way that requires developers to manually specify it.

Specifically, when using --with-debuginfo, I prefer to have developers manually specify RUSTFLAGS="-Csplit-debuginfo=off" rather than having maturin automatically set it.

This commit introduced a regression on FreeBSD, which I couldn’t reproduce on Windows. I’m unsure if it’s a sporadic issue. Could we re-run the CI?

messense commented 2 months ago

I'm concerned that there is no simple and robust way to check the value of split-debuginfo

It's handled by cargo-config2 which is already used in maturin.

Specifically, when using --with-debuginfo, I prefer to have developers manually specify RUSTFLAGS="-Csplit-debuginfo=off" rather than having maturin automatically set it.

I disagree, it should provide a seamless user experience by default w/o asking user to look up how to set the right RUSTFLAGS.

PyO3 / maturin