Merge pull request #2998 from creack/fix_json_display

Fix json display

api.go

@@ -960,11 +960,20 @@ func postBuild(srv *Server, version float64, w http.ResponseWriter, r *http.Requ
 		return err
 	}
 
-	if version > 1.6 {
+	if version >= 1.8 {
 		w.Header().Set("Content-Type", "application/json")
 	}
-	sf := utils.NewStreamFormatter(version > 1.6)
-	b := NewBuildFile(srv, utils.NewWriteFlusher(w), !suppressOutput, !noCache, rm, sf)
+	sf := utils.NewStreamFormatter(version >= 1.8)
+	b := NewBuildFile(srv,
+		&StdoutFormater{
+			Writer:          utils.NewWriteFlusher(w),
+			StreamFormatter: sf,
+		},
+		&StderrFormater{
+			Writer:          utils.NewWriteFlusher(w),
+			StreamFormatter: sf,
+		},
+		!suppressOutput, !noCache, rm, utils.NewWriteFlusher(w), sf)
 	id, err := b.Build(context)
 	if err != nil {
 		if sf.Used() {

buildfile.go

@@ -36,15 +36,19 @@ type buildFile struct {
 	tmpContainers map[string]struct{}
 	tmpImages     map[string]struct{}
 
-	out io.Writer
-	sf  *utils.StreamFormatter
+	outStream io.Writer
+	errStream io.Writer
+
+	// Deprecated, original writer used for ImagePull. To be removed.
+	outOld io.Writer
+	sf     *utils.StreamFormatter
 }
 
 func (b *buildFile) clearTmp(containers map[string]struct{}) {
 	for c := range containers {
 		tmp := b.runtime.Get(c)
 		b.runtime.Destroy(tmp)
-		fmt.Fprintf(b.out, "Removing intermediate container %s\n", utils.TruncateID(c))
+		fmt.Fprintf(b.outStream, "Removing intermediate container %s\n", utils.TruncateID(c))
 	}
 }
 
@@ -53,7 +57,7 @@ func (b *buildFile) CmdFrom(name string) error {
 	if err != nil {
 		if b.runtime.graph.IsNotExist(err) {
 			remote, tag := utils.ParseRepositoryTag(name)
-			if err := b.srv.ImagePull(remote, tag, b.out, b.sf, nil, nil, true); err != nil {
+			if err := b.srv.ImagePull(remote, tag, b.outOld, b.sf, nil, nil, true); err != nil {
 				return err
 			}
 			image, err = b.runtime.repositories.LookupImage(name)
@@ -101,11 +105,7 @@ func (b *buildFile) CmdRun(args string) error {
 		if cache, err := b.srv.ImageGetCached(b.image, b.config); err != nil {
 			return err
 		} else if cache != nil {
-			if b.sf.Used() {
-				b.out.Write(b.sf.FormatStatus("", " ---> Using cache"))
-			} else {
-				fmt.Fprintf(b.out, " ---> Using cache\n")
-			}
+			fmt.Fprintf(b.outStream, " ---> Using cache\n")
 			utils.Debugf("[BUILDER] Use cached version")
 			b.image = cache.ID
 			return nil
@@ -369,6 +369,34 @@ func (b *buildFile) CmdAdd(args string) error {
 	return nil
 }
 
+type StdoutFormater struct {
+	io.Writer
+	*utils.StreamFormatter
+}
+
+func (sf *StdoutFormater) Write(buf []byte) (int, error) {
+	formattedBuf := sf.StreamFormatter.FormatStatus("", "%s", string(buf))
+	n, err := sf.Writer.Write(formattedBuf)
+	if n != len(formattedBuf) {
+		return n, io.ErrShortWrite
+	}
+	return len(buf), err
+}
+
+type StderrFormater struct {
+	io.Writer
+	*utils.StreamFormatter
+}
+
+func (sf *StderrFormater) Write(buf []byte) (int, error) {
+	formattedBuf := sf.StreamFormatter.FormatStatus("", "%s", "\033[91m"+string(buf)+"\033[0m")
+	n, err := sf.Writer.Write(formattedBuf)
+	if n != len(formattedBuf) {
+		return n, io.ErrShortWrite
+	}
+	return len(buf), err
+}
+
 func (b *buildFile) run() (string, error) {
 	if b.image == "" {
 		return "", fmt.Errorf("Please provide a source image with `from` prior to run")
@@ -381,11 +409,8 @@ func (b *buildFile) run() (string, error) {
 		return "", err
 	}
 	b.tmpContainers[c.ID] = struct{}{}
-	if b.sf.Used() {
-		b.out.Write(b.sf.FormatStatus("", " ---> Running in %s", utils.TruncateID(c.ID)))
-	} else {
-		fmt.Fprintf(b.out, " ---> Running in %s\n", utils.TruncateID(c.ID))
-	}
+	fmt.Fprintf(b.outStream, " ---> Running in %s\n", utils.TruncateID(c.ID))
+
 	// override the entry point that may have been picked up from the base image
 	c.Path = b.config.Cmd[0]
 	c.Args = b.config.Cmd[1:]
@@ -394,7 +419,7 @@ func (b *buildFile) run() (string, error) {
 
 	if b.verbose {
 		errCh = utils.Go(func() error {
-			return <-c.Attach(nil, nil, b.out, b.out)
+			return <-c.Attach(nil, nil, b.outStream, b.errStream)
 		})
 	}
 
@@ -436,11 +461,7 @@ func (b *buildFile) commit(id string, autoCmd []string, comment string) error {
 			if cache, err := b.srv.ImageGetCached(b.image, b.config); err != nil {
 				return err
 			} else if cache != nil {
-				if b.sf.Used() {
-					b.out.Write(b.sf.FormatStatus("", " ---> Using cache"))
-				} else {
-					fmt.Fprintf(b.out, " ---> Using cache\n")
-				}
+				fmt.Fprintf(b.outStream, " ---> Using cache\n")
 				utils.Debugf("[BUILDER] Use cached version")
 				b.image = cache.ID
 				return nil
@@ -454,14 +475,10 @@ func (b *buildFile) commit(id string, autoCmd []string, comment string) error {
 			return err
 		}
 		for _, warning := range warnings {
-			fmt.Fprintf(b.out, " ---> [Warning] %s\n", warning)
+			fmt.Fprintf(b.outStream, " ---> [Warning] %s\n", warning)
 		}
 		b.tmpContainers[container.ID] = struct{}{}
-		if b.sf.Used() {
-			b.out.Write(b.sf.FormatStatus("", " ---> Running in %s", utils.TruncateID(container.ID)))
-		} else {
-			fmt.Fprintf(b.out, " ---> Running in %s\n", utils.TruncateID(container.ID))
-		}
+		fmt.Fprintf(b.outStream, " ---> Running in %s\n", utils.TruncateID(container.ID))
 		id = container.ID
 		if err := container.EnsureMounted(); err != nil {
 			return err
@@ -527,22 +544,22 @@ func (b *buildFile) Build(context io.Reader) (string, error) {
 
 		method, exists := reflect.TypeOf(b).MethodByName("Cmd" + strings.ToUpper(instruction[:1]) + strings.ToLower(instruction[1:]))
 		if !exists {
-			b.out.Write(b.sf.FormatStatus("", "# Skipping unknown instruction %s", strings.ToUpper(instruction)))
+			fmt.Fprintf(b.errStream, "# Skipping unknown instruction %s\n", strings.ToUpper(instruction))
 			continue
 		}
 
 		stepN += 1
-		b.out.Write(b.sf.FormatStatus("", "Step %d : %s %s", stepN, strings.ToUpper(instruction), arguments))
+		fmt.Fprintf(b.outStream, "Step %d : %s %s\n", stepN, strings.ToUpper(instruction), arguments)
 
 		ret := method.Func.Call([]reflect.Value{reflect.ValueOf(b), reflect.ValueOf(arguments)})[0].Interface()
 		if ret != nil {
 			return "", ret.(error)
 		}
 
-		b.out.Write(b.sf.FormatStatus("", " ---> %s", utils.TruncateID(b.image)))
+		fmt.Fprintf(b.outStream, " ---> %s\n", utils.TruncateID(b.image))
 	}
 	if b.image != "" {
-		b.out.Write(b.sf.FormatStatus("", "Successfully built %s", utils.TruncateID(b.image)))
+		fmt.Fprintf(b.outStream, "Successfully built %s\n", utils.TruncateID(b.image))
 		if b.rm {
 			b.clearTmp(b.tmpContainers)
 		}
@@ -551,17 +568,19 @@ func (b *buildFile) Build(context io.Reader) (string, error) {
 	return "", fmt.Errorf("An error occurred during the build\n")
 }
 
-func NewBuildFile(srv *Server, out io.Writer, verbose, utilizeCache, rm bool, sf *utils.StreamFormatter) BuildFile {
+func NewBuildFile(srv *Server, outStream, errStream io.Writer, verbose, utilizeCache, rm bool, outOld io.Writer, sf *utils.StreamFormatter) BuildFile {
 	return &buildFile{
 		runtime:       srv.runtime,
 		srv:           srv,
 		config:        &Config{},
-		out:           out,
+		outStream:     outStream,
+		errStream:     errStream,
 		tmpContainers: make(map[string]struct{}),
 		tmpImages:     make(map[string]struct{}),
 		verbose:       verbose,
 		utilizeCache:  utilizeCache,
 		rm:            rm,
 		sf:            sf,
+		outOld:        outOld,
 	}
 }

commands.go

@@ -229,6 +229,8 @@ func (cli *DockerCli) CmdBuild(args ...string) error {
 	if context != nil {
 		headers.Set("Content-Type", "application/tar")
 	}
+	// Temporary hack to fix displayJSON behavior
+	cli.isTerminal = false
 	err = cli.stream("POST", fmt.Sprintf("/build?%s", v.Encode()), body, cli.out, headers)
 	if jerr, ok := err.(*utils.JSONError); ok {
 		return &utils.StatusError{Status: jerr.Message, StatusCode: jerr.Code}

integration/buildfile_test.go

@@ -266,7 +266,7 @@ func buildImage(context testContextTemplate, t *testing.T, eng *engine.Engine, u
 	}
 	dockerfile := constructDockerfile(context.dockerfile, ip, port)
 
-	buildfile := docker.NewBuildFile(srv, ioutil.Discard, false, useCache, false, utils.NewStreamFormatter(false))
+	buildfile := docker.NewBuildFile(srv, ioutil.Discard, ioutil.Discard, false, useCache, false, ioutil.Discard, utils.NewStreamFormatter(false))
 	id, err := buildfile.Build(mkTestContext(dockerfile, context.files, t))
 	if err != nil {
 		return nil, err
@@ -516,7 +516,7 @@ func TestForbiddenContextPath(t *testing.T) {
 	}
 	dockerfile := constructDockerfile(context.dockerfile, ip, port)
 
-	buildfile := docker.NewBuildFile(srv, ioutil.Discard, false, true, false, utils.NewStreamFormatter(false))
+	buildfile := docker.NewBuildFile(srv, ioutil.Discard, ioutil.Discard, false, true, false, ioutil.Discard, utils.NewStreamFormatter(false))
 	_, err = buildfile.Build(mkTestContext(dockerfile, context.files, t))
 
 	if err == nil {
@@ -562,7 +562,7 @@ func TestBuildADDFileNotFound(t *testing.T) {
 	}
 	dockerfile := constructDockerfile(context.dockerfile, ip, port)
 
-	buildfile := docker.NewBuildFile(mkServerFromEngine(eng, t), ioutil.Discard, false, true, false, utils.NewStreamFormatter(false))
+	buildfile := docker.NewBuildFile(mkServerFromEngine(eng, t), ioutil.Discard, ioutil.Discard, false, true, false, ioutil.Discard, utils.NewStreamFormatter(false))
 	_, err = buildfile.Build(mkTestContext(dockerfile, context.files, t))
 
 	if err == nil {

utils/jsonmessage.go

@@ -59,11 +59,11 @@ func (jm *JSONMessage) Display(out io.Writer, isTerminal bool) error {
 		}
 		return jm.Error
 	}
-	endl := ""
+	var endl string
 	if isTerminal {
 		// <ESC>[2K = erase entire current line
 		fmt.Fprintf(out, "%c[2K\r", 27)
-		endl = "\r"
+		endl = "\r\n"
 	}
 	if jm.Time != 0 {
 		fmt.Fprintf(out, "[%s] ", time.Unix(jm.Time, 0))
@@ -79,20 +79,23 @@ func (jm *JSONMessage) Display(out io.Writer, isTerminal bool) error {
 	} else if jm.ProgressMessage != "" { //deprecated
 		fmt.Fprintf(out, "%s %s%s", jm.Status, jm.ProgressMessage, endl)
 	} else {
-		fmt.Fprintf(out, "%s%s\n", jm.Status, endl)
+		fmt.Fprintf(out, "%s%s", jm.Status, endl)
 	}
 	return nil
 }
 
 func DisplayJSONMessagesStream(in io.Reader, out io.Writer, isTerminal bool) error {
-	dec := json.NewDecoder(in)
-	ids := make(map[string]int)
-	diff := 0
+	var (
+		dec  = json.NewDecoder(in)
+		ids  = make(map[string]int)
+		diff = 0
+	)
 	for {
-		jm := JSONMessage{}
-		if err := dec.Decode(&jm); err == io.EOF {
-			break
-		} else if err != nil {
+		var jm JSONMessage
+		if err := dec.Decode(&jm); err != nil {
+			if err == io.EOF {
+				break
+			}
 			return err
 		}
 		if (jm.Progress != nil || jm.ProgressMessage != "") && jm.ID != "" {