setuptools icon indicating copy to clipboard operation
setuptools copied to clipboard

[FR] On Windows `pyproject.toml` needs pre-processing of paths as linker flags from `ext-modules` directive aren't handled correctly by linker

Open skirpichev opened this issue 10 months ago • 3 comments

setuptools version

75.6.0

Python version

3.13

OS

Window$

Additional environment information

No response

Description

I tried to switch my project to use new ext-modules directive, but without success: https://github.com/diofant/python-gmp/pull/124

It seems that linker flags passed incorrectly. Here is a fragment from build log:

2025-01-04T05:28:32.2403885Z   "C:\Program Files\Microsoft Visual Studio\2022\Enterprise\VC\Tools\MSVC\14.42.34433\bin\HostX86\x64\link.exe" \
/nologo /INCREMENTAL:NO /LTCG /DLL /MANIFEST:EMBED,ID=2 /MANIFESTUAC:NO \
/LIBPATH:.local/lib \
[...]

As you can see, setuptools pass /LIBPATH:.local/lib (that's come from library-dirs of the ext-modules directive). But linker doesn't recognize it, perhaps because it doesn't follows Windows path convention with \ separators. (But note that compiler works with added include-dirs.)

Expected behavior

Successful build:)

How to Reproduce

I did a draft pr to show problem in CI: https://github.com/diofant/python-gmp/pull/124

Output

Sorry, I can't reproduce that locally.

skirpichev avatar Jan 04 '25 06:01 skirpichev

Thanks @skirpichev, as far as I understand the reason why the problem does not occur when using setup.py is because setup.py is programmatically choosing the right path separator depending on the OS. But if you use static strings in setup.py I believe that you are going to observe the same problem (so in this sense, the behaviour is backwards compatible).

So I am reclassifying this issue as a feature request to implement some sort of dynamic processing of paths.

abravalheri avatar Jan 08 '25 18:01 abravalheri

setup.py is programmatically choosing the right path separator depending on the OS

Exactly.

if you use static strings in setup.py I believe that you are going to observe the same problem

Well, docs says about include_dirs: "list of directories to search for C/C++ header files (in Unix form for portability)". So, I suspect this should work in a string form too.

But the library_dirs option has no such note.

skirpichev avatar Jan 09 '25 03:01 skirpichev

I suppose we would need something like os.fspath(Path(x)) to make it work.

There would be a couple of places where it would be possible to tackle this:

  1. In pypa/distutils:

    • Pre-processing in the extension object creation
      • See https://github.com/pypa/distutils/blob/ff11eed0c36b35bd68615a8ebf36763b7c8a6f28/distutils/extension.py#L126-L140
    • When creating the CLI arguments for the compiler
      • See https://github.com/pypa/distutils/blob/ff11eed0c36b35bd68615a8ebf36763b7c8a6f28/distutils/_msvccompiler.py#L264-L274
      • See https://github.com/pypa/distutils/blob/ff11eed0c36b35bd68615a8ebf36763b7c8a6f28/distutils/_msvccompiler.py#L579-L585
  2. In pypa/setuptools:

    • Pre-processing in the extension object creation:
      • See https://github.com/pypa/setuptools/blob/a443f7647ba35ef3f5c05513a23766cf4d4130bf/setuptools/extension.py#L143-L159
    • Pre-processing in the TOML parsing:
      • See https://github.com/pypa/setuptools/blob/a443f7647ba35ef3f5c05513a23766cf4d4130bf/setuptools/config/_apply_pyprojecttoml.py#L238-L239

Lately, there has been some changes in distutils to accept Pathlib objects, so maybe this would fit in pypa/distutils?

Also the deeper the layer you go the more impactful the change is and covers more use cases.

On the other hand if this is a controversial change, we might want to keep it in a very superficial layer (e.g. that affects only pyproject.toml).

If you or any other member of the community has any proposal for the implementation please let us know also feel free to submit a PR to either pypa/setuptools or pypa/distutils[^1]

[^1]: I am not a contributor to pypa/distutils, so they might have other recommendations/options/tips about this.

abravalheri avatar Jan 21 '25 11:01 abravalheri