ENH: Don't add system library directories to rpath

[DWesl]

Last of the patches from #73
Might close pypa/setuptools#3257
Might also close pypa/setuptools#3450, depending on what the actual problem looks like

Dual purposes here:

• On platforms like Windows or Cygwin that don't have rpath, try to avoid adding things to rpath (Support Cygwin patches #73, maybe [BUG] Recent release introduced a breaking change on MingW setuptools#3450)
• Some distribution binary package makers require that no shared library list a system library directory (/lib, /lib64, /usr/lib, /usr/lib64) in its rpath because these directories are always in the runtime linker search path; this patch simplifies the code to ensure the shared library can find its dependencies at runtime ([BUG] 62.0.0: setuptools._distutils.unixccompiler hardcodes on linking rpath setuptools#3257)

[isuruf]

You are assuming that LIBDIR is in the dynamic linker's default search directories which is not true for some python distributions.

[DWesl]

So this should stay a distribution patch? Should there be a note for packagers in the setuptools release notes that any patches on the CPython distutils should be duplicated on setuptools._distutils? (Yes this is implied by "setuptools now uses its own copy of distutils", but not everyone makes that connection).

[jaraco]

So this should stay a distribution patch?

I’d say no. The goal of Setuptools adoption of distutils is to remove the need for distribution patches. If a distribution wishes to customize the behavior, Setuptools would like to devise a hook such that a distribution can configure that behavior and not have to patch it… although one supported way of customizing behavior is through a patch to sysconfig.

[DWesl]

You are assuming that LIBDIR is in the dynamic linker's default search directories which is not true for some python distributions.

Is there a way to access the dynamic linker's default search directories from python?

[jaraco]

You are assuming that LIBDIR is in the dynamic linker's default search directories which is not true for some python distributions.

Is there a way to access the dynamic linker's default search directories from python?

I don't understand this concern well enough to comment. I'm hoping @isuruf or someone else can comment.

[jaraco]

Would it be possible to add a test or tests that capture the missed expectation? That way, even if this solution ends up not being the right one, a subsequent change would still honor the concerns this change is attempting to address?

[isuruf]

Is there a way to access the dynamic linker's default search directories from python?

I don't think there is any. I guess there are couple of ways to allow this to work for your use-case.

1. Introduce a sysconfig variable that downstream python maintainers add to their _sysconfigdata* files
2. Check for LIBDIR and check that LIBDIR starts with /usr/lib.

[DWesl]

Would it be possible to add a test or tests that capture the missed expectation? That way, even if this solution ends up not being the right one, a subsequent change would still honor the concerns this change is attempting to address?

I think so; the rough outline would be to create a C Extension module linked against some shared library in LIBDIR, check its RUNPATH (not sure how to do that) to make sure that's unset, then try to import it to check whether the runtime loader can find all the dependencies. The first and third bits might already be in the test suite.

Is there a way to access the dynamic linker's default search directories from python?

I don't think there is any. I guess there are couple of ways to allow this to work for your use-case.

1. Introduce a sysconfig variable that downstream python maintainers add to their _sysconfigdata* files

2. Check for LIBDIR and check that LIBDIR starts with /usr/lib.

That second one looks like a good starting point.

[DWesl]

Changes suggested by black in CI

[DWesl]

More suggestions from black

[jaraco]

Is there a way to access the dynamic linker's default search directories from python?

I don't think there is any. I guess there are couple of ways to allow this to work for your use-case.

1. Introduce a sysconfig variable that downstream python maintainers add to their _sysconfigdata* files

2. Check for LIBDIR and check that LIBDIR starts with `/usr/lib`.

I like idea 1 best, because it gives the end user more control, but the second idea may be suitable too if it captures the essential concern.

[jaraco]

What's the status of this PR? @DWesl are you still working on it? It looks like there was maybe just one lingering concern.

[DWesl]

Could you remind me what that concern is? It's been a bit since I worked on this.

One concern would be all the new tests failing. I should figure out how to compile extensions on Linux, and whether renaming an so causes problems

[jaraco]

After scanning through the messages, I guess I was mistaken. I see you addressed adding a test and of the two options suggested by isuruf, you elected the second. So now it's just a matter of making sure the tests pass. I'll work on it.

[jaraco]

The tests don't appear to be passing. I see four failures in build_ext:

FAILED distutils/tests/test_build_ext.py::TestBuildExt::test_build_ext[False] - AssertionError: assert ('RPATH' not in '\nDynamic s...       0x0\n'
FAILED distutils/tests/test_build_ext.py::TestBuildExt::test_build_ext[True] - distutils.errors.LinkError: command '/usr/bin/gcc' failed with exit code 1
FAILED distutils/tests/test_build_ext.py::TestParallelBuildExt::test_build_ext[False] - AssertionError: assert ('RPATH' not in '\nDynamic s...       0x0\n'
FAILED distutils/tests/test_build_ext.py::TestParallelBuildExt::test_build_ext[True] - distutils.errors.LinkError: command '/usr/bin/gcc' failed with exit code 1

Can you investigate?

[jaraco]

@DWesl Let me know if you want to continue working on this, or if we should abandon it.

[DWesl]

I don't understand why copying /usr/lib/libz.so to /tmp/libxx_z.so and trying to link with -L/tmp -lxx_z fails, so I'm not going to be a lot of help finishing.

If you know a library installed outside /usr/lib*/ on the Linux GHA runners, I can use that get a working test, otherwise it might be simplest to drop the test of rpath for non-system library directories.

EDIT: If you've already got a test of RPATH that this behavior won't break, then this PR is done barring removal of that part of the test.

[DWesl]

Alternately, you could declare this PR tests the new behavior, and tests of old behavior are out of scope, at which point I delete the part of the test that checks the old behavior

[DWesl]

I dropped the test of old behavior, so that test passes, but now ruff fails with the extremely informative message "file would be changed". As I can't run ruff on my machine, I'm leaving that for someone else.

[jaraco]

@isuruf or @lazka Would you be willing to check this change for reasonableness? Tests are passing, but I'm a little uneasy to accept it without a second set of eyes more familiar with the domain. Thanks.

[lazka]

I don't have much experience with rpath (and why one would set it via setuptools), but I wonder if setup.py explicitly requests rpath to be set why remove some of them again? And what is so special about LIBDIR, and /usr/lib with regards to rpath (what about /usr/lib64 used by fedora?)? and the tests seem unfinished (parametrize with one value, left over sleep call)

Regarding the cygwin part here, I was trying to fix it back then in #150 but missed that cygwincompiler is not actually used outside of tests (see #185) so I think we should move the cygwin check to ignore runtime_library_dirs to unixcompiler as was initially intended.

[isuruf]

if setup.py explicitly requests rpath to be set why remove some of them again?

I think the main reason is that some distributions fail when creating a RPM/DEB package when there is a rpath for the system library dir. See the issues that OP posted. Usually, packages in setup.py add this rpath so that when building packages locally, the package works without having to set LD_LIBRARY_PATH which is a bad thing to do. Ideally packages should only add RPATH if the path is not a package in the dynamic linker's path.

And what is so special about LIBDIR, and /usr/lib with regards to rpath (what about /usr/lib64 used by fedora?)?

/usr/lib and /usr/lib64 are in the dynamic linker's default paths to check. RPATHs just adds to the paths to check, so technically adding them is not necessary.

I'm not sure if removing these directories is the correct thing to do, but I don't think it would break anything.

[DWesl]

if setup.py explicitly requests rpath to be set why remove some of them again?

I think the main reason is that some distributions fail when creating a RPM/DEB package when there is a rpath for the system library dir. See the issues that OP posted. Usually, packages in setup.py add this rpath so that when building packages locally, the package works without having to set LD_LIBRARY_PATH which is a bad thing to do. Ideally packages should only add RPATH if the path is not a package in the dynamic linker's path.

Specifically, it looks like it is distrubution policy not to release a package with the system library paths in the rpath of any shared library, because the system library paths are always searched outside certain specific situations. I suspect they don't like the redundancy, as the space gains would be minor.

And what is so special about LIBDIR, and /usr/lib with regards to rpath (what about /usr/lib64 used by fedora?)?

/usr/lib and /usr/lib64 are in the dynamic linker's default paths to check. RPATHs just adds to the paths to check, so technically adding them is not necessary.

I'm not sure if removing these directories is the correct thing to do, but I don't think it would break anything.

I think the LIBDIR check is a simple check for whether python is a system python, so this doesn't break pythons installed to alternate directories not in the default search paths. That seems to have been inherited from the Fedora patch mentioned in #73.

Since my motivation (crashes on Cygwin) seems to have been alleviated in other ways, the main alternatives would be leaving it to distributions that have such policies to patch setuptools on their own.