Respect CXX when parsing linker parameters for UNIX c++ targets #356
Conversation
Previously, when parsing linker parameters for C++ targets, the CC variable was used to determine what the "prefix" of the command was in order to determine what the linker arguments were. If the value of LDCXXSHARED did not match CC, the first argument would be dropped as it was assumed to be the linker command. However, if the command was a wrapper, such as ccache, it could lead to compile problems as the generated command would be incorrect. In the following scenario: LDCXXSHARED="ccache g++ -shared -Wl,--enable-new-dtags" CC="ccache gcc" CXX="ccache g++" The command would be incorrectly parsed to: ccache g++ g++ -shared -Wl,--enable-new-dtags Now, the CXX value is used to improve the chances of parsing the linker arguments correctly to generate: ccache g++ -shared -Wl,--enable-new-dtags LDCXXSHARED and CXX still need to be in sync either in the environment or within the sysconfig variables in the CPython build for parsing to work correctly. The CXX value is now also respected when linking executable binaries. Signed-off-by: Vincent Fazio <vfazio@gmail.com>
Signed-off-by: Vincent Fazio <vfazio@gmail.com>
vfazio
force-pushed
the
vfazio-cxx-linking-arg-parse
branch
from
c85dcbd to
c0d6d71
Compare
vfazio
changed the title
|
@jaraco Mind taking a glance at this and the related issue? |
|
This is excellent. It makes the code even more symmetric and captures the failure clearly. The thorough analysis in the reported issue gives me high confidence this is the right fix. Thank you for everything. |
derp, except it looks like its breaking mingwin builds.. i'll probably need help debugging that maybe i should ignore the test on that platform if they're not respecting the values the same way? |
No problem. Please take a look and if the fix isn't obvious, feel free to tag in |
Address the following: * use the compiler's executable extension in asserts * remove any environment variables that may have been injected by CI * add a pragma to ignore a line without coverage Signed-off-by: Vincent Fazio <vfazio@gmail.com>
|
Ok, may be ready for another run. I don't have a mingwin box to putz with but my guess is it's the CXX environment variable that is being injected via the CI pipeline. I guess we'll find out what else I missed after this |
|
@jaraco do you mind kicking off another pipeline at your convenience? Hopefully, if everything succeeds, I'd like to backport this to fix some build issues we're seeing in Buildroot until it gets pulled into a new version of setuptools. Thanks! |
I should be able to get it rolled out in Setuptools in short order, if that helps. |
That would be really helpful! Thanks! I'm always a bit nervous when touching important packages due to the chance to ripple and cripple, so hopefully this changeset is concise, reasonable, and doesn't end up causing headaches for users. |
Exactly. And most importantly, it adds a new test case that will prevent from regression, so we're continuing to iterate toward perfection. Thanks for the contrib! |
|
Setuptools fix pending in pypa/setuptools#4959. |
Previously, when parsing linker parameters for C++ targets, the CC
variable was used to determine what the "prefix" of the command was in
order to determine what the linker arguments were.
If the value of LDCXXSHARED did not match CC, the first argument would
be dropped as it was assumed to be the linker command.
However, if the command was a wrapper, such as ccache, it could lead to
compile problems as the generated command would be incorrect.
In the following scenario:
LDCXXSHARED="ccache g++ -shared -Wl,--enable-new-dtags"
CC="ccache gcc"
CXX="ccache g++"
The command would be incorrectly parsed to:
"ccache g++ g++ -shared -Wl,--enable-new-dtags"
Now, the CXX value is used to improve the chances of parsing the linker
arguments correctly to generate:
"ccache g++ -shared -Wl,--enable-new-dtags"
LDCXXSHARED and CXX still need to be in sync either in the environment
or within the sysconfig variables in the CPython build for parsing to
work correctly.
The CXX value is now also respected when linking executable binaries.
closes #355