GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — bpo-41818: Add os.login_tty() for *nix. by 8vasu · Pull Request #23740 · python/cpython

Conversation

@8vasu

@8vasu 8vasu commented Dec 11, 2020 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor

This follows #23536. Also, see #23546, #23686.

os.login_tty() calls the native login_tty() when it is available; otherwise, if setsid() is available and if TIOCSCTTY or ttyname() are available, it runs generic code to emulate login_tty().

Post #23686, #23546, #23740 goals:

  1. add test_winsize() to "Lib/test/test_pty.py";
  2. major revision of "Lib/pty.py", which has not happened since the beginning of the millennium.

Signed-off-by: Soumendra Ganguly [email protected]

https://bugs.python.org/issue41818

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@the-knights-who-say-ni the-knights-who-say-ni added the CLA signed label Dec 11, 2020
@bedevere-bot bedevere-bot added the awaiting review label Dec 11, 2020
This was referenced Dec 11, 2020
bpo-41818: Make additions to the tty module #23546
Closed
bpo-41818: Add termios.tcgetwinsize(), termios.tcsetwinsize(). Update docs. #23686
Merged
@8vasu

8vasu commented Dec 11, 2020 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor Author

@asvetlov Here is the implementation of os.login_tty() that you had suggested. Thank you for being patient.

Edit: This will help us make the code in "Lib/pty.py" and "Lib/test/test_pty.py" much simpler. cc: @ethanfurman @aeros @gpshead
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@8vasu

8vasu commented Dec 14, 2020

Copy link
Copy Markdown
Contributor Author

@zoulasc Sir, can you please take a look at the changes that I made to posixmodule.c? os_login_tty_impl() is the function that I mentioned earlier. Thank you for your kind help.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
zoulasc
zoulasc approved these changes Dec 14, 2020
View reviewed changes

@zoulasc zoulasc left a comment

Copy link
Copy Markdown

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@bedevere-bot bedevere-bot added awaiting core review and removed awaiting review labels Dec 14, 2020
@8vasu

8vasu commented Dec 14, 2020

Copy link
Copy Markdown
Contributor Author

@zoulasc Thank you, sir.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@8vasu
Soumendra Ganguly: add os.login_tty() for *nix.
ddc3219 
Signed-off-by: Soumendra Ganguly <[email protected]>
@8vasu
8vasu force-pushed the os_login_tty branch from 59d22bf to ddc3219 Compare December 17, 2020 09:25
@8vasu

8vasu commented Dec 17, 2020

Copy link
Copy Markdown
Contributor Author

There were 2 commits that only changed the wording in the news file; rebased to have a single commit.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@github-actions

github-actions Bot commented Jan 17, 2021

Copy link
Copy Markdown

This PR is stale because it has been open for 30 days with no activity.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@github-actions github-actions Bot added the stale Stale PR or inactive for long period of time. label Jan 17, 2021
@8vasu

8vasu commented Jan 27, 2021

Copy link
Copy Markdown
Contributor Author

Still waiting for core review.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@github-actions github-actions Bot removed the stale Stale PR or inactive for long period of time. label Jan 28, 2021
@github-actions

github-actions Bot commented Feb 28, 2021

Copy link
Copy Markdown

This PR is stale because it has been open for 30 days with no activity.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@github-actions github-actions Bot added the stale Stale PR or inactive for long period of time. label Feb 28, 2021
@8vasu

8vasu commented Aug 27, 2021

Copy link
Copy Markdown
Contributor Author

@gpshead This follows #23686; I need to update Modules/clinic/posixmodule.c.h but that can be done quickly using argument clinic when necessary. I request you to please review this if/when you have time.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@gpshead
Update os.rst versionadded to 3.11
b6267a6
gpshead
gpshead reviewed Aug 27, 2021
View reviewed changes
Comment thread configure
rm -f core conftest.err conftest.$ac_objext conftest.$ac_ext

# check for openpty and forkpty
# check for openpty, login_tty, and forkpty

@gpshead gpshead Aug 27, 2021

Copy link
Copy Markdown
Member

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?)

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@8vasu 8vasu Aug 27, 2021

Copy link
Copy Markdown
Contributor Author

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.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@8vasu 8vasu Aug 28, 2021

Copy link
Copy Markdown
Contributor Author

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!

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@gpshead gpshead self-assigned this Aug 27, 2021
8vasu added 2 commits August 28, 2021 01:39
@8vasu
Add check for login_tty() to configure.ac
e372f55 
Signed-off-by: Soumendra Ganguly <[email protected]>
@8vasu
Merge remote-tracking branch 'dummy/os_login_tty' into os_login_tty
a70889f
@8vasu

8vasu commented Nov 15, 2021

Copy link
Copy Markdown
Contributor Author

@gpshead Any updates on this?
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@asvetlov

asvetlov commented Nov 18, 2021

Copy link
Copy Markdown
Contributor

I see a merge conflict now. Please fix.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@8vasu
Fix merge conflict in Modules/clinic/posixmodule.c.h
ec3b44b 
Signed-off-by: Soumendra Ganguly <[email protected]>
@8vasu
8vasu force-pushed the os_login_tty branch from fefc1fd to ec3b44b Compare November 20, 2021 06:11
@8vasu 8vasu closed this Nov 20, 2021
@8vasu
8vasu deleted the os_login_tty branch November 20, 2021 07:20
@8vasu 8vasu mentioned this pull request Nov 20, 2021
bpo-41818: Add os.login_tty() for *nix. #29658
Merged
@8vasu

8vasu commented Nov 20, 2021

Copy link
Copy Markdown
Contributor Author

@asvetlov @gpshead The merge conflict in posixmodule.c.h was easy to fix but the ones in configure were a little harder; I realized that I had unknowingly added some unnecessary changes to configure since I did not have the "autoconf-archive" package installed; I decided to start fresh instead of rebasing; please check #29658. All other code remains unchanged.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@8vasu 8vasu mannequin mentioned this pull request May 5, 2022
Lib/pty.py major revision #85984
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@gpshead gpshead gpshead left review comments
+1 more reviewer
@zoulasc zoulasc zoulasc approved these changes
Reviewers whose approvals may not affect merge requirements

Uh oh!

There was an error while loading. Please reload this page.

Assignees

@gpshead gpshead

Labels

awaiting core review stale Stale PR or inactive for long period of time.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

6 participants

@8vasu @asvetlov @gpshead @zoulasc @the-knights-who-say-ni @bedevere-bot
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.