Skip to content

gh-98718: re-use already calculated is_python_build from getpath#98719

Open
FFY00 wants to merge 1 commit into
python:mainfrom
FFY00:bpo-98718-is_python_build
Open

gh-98718: re-use already calculated is_python_build from getpath#98719
FFY00 wants to merge 1 commit into
python:mainfrom
FFY00:bpo-98718-is_python_build

Conversation

@FFY00

@FFY00 FFY00 commented Oct 26, 2022

Copy link
Copy Markdown
Member

This also gets rid of the duplicated logic.

@FFY00
FFY00 requested review from jaraco and zooba October 26, 2022 13:19
@FFY00 FFY00 changed the title bpo-98718: re-use already calculated is_python_build from getpath gh-98718: re-use already calculated is_python_build from getpath Oct 26, 2022
@FFY00
FFY00 force-pushed the bpo-98718-is_python_build branch from 665bcf7 to 080a70a Compare October 26, 2022 13:21
@zooba

zooba commented Oct 26, 2022

Copy link
Copy Markdown
Member

Seems fine to me, though I assume some others have more opinions about tacking stuff onto the sys module. I think it's fine.

@FFY00

FFY00 commented Oct 26, 2022

Copy link
Copy Markdown
Member Author

I think the biggest win here is really unifying the logic in one place.

@serhiy-storchaka

Copy link
Copy Markdown
Member

Does not it need documentation?

Would not it be more useful to expose build_prefix (None or not set if is not Python ) rather of simple boolean _is_python_build?

@jaraco

jaraco commented Nov 6, 2022

Copy link
Copy Markdown
Member

Does not it need documentation?

As proposed, I see it as an internal implementation detail, so wouldn't be documented.

Would not it be more useful to expose build_prefix (None or not set if is not Python ) rather of simple boolean _is_python_build?

+1. is_python_build() could still return bool(sys._build_prefix).

@jaraco jaraco left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM, but I'm unsure about the correctness. Is sys._is_python_build always a correct indicator at runtime? How can we have more confidence it's the correct value in different scenarios?

@zooba

zooba commented Nov 7, 2022

Copy link
Copy Markdown
Member

Is sys._is_python_build always a correct indicator at runtime?

Nope, but we don't have a proper one. Any marker file could disappear or be copied, so we may as well look for the things we actually care about (stdlib modules) and guess, rather than look for something that's there solely to help us guess.

At least if we expose it properly, it'll be easier to change it to something more reliable if we come up with one.

Signed-off-by: Filipe Laíns <lains@riseup.net>
@FFY00

FFY00 commented Nov 9, 2022

Copy link
Copy Markdown
Member Author

Does not it need documentation?

It's missing a news entry, but other than that, no, it's an internal change.

Would not it be more useful to expose build_prefix (None or not set if is not Python ) rather of simple boolean _is_python_build?

We can do that. The reason I didn't do it here was because I didn't want to add any more public API, so I just used the already exported _is_build_prefix. I opened #99273 following this suggestion.

@FFY00
FFY00 force-pushed the bpo-98718-is_python_build branch from 080a70a to eaa03ca Compare August 8, 2023 01:27
@github-actions

Copy link
Copy Markdown

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

@github-actions github-actions Bot added the stale Stale PR or inactive for long period of time. label Apr 11, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants