Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork32k
gh-107658: Use same logic to look for exec_prefix as for prefix#107661
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
base:main
Are you sure you want to change the base?
Conversation
and look for landmark for exec_prefix in library_dir
bedevere-bot commentedAug 5, 2023
Most changes to Pythonrequire a NEWS entry. Please add it using theblurb_it web app or theblurb command-line tool. |
ghost commentedAug 5, 2023 • edited by ghost
Loading Uh oh!
There was an error while loading.Please reload this page.
edited by ghost
Uh oh!
There was an error while loading.Please reload this page.
@jpe please may you add a NEWS entry? |
@@ -0,0 +1 @@ | |||
Use same logic to look for exec_prefix as for prefix |
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.
Please explain the new logic here.
if library and not exec_prefix: | ||
library_dir = dirname(library) | ||
if PLATSTDLIB_LANDMARK: | ||
if isdir(joinpath(library_dir, PLATSTDLIB_LANDMARK)): |
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.
This check could also go at line 605 (pre-change; 611 post-change), where it would be with the rest of the search forexec_prefix
.
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.
My intent was to mimic the blocks that detect prefix -- the block above corresponds to the block for prefix at 553 (the leading comment is nearly identical) and the block for exec_prefix following it corresponds to the block for prefix at 583. I can see how it might be better to rearrange some of the logic and possibly to create a function that's called for each, but the current patch minimizes the number of changes.
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 can see what the intent was, but now it's harder to follow because there are two apparently independent blocks for calculatingexec_prefix
with no obvious reason. So better to merge it into a single block.
Uh oh!
There was an error while loading.Please reload this page.
Look for landmark for exec_prefix in library_dir