Repository navigation
bpo-10483: http.server - what is executable on Windows - #29624
MaxwellDupre wants to merge 10 commits into
Conversation
Sorry, something went wrong.
|
Hello, and thanks for your contribution! I'm a bot set up to make sure that the project can legally accept this contribution by verifying everyone involved has signed the PSF contributor agreement (CLA). Recognized GitHub usernameWe couldn't find a bugs.python.org (b.p.o) account corresponding to the following GitHub usernames: This might be simply due to a missing "GitHub Name" entry in one's b.p.o account settings. This is necessary for legal reasons before we can look at this contribution. Please follow the steps outlined in the CPython devguide to rectify this issue. You can check yourself to see if the CLA has been received. Thanks again for the contribution, we look forward to reviewing it! |
Sorry, something went wrong.
| if interp.lower().endswith("w.exe"): | ||
| # On Windows, use python.exe, not pythonw.exe | ||
| interp = interp[:-5] + interp[-4:] |
There was a problem hiding this comment.
This check isn't necessary. The child process is spawned using pipes for standard I/O, which works fine with the non-console "pythonw.exe" executable. This is probably left over from when it called os.popen2() (link to Guido's revision that added this code 21 years ago).
Even if it did matter, it should only check for exactly "pythonw.exe" nowadays, e.g. os.path.normcase(os.path.basename(interp)) == os.path.normcase("pythonw.exe").
Sorry, something went wrong.
There was a problem hiding this comment.
I have made the change in local pushed to Github but I cant figure out how to push it to Python/cpython from my copy, can you give me a clue?
Sorry, something went wrong.
|
Hi Eryk,
Should I remove it from the change? If yes, how do I do that?
Revert/cancel and resubmit another change/pull? Or another change and
squash the two?
Regards,
Mike
…On Sat, 4 Dec 2021 at 22:41, Eryk Sun ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In Lib/http/server.py
<#29624 (comment)>:
> + if interp.lower().endswith("w.exe"):
+ # On Windows, use python.exe, not pythonw.exe
+ interp = interp[:-5] + interp[-4:]
This check isn't necessary. The child process is spawned using pipes for
standard I/O, which works fine with the non-console "pythonw.exe"
executable. This is probably left over from when it called os.popen2() (
link
<e7d6b0a#diff-590102ca60c804a5ea71a9a5e03b0ae0b5806be5f2452e2ca726cdeb6092558fR216-R234>
to Guido's revision that added this code 21 years ago).
Even if it did matter, it should only check for exactly "pythonw.exe"
nowadays, e.g. os.path.normcase(os.path.basename(interp)) ==
os.path.normcase("pythonw.exe").
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#29624 (review)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ADLRPZACEXJJP6KC7DXJAPTUPKKKPANCNFSM5IKHF6GA>
.
Triage notifications on the go with GitHub Mobile for iOS
<https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675>
or Android
<https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.
|
Sorry, something went wrong.
|
Mike, make the change in your repo, then push the PR to github again. |
Sorry, something went wrong.
| self.replace_paths = replace_paths if replace_paths is not None else [] | ||
| self.processed_paths = [] # Used in debugging only | ||
| self.recurse = recurse | ||
|
|
There was a problem hiding this comment.
no need to set self.recurse twice
Sorry, something went wrong.
There was a problem hiding this comment.
I have made the change in local repro.
Sorry, something went wrong.
| @@ -0,0 +1,171 @@ | |||
| --- a/Lib/modulefinder.py | |||
There was a problem hiding this comment.
the issue1284670.patch file should not be part of the PR, and I'm guessing that _bootstrap_python should also not be part of the PR
Sorry, something went wrong.
There was a problem hiding this comment.
I have tried to delete in my Github copy but I can only delete _bootstrap_python. Github doesn't allow me to delete the patch file in my cpython copy. How do I get rid of it?
Sorry, something went wrong.
|
A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated. Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request. |
Sorry, something went wrong.
|
Please add a NEWS entry (you can use https://blurb-it.herokuapp.com/ for it). And you also included _bootstrap_python by mistake. |
Sorry, something went wrong.
|
Sorry for the mess. Its better I close this PR and I will submit a cleaner PR later. Thanks for your comments. |
Sorry, something went wrong.
mikecmcleod
https://bugs.python.org/issue10483