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-20806: os.times document points to wrong section of non-Linux manual #15479

Merged
merged 4 commits into from
Sep 7, 2019

Conversation

nanjekyejoannah
Copy link
Contributor

@nanjekyejoannah nanjekyejoannah commented Aug 25, 2019

Change reference from times(2) to times(3p) .

https://bugs.python.org/issue20806

@nanjekyejoannah
Copy link
Contributor Author

No need for news entry .

Copy link
Contributor

@aeros aeros left a comment

Choose a reason for hiding this comment

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

Thanks for the PR @nanjekyejoannah.

To determine what the correct man page should be, I compared times(2) and times(3p). Also, I referred to the man page conventions from man-pages(7).

The two pages have a number of similarities, but from what I can tell, it looks like times(2) is the correct one to link to in this context. From man-pages(7), manual sections with a (2) refer to system calls and those with a (3) refer to library functions . Since this is within the os module, I think it makes more sense to refer to man page for the system version if it exists. This can be seen in the other parts of os.rst, where it links to open(2) instead of open(3p) and access(2) instead of access(3p).

Additionally, only times(2) mentions the OS standards that it conforms to (since times(3p) is referring to the C library function):

CONFORMING TO 
       POSIX.1-2001, POSIX.1-2008, SVr4, 4.3BSD.

Based on the above conclusions, I think this section should remain as is, and link to times(2) instead of times(3p). However, I'll admit that I'm not the most experienced when it comes to the man page conventions. So, I'll defer this to the experts.

From the blame, it looks like @benjaminp added this in as part of the conversion from svn to github, so there's not an easy way of telling who wrote this section initially. Since there's no current os expert, I'll notify Serhiy since he is the expert for os.path.

/cc @serhiy-storchaka

Edit: There is also a substantial argument that times(3p) would apply more universally (as mentioned by R. David Murray) since it is part of the "POSIX Programmer's Manual" whereas times(2) is part of the "Linux Programmer's Manual". But if that's the deciding factor, open(2) should be changed to open(3p) and access(2) should be changed to access(3p), as well as any others which are currently using (2) and have an equivalent in (3p). There seems to be decent arguments for using either one.

Co-Authored-By: Serhiy Storchaka <storchaka@gmail.com>
@nanjekyejoannah
Copy link
Contributor Author

@serhiy-storchaka , resolved.

@serhiy-storchaka
Copy link
Member

For open it is even more complicated. (2) is Linux Programmer's Manual, (3) refers to Tcl Built-In Commands, and (3p) is a Debian specific section name.

@nanjekyejoannah
Copy link
Contributor Author

For open it is even more complicated. (2) is Linux Programmer's Manual, (3) refers to Tcl Built-In Commands, and (3p) is a Debian specific section name.

Are you suggesting we also look at improving the open documentation too as part of this PR?

@aeros
Copy link
Contributor

aeros commented Aug 29, 2019

@serhiy-storchaka:

For open it is even more complicated. (2) is Linux Programmer's Manual, (3) refers to Tcl Built-In Commands, and (3p) is a Debian specific section name.

Interesting, I wasn't aware that (3p) was sometimes used for Debian specific utilities. Is there a particular source you would recommend for finding out that information? The resource I usually use, man7.org, did not have any mention of Debian on their open(3p) page: http://man7.org/linux/man-pages/man3/open.3p.html.

@nanjekyejoannah:

Are you suggesting we also look at improving the open documentation too as part of this PR?

Out of the options Serhiy mentioned, (2) seems to be the most relevant to OS, which is the one the page currently uses. As far as I'm aware, it wouldn't too much sense to refer to a man page that is specific to a particular Linux distribution (in this case, Debian).

@ammaraskar
Copy link
Member

(3p) was sometimes used for Debian specific utilities.

I've never seen it used in that context. AFAIK, 3 in general is libc functions and then 3p is the posix libc documentation.

@aeros
Copy link
Contributor

aeros commented Aug 31, 2019

@ammaraskar:

I've never seen it used in that context. AFAIK, 3 in general is libc functions and then 3p is the posix libc documentation.

That was similar to my interpretation as well, which is why I asked @serhiy-storchaka this question:

For open it is even more complicated. (2) is Linux Programmer's Manual, (3) refers to Tcl Built-In Commands, and (3p) is a Debian specific section name.

Is there a particular source you would recommend for finding out that information?

@nanjekyejoannah
Copy link
Contributor Author

@serhiy-storchaka, I was wondering If I have addressed all reviews on this PR or I have missed something.

@miss-islington
Copy link
Contributor

Thanks @nanjekyejoannah for the PR, and @serhiy-storchaka for merging it 🌮🎉.. I'm working now to backport this PR to: 3.7.
🐍🍒⛏🤖

@miss-islington
Copy link
Contributor

Thanks @nanjekyejoannah for the PR, and @serhiy-storchaka for merging it 🌮🎉.. I'm working now to backport this PR to: 3.8.
🐍🍒⛏🤖

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Sep 7, 2019
…thonGH-15479)

(cherry picked from commit 3ccdbc3)

Co-authored-by: Joannah Nanjekye <33177550+nanjekyejoannah@users.noreply.github.com>
@bedevere-bot
Copy link

GH-15722 is a backport of this pull request to the 3.8 branch.

@bedevere-bot
Copy link

GH-15723 is a backport of this pull request to the 3.7 branch.

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Sep 7, 2019
…thonGH-15479)

(cherry picked from commit 3ccdbc3)

Co-authored-by: Joannah Nanjekye <33177550+nanjekyejoannah@users.noreply.github.com>
@serhiy-storchaka
Copy link
Member

I left the PR open for some time just for the case if other core developers have to add something.

Thank you for your contribution @nanjekyejoannah.

miss-islington added a commit that referenced this pull request Sep 7, 2019
…-15479)

(cherry picked from commit 3ccdbc3)

Co-authored-by: Joannah Nanjekye <33177550+nanjekyejoannah@users.noreply.github.com>
miss-islington added a commit that referenced this pull request Sep 7, 2019
…-15479)

(cherry picked from commit 3ccdbc3)

Co-authored-by: Joannah Nanjekye <33177550+nanjekyejoannah@users.noreply.github.com>
@nanjekyejoannah nanjekyejoannah deleted the issue20806 branch September 7, 2019 11:41
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.

8 participants