Skip to content
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

deno install should bundle before installation #5069

Open
bartlomieju opened this issue May 4, 2020 · 9 comments · Fixed by #5276
Open

deno install should bundle before installation #5069

bartlomieju opened this issue May 4, 2020 · 9 comments · Fixed by #5276
Labels
cli related to cli/ dir feat new feature (which has been agreed to/accepted)

Comments

@bartlomieju
Copy link
Member

deno install is a utility that allows to install any module as an executable by creating shell script that invokes actual module.

Currently it works by caching all dependencies of the module in $DENO_DIR and adding entry point local URL to the shell script.

Huge caveat in this approach is that if user reloads dependencies installed script might break (especially for untagged imports...).
To mitigate this problem the script should be bundled into a single file during installation.

This way installed script can be made into standalone utilities that do not share cache with other development projects and are completely standalone.

CC @ry

@bartlomieju bartlomieju added the feat new feature (which has been agreed to/accepted) label May 4, 2020
@nayeemrmn
Copy link
Collaborator

nayeemrmn commented May 6, 2020

It's challenging to make this a self-contained file still, since it needs to both be a shell script and contain the bundle somehow. For unix you can hack this with:

#!/bin/sh
":" //; exec "deno" "run" [baked options]... "$0" [baked args]... "$@"
;

<bundle output>

In the shell context this is equivalent to:

":" //
exec "deno" "run" [baked options]... "$0" [baked args]... "$@"

And in the JS context it is:

":";
<bundle output>

😝

@ry
Copy link
Member

ry commented May 6, 2020

For a first version, I think it would be okay if the bundled lived in a separate file, neighboring the executable script.

@kitsonk
Copy link
Contributor

kitsonk commented Nov 2, 2020

We reverted this, so we need to re-open.

@kitsonk kitsonk reopened this Nov 2, 2020
@kitsonk kitsonk added the cli related to cli/ dir label Nov 2, 2020
@kitsonk
Copy link
Contributor

kitsonk commented Nov 2, 2020

I suspect that something like #5276 might be able to be re-landed now the refactor is done.

@bartlomieju
Copy link
Member Author

@kt3k could you update and reopen your PR?

@kt3k
Copy link
Member

kt3k commented Nov 9, 2020

@bartlomieju The previous branch seems unusable (because many things have been changed), but I'll try a new one in another branch 👍 (It may take a little while)

@kt3k
Copy link
Member

kt3k commented Nov 9, 2020

By the way I still see some issues about bundling ( https://github.com/denoland/deno/issues?q=is%3Aissue+is%3Aopen+bundle ). Do you think we need an option for enabling/disabling bundling at install time?

@bartlomieju
Copy link
Member Author

@kt3k I think adding a flag is a sensible solution

@kitsonk
Copy link
Contributor

kitsonk commented Nov 9, 2020

Yes, I even think with a lot of issues resolved, people may still not want to bundle. I would suggest though that bundling be t the default, and that the option deno install --no-bundle be used to opt out.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cli related to cli/ dir feat new feature (which has been agreed to/accepted)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants