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.

Sign up for GitHub

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

Jump to bottom

Run shutdown-agents after a cljsbuild once if :notify-command is used #453

Closed
danielcompton opened this issue Oct 20, 2016 · 1 comment
Closed

Run shutdown-agents after a cljsbuild once if :notify-command is used #453

danielcompton opened this issue Oct 20, 2016 · 1 comment

Comments

@danielcompton
Copy link
Contributor

danielcompton commented Oct 20, 2016 •
edited
Loading

:notify-command uses a future through cljsbuild.util/process-start. If a notify-command is run during a cljsbuild once run, then the cljsbuild process will stay running for another minute until the agents are shut down.

You can reproduce this on the Reagent project by running

$ lein with-profile prod-test do clean, cljsbuild once
@miikka
Copy link

miikka commented Dec 13, 2016

lein cljsbuild test has the same problem, because the test commands are run with cljsbuild.util/process-start as well.

sfnelson pushed a commit to montoux/lein-cljsbuild that referenced this issue Apr 13, 2017
Fix emezeske#453: use ProcessBuilder for output redirect
149e363 
address emezeske#171. This caused a regression in testing and notify behaviour that was
reported as emezeske#453, where subprocesses would hang for 30 seconds after the
subprocess completed.

This commit avoids the need for pump threads to copy data from subprocess
streams by using ProcessBuilder's existing Redirect functionality to either
write redirect stream output to files directly, or to inherit System.out and
System.err from the main process. This means that `lein test` etc now terminate
in a reasonable time again, and avoids the call to `exit` that was causing
problems for trampoline tasks.
sfnelson pushed a commit to montoux/lein-cljsbuild that referenced this issue Apr 13, 2017
Fix emezeske#453: use ProcessBuilder for output redirect
935a960 
Pull request emezeske#436 removes the call to exit that was added to address emezeske#171. This
caused a regression in testing and notify behaviour that was reported as emezeske#453,
where subprocesses would hang for 30 seconds after the subprocess completed.

This commit avoids the need for pump threads to copy data from subprocess
streams by using ProcessBuilder's existing Redirect functionality to either
write redirect stream output to files directly, or to inherit System.out and
System.err from the main process. This means that `lein test` etc now terminate
in a reasonable time again, and avoids the call to `exit` that was causing
problems for trampoline tasks.
@sfnelson sfnelson mentioned this issue Apr 13, 2017
Merged
@mneise mneise closed this as completed in 4ba27e5 Apr 24, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@miikka @danielcompton