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

deps: Including additional binaries with npm? #13640

Closed
zkat opened this issue Jun 12, 2017 · 25 comments
Closed

deps: Including additional binaries with npm? #13640

zkat opened this issue Jun 12, 2017 · 25 comments
Labels
discuss Issues opened for discussions and feedbacks. npm Issues and PRs related to the npm client dependency or the npm registry.

Comments

@zkat
Copy link
Contributor

zkat commented Jun 12, 2017

Hey y'all!

So, the npm team is planning on bundling npx with the standard npm distribution sometime soonish. We figured we should let y'all know asap so we can start having a conversation about how this would affect your own distribution (if at all), and if there's anything you need from us to get this to work -- since right now, we only ship the npm binary, and this would mean an additional npx getting installed into users' bin dir.

idk if there's much to say here besides that! If this is just fine, feel free to close it, and we'll give y'all a heads up when we push a version of npm with npx bundled into it (in the next month or so). We think npx is gonna be a pretty core/important tool for npm users, since it'll give everyone easier access to the various CLI tools available in the npm registry, as well as solve a long-standing problem where there's been no standardised way to invoke devDependency binaries while working interactively.

@mscdex
Copy link
Contributor

mscdex commented Jun 12, 2017

To be clear, "binaries" here doesn't mean actual compiled executables/addons but instead node script files, right?

@mscdex mscdex added npm Issues and PRs related to the npm client dependency or the npm registry. discuss Issues opened for discussions and feedbacks. labels Jun 12, 2017
@zkat
Copy link
Contributor Author

zkat commented Jun 12, 2017

@mscdex yes, I'm using binaries in the npm parlance sense -- executables. In this case, it's definitely just a node script.

@bnoordhuis
Copy link
Member

Did you check for name conflicts with existing programs? I didn't find anything called 'npx' in apt (debian) but that's not exhaustive.

@targos
Copy link
Member

targos commented Jun 13, 2017

No result on Fedora with dnf search npx and dnf provides npx.

@refack
Copy link
Contributor

refack commented Jun 13, 2017

Nothing on chocolatey 🤣

@Fishrock123
Copy link
Contributor

There is no default formula for any npx binaries.

This does mean shipping another dep though with all the same weird caveats that shipping npm has (albeit npx is much smaller because it used npm).

@gibfahn
Copy link
Member

gibfahn commented Jun 14, 2017

This does mean shipping another dep though with all the same weird caveats that shipping npm has

Presumably it'll be included in the npm source, so it won't be a separate folder in deps/ or anything, just another dependency in our npm updates.

@bnoordhuis
Copy link
Member

I interpreted that comment as needing to update installers, etc. I'm curious if distro vendors would package it.

@Fishrock123
Copy link
Contributor

Presumably it'll be included in the npm source, so it won't be a separate folder in deps/ or anything, just another dependency in our npm updates.

I do not think this is the case, npx is it's own module?

@zkat
Copy link
Contributor Author

zkat commented Jun 17, 2017

I checked this originally when I chose the npx name, and there doesn't seem to be any such package on the couple of distros I checked, nor any real conflicts anywhere. I'm also the owner of npm.im/npx itself, so it's fine to clobber it with npm's own (and vice-versa).

It will be included with the npm sources, and installed alongside the regular npm binary. It will be an additional bin entry in npm's own package.json (essentially a re-export script). We're bundling npx with npm -- so it won't be an extra entry in deps/, it'll be in deps/npm/node_modules/npx.

@MylesBorins
Copy link
Contributor

it looks like this is going to be coming in a future semver minor of npm. If we plan to ship this we need to ensure it ends up in the binary as expected and is installed properly by the double click installers

@refack
Copy link
Contributor

refack commented Jul 9, 2017

@zkat what do you see the install formula being? i.e. if the npm lands as a zip, what will create the binary shims, or will they be already packed in the zip?

@richardlau
Copy link
Member

richardlau commented Jul 9, 2017

For our Unix tools/install.py this is the relevant bit that creates/removes the npm bin symlinks:

node/tools/install.py

Lines 92 to 99 in 23498f2

# create/remove symlink
link_path = abspath(install_path, 'bin/npm')
if action == uninstall:
action([link_path], 'bin/npm')
elif action == install:
try_symlink('../lib/node_modules/npm/bin/npm-cli.js', link_path)
else:
assert(0) # unhandled action type

@refack
Copy link
Contributor

refack commented Jul 9, 2017

The windows MSI installer currently has only directives for the npm shims - https://github.com/nodejs/node/blob/master/tools/msvs/msi/product.wxs#L210-L216

@zkat
Copy link
Contributor Author

zkat commented Jul 9, 2017

I haven't added the built-in shims yet, but I think it's safe to assume right now that they'll mirror the npm ones, but replacing npm with npx. I'm literally just gonna copy them over and frob them into npx-ness, but I want to confirm that this is the right thing to do before I get around to that. This bit of npm is newer to me.

@zkat zkat mentioned this issue Jul 11, 2017
4 tasks
@zkat
Copy link
Contributor Author

zkat commented Jul 11, 2017

The PR is up for your consideration, curiosity, and merge bit ;)

@zkat zkat mentioned this issue Jul 14, 2017
4 tasks
@gustavnikolaj
Copy link

gustavnikolaj commented Jul 17, 2017

I am not sure this is the right place to raise this, or if I am in any position to do so, but as I haven't seen anyone present this view, I am going to take the liberty to do so anyway.

Something like npm obviously needs to be bundled with node, but I don't think the same is true for a package like npx. Why is it not just a userland package that people can opt to install themselves?

If this question has been discussed in other places, I apologize for missing it.

To make it perfectly clear, I am an npm-fanboy myself (I think npm is far more attractive than yarn etc) and I'm most likely going to be very happy with npx as well. I built a similar thing myself - my version was called nr and was a lot simpler - just a bash function around npm bin.

@refack
Copy link
Contributor

refack commented Jul 17, 2017

I am not sure this is the right place to raise this, or if I am in any position to do so, but as I haven't seen anyone present this view, I am going to take the liberty to do so anyway.

Something like npm obviously needs to be bundled with node, but I don't think the same is true for a package like npx. Why is it not just a userland package that people can opt to install themselves?

  1. IMHO users' input is of equal or even higher value than the academic musings of the inner-circle people, so more power to you ✊

  2. (just my "academic" musings) in node core's context, it's npm's choice. If "they" decided to bundle it in the default bundle, then we should follow suit. So maybe a better thread is npx: bundle npx with npm itself npm/npm#17685 🤷‍♂️

P.S. As I mentioned above I also implemented a similar "hack" by adding :./node_codule/.bin: to my PATH.

@evanlucas
Copy link
Contributor

I am +1 to bundling npx. At Help.com, we use the official tarballs in docker images and our CI uses a docker-compose like tool to run our test suites. To avoid manually installing all of our dependencies twice (to get the tool to run the test suite and in the docker container), we have to npm install <tool> and then run it. Including npx would greatly simplify this process for us as we wouldn't have to bake it into the docker image ourselves.

@mohsen1
Copy link

mohsen1 commented Aug 11, 2017

Sorry but I don't use npx and don't want my cli autocomplete polluted with it. Installing npx from npm isn't that much of hassle and if someone wants to use it they can install it in a few seconds.

@gibfahn
Copy link
Member

gibfahn commented Aug 11, 2017

Sorry but I don't use npx and don't want my cli autocomplete polluted with it. Installing npx from npm isn't that much of hassle and if someone wants to use it they can install it in a few seconds.

Seems like something to take up with npm (see npm/npm#17685). AIUI the npm team think this should be available by default, and if you don't like it you can always uninstall it in a few seconds.

To be clear, I haven't actually used npx, so I don't have an opinion on bundling it.

@TimothyGu
Copy link
Member

@mohsen1 Your CLI autocomplete isn't polluted by default, only by choice IIRC.

@gibfahn
Copy link
Member

gibfahn commented Aug 12, 2017

Your CLI autocomplete isn't polluted by default, only by choice IIRC.

What kind of autocomplete do you mean?

If you do np<Tab> in a shell with "binary-in-path" autocomplete then there is now an extra binary (not that it makes a difference, it's easier to just type npm).

image

@TimothyGu
Copy link
Member

@gibfahn https://www.npmjs.com/package/npx#shell-auto-fallback

@richardlau
Copy link
Member

npx landed with npm 5.3.0 in #14235 and released in Node.js 8.2.0.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discuss Issues opened for discussions and feedbacks. npm Issues and PRs related to the npm client dependency or the npm registry.
Projects
None yet
Development

No branches or pull requests