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

fix: correctly find npm on Python 3.12 (win) #47

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

agoose77
Copy link

@agoose77 agoose77 commented Nov 20, 2023

On Python 3.12.0, shutil.which finds the Bash npm shell script rather than npm.CMD when the system is Windows. This PR effectively looks for PATHEXT-suffixed binaries and takes them in preference.

Fixes #46

@agoose77
Copy link
Author

@pradyunsg not sure what your notification setup is, so I'm manually pinging you. Apologies if you're already tracking this.

@pradyunsg
Copy link
Owner

Ah, so it's python/cpython#109590.

@agoose77
Copy link
Author

@pradyunsg is there anything you need me to do here? I'm under the impression you'll want to think about this, but let me know if there's any further work to be done on my part!

@pradyunsg
Copy link
Owner

No, I think I've got everything here from you -- thanks for filing this, it's genuinely appreciated! It would've taken me quite a while to realise this was a CPython bug without a prompt from this PR and this fix.

I have to spend some time context switching back into this project sometime in the next few weeks to catch up on what's borked and all that -- I've got a lot of life happening over the past few weeks for me and being a volunteer maintainer on OSS stuff comes lower in the hierarchy of things for me. :)

@agoose77
Copy link
Author

agoose77 commented Nov 27, 2023

I have to spend some time context switching back into this project sometime in the next few weeks to catch up on what's borked and all that -- I've got a lot of life happening over the past few weeks for me and being a volunteer maintainer on OSS stuff comes lower in the hierarchy of things for me. :)

Totally get it — good luck with IRL! We'll move forward over at EB with working wheels, and pick up the sdist side of things when we next circle back.

Equally, I'm happy to take on some maintainership here, so let me know if you're interested. My outgoing role is on scikit-hep/awkward, and I'm also involved in the EB project (e.g. executablebooks/jupyterlab-myst).

@agoose77
Copy link
Author

@pradyunsg do you perhaps have any more time to take a look at this PR? Let me know if there's more I can/should do here.

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.

Build failures on Windows
2 participants