tessel / t2-cli

Tessel 2 Command Line Interface
MIT License
114 stars 56 forks source link

Improve install experience of t2-cli on Linux #727

Closed johnnyman727 closed 8 years ago

johnnyman727 commented 8 years ago

I was trying to install T2 on Ubuntu 14.04. To do it properly, you have to work around a lot of little quirks. The purpose of this issue is to start a discussion about how to make the best experience such that the install is as close to npm install t2-cli -g as possible (this issue should arguably be on the t2-start repo but I figure we can follow up there with any changes we decide on).

I was working off the start experience and the first line tells me to:

If you are running Ubuntu 13.10 or later, you can run this:

apt-get install nodejs nodejs-legacy

I think we should probably not advise people to do this because it will install Node@0.10.x which is too old to run the CLI. Instead, I think we should advise people to install via the instructions from nodejs.org:

curl -sL https://deb.nodesource.com/setup_4.x | sudo -E bash -
sudo apt-get install -y nodejs

It is a little sketchy to pipe things from the internet into the shell but folks can do them in separate commands if they want to read through the source code first.

Then I got to this line of the start experience:

apt-get install libusb-1.0-0-dev libudev-dev

This worked just fine (with the addition of sudo) but it would be great if we could somehow automate this installing with the installation of t2-cli from npm. I don't know enough about npm at the moment to know what options exist.

Then I got to the final, most troublesome line of the start experience:

npm install -g t2-cli

I immediately ran into EACCES errors so I figured I might want to try installing with sudo. I've been bit by npm permissions issues enough to know it's not a good idea but I was trying to run through what the average user would do. When I ran with sudo, I still got EACCES errors as described by this issue. At this point, I decided to modify npm global installs to install in a directory that won't have permissions issue. Being the lazy developer that I am, I decided to just run the usage instructions from this script:

wget -O- https://raw.githubusercontent.com/glenpike/npm-g_nosudo/master/npm-g-nosudo.sh | sh

If I had any other packages already globally installed using sudo, I would then have to run source ~/.bashrc to get them on the path but I didn't so I skipped that instruction.

Finally, I re-ran npm install t2-cli -g again and it installed without a problem. Then I ran t2 list as a smoke test and got this output:

INFO Searching for nearby Tessels...
ERR! Please run `sudo t2 install-drivers` to fix device permissions.
(Error: could not open USB device.)
WARN Detected a Tessel that may be booting.

So, per the instructions printed, I ran sudo t2 install-driver but, of course, t2 can't be found when run as sudo because apparently the $PATH used with sudo is different than the path without??? So then I ran just t2 install-drivers and got this output:

jmckay@iceye-jmckay:~/Code$ t2 install-drivers
INFO Could not write to /etc/udev/rules.d/85-tessel.rules
INFO Run `sudo t2 install-drivers`

Ah so I need root permissions to write to the udev rules but I can't run t2 as root! So following a cue from this comment, I manually created the udev rule file:

sudo nano /etc/udev/rules.d/85-tessel.rules

and pasted these contents:

SUBSYSTEM=="usb", ATTRS{idVendor}=="1209", ATTRS{idProduct}=="7551", ENV{ID_MM_DEVICE_IGNORE}="1", MODE="0666", GROUP="plugdev"

Then, I reloaded the udev rules:

sudo udevadm control --reload-rules

Finally, the CLI was functional.

In conclusion, I think we to change the current install process which is:

apt-get install nodejs nodejs-legacy
apt-get install libusb-1.0-0-dev libudev-dev
npm install -g t2-cli

which, in reality, is actually:

curl -sL https://deb.nodesource.com/setup_4.x | sudo -E bash -
sudo apt-get install -y nodejs
sudo apt-get install libusb-1.0-0-dev libudev-dev
wget -O- https://raw.githubusercontent.com/glenpike/npm-g_nosudo/master/npm-g-nosudo.sh | sh
source ~/.bashrc
npm install t2-cli -g
sudo nano /etc/udev/rules.d/85-tessel.rules
sudo udevadm control --reload-rules

as close as possible into:

npm install t2-cli -g

Thoughts?

Student007 commented 8 years ago

@johnnyman727 nice work :+1: My two cents: I would vote for using NVM together with npm-g-nosudo.sh. In the case there is still someone out there is using NodeJS as root ... the sudo npm install t2-cli -g would also work.

jimbojw commented 8 years ago

I'm working through this right now, also trying to get to the simplest possible successful setup. I'm starting from non-root nvm with latest Node.

$ nvm use default
Now using node v6.1.0 (npm v3.8.6)
$ sudo apt-get install libusb-1.0-0-dev libudev-dev
$ npm install -g t2-cli

Attempt to t2 list and discover issue:

$ t2 list
INFO Searching for nearby Tessels...
ERR! Please run `sudo t2 install-drivers` to fix device permissions.
(Error: could not open USB device.)
WARN Detected a Tessel that may be booting.
WARN No Tessels Found.

This almost works:

$ sudo PATH=$PATH bash -c 't2 install-drivers'
INFO udev rules installed to /etc/udev/rules.d/85-tessel.rules
INFO Done. Unplug and re-plug Tessel to update permissions.

However, the rules it creates are different than those reported by @johnnyman727.

$ cat /etc/udev/rules.d/85-tessel.rules 
+ATTRS{idVendor}=="1209", ATTRS{idProduct}=="7551", ENV{ID_MM_DEVICE_IGNORE}="1", MODE="0666"

Running t2 list through sudo bash works:

$ sudo PATH=$PATH bash -c 't2 list'
INFO Searching for nearby Tessels...
        USB     Tessel-02A35FB40D3E

But running t2 list straight up crashes with no output:

$ t2 list
INFO Crash Reported: http://crash-reporter.tessel.io/

I suspect I'm seeing issue #730, otherwise I'd post the crash info.

I tried overwriting the 85-tessel.rules with johnnyman's and reloading, but t2 list still crashes with no info. That's as far as I've gotten.

theirnameissam commented 8 years ago

@jimbojw How can I undo this????

jimbojw commented 8 years ago

The install-drivers script[1] does two things:

So, to undo the effects of driver installation, I would guess that these commands would do it:

$ sudo rm /etc/udev/rules.d/85-tessel.rules
$ sudo udevadm control --reload-rules

I haven't tested these commands, YMMV. Hope this helps!

[1] https://github.com/tessel/t2-cli/blob/7add2f8b8f42dfabddce982fc8e932ecf7e75799/bin/tessel-install-drivers.js

johnnyman727 commented 8 years ago

Thanks for looking into this @jimbojw. Do you have any suggestions about how we might be able to make the install easier?

jimbojw commented 8 years ago

Well, I don't actually have mine working yet. It's crashing with no indication as to the issue :)

Given that the tessel-install-drivers.js just writes 85-tessel.rules and restarts the service, having people do those two commands may be simpler than fighting with running node as root.

This is untested, but suppose t2 had an echo-drivers command that echo'd the contents of 85-tessel.rules. The following would do the same thing as running t2 install-drivers but without the hassle of getting node to run under root:

$ t2 echo-drivers | sudo dd /etc/udev/rules.d/85-tessel.rules
$ sudo udevadm control --reload-rules

The brave can do something like this today by curling the source from github:

$ curl https://raw.githubusercontent.com/tessel/t2-cli/0.0.19/resources/85-tessel.rules | \
    sudo dd /etc/udev/rules.d/85-tessel.rules

My $0.02.

jimbojw commented 8 years ago

Update: After hacking the logger to print stack before reporting the crash, I discovered that update notifier config file permissions were the cause of my crashes.

When you install the t2-cli package globally, it creates a file in your ~/.config/configstore/ directory called update-notifier-t2-cli.json.

$ ls -l ~/.config/configstore/update-notifier-t2-cli.json 
-rw-------  1 jimbo jimbo   55 May 11 05:36 update-notifier-t2-cli.json
$ cat ~/.config/configstore/update-notifier-t2-cli.json 
{
        "optOut": false,
        "lastUpdateCheck": 1462959391730
}

So far so good. But once you use sudo to install drivers as root, the permissions change. Now instead of you having read/write access to that file, it belongs to root:

$ ls -l ~/.config/configstore/update-notifier-t2-cli.json 
-rw------- 1 root root 55 May 11 05:38 /home/jimbo/.config/configstore/update-notifier-t2-cli.json

After this change, all attempts to use t2 as non-root users will crash with no output. By hacking the logger, I was able to get to this stack trace.

Error: EACCES: permission denied, open '/home/jimbo/.config/configstore/update-notifier-t2-cli.json'
You don't have access to this file.

    at Error (native)
    at Object.fs.openSync (fs.js:634:18)
    at Object.fs.readFileSync (fs.js:502:33)
    at Object.create.all.get (t2-cli/node_modules/configstore/index.js:35:26)
    at Object.Configstore (t2-cli/node_modules/configstore/index.js:28:44)
    at new UpdateNotifier (t2-cli/node_modules/update-notifier/index.js:34:17)
    at module.exports (t2-cli/node_modules/update-notifier/index.js:108:23)
    at Object.<anonymous> (t2-cli/bin/tessel-2.js:22:1)
    at Module._compile (module.js:541:32)
    at Object.Module._extensions..js (module.js:550:10)

I haven't tried the instructions on the getting started page verbatim, because I use nvm. But according to issue #714, this is an extremely common problem.

You can get out of this state by removing the update-config-t2-cli.json file entirely.

$ sudo rm  ~/.config/configstore/update-notifier-t2-cli.json

It will automatically be recreated with the correct ownership the next time you run a t2 command.

@klombomb add this to your list of "undo" steps.

tikurahul commented 8 years ago

@jimbojw FWIW, you can also use the CLI to temporarily turn off the crash reporter, when you want to look at the problem given the patch to actually print the root crash has not landed in 0.0.19 yet.

jimbojw commented 8 years ago

Update: The following installation steps work for me (Ubuntu user with node installed via nvm):

$ npm install -g t2-cli@0.0.20
$ sudo PATH=$PATH bash -c 't2 install-drivers'
$ sudo chown $USER:$USER $HOME/.config/configstore/update-notifier-t2-cli.json

The last line is necessary to restore the ownership of the update-notifier backing file after running install-drivers as root. Without this line, t2 list appears to work without crashing as of v0.0.20. However it would be impossible to overwrite the update notifier settings (even the last checked timestamp) as your regular non-root user without it.

The root cause of this bug is in write-file-atomic, which is used by configstore, which is used by update-notifier. I have created a bug to track this issue: https://github.com/npm/write-file-atomic/issues/11

johnnyman727 commented 8 years ago

@jimbojw so running t2 as sudo even once will overwrite the permissions of the update-notifier config file? That totally sucks. I think we may even want to get rid of update-notifier if that's the case. It's a big problem with a small gain if we fix it (I doubt that npm team will ever get around to looking at your issue).

Here is my recommendation for how we can make the install process smoother:

  1. Remove the update-notifier dependency until such time that the issues we described are fixed. Edit: Do not run update-notifier when the install-drivers command is run: https://github.com/tessel/t2-cli/pull/740
  2. Update the Node/npm install instructions to 4.x using the script I linked to above.
  3. Keep the apt-get install instructions as-is and let the user choose whether or not to use sudo.
  4. Recommend they install with npm install t2-cli -g and make very clear that they should fix npm permission issues rather than try to install with sudo if they encounter EACCES errors. We can link to the npm blog post on fixing permissions issues.
  5. Recommend they run sudo t2 install-drivers (and make clear that the use of sudo is for this one-time operation only).

@jimbojw what do you think about this?

johnnyman727 commented 8 years ago

Implemented here: https://github.com/tessel/t2-start/pull/120

jimbojw commented 8 years ago

Sounds good to me. I can't speak to the npm permission issues (4) because I haven't experienced the kind that you're describing, only the kind that arises from sudo + update-notifier.

I think you may be setting yourself up for a different problem with #740 however. I'm thinking of the case where the user has decided to run everything as root all the time. In that case, they actually would want to run update-notifier as well. Running everything as root is generally a bad idea, of course, but unless it's explicitly disallowed, a user might be surprised to see that update-notifier is not working for them as root.

To reduce the surprise, you might want to add an INFO notice informing the user that update-notifier is being skipped because the tool is running as root.

[WARN] Running t2 commands as root is discouraged.
[INFO] Skipping update-notifier check for root user.

This output may make sense only for commands other than install-drivers. Just my $0.02 :)

johnnyman727 commented 8 years ago

@jimbojw I'm going to play the #lazy card and say that we'd gladly accept a pull request for warnings when using sudo with a non-install-drivers command. I think overall, it's not a huge issue if update-notifier isn't working for the small minority of folks who run everything with sudo.

For now though, since https://github.com/tessel/t2-start/pull/120 is merged, I'll consider this issue closed.

Thanks for all your help!