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

docs: GNU symbol visibility option #994

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

aaron-bray
Copy link
Contributor

@aaron-bray aaron-bray commented Jun 7, 2023

Address issue #856

docs/usage.rst Outdated Show resolved Hide resolved
docs/usage.rst Outdated Show resolved Hide resolved
@aaron-bray aaron-bray force-pushed the 856-Document-SKBUILD_GNU_SKIP_LOCAL_SYMBOL_EXPORT_OVERRIDE branch 4 times, most recently from 2e7b2d2 to 76d8540 Compare June 7, 2023 18:56
@aaron-bray aaron-bray requested a review from jcfr June 7, 2023 19:29
`CMake option <https://scikit-build.readthedocs.io/en/latest/usage.html#cmake-configure-options>`_
or
`setup.py option <https://scikit-build.readthedocs.io/en/latest/usage.html#scikit-build-options>`_.

Copy link
Contributor

@jcfr jcfr Jun 7, 2023

Choose a reason for hiding this comment

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

Suggested change
Note that for Python >= 3.9, setting the option `SKBUILD_GNU_SKIP_LOCAL_SYMBOL_EXPORT_OVERRIDE` to `ON` allow to ...

It would be nice to further improve to provide some historical contest based on the summary available at #703 (comment)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added a little more detail as to how this works and on what systems.

docs/usage.rst Outdated Show resolved Hide resolved
@aaron-bray aaron-bray force-pushed the 856-Document-SKBUILD_GNU_SKIP_LOCAL_SYMBOL_EXPORT_OVERRIDE branch from 76d8540 to a98311f Compare June 8, 2023 00:15
@aaron-bray aaron-bray requested a review from jcfr June 8, 2023 00:16
@aaron-bray aaron-bray force-pushed the 856-Document-SKBUILD_GNU_SKIP_LOCAL_SYMBOL_EXPORT_OVERRIDE branch from 06ab36d to 2410b1c Compare June 8, 2023 00:17
@codecov
Copy link

codecov bot commented Jun 8, 2023

Codecov Report

Merging #994 (62bf0d7) into main (d06e872) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main     #994   +/-   ##
=======================================
  Coverage   86.26%   86.26%           
=======================================
  Files          33       33           
  Lines        1587     1587           
  Branches      351      351           
=======================================
  Hits         1369     1369           
  Misses        155      155           
  Partials       63       63           

Controlling exported symbol visibility
--------------------------------------

When using a GNU based compiler on Linux, scikit-build will reduce the binary size by removing all the native exported methods.
Copy link
Contributor

Choose a reason for hiding this comment

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

This is part of PythonExtensions, not all of scikit-build, right? Most or all pybind11 and nanobind projects do not use PythonExtensions. Also, FindPythonInterp and FindPythonLibs is being removed (sort-of) from CMake 3.27; users using FindPython won't be affected by this variable either, I believe.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jcfr is there another caveot I am missing as to when this would be used?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants