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

_toolchain: Properly set compiler/linker executables on Gentoo #632

Merged
merged 1 commit into from
Sep 25, 2021
Merged

_toolchain: Properly set compiler/linker executables on Gentoo #632

merged 1 commit into from
Sep 25, 2021

Conversation

ajelinski
Copy link
Contributor

The test_toolchain_cxx.py tests definitely use compiler and linker set
with _so_cxx-suffixed parameters. After this change tests use a proper
executable instead of c++.

@whitequark
Copy link
Member

The tests are failing.

The `test_toolchain_cxx.py` tests on Gentoo definitely use compiler and
linker set with `_so_cxx`-suffixed executables. Tests use a proper
executable instead of `c++` after this change.

Signed-off-by: Adam Jeliński <ajelinski@antmicro.com>
@ajelinski
Copy link
Contributor Author

At first I was extremely confused by the error:

ValueError: unknown executable 'compiler_so_cxx' for class UnixCCompiler

as it worked and helped in my case.

After doing some research it seems the whole thing is quite Gentoo-specific. Python is built there after applying quite a few patches and adding {compiler,linker}_so_cxx to UnixCCompiler is in one of these patches: https://github.com/OpenMandrivaAssociation/python/blob/ccf96eae53697ec09e8d03ad46cd8be4ce044548/0005-Improve-distutils-C-support.patch#L243-L246 (that's not the latest version of the patch but these lines haven't changed).

I've modified the commit to try to set them but skip raising the exception if not available.

@ajelinski ajelinski changed the title _toolchain: Use sysconfig's CXX tool for shared objects too _toolchain: Properly set compiler/linker executables on Gentoo Sep 23, 2021
@codecov
Copy link

codecov bot commented Sep 25, 2021

Codecov Report

Merging #632 (4f53bf6) into master (23a44f3) will increase coverage by 0.01%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #632      +/-   ##
==========================================
+ Coverage   81.46%   81.47%   +0.01%     
==========================================
  Files          49       49              
  Lines        6446     6450       +4     
  Branches     1287     1287              
==========================================
+ Hits         5251     5255       +4     
  Misses       1008     1008              
  Partials      187      187              
Impacted Files Coverage Δ
nmigen/_toolchain/cxx.py 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 23a44f3...4f53bf6. Read the comment docs.

Copy link
Member

@whitequark whitequark left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thank you!

@whitequark whitequark merged commit da8a492 into amaranth-lang:master Sep 25, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants