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

Spawn go daemon does not work with Node.js v9.9.0+ #223

Closed
alanshaw opened this issue Mar 29, 2018 · 14 comments
Closed

Spawn go daemon does not work with Node.js v9.9.0+ #223

alanshaw opened this issue Mar 29, 2018 · 14 comments
Labels
exp/expert Having worked on the specific codebase is important help wanted Seeking public contribution on this issue kind/bug A bug in existing code (including security flaws) P0 Critical: Tackled by core team ASAP

Comments

@alanshaw
Copy link
Member

.spawn never calls back.

@victorb
Copy link
Member

victorb commented Mar 29, 2018

We're already testing js-ipfsd-ctl on nodejs 9.2.0. Is there a specific version you're thinking about?

@alanshaw
Copy link
Member Author

Possibly, I'm on 9.10.0. I'll add a reproducible example when I get a sec.

@victorb
Copy link
Member

victorb commented Mar 29, 2018

I see. I added a test-run with 9.10.0 activated (together with the old ones we're already running): https://ci.ipfs.team/blue/organizations/jenkins/IPFS%2Fjs-ipfsd-ctl/detail/master/40/pipeline/

@victorb
Copy link
Member

victorb commented Mar 29, 2018

@victorb
Copy link
Member

victorb commented Mar 29, 2018

On Jenkins (linux at least), the node-gyp builds are not even working properly: https://ci.ipfs.team/blue/organizations/jenkins/IPFS%2Fjs-ipfsd-ctl/detail/master/41/pipeline/13

@dryajov
Copy link
Member

dryajov commented Mar 29, 2018

Taking a look now as well.

@dryajov
Copy link
Member

dryajov commented Mar 29, 2018

BTW - everything up until 9.8.0 seems to work as expected (I'm on 9.0.0 myself), v9.10.0 is pretty broken.

@dryajov dryajov changed the title Spawn go daemon does not work with Node.js 9 Spawn go daemon does not work with Node.js v9.8.0+ Mar 29, 2018
@dryajov dryajov changed the title Spawn go daemon does not work with Node.js v9.8.0+ Spawn go daemon does not work with Node.js v9.9.0+ Mar 29, 2018
@dryajov dryajov added kind/bug A bug in existing code (including security flaws) exp/expert Having worked on the specific codebase is important P0 Critical: Tackled by core team ASAP status/ready Ready to be worked help wanted Seeking public contribution on this issue labels Mar 29, 2018
@victorb
Copy link
Member

victorb commented Apr 3, 2018

This is probably related to jbenet/node-subcomandante#6

@travisperson
Copy link
Member

We just published v1.2.0 for node-subcomandante, which should fix this issue. I ran the tests locally, but am running into what I believe is a type check issue when running js-ipfs in proc.

ipfs/js-ipfs#1331

@travisperson
Copy link
Member

Package ipfsd-ctl@0.32.1 wants ipfs-repo@^0.19.0
 from .
 resolves to ./node_modules/ipfs-repo@0.19.0

Package ipfs-repo@0.19.0 wants ipfs-block@~0.7.1
 from ./node_modules/ipfs-repo
 resolves to ./node_modules/ipfs-repo/node_modules/ipfs-block@0.7.1

Package ipfs@0.28.2 wants ipfs-repo@~0.18.7
 from ./node_modules/ipfs
 resolves to ./node_modules/ipfs/node_modules/ipfs-repo@0.18.7

Package ipfs-repo@0.18.7 wants ipfs-block@~0.6.1
 from ./node_modules/ipfs/node_modules/ipfs-repo
 resolves to ./node_modules/ipfs-block@0.6.1

Package ipfs@0.28.2 wants ipfs-block@~0.6.1
 from ./node_modules/ipfs
 resolves to ./node_modules/ipfs-block@0.6.1

This resolves down into:

  • ipfsd-ctl wants ipfs-repo@0.19.0, and ipfs wants ipfs-repo@0.18.7,
  • ipfs-repo@0.19.0 uses ipfs-block@0.7.1
  • ipfs-repo@0.18.7 uses ipfs-block@0.6.1

Both of these packages check blocks differently, so they are not compatible.

Downgrading the ipfs-repo dependency of ipfsd-ctl to ipfs-repo@0.18.7 fixes the issue and all tests now pass on Node.js v9.9.0!

@dryajov
Copy link
Member

dryajov commented May 2, 2018

Weird this would show only under v9.9.0? I've seen the block error pop up with previous versions. Does using the correct dependencies (npm link?) fix the issue as well?

@travisperson
Copy link
Member

Weird this would show only under v9.9.0?

Sorry if this was confusing. The block error has nothing to do with Node.js v9.9.0 or node-subcomandante, I'm just unable to verify that the newly published version of node-subcomandante works in master due to the block issue without downgrading the dependency ipfs-repo.

Everything works correctly in Node.js v9.9.0 if I downgrade ipfs-repo to a compatible version that works with the js-ipfs dependency (v0.28.2).

@dryajov
Copy link
Member

dryajov commented May 2, 2018

@travisperson oh! Got you, thanks for clarifying - this makes sense then 👍

@hugomrdias
Copy link
Member

@alanshaw can i close this? jenkins is running node 10 and tests are passing

@ghost ghost removed the status/ready Ready to be worked label Sep 19, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
exp/expert Having worked on the specific codebase is important help wanted Seeking public contribution on this issue kind/bug A bug in existing code (including security flaws) P0 Critical: Tackled by core team ASAP
Projects
None yet
Development

No branches or pull requests

5 participants