Skip to content

gh-133537: only use console when available #133538

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

Merged
merged 2 commits into from
May 7, 2025
Merged

Conversation

maxbachmann
Copy link
Contributor

@maxbachmann maxbachmann commented May 6, 2025

Alternatively to always returning false the function could only be available when the partition supports console io. Similar to the functions in winconsoleio which are only available when the partition supports console io.

Copy link
Contributor

@sharktide sharktide left a comment

Choose a reason for hiding this comment

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

Please explain your changes and how they help in a more understandable way so the review process can go a lot faster :) TiA

@maxbachmann
Copy link
Contributor Author

maxbachmann commented May 6, 2025

GetConsoleMode is only available when HAVE_WINDOWS_CONSOLE_IO is defined. Using it without the guard fails to compile on platforms like the xbox where console io isn't available.

I wasn't sure whether this should get a news entry. If it should I can add one.

Copy link
Contributor

@sharktide sharktide left a comment

Choose a reason for hiding this comment

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

I wasn't sure wheter this should get a news entry. If it should I can add one

In this case it doesn't really matter, since XBox and similar partitions aren't commonly used by CPython developers.

@ZeroIntensity what do you think

@ZeroIntensity
Copy link
Member

I have absolutely no idea if we support compiling on XBox (if we do, that's sort of hilarious), but we do accept changes for unsupported platforms, which do need blurb entries. I'm not enough of an expert here to deal with this any further.

@chris-eibl, I heard you know a thing or two about Windows. Would you mind looking at this?

@chris-eibl
Copy link
Member

@maxbachmann already improved building on xbox in the past - #102256 was quite a big task.

This looks good to me, just needs a blurb entry.

Pinging @zooba for final steps.

@chris-eibl chris-eibl added OS-windows interpreter-core (Objects, Python, Grammar, and Parser dirs) build The build process and cross-build labels May 7, 2025
@zooba
Copy link
Member

zooba commented May 7, 2025

Going to have to wait until beta 1 gets released, but yeah, this is fine.

And we totally support building on Xbox, plenty of games have embedded Python in the past and we want that to be able to continue. We don't have a ready release though, but Max has done a ton of work in the past to make it work nicely. Xbox OS is basically just a modified version of Windows1, so everything works the same, but there are some differences in what APIs are available so that it's easier to trust any apps.

Footnotes

  1. Very vague description, but I'm not sure how much I can share publicly about how it's put together.

Copy link
Contributor

@sharktide sharktide left a comment

Choose a reason for hiding this comment

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

This PR seems to be good, but we will have to wait until Beta 1 gets released as zooba said

@zooba zooba merged commit 1460cce into python:main May 7, 2025
43 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build The build process and cross-build interpreter-core (Objects, Python, Grammar, and Parser dirs) OS-windows
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants