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

Allow spawned daemon processes to remain running #57

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

specious
Copy link

@specious specious commented Jan 3, 2022

I'm slightly perplexed as to how this used to work, but testing either way shows that the daemon processes spawned by hyp daemon start have a tendency to terminate if they are launched without disconnecting their stdio from the parent process.

@OskarRubensson
Copy link

This fix works and without it the CLI-tool doesn't work. A new release with this included would be nice.

@davetapley
Copy link

What's the easiest way to test this locally? 🤔

@OskarRubensson
Copy link

What's the easiest way to test this locally? 🤔

Pull this fork to test the fix locally: https://github.com/specious/cli/tree/fix/daemon-start

Copy link

@KougatSundew KougatSundew left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have tested these changes on Fedora 34 & Windows 11. The fix works well.

@specious
Copy link
Author

What's the easiest way to test this locally? 🤔

I pull PR's into a branch to test them like this:

git fetch origin pull/57/head:pr-57
git checkout pr-57

@pcfreak30
Copy link

Please get this merged in. It's giving me headaches :sweat_smile.

@specious
Copy link
Author

specious commented Jul 7, 2022

@mafintosh, could you please take a quick look at this?

@specious
Copy link
Author

specious commented Aug 1, 2022

@pfrazee, no doubt you are busy, but all you need to do is merge and push to npm.

@specious
Copy link
Author

specious commented Aug 1, 2022

@andrewosh, maybe you could take a quick look?

@mafintosh
Copy link
Contributor

@specious hi! we've been focused on making the latest hypercore and hyperdrive (see the -next repos). i suggest you take a look at those. we'll be deprecating the deamon when those land, as they allow for much easier "embedded" workflows so you don't need a daemon at all which is nice.

suggest you join the discord if you wanna know more :)

@specious
Copy link
Author

specious commented Aug 2, 2022

That's quite exciting, but merging this pull request would fix the version that people are currently trying to use.

@ralphtheninja
Copy link

ralphtheninja commented Sep 3, 2022

@mafintosh Can we get this merged and published as a patch? I'm installing this globally with npm i @hyperspace/cli -g and manually editing this file to make it work. I realize you are working on more important things that will be coming out, but there are people relying on this to make it work on node 16.

@originalmk
Copy link

@ralphtheninja Same. I came across this project today and I wanted to give it a try. NodeJS LTS installed, everything fine... but what... it does not work! Would really appreciate if this was merged.

@specious
Copy link
Author

specious commented Sep 5, 2022

For the time being, we can install it directly from this pull request:

npm i -g hypercore-protocol/cli#pull/57/head

@ralphtheninja
Copy link

ralphtheninja commented Sep 6, 2022

For the time being, we can install it directly from this pull request:

I had no clue you could do this. Very nice!

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.

9 participants