Merge lp:~rogpeppe/juju-core/136-uniter-fix-logger into lp:~juju/juju-core/trunk

[Roger Peppe]

worker/uniter: make logger predictable

The stdout pipe is closed too early by os/exec;
do the work ourselves to prevent this.

https://codereview.appspot.com/6774050/

[Roger Peppe (rogpeppe)]

Reviewers: mp+131580_code.launchpad.net, dfc,

Message:
Please take a look.

Description:
worker/uniter: make logger predictable

The stdout pipe is closed too early by os/exec;
do the work ourselves to prevent this.

https://code.launchpad.net/~rogpeppe/juju-core/136-uniter-fix-logger/+merge/131580

(do not edit description out of merge proposal)

Please review this at https://codereview.appspot.com/6774050/

Affected files:
   A [revision details]
   M worker/uniter/context.go

Index: [revision details]
=== added file '[revision details]'
--- [revision details] 2012-01-01 00:00:00 +0000
+++ [revision details] 2012-01-01 00:00:00 +0000
@@ -0,0 +1,2 @@
+Old revision: <email address hidden>
+New revision: <email address hidden>

Index: worker/uniter/context.go
=== modified file 'worker/uniter/context.go'
--- worker/uniter/context.go 2012-10-10 21:46:49 +0000
+++ worker/uniter/context.go 2012-10-26 10:47:21 +0000
@@ -151,17 +151,22 @@
   ps := exec.Command(filepath.Join(charmDir, "hooks", hookName))
   ps.Env = ctx.hookVars(charmDir, toolsDir, socketPath)
   ps.Dir = charmDir
- outReader, err := ps.StdoutPipe()
+ outReader, outWriter, err := os.Pipe()
   if err != nil {
- return err
+ return fmt.Errorf("cannot make logging pipe: %v", err)
   }
- ps.Stderr = ps.Stdout
+ ps.Stdout = outWriter
+ ps.Stderr = outWriter
   logger := &hookLogger{
    r: outReader,
    done: make(chan struct{}),
   }
   go logger.run()
- err = ps.Run()
+ err = ps.Start()
+ outWriter.Close()
+ if err == nil {
+ err = ps.Wait()
+ }
   logger.stop()
   if ee, ok := err.(*exec.Error); ok && err != nil {
    if os.IsNotExist(ee.Err) {

[Gustavo Niemeyer (niemeyer)]

LGTM

https://codereview.appspot.com/6774050/

[Roger Peppe (rogpeppe)]

*** Submitted:

worker/uniter: make logger predictable

The stdout pipe is closed too early by os/exec;
do the work ourselves to prevent this.

R=dfc, niemeyer
CC=
https://codereview.appspot.com/6774050

https://codereview.appspot.com/6774050/

=== modified file 'worker/uniter/context.go'
--- worker/uniter/context.go	2012-10-10 21:46:49 +0000
+++ worker/uniter/context.go	2012-10-26 10:49:29 +0000
@@ -151,17 +151,22 @@
 	ps := exec.Command(filepath.Join(charmDir, "hooks", hookName))
 	ps.Env = ctx.hookVars(charmDir, toolsDir, socketPath)
 	ps.Dir = charmDir
-	outReader, err := ps.StdoutPipe()
+	outReader, outWriter, err := os.Pipe()
 	if err != nil {
-		return err
+		return fmt.Errorf("cannot make logging pipe: %v", err)
 	}
-	ps.Stderr = ps.Stdout
+	ps.Stdout = outWriter
+	ps.Stderr = outWriter
 	logger := &hookLogger{
 		r:    outReader,
 		done: make(chan struct{}),
 	}
 	go logger.run()
-	err = ps.Run()
+	err = ps.Start()
+	outWriter.Close()
+	if err == nil {
+		err = ps.Wait()
+	}
 	logger.stop()
 	if ee, ok := err.(*exec.Error); ok && err != nil {
 		if os.IsNotExist(ee.Err) {