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

bpo-43956: Doc/c-api: Fix default program name #24876

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

joukewitteveen
Copy link
Contributor

@joukewitteveen joukewitteveen commented Mar 15, 2021

This is a minor documentation fix.

The default program name is set in Python/pathconfig.c. It is python on Windows, and python3 elsewhere.

https://bugs.python.org/issue43956

@the-knights-who-say-ni
Copy link

the-knights-who-say-ni commented Mar 15, 2021

Hello, and thanks for your contribution!

I'm a bot set up to make sure that the project can legally accept this contribution by verifying everyone involved has signed the PSF contributor agreement (CLA).

Recognized GitHub username

We couldn't find a bugs.python.org (b.p.o) account corresponding to the following GitHub usernames:

@joukewitteveen

This might be simply due to a missing "GitHub Name" entry in one's b.p.o account settings. This is necessary for legal reasons before we can look at this contribution. Please follow the steps outlined in the CPython devguide to rectify this issue.

You can check yourself to see if the CLA has been received.

Thanks again for the contribution, we look forward to reviewing it!

@joukewitteveen
Copy link
Contributor Author

joukewitteveen commented Apr 2, 2021

Additionally, the documentation currently says:

The argument should point to a zero-terminated wide character string in static storage whose contents will not change for the duration of the program’s execution.

The code, however, duplicates the string, so I am not sure this is true. Because the duplication might be an implementation detail that is intentionally left open to change, I did not remove this sentence.

@joukewitteveen joukewitteveen changed the title Doc/c-api: Fix default program name bpo-43956: Doc/c-api: Fix default program name Apr 27, 2021
@ammaraskar
Copy link
Member

ammaraskar commented Apr 28, 2021

The code, however, duplicates the string, so I am not sure this is true. Because the duplication might be an implementation detail that is intentionally left open to change, I did not remove this sentence.

@vstinner do you have an opinion on this? Looks like this part of your commit made it so that Py_SetProgramName duplicates the string:

argument should point to a zero-terminated wide character string in static
storage whose contents will not change for the duration of the program's
execution. No code in the Python interpreter will change the contents of
this storage.
Copy link
Member

@vstinner vstinner Apr 28, 2021

Choose a reason for hiding this comment

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

I suggest to link to https://docs.python.org/dev/c-api/init_config.html#c.PyConfig.program_name which documents the default value.

Copy link
Contributor Author

@joukewitteveen joukewitteveen May 1, 2021

Choose a reason for hiding this comment

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

Good point. I extended the text with a bit more detail. This includes removing the 'static lifetime' remark. Please let me know if that should be reinstated.

@vstinner
Copy link
Member

vstinner commented May 12, 2021

@joukewitteveen joukewitteveen requested a review from vstinner 5 days ago

I already reviewed the PR. IMO it's a bad idea to document the default program in Py_SetProgramName(). It should link to https://docs.python.org/dev/c-api/init_config.html#c.PyConfig.program_name which is the reference.

Py_SetProgramName() is a legacy API and should be avoided.

@vstinner
Copy link
Member

vstinner commented May 12, 2021

I created bpo-44113: "[C API] Deprecate legacy API for configure Python initialization".

@joukewitteveen
Copy link
Contributor Author

joukewitteveen commented May 12, 2021

@joukewitteveen joukewitteveen requested a review from vstinner 5 days ago

I already reviewed the PR.

But I had updated it since, in response to your comments.

I created bpo-44113: "[C API] Deprecate legacy API for configure Python initialization".

I updated the PR to only document the specifics of Py_SetProgramName(). Even though it may be legacy, there is no reason to have incorrect documentation.

The full program name inference is done in Python/initconfig.c and the
reference documentation is with PyConfig.program_name.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting review docs Documentation in the Doc dir skip news
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants