installer upgrades #512

bartlomieju · 2019-06-19T16:39:45Z

Closes #497.
Closes #501

Blocked on #509.

This PR:

splits installer into separate files leaving flag parsing in installer/mod.ts
removes uninstall command
adds --reload to deno fetch - this ensures that subsequent installation will upgrade executable
fixes shebang missing !
fixes prompt on subsequent installation
supports -d/--dir parameter for custom installation directory

After we land this PR I'll upgrade deno install.

bartlomieju · 2019-06-19T18:33:15Z

@axetroy take a look - looks like I fixed problem in Windows tests.

bartlomieju · 2019-06-20T11:26:10Z

After some thought and especially In light of denoland/deno#2546 I think that uninstall command is superfluous.

If we add ability for custom installation directory we'd need to pass the same directory to uninstall (deno_installer uninstall -d /usr/local/bin file_server). Since uninstalling is just removing a single file it makes no sense to pass installation dir there. So there are two options:
a) remove uninstall command and leave it to user to remove the file
b) try to locale installed file (which file_server or where file_server on Windows) and verify that it's file installed via installer (probably by matching content of the file) and then remove it.

Opinions?

ry · 2019-06-20T12:23:51Z

I think that uninstall command is superfluous.

I agree. For myself at least “rm $(which file_server)” works

(Brevity due to phone)

axetroy · 2019-06-20T12:49:15Z

I think that uninstall command is superfluous.

agree with this.

but I don't think allow different installation directories is a good idea.

This may cause some module to induce users to install in /usr/local/bin instead of the default $HOME/.deno/bin

It makes difficult to manage the installed module.

example1: if the user reinstalls in different directories. There are two identical scripts in the $PATH, how to deal with this？

example2: if there is a conflict with the native command, for example, there is a file ls under /usr/local/bin. At this time, the user installs the ls module to /usr/local/bin, what should I do?

At least for the current stage, I disapprove.

The solution to allowing different installation directories

If the user wants to install in /usr/local/bin, he can move the script manually.

bartlomieju · 2019-06-20T13:05:58Z

This may cause some module to induce users to install in /usr/local/bin instead of the default $HOME/.deno/bin

It makes difficult to manage the installed module.

You have to explicitly tell installer that you want to install somewhere else (eg. --dir /usr/local/bin), installed modules won't have ability to decide where to install themselves.

example1: if the user reinstalls in different directories. There are two identical scripts in the $PATH, how to deal with this？

example2: if there is a conflict with the native command, for example, there is a file ls under /usr/local/bin. At this time, the user installs the ls module to /usr/local/bin, what should I do?

Your shell deals with that, nothing prevents you from doing exactly these things right now.

Some people might want to keep all executable files in single place and don't want to add ~/.deno/bin to their PATH

axetroy · 2019-06-20T13:16:13Z

My point is to keep the computer as simple as possible.

When I need deno, I will go to $HOME/.deno to find the file.
When I don't need deno or want to uninstall it, just remove $HOME/.deno, except for the cache. No residual files.

make it clean and don't mess up your computer.

ry · 2019-06-20T13:39:13Z

@axetroy I'm of your opinion too I prefer everything located in ~/.deno, but I think a flag to specify the directory is fine...

(I kind of wish the cache was still there too...)

bartlomieju · 2019-06-20T14:04:41Z

Okay so I added -d/--dir flag - it tells where to put executable. This said, installed executable still uses default deno cache directory.

(I kind of wish the cache was still there too...)

Maybe we should add DENO_DIR to generated executable file - then each installed module is isolated and can be simply upgraded. WDYT?

ry · 2019-06-20T14:22:40Z

Maybe we should add DENO_DIR to generated executable file - then each installed module is isolated and can be simply upgraded. WDYT?

Eh.. Idk. I'd prefer the DENO_DIR be controlled by the caller.

bartlomieju · 2019-06-20T14:27:36Z

Eh.. Idk. I'd prefer the DENO_DIR be controlled by the caller.

Let's tackle that later. This PR should be landable as is. PTAL and review

ry

Looks good - just one thing...

ry · 2019-06-20T14:29:15Z

installer/mod.ts

+
+OPTIONS:
+ -d, --dir <PATH> Installation directory path (defaults to ~/.deno/bin)
+`);


I wish the flags module was more like Go's... This would be generated automatically.

Agreed, I was looking at porting one of more advanced CLI modules from node:

https://github.com/yargs/yargs

https://github.com/tj/commander.js/blob/master/index.js

https://github.com/mattallty/Caporal.js

But I might as well look into https://golang.org/pkg/flag/

installer/mod.ts

ry

LGTM

ry · 2019-06-20T14:47:28Z

@bartlomieju Can you draft a commit message please?

bartlomieju · 2019-06-20T14:49:05Z

@ry

Upgrade installer

remove uninstall command
add --reload to deno fetch - to ensure subsequent installation upgrades script and deps
fix executable shebang
fix prompt for subsequent installation
support custom installation dir via -d/--dir flag

add test case for reading code from stdin and format update update update update update update update update update update update update update update update improve installer (denoland#512) - remove uninstall command - add --reload to deno fetch - to ensure subsequent installation upgrades script and deps - fix executable shebang - fix prompt for subsequent installation - support custom installation dir via -d/--dir flag typo (denoland#515) bundle/run handles Deno.args better. (denoland#514) fix: pin eslint version for CI (denoland#518) typescript-eslint/typescript-eslint#637 feat: add catjson example (denoland#517) file server should order filenames (denoland#511) update update update update update update update update update update

- remove uninstall command - add --reload to deno fetch - to ensure subsequent installation upgrades script and deps - fix executable shebang - fix prompt for subsequent installation - support custom installation dir via -d/--dir flag Original: denoland/deno_std@b13441f

axetroy and others added 24 commits June 19, 2019 14:41

installer: test installed module run correctly

be1d05c

fix error

13e167c

update

296e52b

update

109924f

update

143c03e

update

9a44ed6

add test

b066a4a

update

880dd11

update

43cd107

update

89bafa1

update

d886fdc

fix install local module

27f6b4f

remove http/file_server.ts test case

a5f6acc

merge master

c695792

add install local module test case

153f9c6

fix test in windows

f0f5a38

add installed local module test case

4b92265

try fix windows test case

a96d1f2

try fix windows test case

38f701d

try fix windows test case

f78763e

split installer

c58c512

fix prompt on subsequent installation

891be4f

fix bad shebang

656d49d

fmt

17e46ca

bartlomieju changed the title ~~chore: split installer~~ installer upgrades Jun 19, 2019

bartlomieju added 4 commits June 19, 2019 18:51

lint & fmt

7ae9645

fix tests

4706dc6

add debug

e266a89

update for windows

a1a317a

remove uninstall command

4b08554

reset CI

1460eaa

bartlomieju added 2 commits June 20, 2019 15:47

fix tests

f8f49c5

support -d/--dir argument

40247f3

remove console.log

aa71f58

bartlomieju mentioned this pull request Jun 20, 2019

add docs to manual for "deno install" denoland/deno#2534

Closed

reset CI

a7a00e6

fmt

24e8c44

ry reviewed Jun 20, 2019

View reviewed changes

bartlomieju added 2 commits June 20, 2019 16:40

factor out isRemoteUrl

571354b

lint

e67a43a

ry approved these changes Jun 20, 2019

View reviewed changes

ry merged commit b13441f into denoland:master Jun 20, 2019

bartlomieju deleted the split_installer branch June 20, 2019 14:57

bartlomieju mentioned this pull request Jun 20, 2019

Installer: missing real test in different platforms #500

Closed

3 tasks

bartlomieju mentioned this pull request Jun 26, 2019

installed programs should be able to take --reload argument denoland/deno#2580

Closed

bartlomieju mentioned this pull request Oct 17, 2019

deno remove/uninstall subcommand denoland/deno#3139

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

installer upgrades #512

installer upgrades #512

bartlomieju commented Jun 19, 2019 •

edited

Loading

bartlomieju commented Jun 19, 2019

bartlomieju commented Jun 20, 2019

ry commented Jun 20, 2019 •

edited

Loading

axetroy commented Jun 20, 2019 •

edited

Loading

bartlomieju commented Jun 20, 2019

axetroy commented Jun 20, 2019

ry commented Jun 20, 2019 •

edited

Loading

bartlomieju commented Jun 20, 2019

ry commented Jun 20, 2019

bartlomieju commented Jun 20, 2019

ry left a comment

ry Jun 20, 2019

bartlomieju Jun 20, 2019

ry left a comment

ry commented Jun 20, 2019

bartlomieju commented Jun 20, 2019

installer upgrades #512

installer upgrades #512

Conversation

bartlomieju commented Jun 19, 2019 • edited Loading

bartlomieju commented Jun 19, 2019

bartlomieju commented Jun 20, 2019

ry commented Jun 20, 2019 • edited Loading

axetroy commented Jun 20, 2019 • edited Loading

The solution to allowing different installation directories

bartlomieju commented Jun 20, 2019

axetroy commented Jun 20, 2019

ry commented Jun 20, 2019 • edited Loading

bartlomieju commented Jun 20, 2019

ry commented Jun 20, 2019

bartlomieju commented Jun 20, 2019

ry left a comment

Choose a reason for hiding this comment

ry Jun 20, 2019

Choose a reason for hiding this comment

bartlomieju Jun 20, 2019

Choose a reason for hiding this comment

ry left a comment

Choose a reason for hiding this comment

ry commented Jun 20, 2019

bartlomieju commented Jun 20, 2019

bartlomieju commented Jun 19, 2019 •

edited

Loading

ry commented Jun 20, 2019 •

edited

Loading

axetroy commented Jun 20, 2019 •

edited

Loading

ry commented Jun 20, 2019 •

edited

Loading