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

[BUG] Unprotected path for objects passed to windows linker #4645

Open
mglisse opened this issue Sep 10, 2024 · 14 comments
Open

[BUG] Unprotected path for objects passed to windows linker #4645

mglisse opened this issue Sep 10, 2024 · 14 comments
Labels
bug Needs Triage Issues that need to be evaluated for severity and status.

Comments

@mglisse
Copy link

mglisse commented Sep 10, 2024

setuptools version

setuptools==73.0.1

Python version

Python 3.13

OS

Windows

Additional environment information

This happens on the conda-forge servers.

Description

An object file is passed to link.exe directly with its unix-like absolute path '/bld/gudhi_1725978620692/work/build/version/python/gudhi/off_utils.obj'. Since it starts with a '/', it is interpreted as an option and ignored.
(In the cl.exe command, the source filename is prefixed by /TpD: and the object by /Fo, so there is no problem there)
(We did not have this issue before because we always ended up with relative paths, which don't start with '/' and just work)

Expected behavior

It would be nice if the object was passed in a safe way. If there existed an option (tentatively called /Input here), it could pass /Input/path/to/file.obj instead of just /path/to/file.obj, but I didn't find it in the doc. More simply, it should be possible to sanitize the path (using functions from pathlib?) before using it, probably rewriting to \path\to\file.obj.

How to Reproduce

This is the build win_64_numpy2python3.13.____cp313 from
conda-forge/gudhi-feedstock#67
(you can get a look at setup.py at https://github.com/GUDHI/gudhi-devel/blob/master/src/python/setup.py.in#L30)
I don't know how to reproduce their exact setup, maybe using an obsolute path somewhere is important.

Output

2024-09-10T14:47:13.1506361Z running build_ext
2024-09-10T14:47:13.1597057Z building 'gudhi.off_utils' extension
2024-09-10T14:47:13.1644903Z "C:\Program Files\Microsoft Visual Studio\2022\Enterprise\VC\Tools\MSVC\14.29.30133\bin\HostX64\x64\cl.exe" /c /nologo /O2 /W3 /GL /DNDEBUG /MD -DNPY_NO_DEPRECATED_API=NPY_1_7_API_VERSION -I%PREFIX%\Lib\site-packages\numpy\_core\include -ID:/bld/gudhi_1725978620692/work/build/version/python/gudhi/ -ID:/bld/gudhi_1725978620692/_h_env/Library/include/eigen3 -ID:/bld/gudhi_1725978620692/_h_env/Library/include -ID:/bld/gudhi_1725978620692/_h_env/Library/include -ID:/bld/gudhi_1725978620692/work/build/version/ext/hera/include -ID:/bld/gudhi_1725978620692/_h_env/Library/include -ID:/bld/gudhi_1725978620692/_h_env/Library/include -ID:/bld/gudhi_1725978620692/work/build/version/include -ID:/bld/gudhi_1725978620692/work/build/version/python/include -I%PREFIX%\include -I%PREFIX%\Include "-IC:\Program Files\Microsoft Visual Studio\2022\Enterprise\VC\Tools\MSVC\14.29.30133\include" "-IC:\Program Files\Microsoft Visual Studio\2022\Enterprise\VC\Auxiliary\VS\include" "-IC:\Program Files (x86)\Windows Kits\10\include\10.0.22621.0\ucrt" "-IC:\Program Files (x86)\Windows Kits\10\\include\10.0.22621.0\\um" "-IC:\Program Files (x86)\Windows Kits\10\\include\10.0.22621.0\\shared" "-IC:\Program Files (x86)\Windows Kits\10\\include\10.0.22621.0\\winrt" "-IC:\Program Files (x86)\Windows Kits\10\\include\10.0.22621.0\\cppwinrt" "-IC:\Program Files (x86)\Windows Kits\NETFXSDK\4.8\include\um" -I%PREFIX%\Library\include -I%PREFIX%\Library\include /d1trimfile:%SRC_DIR% /EHsc /TpD:/bld/gudhi_1725978620692/work/build/version/python/gudhi/off_utils.cpp /Fo/bld/gudhi_1725978620692/work/build/version/python/gudhi/off_utils.obj -DBOOST_RESULT_OF_USE_DECLTYPE -DBOOST_ALL_NO_LIB -DBOOST_SYSTEM_NO_DEPRECATED /std:c++17 /fp:strict -DNOMINMAX -DCGAL_EIGEN3_ENABLED -DCGAL_HEADER_ONLY -DCGAL_USE_GMP -DCGAL_USE_MPFR
2024-09-10T14:47:13.1816038Z off_utils.cpp
2024-09-10T14:47:13.9596360Z creating %SRC_DIR%\build\version\build\python\build
2024-09-10T14:47:13.9617275Z creating %SRC_DIR%\build\version\build\python\build\lib.win-amd64-cpython-313
2024-09-10T14:47:14.0121940Z creating %SRC_DIR%\build\version\build\python\build\lib.win-amd64-cpython-313\gudhi
2024-09-10T14:47:14.1740722Z "C:\Program Files\Microsoft Visual Studio\2022\Enterprise\VC\Tools\MSVC\14.29.30133\bin\HostX64\x64\link.exe" /nologo /INCREMENTAL:NO /LTCG /DLL /MANIFEST:EMBED,ID=2 /MANIFESTUAC:NO /LIBPATH:D:/bld/gudhi_1725978620692/_h_env/Library/lib /LIBPATH:D:/bld/gudhi_1725978620692/_h_env/Library/lib /LIBPATH:%PREFIX%\libs /LIBPATH:%PREFIX% /LIBPATH:%PREFIX%\PCbuild\amd64 "/LIBPATH:C:\Program Files\Microsoft Visual Studio\2022\Enterprise\VC\Tools\MSVC\14.29.30133\lib\x64" "/LIBPATH:C:\Program Files (x86)\Windows Kits\NETFXSDK\4.8\lib\um\x64" "/LIBPATH:C:\Program Files (x86)\Windows Kits\10\lib\10.0.22621.0\ucrt\x64" "/LIBPATH:C:\Program Files (x86)\Windows Kits\10\\lib\10.0.22621.0\\um\x64" /LIBPATH:%PREFIX%\Library\lib /LIBPATH:%PREFIX%\Library\lib gmp.lib mpfr.lib /EXPORT:PyInit_off_utils /bld/gudhi_1725978620692/work/build/version/python/gudhi/off_utils.obj /OUT:build\lib.win-amd64-cpython-313\gudhi\off_utils.cp313-win_amd64.pyd /IMPLIB:/bld/gudhi_1725978620692/work/build/version/python/gudhi\off_utils.cp313-win_amd64.lib
2024-09-10T14:47:14.2703556Z LINK : warning LNK4044: unrecognized option '/bld/gudhi_1725978620692/work/build/version/python/gudhi/off_utils.obj'; ignored
2024-09-10T14:47:14.2714614Z LINK : warning LNK4001: no object files specified; libraries used
2024-09-10T14:47:14.2742679Z LINK : warning LNK4068: /MACHINE not specified; defaulting to X64
2024-09-10T14:47:14.2798709Z LINK : error LNK2001: unresolved external symbol PyInit_off_utils
2024-09-10T14:47:14.2822359Z /bld/gudhi_1725978620692/work/build/version/python/gudhi\off_utils.cp313-win_amd64.lib : fatal error LNK1120: 1 unresolved externals

(full output at https://dev.azure.com/conda-forge/84710dde-1620-425b-80d0-4cf5baca359d/_apis/build/builds/1024741/logs/121)

@mglisse mglisse added bug Needs Triage Issues that need to be evaluated for severity and status. labels Sep 10, 2024
@abravalheri
Copy link
Contributor

abravalheri commented Sep 10, 2024

Hi @mglisse. Is this a bug in setuptools or a feature request?
I.e., does it happen regardless of the customisations in setup.py (e.g. it would happen with a simplified reproducer with no custom options/arguments in Extension), or it happens because the setup.py script is explicitly passing strings like '/bld/gudhi_1725978620692/work/build/version/python/gudhi/off_utils.obj' as a path?1

Anyway, it would be nice if you can provide a simplified reproducer. If you could please have a look on this document and try to create a very small toy example that only uses setuptools itself without all the extra stuff in https://github.com/GUDHI/gudhi-devel/blob/master/src/python/setup.py.in#L30 and without the extra templating, that would be ideal.

Footnotes

  1. The later would be a bug in that specific setup.py, not exactly on setuptools, and I would see this issue as a feature request for some form of preventing a setup.py from doing that.

@mglisse
Copy link
Author

mglisse commented Sep 10, 2024

Hi @mglisse. Is this a bug in setuptools or a feature request?

I guess that's up to you 😉

I.e., does it happen regardless of the customisations in setup.py (e.g. it would happen with a simplified reproducer with no custom options/arguments in Extension), or it happens because the setup.py script is explicitly passing strings like '/bld/gudhi_1725978620692/work/build/version/python/gudhi/off_utils.obj' as a path?

I don't think setup.py is explicitly passing such a string. All it gives is the source, which apparently starts with "D:"
2024-09-10T14:47:09.4819616Z [ 6/14] Cythonizing D:/bld/gudhi_1725978620692/work/build/version/python/gudhi/off_utils.pyx
I believe the other paths are derived from it, changing just the extension. The .cpp produced by cython and compiled by cl.exe is D:/bld/gudhi_1725978620692/work/build/version/python/gudhi/off_utils.cpp, while the command tells cl.exe to call the object file /bld/gudhi_1725978620692/work/build/version/python/gudhi/off_utils.obj, somehow "D:" disappeared (could be a red herring, but it looks surprising). My impression (I may be completely wrong) is that setuptools (or something related) was fed a valid and safe path for the sources and produced an unsafe path for the object.

Anyway, it would be nice if you can provide a simplified reproducer.

I'll try, but as a linux user, it is not easy (have to locate a suitable windows VM, install what I need, etc, before I can even try to create a small reproducer), so I wanted to at least check if the description made the issue obvious to someone familiar with setuptools.

@mglisse
Copy link
Author

mglisse commented Sep 11, 2024

Ok, in the empty directory C:/Users/ci/bug1, I create setup.py that contains

from setuptools import setup, Extension, find_packages
ext_modules = [Extension('xyz',sources=['C:/Users/ci/bug1/xyz.cc'],language='c++')]
setup(
    name='example',
    version='0.1.0',
    packages=find_packages(),
    ext_modules = ext_modules,
)

and xyz.cc that contains

void fgh(){}

Then I run python setup.py build_ext --inplace. In all cases I will use setuptools 74.1.2.
If I use python 3.12.1, things work (well, there is an error in the end because xyz.cc is nonsense, but that's fine)

running build_ext
building 'xyz' extension
creating build
creating build\temp.win32-cpython-312
creating build\temp.win32-cpython-312\Release
creating build\temp.win32-cpython-312\Release\Users
creating build\temp.win32-cpython-312\Release\Users\ci
creating build\temp.win32-cpython-312\Release\Users\ci\bug1
"C:\Program Files\Microsoft Visual Studio\2022\Community\VC\Tools\MSVC\14.38.33130\bin\HostX86\x86\cl.exe" /c /nologo /O2 /W3 /GL /DNDEBUG /MD -IC:\Users\ci\AppData\Local\Programs\Python\Python312\include -IC:\Users\ci\AppData\Local\Programs\Python\Python312\Include "-IC:\Program Files\Microsoft Visual Studio\2022\Community\VC\Tools\MSVC\14.38.33130\include" "-IC:\Program Files\Microsoft Visual Studio\2022\Community\VC\Tools\MSVC\14.38.33130\ATLMFC\include" "-IC:\Program Files\Microsoft Visual Studio\2022\Community\VC\Auxiliary\VS\include" "-IC:\Program Files (x86)\Windows Kits\10\include\10.0.22621.0\ucrt" "-IC:\Program Files (x86)\Windows Kits\10\\include\10.0.22621.0\\um" "-IC:\Program Files (x86)\Windows Kits\10\\include\10.0.22621.0\\shared" "-IC:\Program Files (x86)\Windows Kits\10\\include\10.0.22621.0\\winrt" "-IC:\Program Files (x86)\Windows Kits\10\\include\10.0.22621.0\\cppwinrt" "-IC:\Program Files\Microsoft Visual Studio\2022\Community\VC\Tools\MSVC\14.38.33130\include" "-IC:\Program Files\Microsoft Visual Studio\2022\Community\VC\Tools\MSVC\14.38.33130\ATLMFC\include" "-IC:\Program Files\Microsoft Visual Studio\2022\Community\VC\Auxiliary\VS\include" "-IC:\Program Files (x86)\Windows Kits\10\include\10.0.22621.0\ucrt" "-IC:\Program Files (x86)\Windows Kits\10\\include\10.0.22621.0\\um" "-IC:\Program Files (x86)\Windows Kits\10\\include\10.0.22621.0\\shared" "-IC:\Program Files (x86)\Windows Kits\10\\include\10.0.22621.0\\winrt" "-IC:\Program Files (x86)\Windows Kits\10\\include\10.0.22621.0\\cppwinrt" /EHsc /TpC:/Users/ci/bug1/xyz.cc /Fobuild\temp.win32-cpython-312\Release\Users/ci/bug1/xyz.obj
xyz.cc
creating C:\Users\ci\bug1\build\lib.win32-cpython-312
"C:\Program Files\Microsoft Visual Studio\2022\Community\VC\Tools\MSVC\14.38.33130\bin\HostX86\x86\link.exe" /nologo /INCREMENTAL:NO /LTCG /DLL /MANIFEST:EMBED,ID=2 /MANIFESTUAC:NO /LIBPATH:C:\Users\ci\AppData\Local\Programs\Python\Python312\libs /LIBPATH:C:\Users\ci\AppData\Local\Programs\Python\Python312 /LIBPATH:C:\Users\ci\AppData\Local\Programs\Python\Python312\PCbuild\win32 "/LIBPATH:C:\Program Files\Microsoft Visual Studio\2022\Community\VC\Tools\MSVC\14.38.33130\ATLMFC\lib\x86" "/LIBPATH:C:\Program Files\Microsoft Visual Studio\2022\Community\VC\Tools\MSVC\14.38.33130\lib\x86" "/LIBPATH:C:\Program Files (x86)\Windows Kits\10\lib\10.0.22621.0\ucrt\x86" "/LIBPATH:C:\Program Files (x86)\Windows Kits\10\\lib\10.0.22621.0\\um\x86" "/LIBPATH:C:\Program Files\Microsoft Visual Studio\2022\Community\VC\Tools\MSVC\14.38.33130\ATLMFC\lib\x86" "/LIBPATH:C:\Program Files\Microsoft Visual Studio\2022\Community\VC\Tools\MSVC\14.38.33130\lib\x86" "/LIBPATH:C:\Program Files (x86)\Windows Kits\10\lib\10.0.22621.0\ucrt\x86" "/LIBPATH:C:\Program Files (x86)\Windows Kits\10\\lib\10.0.22621.0\\um\x86" /EXPORT:PyInit_xyz build\temp.win32-cpython-312\Release\Users/ci/bug1/xyz.obj /OUT:build\lib.win32-cpython-312\xyz.cp312-win_amd64.pyd /IMPLIB:build\temp.win32-cpython-312\Release\Users/ci/bug1\xyz.cp312-win_amd64.lib
LINK : error LNK2001: unresolved external symbol PyInit_xyz

If on the other hand I use python 3.13.0rc2

running build_ext
building 'xyz' extension
"C:\Program Files\Microsoft Visual Studio\2022\Community\VC\Tools\MSVC\14.38.33130\bin\HostX86\x86\cl.exe" /c /nologo /O2 /W3 /GL /DNDEBUG /MD -IC:\Users\ci\AppData\Local\Programs\Python\Python313\include -IC:\Users\ci\AppData\Local\Programs\Python\Python313\Include "-IC:\Program Files\Microsoft Visual Studio\2022\Community\VC\Tools\MSVC\14.38.33130\include" "-IC:\Program Files\Microsoft Visual Studio\2022\Community\VC\Tools\MSVC\14.38.33130\ATLMFC\include" "-IC:\Program Files\Microsoft Visual Studio\2022\Community\VC\Auxiliary\VS\include" "-IC:\Program Files (x86)\Windows Kits\10\include\10.0.22621.0\ucrt" "-IC:\Program Files (x86)\Windows Kits\10\\include\10.0.22621.0\\um" "-IC:\Program Files (x86)\Windows Kits\10\\include\10.0.22621.0\\shared" "-IC:\Program Files (x86)\Windows Kits\10\\include\10.0.22621.0\\winrt" "-IC:\Program Files (x86)\Windows Kits\10\\include\10.0.22621.0\\cppwinrt" "-IC:\Program Files\Microsoft Visual Studio\2022\Community\VC\Tools\MSVC\14.38.33130\include" "-IC:\Program Files\Microsoft Visual Studio\2022\Community\VC\Tools\MSVC\14.38.33130\ATLMFC\include" "-IC:\Program Files\Microsoft Visual Studio\2022\Community\VC\Auxiliary\VS\include" "-IC:\Program Files (x86)\Windows Kits\10\include\10.0.22621.0\ucrt" "-IC:\Program Files (x86)\Windows Kits\10\\include\10.0.22621.0\\um" "-IC:\Program Files (x86)\Windows Kits\10\\include\10.0.22621.0\\shared" "-IC:\Program Files (x86)\Windows Kits\10\\include\10.0.22621.0\\winrt" "-IC:\Program Files (x86)\Windows Kits\10\\include\10.0.22621.0\\cppwinrt" /EHsc /TpC:/Users/ci/bug1/xyz.cc /Fo/Users/ci/bug1/xyz.obj
xyz.cc
creating C:\Users\ci\bug1\build\lib.win32-cpython-313
"C:\Program Files\Microsoft Visual Studio\2022\Community\VC\Tools\MSVC\14.38.33130\bin\HostX86\x86\link.exe" /nologo /INCREMENTAL:NO /LTCG /DLL /MANIFEST:EMBED,ID=2 /MANIFESTUAC:NO /LIBPATH:C:\Users\ci\AppData\Local\Programs\Python\Python313\libs /LIBPATH:C:\Users\ci\AppData\Local\Programs\Python\Python313 /LIBPATH:C:\Users\ci\AppData\Local\Programs\Python\Python313\PCbuild\win32 "/LIBPATH:C:\Program Files\Microsoft Visual Studio\2022\Community\VC\Tools\MSVC\14.38.33130\ATLMFC\lib\x86" "/LIBPATH:C:\Program Files\Microsoft Visual Studio\2022\Community\VC\Tools\MSVC\14.38.33130\lib\x86" "/LIBPATH:C:\Program Files (x86)\Windows Kits\10\lib\10.0.22621.0\ucrt\x86" "/LIBPATH:C:\Program Files (x86)\Windows Kits\10\\lib\10.0.22621.0\\um\x86" "/LIBPATH:C:\Program Files\Microsoft Visual Studio\2022\Community\VC\Tools\MSVC\14.38.33130\ATLMFC\lib\x86" "/LIBPATH:C:\Program Files\Microsoft Visual Studio\2022\Community\VC\Tools\MSVC\14.38.33130\lib\x86" "/LIBPATH:C:\Program Files (x86)\Windows Kits\10\lib\10.0.22621.0\ucrt\x86" "/LIBPATH:C:\Program Files (x86)\Windows Kits\10\\lib\10.0.22621.0\\um\x86" /EXPORT:PyInit_xyz /Users/ci/bug1/xyz.obj /OUT:build\lib.win32-cpython-313\xyz.cp313-win_amd64.pyd /IMPLIB:/Users/ci/bug1\xyz.cp313-win_amd64.lib
LINK : warning LNK4044: unrecognized option '/Users/ci/bug1/xyz.obj'; ignored
LINK : warning LNK4001: no object files specified; libraries used
LINK : warning LNK4068: /MACHINE not specified; defaulting to X86
   Creating library /Users/ci/bug1\xyz.cp313-win_amd64.lib and object /Users/ci/bug1\xyz.cp313-win_amd64.exp
LINK : error LNK2001: unresolved external symbol __DllMainCRTStartup@12
xyz.cp313-win_amd64.exp : error LNK2001: unresolved external symbol _PyInit_xyz

@abravalheri
Copy link
Contributor

abravalheri commented Sep 11, 2024

OK, perfect. Thank you very much for the reproducer @mglisse. I think we should be able to use this to investigate (and probably liaise with pypa/distutils to find a solution).

/cc @jaraco.

@abravalheri
Copy link
Contributor

abravalheri commented Sep 11, 2024

@mglisse, one thing that I just noticed: in your reproducer you use an absolute path as source for the extension object and backslashes. However the setuptools docs explicitly says you should use a relative path, Unix-style:

sources (list[str]) – list of source filenames, relative to the distribution root (where the setup script lives), in Unix form (slash-separated) for portability. Source files may be C, C++, SWIG (.i), platform-specific resource files, or whatever else is recognized by the “build_ext” command as source for a Python extension.

https://setuptools.pypa.io/en/latest/userguide/ext_modules.html#extension-api-reference

Can the problem that you are facing be explained by this mismatch?

@mglisse
Copy link
Author

mglisse commented Sep 11, 2024

in your reproducer you use an absolute path as source for the extension object and backslashes.

I think I only used forward slashes.

However the setuptools docs explicitly says you should use a relative path, Unix-style:

Ah... Thanks for the pointer.
That's surprising since in our case in has been working fine for years, but I guess the restriction is probably caused by more complicated cases.
Still, I wonder if a small tweak somewhere would be enough to avoid dropping "C:" from the path, even if this remains officially unsupported.

Can the problem that you are facing be explained by this mismatch?

That seems likely. We will see if we can adapt our scripts to use a relative path (it isn't trivial, setup.py is in a build directory, unlike the sources it references).

@abravalheri
Copy link
Contributor

I think I only used forward slashes.

Sorry, my bad, you are right.

Still, I wonder if a small tweak somewhere would be enough to avoid dropping "C:" from the path, even if this remains officially unsupported.

Please feel free to propose a PR either in pypa/setuptools or pypa/distutils repository.
I suppose this restriction is intentional so that all sources are contained in the project directory.
Since the problem happens in 3.13 but not in 3.12, I conjecture that there might have been a change in some function in 3.13 that was previously was documented to work only with relative paths but was still able to handle absolute paths.

That seems likely. We will see if we can adapt our scripts to use a relative path (it isn't trivial, setup.py is in a build directory, unlike the sources it references).

Thank you very much for having a look on that!

@abravalheri
Copy link
Contributor

abravalheri commented Sep 11, 2024

That seems likely. We will see if we can adapt our scripts to use a relative path (it isn't trivial, setup.py is in a build directory, unlike the sources it references).

Maybe you can try map-ing the extension sources with str(pathlib.Path(...).relative_to(...))?

@mglisse
Copy link
Author

mglisse commented Sep 11, 2024

I suppose this restriction is intentional so that all sources are contained in the project directory.

Ideally, we would have the original sources in some read-only directory somewhere, and everything generated (setup.py by cmake, off_utils.cpp by cython, etc) in a build directory elsewhere. Asking that everything is together requires either writing to the source directory, or copying the sources to the build directory, that's doable but not the cleanest.

Since the problem happens in 3.13 but not in 3.12, I conjecture that there might have been a change in some function in 3.13 that was previously was documented to work only with relative paths but was still able to handle absolute paths.

With 3.12, the object files are sent to a temporary directory that looks like the concatenation of build\temp.win32-cpython-312\Release with the source path (minus "C:"). With 3.13, it does not seem to create such a temporary directory at all (it wants to write the object next to its source). Yes, maybe the python function creating the concatenated path now produces an error and causes this bad path to appear...

That seems likely. We will see if we can adapt our scripts to use a relative path (it isn't trivial, setup.py is in a build directory, unlike the sources it references).

Maybe you can try map-ing the extension sources with str(pathlib.Path(...).relative_to(...))?

That's quite ugly, but possibly a good way to postpone having to rework our build system, thanks.
(we probably need walk_up=True, and have to cross our fingers that nobody puts the build directory on a different drive than the sources)

@mglisse
Copy link
Author

mglisse commented Sep 11, 2024

With 3.12, the object files are sent to a temporary directory that looks like the concatenation of build\temp.win32-cpython-312\Release with the source path (minus "C:"). With 3.13, it does not seem to create such a temporary directory at all (it wants to write the object next to its source). Yes, maybe the python function creating the concatenated path now produces an error and causes this bad path to appear...

From https://docs.python.org/3.13/library/os.path.html#os.path.isabs

Changed in version 3.13: On Windows, returns False if the given path starts with exactly one (back)slash.

Now looking at the code in distutils to produce the path of object files

def _make_out_path(self, output_dir, strip_dir, src_name):
base, ext = os.path.splitext(src_name)
base = self._make_relative(base)
try:
new_ext = self.out_extensions[ext]
except LookupError:
raise UnknownFileError(f"unknown file type '{ext}' (from '{src_name}')")
if strip_dir:
base = os.path.basename(base)
return os.path.join(output_dir, base + new_ext)
@staticmethod
def _make_relative(base):
"""
In order to ensure that a filename always honors the
indicated output_dir, make sure it's relative.
Ref python/cpython#37775.
"""
# Chop off the drive
no_drive = os.path.splitdrive(base)[1]
# If abs, chop off leading /
return no_drive[os.path.isabs(no_drive) :]

First it explicitly strips "C:". Then if what remains is an absolute path (changed in 3.13!) it strips one initial / or \. Finally, it uses this as second argument of os.path.join which, if the second argument is an absolute path, ignores the first (the broken case).

I find the new behavior of os.path in Python 3.13 a bit confusing. '/a/b' is considered not absolute by isabs, but it is considered absolute by join 😕 Probably it is considered kind of invalid, a relative path should not start with "/" and an absolute path should have a drive. If _make_relative wants to test if the path starts with a (back)slash, maybe it could test that instead of isabs, or test isabs(base), or even use str.lstrip since there could be multiple slashes?

@abravalheri
Copy link
Contributor

abravalheri commented Sep 11, 2024

If _make_relative wants to test if the path starts with a (back)slash, maybe it could test that instead of isabs, or test isabs(base), or even use str.lstrip since there could be multiple slashes?

That is something that can be discussed with the maintainers of pypa/distutils. I checked that the statement about Extension expecting a relative path seems to be 24 years old 😅, so not sure how much they want to flexibilize that recommendation.

@abravalheri
Copy link
Contributor

@jaraco, we might have to start testing on 3.13 to see if this change in behaviour of the stdlib affects other parts of setuptools/distutils and check if it will cause further disruption.

@jaraco
Copy link
Member

jaraco commented Sep 11, 2024

I'm about to roll out 3.13 testing on all skeleton projects. jaraco/skeleton#141 jaraco/skeleton#146.

@mglisse
Copy link
Author

mglisse commented Sep 13, 2024

Maybe you can try map-ing the extension sources with str(pathlib.Path(...).relative_to(...))?

That does give something "relative", but the code doesn't expect a relative path with a bunch of ../ at the start. Appending it to the build directory may send to unexpected places.

Monkey patching _make_relative is tempting and works (it took me a while to understand I had to patch distutils and not setuptools._distutils); creating a symlink to the source directory should allow referring to them with relative paths and would be easy if only windows 10 did not exist; the other solutions are a bit more intrusive 😞

Meanwhile I filed pypa/distutils#297, hopefully it is simple (1-liner patch) and focused (1 (private) function does not do what its doc says) enough that they will be willing to patch it, even if the motivation comes from some unsupported use case.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Needs Triage Issues that need to be evaluated for severity and status.
Projects
None yet
Development

No branches or pull requests

3 participants