bpo-32398: Remove OSX c++ linking workaround in distutils. #4965
Conversation
When compiling GDAL with python support, before the removed code we have ``` self.compiler_cxx = ['/bin/sh', '/usr/x86_64-pc-linux-gnu/bin/libtool', '--mode=compile', '--tag=CXX', 'x86_64-pc-linux-gnu-c++'] linker = ['x86_64-pc-linux-gnu-cc', '-pthread', '-shared', '-march=native', '-pipe', '-O2', '-Wall', '-Wdeclaration-after-statement', '-Wextra', '-Winit-self', '-Wunused-parameter', '-Wmissing-prototypes', '-Wmissing-declarations', '-Wformat', '-Werror=format-security', '-Wno-format-nonliteral', '-Wlogical-op', '-Wshadow', '-Werror=vla', '-Wdeclaration-after-statement', '-Wnull-dereference', '-Wduplicated-cond', '-DOGR_ENABLED', '-march=native', '-pipe', '-O2', '-I/var/tmp/paludis/build/sci-libs-gdal-2.1.1/work/gdal-2.1.1/port', '-DGDAL_COMPILATION'] ``` and after the removed code ``` linker = ['/bin/sh', '-pthread', '-shared', '-march=native', '-pipe', '-O2', '-Wall', '-Wdeclaration-after-statement', '-Wextra', '-Winit-self', '-Wunused-parameter', '-Wmissing-prototypes', '-Wmissing-declarations', '-Wformat', '-Werror=format-security', '-Wno-format-nonliteral', '-Wlogical-op', '-Wshadow', '-Werror=vla', '-Wdeclaration-after-statement', '-Wnull-dereference', '-Wduplicated-cond', '-DOGR_ENABLED', '-march=native', '-pipe', '-O2', '-I/var/tmp/paludis/build/sci-libs-gdal-2.1.1/work/gdal-2.1.1/port', '-DGDAL_COMPILATION'] ``` which leads to next [error](https://www.mail-archive.com/freebsd-ports@freebsd.org/msg41030.html): ``` /bin/sh: -d: invalid option ``` Some [workarounds](https://www.michael-joost.de/gdal_install.html) have been found, but anyway the code removed: 1. ...is a hack about OSX but there is no platform checking, 2. ...assumes linker and compiler commands have similar structure and environment settings (no documentation reference found about that), and 3. ...assumes `env`, if used, does not come with any modifier.
|
Hello, and thanks for your contribution! I'm a bot set up to make sure that the project can legally accept your contribution by verifying you have signed the PSF contributor agreement (CLA). Unfortunately we couldn't find an account corresponding to your GitHub username on bugs.python.org (b.p.o) to verify you have signed the CLA (this might be simply due to a missing "GitHub Name" entry in your b.p.o account settings). This is necessary for legal reasons before we can look at your contribution. Please follow the steps outlined in the CPython devguide to rectify this issue. Thanks again to your contribution and we look forward to looking at it! |
esuarezsantana
changed the title
There was a problem hiding this comment.
I don't know whether this test is still needed for macOS and we don't do any regular testing with C++ on macOS. So, without more investigative work, I would be reluctant to remove it. Perhaps @ronaldoussoren has an opinion. I also don't know whether this code is needed on other platforms; I would guess not because I'm guessing the use of 'env' is or was part of Apple's universal compile/link wrappers around cc/gcc/clang. If it is causing problems elsewhere (after all these years), I would be OK with adding a platform == 'darwin' to it.
|
A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated. Once you have made the requested changes, please leave a comment on this pull request containing the phrase |
|
How about something like - linker[i] = self.compiler_cxx[i]
+ if linker[i] != self.compiler_cxx[i]:
+ # issue a warning
+ linker[i] = self.compiler_cxx[i]so this workaround may be removed in the future. And in the meanwhile, workaround the workaround |
|
I don't know if the patch is still needed. It used to be necessary to use a C++ compiler in the linking phase when compiling C++ code, and not use the C compiler for that because the C++ compiler would do some additional work. I don't know if that's still needed. The block of code that this patch uses is platform independent, but was added for macOS because it is more likely to use /usr/bin/env in a compiler invocation there (or at least it used to be). In particular, we started out with using "/usr/bin/env cc" as the compiler and linker, the code block skips the call to "/usr/bin/env" and extracts the name of the real executable. The issue on bugs.python.org is not clear enough to understand what's going on, and therefore I can't give advise on what would be the best way forward. The issue appears to be related to cross building, but doesn't mention what the relevant settings w.r.t. compilers are there (that is, the values of CC, CXX, ... in the Makefile) BTW. I though that discussions about issues would be kept on bugs.python.org and only discussions about code would be on Github. P.S. Issue that introduced this code: https://bugs.python.org/issue1488098 |
|
@esuarezsantana, please respond on the bug tracker answering @ronaldoussoren's questions. Thank you! |
|
I'm going to close this as it appears to be abandoned. If the original author is still interested, the questions and concerns can be discussed on the bug tracker and this PR can be reopened if it's the proper change. Thanks! |
When compiling GDAL with python support, before the removed code we have
and after the removed code
which leads to next error:
Some workarounds have been found, but anyway the code removed:
...is a hack about OSX but there is no platform checking,
...assumes linker and compiler commands have similar structure and
environment settings (no documentation reference found about that), and
...assumes
env, if used, does not come with any modifier.https://bugs.python.org/issue32398