feat: add 'ipfs' to PATH on macOS and Linux #896

hacdias · 2019-04-05T14:48:04Z

This adds ipfs to the PATH on Linux and macOS. Will do the Windows version on a different PR. Right now, this is what's done:

On macOS, on first run and on Linux, during install:
- If /usr/local/bin/ipfs doesn't exist, we symlink to our ipfs.sh file. Otherwise we do nothing.
During uninstall, on Linux, we remove the symlink.

Addresses #727.

License: MIT Signed-off-by: Henrique Dias <hacdias@gmail.com>

hacdias · 2019-04-06T11:59:20Z

@lidel @olizilla need your feedback about what we currently do: I don't feel great about the idea of doing nothing when the user has IPFS already, but I think we have margin to play since we use a script and don't link directly to the binary.

Could you please review the scripts to see if they're OK?

License: MIT Signed-off-by: Henrique Dias <hacdias@gmail.com>

hacdias · 2019-04-06T12:17:21Z

WIP update on Linux: only working on deb.

hacdias · 2019-04-06T12:31:02Z

As per electron-userland/electron-builder#3436 it seems we can't run install scripts on some platforms. Ideally we could do everything on runtime. @lidel is there a way on Linux to ask permission temporarily so we can modify /usr/local/bin?

License: MIT Signed-off-by: Henrique Dias <hacdias@gmail.com>

This fixes .deb to properly create/remove symlinks to ipfs and ipfs-desktop during the install. Previous version created broken symlink, and wrapper script was not included in app bundle extracted into /opt/ We now include wrapper script in resources/bin/ipfs.sh Uninstall removes broken symlinks, making it safe if someone has ipfs installed by other means

lidel

I did quick smoke-test and found issues with afterInstall in .deb:

ipfs-desktop was no longer added to PATH, symlink at /usr/local/bin/ipfs-desktop was no longer created during install (missing from custom after-install.sh)
Created symlink for ipfs pointed at non-existing file:

$ ls /usr/local/bin/ipfs
lrwxrwxrwx 1 root staff 27 Apr  6 21:57 /usr/local/bin/ipfs -> /var/lib/assets/bin/ipfs.sh

$ ls /var/lib/assets/bin/ipfs.sh
ls: cannot access '/var/lib/assets/bin/ipfs.sh': No such file or directory

I fixed above and ensured wrapper script is included in bundled app under resources/bin/ipfs.sh – it should work now as expected, at least with .deb (I tested on Debian).

Note that I changed assets/bin/ipfs.sh – it did not work correctly on Linux, I assumed was also broken on MacOS (cc @olizilla), but let me know if I introduced any regressions.
If so, we may need to set different path in ipfs= on each platform.

@lidel is there a way on Linux to ask permission temporarily so we can modify /usr/local/bin?

Some systems have graphical sudo installed, such as gksudo/gksu, but that is not always enough, user needs to be added to privileged group first.

There is no easy way, because regular desktop app asking for root is considered a bad practice and will raise serious eyebrows. Adding symlink on install is the only viable option on Linux IMO.

License: MIT Signed-off-by: Henrique Dias <hacdias@gmail.com>

hacdias · 2019-04-07T08:29:40Z

@lidel yes, there were some issues on macOS, but they're fixed now 😄 Thanks for the Linux fixes.

@olizilla would you mind to try building and installing?

electron-builder.yml

License: MIT Signed-off-by: Henrique Dias <hacdias@gmail.com>

assets/bin/ipfs.sh

olizilla · 2019-04-08T09:00:55Z

electron-builder.yml

+deb:
+  afterInstall: assets/build/after-install.sh
+  afterRemove: assets/build/after-remove.sh
+
 snap:


How does it work for snap installs?

As mentioned above, it doesn't. Only on deb for now.

olizilla · 2019-04-08T09:03:52Z

src/early/ipfs-script.js

+  }
+
+  // Don't make any changes if IPFS already exists...
+  if (fs.existsSync('/usr/local/bin/ipfs')) {


install.sh from a go-ipfs distribution will try to install in either /usr/local/bin or /usr/bin https://github.com/ipfs/go-ipfs/blob/622dbf7f266e513c6de00cddc5cb851aa5cbfe61/cmd/ipfs/dist/install.sh#L9

perhaps we should try and exec ipfs here to see if it exists on the users path.

olizilla · 2019-04-08T09:05:43Z

src/early/ipfs-script.js

+
+  // Don't make any changes if IPFS already exists...
+  if (fs.existsSync('/usr/local/bin/ipfs')) {
+    logger.info('[ipfs on path] was not added, already exists')


For a subsequent PR, we could prompt the user here.

"you appear to have a version of the ipfs command line tool already. Would you like to let ipfs-desktop replace it with the a version that it can keep up to date?"

...that kind of thing.

Yes, that's the goal. I'm just making this one as simple as possible.

Co-Authored-By: hacdias <hacdias@gmail.com>

License: MIT Signed-off-by: Marcin Rataj <lidel@lidel.org>

olizilla · 2019-04-09T12:19:35Z

woo haa! It works!

$ cat `which ipfs`
#!/usr/bin/env bash

# Get the full path of "resources/" directory (resolving the symlink if needed)
resources=$(dirname "$(dirname "$(test -L "$0" && readlink "$0" || echo "$0")")")
# Get the full path to ipfs binary bundled with ipfs-desktop
ipfs="$resources/app.asar.unpacked/node_modules/go-ipfs-dep/go-ipfs/ipfs"

exec "$ipfs" "$@"

$ ipfs version
ipfs version 0.4.19

hacdias · 2019-04-09T13:48:25Z

I'm merging this and I've left a comment on the issue: #727 (comment)

hacdias added 2 commits April 5, 2019 15:22

wip: add ipfs script for macOS and Linux

29c6ab7

License: MIT Signed-off-by: Henrique Dias <hacdias@gmail.com>

wip: add successfully on macOS

212fdc0

License: MIT Signed-off-by: Henrique Dias <hacdias@gmail.com>

ghost assigned hacdias Apr 5, 2019

ghost added the in progress label Apr 5, 2019

hacdias changed the title ~~WIP: Add ipfs to PATH~~ wip: add 'ipfs' to PATH Apr 5, 2019

feat: add after install && after remove on linux

f5f67b7

License: MIT Signed-off-by: Henrique Dias <hacdias@gmail.com>

hacdias changed the title ~~wip: add 'ipfs' to PATH~~ feat: add 'ipfs' to PATH Apr 6, 2019

hacdias requested review from olizilla and lidel April 6, 2019 11:59

hacdias changed the title ~~feat: add 'ipfs' to PATH~~ feat: add 'ipfs' to PATH on macOS and Linux Apr 6, 2019

hacdias changed the title ~~feat: add 'ipfs' to PATH on macOS and Linux~~ wip: add 'ipfs' to PATH on macOS and Linux Apr 6, 2019

fix: move to deb

b44a81f

License: MIT Signed-off-by: Henrique Dias <hacdias@gmail.com>

hacdias added 2 commits April 6, 2019 14:23

feat: move scripts and run on rpm too

acefd45

License: MIT Signed-off-by: Henrique Dias <hacdias@gmail.com>

fix: rpm keys

d4152c1

hacdias changed the title ~~wip: add 'ipfs' to PATH on macOS and Linux~~ feat: add 'ipfs' to PATH on macOS and Linux Apr 6, 2019

hacdias changed the title ~~feat: add 'ipfs' to PATH on macOS and Linux~~ wip: add 'ipfs' to PATH on macOS and Linux Apr 6, 2019

fix: remove unnecessary redirections

5549261

License: MIT Signed-off-by: Henrique Dias <hacdias@gmail.com>

hacdias changed the title ~~wip: add 'ipfs' to PATH on macOS and Linux~~ feat: add 'ipfs' to PATH on macOS and Linux Apr 6, 2019

ghost assigned lidel Apr 6, 2019

lidel reviewed Apr 6, 2019

View reviewed changes

hacdias added 2 commits April 7, 2019 09:26

fix(macOS): link to script

bbbc542

License: MIT Signed-off-by: Henrique Dias <hacdias@gmail.com>

fix(macOS): ignore ipfs cli on dev

9dee945

License: MIT Signed-off-by: Henrique Dias <hacdias@gmail.com>

hacdias commented Apr 7, 2019

View reviewed changes

electron-builder.yml Outdated Show resolved Hide resolved

fix(build): extraFiles --> extraResources

7c5f6bb

License: MIT Signed-off-by: Henrique Dias <hacdias@gmail.com>

olizilla requested changes Apr 8, 2019

View reviewed changes

olizilla and others added 2 commits April 8, 2019 11:51

Update assets/bin/ipfs.sh

665e99b

Co-Authored-By: hacdias <hacdias@gmail.com>

chore: add comments on what ipfs.sh does

7e8c171

License: MIT Signed-off-by: Marcin Rataj <lidel@lidel.org>

olizilla approved these changes Apr 9, 2019

View reviewed changes

hacdias mentioned this pull request Apr 9, 2019

Provide ipfs command line tools on install #727

Closed

hacdias merged commit 10d964e into master Apr 9, 2019

hacdias deleted the feat/ipfs-script branch April 9, 2019 13:49

ghost removed the in progress label Apr 9, 2019

MarcelRaschke mentioned this pull request Sep 28, 2022

[Snyk] Upgrade fs-extra from 9.0.1 to 10.1.0 meonBot/ipfs-desktop#40

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: add 'ipfs' to PATH on macOS and Linux #896

feat: add 'ipfs' to PATH on macOS and Linux #896

hacdias commented Apr 5, 2019 •

edited

Loading

hacdias commented Apr 6, 2019

hacdias commented Apr 6, 2019

hacdias commented Apr 6, 2019

lidel left a comment •

edited

Loading

hacdias commented Apr 7, 2019 •

edited

Loading

olizilla Apr 8, 2019

hacdias Apr 8, 2019

olizilla Apr 8, 2019

olizilla Apr 8, 2019

hacdias Apr 8, 2019

olizilla commented Apr 9, 2019

hacdias commented Apr 9, 2019

feat: add 'ipfs' to PATH on macOS and Linux #896

feat: add 'ipfs' to PATH on macOS and Linux #896

Conversation

hacdias commented Apr 5, 2019 • edited Loading

hacdias commented Apr 6, 2019

hacdias commented Apr 6, 2019

hacdias commented Apr 6, 2019

lidel left a comment • edited Loading

Choose a reason for hiding this comment

hacdias commented Apr 7, 2019 • edited Loading

olizilla Apr 8, 2019

Choose a reason for hiding this comment

hacdias Apr 8, 2019

Choose a reason for hiding this comment

olizilla Apr 8, 2019

Choose a reason for hiding this comment

olizilla Apr 8, 2019

Choose a reason for hiding this comment

hacdias Apr 8, 2019

Choose a reason for hiding this comment

olizilla commented Apr 9, 2019

hacdias commented Apr 9, 2019

hacdias commented Apr 5, 2019 •

edited

Loading

lidel left a comment •

edited

Loading

hacdias commented Apr 7, 2019 •

edited

Loading