-
Notifications
You must be signed in to change notification settings - Fork 865
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
Provide ipfs
command line tools on install
#727
Comments
Can you tackle this next... it'd really help us with the "easy install" story for ipfs-inactive/website#299 |
Some notes from IRC about Symlinking to
|
Thanks for bringing all the thoughts here @lidel. Another thing I want to mention is about Windows: on Windows we actually need to add a directory to the PATH since there's no default folder to put binaries into on Windows. And I'm not yet sure if we need elevated permissions during installation. Windows actually has two PATHs: a system-wide one and a user-specific one. I'll investigate a bit more about that. On other news, we probably need to take in account #836. If we are going to add it to the PATH, perhaps we should solve both issues at once since they're closely related. Could you please comment on those? /cc @lidel @olizilla |
I like the general idea from #836! What if we dont execute
|
I personally enjoy the idea of having that wrapper around the On the first run, we could add "our" script to the |
If we manually add We could detect if To keep it simple, let's just assume user wants ipfs-desktop to do the thing (default to bundled version without any prompts). |
Ref for myself: https://nsis.sourceforge.io/Path_Manipulation#Warning |
Great stuff! To check we are on the same page, I don't think we should change the definition of the users I strongly agree that we should keep this simple, and for the default case where On first run it attempt to symlink a shell script
I'm in favour of @lidel's suggestion of adding a wrapper that we can use to manage which |
So, let's come to an agreement of what we need to do to close this issue.
You don't agree on changing the
Does it bundle My suggestion for now:
Other option I like is: to make Or make an |
@hacdias please review the code in atom that does this. You get to decide how it works on windows. I recommend checking how some other apps do it. As much as possible the process should be the same on macOS vs Linux, so there are fewer variations to test. |
@olizilla on Windows it seems they just don't add anything to the PATH. On macOS, they add it on the first run. See here.. On Linux, they add it during installation. See here. Basically what I've suggested.
As @lidel mentioned, you can't modify What's still to decide is what to put there: the bundled version? Ignore anything that already exists? We could assume that someone using IPFS Desktop doesn't install This would not take in account #836 though. |
Get the simplest thing working first. We're gonna need tests for the 3 different paths. Link to a shellscript that gives us enough indirection to change how it works later / update the binary without needing to update the link. |
So, we now have this enabled on all packages we ship except AppImage. It isn't possible with AppImage unfortunately. At least I haven't found a way to do so. Although I've found an interesting solution that could help us making the process more uniform on Linux and macOS. If we did it on runtime, we could use this package ( What do you think @lidel? I believe it's not common on Linux to ask on runtime for permissions, but it'd be much easier and give more flexibility (like reverting the option in the interface). And it would work with all installers. |
@hacdias I must agree, unifying everything around The only ask for Linux runtime would be to remove the "surprise factor" caused by sudo call.
|
Atom.app provides the
apm
command when it is installed, as do many other apps. It would be rad if installing IPFS Desktop added theipfs
command to your PATH if it's not already present.We already bundle the binaries, so it's just a matter of figuring out how to make them available.
The text was updated successfully, but these errors were encountered: