From de4429f70d51056c3ec32072f3f18898e36171b3 Mon Sep 17 00:00:00 2001 From: "Guillaume J. Charmes" Date: Mon, 2 Dec 2013 11:43:41 -0800 Subject: [PATCH 1/4] Do not format at each write but use a Writer instead (build) --- api.go | 15 ++++++++-- buildfile.go | 83 ++++++++++++++++++++++++++++++++-------------------- 2 files changed, 63 insertions(+), 35 deletions(-) diff --git a/api.go b/api.go index 6660d396ab..da16044410 100644 --- a/api.go +++ b/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() { diff --git a/buildfile.go b/buildfile.go index 92b3297843..e8f178fbbd 100644 --- a/buildfile.go +++ b/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, } } From 6ea3b9651b3793e31a320926472ff23383a7b915 Mon Sep 17 00:00:00 2001 From: "Guillaume J. Charmes" Date: Mon, 2 Dec 2013 11:47:13 -0800 Subject: [PATCH 2/4] Fix displayJson behavior (dont add newline) --- commands.go | 2 ++ utils/jsonmessage.go | 4 ++-- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/commands.go b/commands.go index e24b7aeece..19e7110e9d 100644 --- a/commands.go +++ b/commands.go @@ -228,6 +228,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} diff --git a/utils/jsonmessage.go b/utils/jsonmessage.go index a0aeba2e7a..be74033f7f 100644 --- a/utils/jsonmessage.go +++ b/utils/jsonmessage.go @@ -63,7 +63,7 @@ func (jm *JSONMessage) Display(out io.Writer, isTerminal bool) error { if isTerminal { // [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,7 +79,7 @@ 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 } From 5cd09dc1158c62754641dce9c59a735c41e59722 Mon Sep 17 00:00:00 2001 From: "Guillaume J. Charmes" Date: Mon, 2 Dec 2013 11:49:11 -0800 Subject: [PATCH 3/4] small reformatting jsonmessage --- utils/jsonmessage.go | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/utils/jsonmessage.go b/utils/jsonmessage.go index be74033f7f..809524cd28 100644 --- a/utils/jsonmessage.go +++ b/utils/jsonmessage.go @@ -59,7 +59,7 @@ func (jm *JSONMessage) Display(out io.Writer, isTerminal bool) error { } return jm.Error } - endl := "" + var endl string if isTerminal { // [2K = erase entire current line fmt.Fprintf(out, "%c[2K\r", 27) @@ -85,14 +85,17 @@ func (jm *JSONMessage) Display(out io.Writer, isTerminal bool) error { } 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 != "" { From 98ed1dc433e423b93ae0619c5a2c474ccec1454d Mon Sep 17 00:00:00 2001 From: "Guillaume J. Charmes" Date: Mon, 2 Dec 2013 12:51:37 -0800 Subject: [PATCH 4/4] Fix unit test with new buildfile prototype --- integration/buildfile_test.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/integration/buildfile_test.go b/integration/buildfile_test.go index 8975aa7ec1..242bf9f412 100644 --- a/integration/buildfile_test.go +++ b/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 {