dundalek / closh

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

Move lumo into git submodule or subtree? #28

Closed mnewt closed 6 years ago

mnewt commented 6 years ago

Should the lumo repo be imported as a subproject of closh?

If that seems like a good idea, should it be a submodule or subtree? I don't feel strongly either way. I guess I lean toward subtree.

I'm happy to send a PR

dundalek commented 6 years ago

My initial reason not to put lumo in package.json dependencies was to save space by not downloading duplicates (I suspected early adopters would have lumo already installed). But this decision seems to still cause trouble, especially on mac. Therefore I am cool to put lumo into package.json now.

mnewt commented 6 years ago

Huh. I didn't think it would be enough to put lumo-cljs in package.json since it just includes the binary, not the source. Is there something I'm missing?

dundalek commented 6 years ago

Oh sorry, I misunderstood. However, I am not sure what added benefit would building lumo from source have?

It would be cool though if we could hook into lumo's build process and bundle lumo and closh into a single executable.

mnewt commented 6 years ago

I don't have any particular feelings about building lumo from source, but parts of its source are required unless I am misunderstanding something (which is quite possible).

Here is my understanding: When closh gets built, it doesn't just need the lumo executable--it needs the source too:

project.clj:

  :cljsbuild {:builds [{:id "main"
                        :source-paths ["src" "../lumo/src/cljs/snapshot"]

src/closh/eval.cljs:

(ns closh.eval
  (:require [lumo.repl]))

That source isn't contained in the npm lumo-cljs package, only the binary. So to build closh, the lumo sources are necessary. Right...?

So my thinking was to simply pull the lumo sources into the closh repo for the above reasons, instead of having them sit somewhere else on the filesystem.

Regarding building the lumo binary, I had a glance at it and it uses nexe to build it. It should be possible to do the same for closh.

dundalek commented 6 years ago

I see. If I remember correctly I had to add those lumo source dirs for the test runner to run. Now that we are running in lumo that may no longer be needed.

Actually the closh does not get pre-compiled. Lumo loads and compiles cljs files directly when it starts. It uses caching so it should do the ccompilation only once.

It would be cool to compile closh with lumo so that it can be possible to run it on stock node. I am not sure if that is possible, lumo might patch V8 or something, would need to dig deeper to know for sure.

mnewt commented 6 years ago

I'll close this for now. There's plenty of other work to do before building a standalone closh.