dundalek / closh

Bash-like shell based on Clojure
Eclipse Public License 1.0
1.62k stars 66 forks source link

Reconsider closh and lumo distribution #42

Open dundalek opened 6 years ago

dundalek commented 6 years ago

As I am working on the signal handling I've been running into an issue that lumo installed via npm does not start directly but inside a js script running in node which spawns the lumo child process. Therefore pressing ctrl+c interrupts the parent process and pipe to the lumo process gets closed and closh errors out and exits.

To solve the issue we could do native exec to replace the process (using something like kexec, but it does not work on windows). Or run run the script directly with the lumo binary without the node wrapper.

There are couple of ways we could go: 1) Keep using npm, but make the shell script that would use exec to run closh on lumo. 2) Stop using npm to install, distribute lumo with closh and run directly. 3) It might worth to try compile closh into a single binary using the lumo's build process and distribute a single executable.

arichiardi commented 6 years ago

Regarding 3, there is a very nice PR by @hlolli that makes packaging possible.

dundalek commented 6 years ago

Thanks @arichiardi, I was not aware of this, it may be useful. I will take a look into it.

dundalek commented 6 years ago

Sharing my progress: I was able to package a sample lumo project into a single executable thanks to pkg-lumo which uses nexe (as does lumo itself). Unfortunately closh uses some native modules which are supported only in latest version of nexe.

So my next steps towards this will be: 1) Rebase Antonio's nexe patches on top of nexe master 2) Try to bundle closh with pkg-lumo using that latest nexe (.node files will still be sideloaded) 3) If that works then bundle .node files as resources into the binary and write them somewhere to disk during startup.

mnewt commented 6 years ago

Nice! Bleeding edge technology :)

hlolli commented 6 years ago

@dundalek I never understood why you need the nexe master, as far as I see, they changed to typescript and much of their api is totally different since Anton branched his fork, not to mention changes Anton did himself on the fork.

To make this kind of patching easier in the future, I plan to implement overrides as you can do in shadow-cljs, this should be easy to implement (basically telling pkg-lumo to replace a specific .js file from node_modules in pre-compilation in the nexe script).

dundalek commented 6 years ago

@hlolli It seemed to me those native modules only worked in nexe master (as I read through nexe's issues). But I will give it a shot again. If it would work with lumo and pkg-lumo's version that would be the simplest.

hlolli commented 6 years ago

Yes, but if you look at how they support native modules https://github.com/nexe/nexe/blob/02481f069c9462f6e0571974f603210a2578fe51/src/bundling/embed-node.ts#L34-L38 then they do what you can do yourself, just spit the file onto the disk. Their solution is very bad if you have binary say in /usr/bin with sudo access, then useing their automatic spitting is going to be problematic. The solution for closh I think should be to spit the .node file into a dot directory in the home directory, that way you can also control the version conflicts, because you'd always want to use the .node file that was generated compileing that specific binary. This should work

  1. spit .node
  2. require it There even a special case for dynamic libs in the requirePatch I wrote which I didn't touch, so it should alway try to find that file from the disk and never from the base64 table in the binary.

Btw, I'm now makeing the preperations to add 1.8.0 to pkg-lumo. You think it would make sense to try to search for "pkg-lumo" for pkg-lumo options in the package.json before opening the prompt? I personally don't like promting the config, but I'm unsure of alternative...

hlolli commented 6 years ago

And as you can see in my demo, you can slurp the resource file contents from the binary like this https://github.com/hlolli/lumo-quiz-game/blob/master/src/lumo_quiz_game/main.cljs#L31-L36 [lumo.io :as io] had some powers to read and convert the base64 data to string which is bundled from within the binary, don't have it on top of my head where exacly those powers are hidden :)

dundalek commented 6 years ago

I was thinking about the solution to spit the .node files into a dot directory in home as you say anyway. And I can patch the module's js files to look in the right location, so I see no problem there.

Thanks, that's a great idea to include and spit those files manually, therefore current lumo's version of nexe can be used. I am going to try that. :+1:

As for config, the usual way is to pass an option to a config file, e.g. pkg-lumo -c path/to/pkg-lumo.json. And since you already have the prompt it would be cool to save those entered values into some default file like .pkg-lumo.json so the next time it can be loaded and run without the prompt. I think a separate file works fine, I am generally not a fan of nesting custom configs inside package.json.

mnewt commented 6 years ago

@dundalek I was thinking about playing around with this a little. There are so many ways to do it, maybe I'll stumble on one that works. Were you able to compile closh to js?

dundalek commented 6 years ago

@mnewt My last progress is in this branch: https://github.com/dundalek/closh/tree/feature/build

It works by including native modules as resources in nexe compilation and spitting them on the disk where they can be loaded with process.dlopen (I forked deasync and sqlite so that look in the right location).

However there are two problems with it: 1) the bundled native modules are corrupted and I get Error: /home/ubuntu/.closh/cache/modules/v0.2.0/deasync.node: ELF file's phentsize not the expected size 2) node modules are not bundled, so when I move the resulting executable somewhere I get No such namespace: glob, could not locate glob.cljs, glob.cljc, or JavaScript source providing "glob"

hlolli commented 6 years ago

@Jakub are you useing node v8.5.0 ? Could be that the .node is compiled on your computer locally with different node version. Been too busy to update to lumo-1.8.0 with node 9.2.0, but I'll do it soon.

The glob missing is mysterious... :/ should try to analyze what's in the node_modules directory that's bundled (it's copied to target/node_modules during bundling in index.js)

On 5 March 2018 at 16:13, Jakub Dundalek notifications@github.com wrote:

@mnewt https://github.com/mnewt My last progress is in this branch: https://github.com/dundalek/closh/tree/feature/build

It works by including native modules as resources in nexe compilation and spitting them on the disk where they can be loaded with process.dlopen (I forked deasync and sqlite so that look in the right location).

However there are two problems with it:

  1. the bundled native modules are corrupted and I get Error: /home/ubuntu/.closh/cache/modules/v0.2.0/deasync.node: ELF file's phentsize not the expected size
  2. node modules are not bundled, so when I move the resulting executable somewhere I get No such namespace: glob, could not locate glob.cljs, glob.cljc, or JavaScript source providing "glob"

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/dundalek/closh/issues/42#issuecomment-370450416, or mute the thread https://github.com/notifications/unsubscribe-auth/AFyxgv4POfBdZXotY1Gr_ER5iY5j9sN1ks5tbVYBgaJpZM4RN0fI .

dundalek commented 6 years ago

@hlolli I disabled the require patch otherwise I had other compilation errors. I hoped rollup would bundle all the js files together.

hlolli commented 6 years ago

yup, useing rollup to bundle all the js files sounds like a better solution

On 5 March 2018 at 18:12, Jakub Dundalek notifications@github.com wrote:

@hlolli https://github.com/hlolli I disabled the require patch otherwise I had other compilation errors. I hoped rollup would bundle all the js files together.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/dundalek/closh/issues/42#issuecomment-370491796, or mute the thread https://github.com/notifications/unsubscribe-auth/AFyxgidAJguhfYLEkLLR0EerJcV6oKJvks5tbXHvgaJpZM4RN0fI .

arichiardi commented 6 years ago

Or you can use lumo as compiler to bundle its bundler :smile:

Likes apart I am really happy this train has left. If you folks come up with a better way to package lumo itself (I have been longing for boot's fileset caching for instance), please ping me ok?

dundalek commented 6 years ago

@arichiardi I pretty much give up on this at this moment. There are just so many issues that I keep bumping into. I will wait on this and hopefully someone smarter than me can figure something out. I wonder whether upgrading to latest nexe would help, since it seems they fixed many issues since 1.x. But lumo has its own fork of nexe, so things are getting complicated and over my head.