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

gh-109054: Document configure variables #109224

Merged
merged 3 commits into from
Sep 20, 2023
Merged

Conversation

vstinner
Copy link
Member

@vstinner vstinner commented Sep 10, 2023

@bedevere-bot bedevere-bot added awaiting core review docs Documentation in the Doc dir labels Sep 10, 2023
@vstinner vstinner changed the title gh-109054: Document configue options gh-109054: Document configue variables Sep 10, 2023
@AlexWaygood AlexWaygood changed the title gh-109054: Document configue variables gh-109054: Document configure variables Sep 10, 2023
@vstinner
Copy link
Member Author

I'm not sure if adding a new dedicated Variables section is the best home for all these variables.

I chose to use .. cmdoption:: NAME syntax to avoid name conflicts with compiler flags defined with .. envvar:: NAME such as: https://docs.python.org/dev/using/configure.html#compiler-flags

I used ./configure --help | grep '^\s\+[A-Z]\{2,\}' command to list all configure variables, see @erlend-aasland's comment: #109101 (comment)

Initially, I just wrote this PR to document LIBATOMIC :-)

Doc/using/configure.rst Outdated Show resolved Hide resolved
Doc/using/configure.rst Outdated Show resolved Hide resolved
Doc/using/configure.rst Outdated Show resolved Hide resolved
Doc/using/configure.rst Outdated Show resolved Hide resolved
Doc/using/configure.rst Outdated Show resolved Hide resolved
@vstinner
Copy link
Member Author

I reorganized the doc. I'm not sure between envvar and cmdoption. The current doc uses envvar.

cc @hugovk

@vstinner
Copy link
Member Author

@erlend-aasland: it seems like the whole "3.3.4. Libraries Options" section is new in Python 3.11, all xxx_CFLAGS and xxx_LIBS, no?

@vstinner
Copy link
Member Author

PROFILE_TASK was added in Python 3.8.

@erlend-aasland
Copy link
Contributor

@erlend-aasland: it seems like the whole "3.3.4. Libraries Options" section is new in Python 3.11, all xxx_CFLAGS and xxx_LIBS, no?

Yes, it came with the setup.py migration we did.

Name for machine-dependent library files.


Libraries Options
Copy link
Contributor

Choose a reason for hiding this comment

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

How about Third-party dependencies or Options for third-party dependencies?

Copy link
Member Author

Choose a reason for hiding this comment

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

Go for "Options for third-party dependencies".

@vstinner
Copy link
Member Author

@erlend-aasland: I updated my PR, would you mind to review it again?

Comment on lines 329 to 328
.. cmdoption:: LIBATOMIC

Linker flags when ``cpython/pyatomic.h`` header file is used.

Default: ``LIBATOMIC='-latomic'`` if ``libatomic`` is needed, or
``LIBATOMIC=''`` otherwise (also the default when Python is cross-compiled).

.. versionadded:: 3.13

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 going away with #109344:

Suggested change
.. cmdoption:: LIBATOMIC
Linker flags when ``cpython/pyatomic.h`` header file is used.
Default: ``LIBATOMIC='-latomic'`` if ``libatomic`` is needed, or
``LIBATOMIC=''`` otherwise (also the default when Python is cross-compiled).
.. versionadded:: 3.13

Copy link
Member Author

Choose a reason for hiding this comment

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

I understand that you want me to remove this variable in the doc, so I removed it.

@vstinner
Copy link
Member Author

vstinner commented Sep 13, 2023

Oh sorry for the noise, I tried a Git tool which messed up with my PR. I fixed my PR.

@vstinner
Copy link
Member Author

PR rebased on the main branch.

Copy link
Member

@ezio-melotti ezio-melotti left a comment

Choose a reason for hiding this comment

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

I would keep the PR focused and handle capitalization issues in a separate PR.
New titles should follow the existing conventions, even if it doesn't match the preferred capitalization specified by the devguide. A follow-up PR can fix it.

Doc/using/configure.rst Outdated Show resolved Hide resolved
@vstinner
Copy link
Member Author

I would keep the PR focused and handle capitalization issues in a separate PR.

Hum, ok, I reverted these changes. I didn't expect that changing a few titles would be so controversial :-)

@vstinner
Copy link
Member Author

@erlend-aasland @hugovk @ezio-melotti: I rebased my PR on the main branch, squashed commits, I reverted title casing change (don't use Titlecase Anymore). Please review the updated PR.

@ezio-melotti: I applied your sugsestions to use :samp:, thanks.

@hugovk
Copy link
Member

hugovk commented Sep 19, 2023

Looks good, although I'll leave approval for someone who knows about these flags 👍

Copy link
Member

@AA-Turner AA-Turner left a comment

Choose a reason for hiding this comment

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

Looks good Sphinx-wise. (But same caveat as Hugo wrt the flags)

A

Doc/using/configure.rst Outdated Show resolved Hide resolved
@vstinner vstinner merged commit d41d2e6 into python:main Sep 20, 2023
@vstinner vstinner deleted the doc_configure_vars branch September 20, 2023 16:51
@vstinner
Copy link
Member Author

Thanks for the reviews. If someone has more remarks, feel free to propose a PR :-)

@erlend-aasland
Copy link
Contributor

LGTM! Thanks for documenting these things.

csm10495 pushed a commit to csm10495/cpython that referenced this pull request Sep 28, 2023
Glyphack pushed a commit to Glyphack/cpython that referenced this pull request Sep 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs Documentation in the Doc dir skip news
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants