GMOD / jbrowse

JBrowse 1, a full-featured genome browser built with JavaScript and HTML5. For JBrowse 2, see https://github.com/GMOD/jbrowse-components.
http://jbrowse.org
Other
460 stars 199 forks source link

Bower should not need to be installed globally #873

Closed hexylena closed 6 years ago

hexylena commented 7 years ago

Setup.sh should be fixed to also look for bower in node_modules/.bin and npm install if it isn't there. This would allow non-primary devs to setup jbrowse without having to learn what bower is and how to install it.

Nor should it try to: $npm_executable install -g bower

nathandunn commented 7 years ago

I think that is reasonable. It just needs to detect globally and then locally (or vice versa). If it fails it can install locally.

nathandunn commented 7 years ago

Are you able to create a PR for it?

hexylena commented 7 years ago

Yeah, but not this week. It isn't a high priority.

Nathan Dunn notifications@github.com schrieb am Do., 23. März 2017, 19:16:

Are you able to create a PR for it?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/GMOD/jbrowse/issues/873#issuecomment-288831423, or mute the thread https://github.com/notifications/unsubscribe-auth/AAb_u2QKPZcvZGWAtpShfI6AkHZMOGEGks5rosUfgaJpZM4MnKPB .

nathandunn commented 7 years ago

Agreed. But good to have in there anyway.

cmdcolin commented 7 years ago

Possible patch/branch https://github.com/GMOD/jbrowse/tree/bower_local

cmdcolin commented 7 years ago

I think the thing where node is made an alias for nodejs does not seem to be working either (will still give error from bower if the node is named nodejs), so it may just be worth removing(?)

nathandunn commented 7 years ago

@cmdcolin A couple of thoughts.

1 - I definitely think we should do this. Ran into it the other day and I agree that the global install is un-necessary. 2- I'm also thinking, should we just use NPM and replace bower entirely? Its basically this: https://docs.npmjs.com/files/package.json#git-urls-as-dependencies

Either way, issue a PR and thanks for looking into this.

nathandunn commented 7 years ago

I'm not sure what the node vs nodejs thing is, other than that I and other users ran into it. Some linux versions seemed to use one versus the other.

cmdcolin commented 7 years ago

We can get close to fully installing stuff from npm, but it's not exactly there yet

https://github.com/GMOD/jbrowse/tree/replace_bower_with_npm

nathandunn commented 7 years ago

Ahh okay. Well, it was an idea. Still an improvement.

On Mar 31, 2017, at 1:06 PM, Colin Diesh notifications@github.com wrote:

We can get close to fully installing stuff from npm, but it's not exactly there yet

https://github.com/GMOD/jbrowse/tree/replace_bower_with_npm

— You are receiving this because you commented. Reply to this email directly, view it on GitHub, or mute the thread.

cmdcolin commented 7 years ago

Actually, installing from npm instead of bower kind of works now. https://github.com/GMOD/jbrowse/tree/replace_bower_with_npm

Some tests fail, maybe due to updated jDataView library, but the volvox example data loads files using jDataView fine (parts that were dependant on jDataView include bigwig and tabix like vcf files)

cmdcolin commented 7 years ago

Just checkout that branch, run npm install and setup.sh and checkout the volvox sample

cmdcolin commented 7 years ago

If there is interest I can open a PR for this code (swapping bower for npm). It passes travis-CI tests, kind of a big change just due to upgrading dependency versions but seems to work ok

nathandunn commented 7 years ago

Yes. That would be great if you have the time. So long as the dependencies are stable it should be a good idea.

cmdcolin commented 7 years ago

See #886 :)

nathandunn commented 7 years ago

K. I will try to test it today or sometime next week as I have time. Looking at it, it looks good. Thanks for putting it in.

cmdcolin commented 7 years ago

Why is bower needed when jbrowse is precompiled? This needs to be changed I think @nathandunn @enuggetry

cmdcolin commented 7 years ago

It tries to run npm install -g bower which is bad because (a) it's unnecessary and (b) it causes setup.sh to produce a ton of warnings!

Here is what happens on a system where bower is not installed

 ./setup.sh
Legacy scripts wig-to-json.pl and bam-to-json.pl have removed from setup. Their functionality has been superseded by add-bam-track.pl and add-bw-track.pl. If you require the old versions, run 'setup.sh legacy'.
Installing javascript dependencies ...[..................] - normalizeTree: sillnpm WARN checkPermissions Missing write access to /usr/lib/node_modules
npm ERR! Linux 4.4.0-43-Microsoft
npm ERR! argv "/usr/bin/nodejs" "/usr/bin/npm" "install" "-g" "bower"
npm ERR! node v7.9.0
npm ERR! npm  v4.2.0
npm ERR! path /usr/lib/node_modules
npm ERR! code EACCES
npm ERR! errno -13
npm ERR! syscall access

npm ERR! Error: EACCES: permission denied, access '/usr/lib/node_modules'
npm ERR!  { Error: EACCES: permission denied, access '/usr/lib/node_modules'
npm ERR!   errno: -13,
npm ERR!   code: 'EACCES',
npm ERR!   syscall: 'access',
npm ERR!   path: '/usr/lib/node_modules' }
npm ERR!
npm ERR! Please try running this command again as root/Administrator.

npm ERR! Please include the following file with any support request:
npm ERR!     /home/cdiesh/.npm/_logs/2017-05-03T04_06_06_099Z-debug.log
npm WARN checkPermissions Missing write access to /usr/lib/node_modules
npm ERR! Linux 4.4.0-43-Microsoft
npm ERR! argv "/usr/bin/nodejs" "/usr/bin/npm" "install" "-g" "bower"
npm ERR! node v7.9.0
npm ERR! npm  v4.2.0
npm ERR! path /usr/lib/node_modules
npm ERR! code EACCES
npm ERR! errno -13
npm ERR! syscall access

npm ERR! Error: EACCES: permission denied, access '/usr/lib/node_modules'
npm ERR!  { Error: EACCES: permission denied, access '/usr/lib/node_modules'
npm ERR!   errno: -13,
npm ERR!   code: 'EACCES',
npm ERR!   syscall: 'access',
npm ERR!   path: '/usr/lib/node_modules' }
npm ERR!
npm ERR! Please try running this command again as root/Administrator.

npm ERR! Please include the following file with any support request:
npm ERR!     /home/cdiesh/.npm/_logs/2017-05-03T04_06_07_500Z-debug.log
  Bower dependencies already installed.  Type ' install -f --allow-root' to force reinstallation of dependencies.done
Gathering system information ...done
Installing Perl prerequisites ...

It does not cause setup.sh to fail but that is quite the warning

cmdcolin commented 7 years ago

Edit: it doesn't cause setup.sh to fail, but it prints a lot of warnings

nathandunn commented 7 years ago

@cmdcolin I agree that we should not install bower globally. I just haven't had time to fix it, myself. If you have time, I'm happy to test / review anything you build. I think we should do that in favor of #887. If we could do it all in favor of just using straight npm, that would be the best.

WRT to why it is used at all, a lot of users (including all apollo users) don't download / install the precompiled version, because I actually key the versions (including master) to a commit hash of JBrowse because we want to have features, but aren't always keyed to a certain release.

My dream of dreams would be to install JBrowse, load any plugins using npm (and its wrapper of git clone where necessary) if Apollo is pulling down other dependencies and then have it run the closure compiler over all plugins using an simple npm script instead of the makefile to get us a little closer to what other web projects are doing. I think that this is non-trivial, unfortunately, but anything that is progress along the way is great. I see it as:

  1. remove bower global install vs local (should be easy, can't remember what the problem was with not doing it)
  2. remove bower entirely in favor of npm
  3. use npm to run the closure compile on-demand

Obviously any subsequent step removes the necessity for the prior one.

cmdcolin commented 7 years ago

Sorry to raise the alarm, and I think those are valid ideas about packaging, but it just seemed pertinent to not run the bower install step for the jbrowse releases since dependencies are prepackaged in the releases, hence the #887

There is a bower local install here that could be a PR too but I don't think it changes #887 https://github.com/GMOD/jbrowse/tree/bower_local

nathandunn commented 7 years ago

I think that its okay for the "release" install step to have its own build strategy that doesn't include bower or npm, so long as the "un-release" version still relies on bower / npm.

rbuels commented 6 years ago

bower being deprecated, I don't think we need to do anything further here, right guys?