-
-
Notifications
You must be signed in to change notification settings - Fork 30.4k
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-41818: Add os.login_tty() for *nix. #23740
Conversation
@asvetlov Here is the implementation of Edit: This will help us make the code in "Lib/pty.py" and "Lib/test/test_pty.py" much simpler. cc: @ethanfurman @aeros @gpshead |
@zoulasc Sir, can you please take a look at the changes that I made to posixmodule.c? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
@zoulasc Thank you, sir. |
Signed-off-by: Soumendra Ganguly <soumendraganguly@gmail.com>
There were 2 commits that only changed the wording in the news file; rebased to have a single commit. |
This PR is stale because it has been open for 30 days with no activity. |
Still waiting for core review. |
This PR is stale because it has been open for 30 days with no activity. |
@@ -12735,7 +12735,7 @@ $as_echo "no" >&6; } | |||
fi | |||
rm -f core conftest.err conftest.$ac_objext conftest.$ac_ext | |||
|
|||
# check for openpty and forkpty | |||
# check for openpty, login_tty, and forkpty |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks like you're directly editing configure
? this is generated code. edit the input configure.ac
instead and regenerate configure. (i suspect might have just forgotten to commit your configure.ac
change to the PR branch?)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sir, thank you for the prompt review. I will be honest; I actually did manually edit configure
. I will learn how to do it and make the requested changes to configure.ac
by Saturday morning.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have edited configure.ac
and generated configure
using autoconf
now. Are there any steps that I might have missed/done improperly? When I generated the configure
script, it added the runstatedir
option to it which was not there before; please let me know if that was not supposed to happen/how to fix it. Also, it says that this branch has "conflicts that must be resolved"; Modules/clinic/posixmodule.c.h is the conflicting file; what is the best way to fix this problem; I want to avoid the XY problem this time!
Signed-off-by: Soumendra Ganguly <soumendraganguly@gmail.com>
@gpshead Any updates on this? |
I see a merge conflict now. Please fix. |
Signed-off-by: Soumendra Ganguly <soumendraganguly@gmail.com>
@asvetlov @gpshead The merge conflict in |
This follows #23536. Also, see #23546, #23686.
os.login_tty()
calls the nativelogin_tty()
when it is available; otherwise, ifsetsid()
is available and ifTIOCSCTTY
orttyname()
are available, it runs generic code to emulatelogin_tty()
.Post #23686, #23546, #23740 goals:
test_winsize()
to "Lib/test/test_pty.py";Signed-off-by: Soumendra Ganguly soumendraganguly@gmail.com
https://bugs.python.org/issue41818