bpo-32593: distutils UnixCCompiler: remove FreeBSD special case by vstinner · Pull Request #5233 · python/cpython

@bapt: This PR is a follow-up of your #5232 (review) comment.

Would you mind to review it and test it?

I don't know how I can test this given after building with the patch I can't find any elf file with rpath assigned.
That said I run the testsuite both prior and after the patch on FreeBSD current with exactly the same results:
383 tests OK.

3 tests failed:
test_gdb test_posix test_uuid

20 tests skipped:
test_curses test_devpoll test_epoll test_msilib test_ossaudiodev
test_smtpnet test_socketserver test_spwd test_startfile
test_timeout test_tix test_tk test_ttk_guionly test_urllib2net
test_urllibnet test_winconsoleio test_winreg test_winsound
test_xmlrpc_net test_zipfile64

This is run on FreeBSD current FreeBSD 12.0-CURRENT #1 r326932M: Mon Dec 18 17:09:51 CET 2017 as a user (not root)

"3 tests failed: test_gdb test_posix test_uuid"

What are these failures? All tests are supposed to pass on FreeBSD. But I'm aware of an issue with test_uuid, a recent regression.

Originating issue for this code is issue 20767. Based on my memory and understanding, the code being proposed for removal was not added (in 3.6) for and was not specific to FreeBSD 9.

Unless something has changed (eg: upstream @ clang/llvm and merged back to all clang versions used in all supported FreeBSD versions), it doesn't appear removing this code is relevant to removing 'EoL OS support'

Having said that, if a (version) unconditional 'freebsd' special case is no longer required at all, removal is fine.

Interesting and thank you for this input, the thing is yes clang rejects -R but does not reject -Wl,-R which will pass the argument directly to the linker like gcc does (I just tested with clang 3.3 and earlier version).

Looking at the rest of the code path, what was failing for that issue is/was the detection of clang as a compiler gcc compatible which is the only situation where -R can be passed to the compiler directly.

So in my opinion this PR is right as this code should be removed (it is wrong), then in addition a proper fix would be added here: https://github.com/python/cpython/blob/master/Lib/distutils/unixccompiler.py#L237 by treating clang as gcc, btw the gcc detection https://github.com/python/cpython/blob/master/Lib/distutils/unixccompiler.py#L209 is wrong as well, in many systems, the gcc compiler is just named cc meaning this test will fail.

@koobs: "it doesn't appear removing this code is relevant to removing 'EoL OS support'"

Sorry, I abused this bpo number. I created this change as a follow up of #5232 (review)

@koobs, @bapt: If the change looks good to you, would you mind to use the GitHub review to "Approve" this PR? By the way, I would be more confident if you both approve the change :-)

So after a deeper look, the change is not ok without a change in the code that finds if the compiler is gcc to make it accept clang as well. see my previous comment.