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

Implement termios.tcgetattr/tcsetattr using P/Invoke #1910

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

BCSharp
Copy link
Member

@BCSharp BCSharp commented Feb 11, 2025

I kept a limited workaround for .NET because using tcsetattr to put stdin into the raw mode does not have the desired effect on .NET (works fine on Mono) — one can still edit the input line using all standard readline control keys. I have a feeling that for console support .NET is using lower level calls than the simple read on a descriptor 0, however, I didn't have time to investigate this in depth. I am submitting this PR with the workaround though before my window of availability for this project narrows.

Copy link
Contributor

@Lamparter Lamparter left a comment

Choose a reason for hiding this comment

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

Can you use LibraryImportAttribute instead?

@slozier
Copy link
Contributor

slozier commented Feb 12, 2025

Can you use LibraryImportAttribute instead?

Unfortunately it's one of those .NET 7+ exclusive features which we cant use (afaik) because of .NET Framework.

@Lamparter
Copy link
Contributor

Lamparter commented Feb 12, 2025

What about using #if !NETFRAMEWORK && NET_7_0_OR_GREATER?
That would be a bit messy though, maybe just leave it for now

@BCSharp BCSharp marked this pull request as draft February 12, 2025 21:41
@BCSharp
Copy link
Member Author

BCSharp commented Feb 12, 2025

What about using #if !NETFRAMEWORK && NET_7_0_OR_GREATER? That would be a bit messy though, maybe just leave it for now

#if NET_7_0_OR_GREATER should suffice. However, this would further multiply the number of prototypes. Because of platform differences, there are already two per call, which would grow to four per call, which we would have to debug, test, and maintain. This module is a niche module, rarely used, and even if so, the calls are infrequent, so I don't think that the performance gain is worth sacrificing code maintainability. Perhaps fcntl.lockf and flock would be more worthwhile. Although I wonder how much performance gain is really there. The runtime stub generation should be happening only once per run.

I thought that the biggest advantage of LimportLibrary is that it allows for AOT compilation, but this is something that is already tricky (if not impossible for any meaningful use) for IronPython. I can be totally wrong here as I have no experience with the IronPython compiler (ipyc, ngen, etc.).

I am putting this PR in draft because #1912 has to be sorted out first.

@Lamparter
Copy link
Contributor

I thought that the biggest advantage of LimportLibrary is that it allows for AOT compilation, but this is something that is already tricky (if not impossible for any meaningful use) for IronPython. I can be totally wrong here as I have no experience with the IronPython compiler (ipyc, ngen, etc.).

Yeah, that's ultimately the goal
Though don't forget that IronPython is powered by the dynamic language runtime 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants