Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

export $SCRIPT as leiningen.script #874

Merged
merged 1 commit into from Dec 4, 2012

Conversation

2 participants
Contributor

arohner commented Dec 2, 2012

Apologies, I've never made a PR for modifying the bash script, I'm not sure what the process is.

This exposes the path where lein started to the clojure code. That's useful in lein2 version of lein-daemon (https://github.com/arohner/lein-daemon/tree/lein2), because it calls lein again, as a background process, to fork.

Owner

technomancy commented Dec 3, 2012

You should use the trampoline mechanism if you want to fork/background the project I think. Have you looked into going that route?

Contributor

arohner commented Dec 3, 2012

I have looked into trampoline, and while it might be possible, I think I'd need to patch leiningen.core.eval/shell-command (or write my own). When starting a daemon, I need to redirect stdin, stdout & stderr, and fork the process.

trampoline is also slightly limiting because currently, lein daemon spawns a child (which has redirected in & out), then the parent can print to the console about current status: waiting for the child, pid has been written, started successfully, etc. With trampoline, that's no longer possible, because there's no parent process around to watch the child.

Owner

technomancy commented Dec 3, 2012

I guess it depends on whether you need the parent to be a JVM or not. If the parent can be a shell script then trampoline definitely the way to go; just write the parent process's script to trampoline-file and it can handle everything. It's also much less wasteful in terms of memory. But if that's not feasible and you need significant logic in the parent then I can add this.

Contributor

arohner commented Dec 3, 2012

I think I need this, sorry. It leads to a much more pleasant experience for the user, in the case of errors.

Owner

technomancy commented Dec 4, 2012

No problem; just wanted to make sure the options had been considered.

technomancy added a commit that referenced this pull request Dec 4, 2012

Merge pull request #874 from arohner/master
export $SCRIPT as leiningen.script

@technomancy technomancy merged commit 8f46783 into technomancy:master Dec 4, 2012

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment