瀏覽代碼

Fix issue where build steps are duplicated in the output

This fixes an issue where the build output for the "Steps" would look like:
```
Step 1: RUN echo hi echo hi
```
instead of
```
Step 1: RUN echo hi
```

Also, I noticed that there were no checks to make sure invalid Dockerfile
cmd flags were caught on cmds that didn't use cmd flags at all. They would
have been caught on the cmds that had flags, but cmds that didn't bother
to add a new code for flags would have just ignored them.  So, I added
checks to each cmd to flag it.

Added testcases for issues.

Signed-off-by: Doug Davis <dug@us.ibm.com>
Doug Davis 10 年之前
父節點
當前提交
08b7f30fcd
共有 3 個文件被更改,包括 101 次插入1 次删除
  1. 56 0
      builder/dispatchers.go
  2. 10 1
      builder/evaluator.go
  3. 35 0
      integration-cli/docker_cli_build_test.go

+ 56 - 0
builder/dispatchers.go

@@ -47,6 +47,10 @@ func env(b *Builder, args []string, attributes map[string]bool, original string)
 		return fmt.Errorf("Bad input to ENV, too many args")
 	}
 
+	if err := b.BuilderFlags.Parse(); err != nil {
+		return err
+	}
+
 	// TODO/FIXME/NOT USED
 	// Just here to show how to use the builder flags stuff within the
 	// context of a builder command. Will remove once we actually add
@@ -97,6 +101,10 @@ func maintainer(b *Builder, args []string, attributes map[string]bool, original
 		return fmt.Errorf("MAINTAINER requires exactly one argument")
 	}
 
+	if err := b.BuilderFlags.Parse(); err != nil {
+		return err
+	}
+
 	b.maintainer = args[0]
 	return b.commit("", b.Config.Cmd, fmt.Sprintf("MAINTAINER %s", b.maintainer))
 }
@@ -114,6 +122,10 @@ func label(b *Builder, args []string, attributes map[string]bool, original strin
 		return fmt.Errorf("Bad input to LABEL, too many args")
 	}
 
+	if err := b.BuilderFlags.Parse(); err != nil {
+		return err
+	}
+
 	commitStr := "LABEL"
 
 	if b.Config.Labels == nil {
@@ -142,6 +154,10 @@ func add(b *Builder, args []string, attributes map[string]bool, original string)
 		return fmt.Errorf("ADD requires at least two arguments")
 	}
 
+	if err := b.BuilderFlags.Parse(); err != nil {
+		return err
+	}
+
 	return b.runContextCommand(args, true, true, "ADD")
 }
 
@@ -154,6 +170,10 @@ func dispatchCopy(b *Builder, args []string, attributes map[string]bool, origina
 		return fmt.Errorf("COPY requires at least two arguments")
 	}
 
+	if err := b.BuilderFlags.Parse(); err != nil {
+		return err
+	}
+
 	return b.runContextCommand(args, false, false, "COPY")
 }
 
@@ -166,6 +186,10 @@ func from(b *Builder, args []string, attributes map[string]bool, original string
 		return fmt.Errorf("FROM requires one argument")
 	}
 
+	if err := b.BuilderFlags.Parse(); err != nil {
+		return err
+	}
+
 	name := args[0]
 
 	if name == NoBaseImageSpecifier {
@@ -210,6 +234,10 @@ func onbuild(b *Builder, args []string, attributes map[string]bool, original str
 		return fmt.Errorf("ONBUILD requires at least one argument")
 	}
 
+	if err := b.BuilderFlags.Parse(); err != nil {
+		return err
+	}
+
 	triggerInstruction := strings.ToUpper(strings.TrimSpace(args[0]))
 	switch triggerInstruction {
 	case "ONBUILD":
@@ -233,6 +261,10 @@ func workdir(b *Builder, args []string, attributes map[string]bool, original str
 		return fmt.Errorf("WORKDIR requires exactly one argument")
 	}
 
+	if err := b.BuilderFlags.Parse(); err != nil {
+		return err
+	}
+
 	workdir := args[0]
 
 	if !filepath.IsAbs(workdir) {
@@ -258,6 +290,10 @@ func run(b *Builder, args []string, attributes map[string]bool, original string)
 		return fmt.Errorf("Please provide a source image with `from` prior to run")
 	}
 
+	if err := b.BuilderFlags.Parse(); err != nil {
+		return err
+	}
+
 	args = handleJsonArgs(args, attributes)
 
 	if !attributes["json"] {
@@ -317,6 +353,10 @@ func run(b *Builder, args []string, attributes map[string]bool, original string)
 // Argument handling is the same as RUN.
 //
 func cmd(b *Builder, args []string, attributes map[string]bool, original string) error {
+	if err := b.BuilderFlags.Parse(); err != nil {
+		return err
+	}
+
 	cmdSlice := handleJsonArgs(args, attributes)
 
 	if !attributes["json"] {
@@ -345,6 +385,10 @@ func cmd(b *Builder, args []string, attributes map[string]bool, original string)
 // is initialized at NewBuilder time instead of through argument parsing.
 //
 func entrypoint(b *Builder, args []string, attributes map[string]bool, original string) error {
+	if err := b.BuilderFlags.Parse(); err != nil {
+		return err
+	}
+
 	parsed := handleJsonArgs(args, attributes)
 
 	switch {
@@ -384,6 +428,10 @@ func expose(b *Builder, args []string, attributes map[string]bool, original stri
 		return fmt.Errorf("EXPOSE requires at least one argument")
 	}
 
+	if err := b.BuilderFlags.Parse(); err != nil {
+		return err
+	}
+
 	if b.Config.ExposedPorts == nil {
 		b.Config.ExposedPorts = make(nat.PortSet)
 	}
@@ -428,6 +476,10 @@ func user(b *Builder, args []string, attributes map[string]bool, original string
 		return fmt.Errorf("USER requires exactly one argument")
 	}
 
+	if err := b.BuilderFlags.Parse(); err != nil {
+		return err
+	}
+
 	b.Config.User = args[0]
 	return b.commit("", b.Config.Cmd, fmt.Sprintf("USER %v", args))
 }
@@ -441,6 +493,10 @@ func volume(b *Builder, args []string, attributes map[string]bool, original stri
 		return fmt.Errorf("VOLUME requires at least one argument")
 	}
 
+	if err := b.BuilderFlags.Parse(); err != nil {
+		return err
+	}
+
 	if b.Config.Volumes == nil {
 		b.Config.Volumes = map[string]struct{}{}
 	}

+ 10 - 1
builder/evaluator.go

@@ -280,7 +280,11 @@ func (b *Builder) dispatch(stepN int, ast *parser.Node) error {
 	original := ast.Original
 	flags := ast.Flags
 	strs := []string{}
-	msg := fmt.Sprintf("Step %d : %s", stepN, original)
+	msg := fmt.Sprintf("Step %d : %s", stepN, strings.ToUpper(cmd))
+
+	if len(ast.Flags) > 0 {
+		msg += " " + strings.Join(ast.Flags, " ")
+	}
 
 	if cmd == "onbuild" {
 		if ast.Next == nil {
@@ -289,6 +293,11 @@ func (b *Builder) dispatch(stepN int, ast *parser.Node) error {
 		ast = ast.Next.Children[0]
 		strs = append(strs, ast.Value)
 		msg += " " + ast.Value
+
+		if len(ast.Flags) > 0 {
+			msg += " " + strings.Join(ast.Flags, " ")
+		}
+
 	}
 
 	// count the number of nodes that we are going to traverse first

+ 35 - 0
integration-cli/docker_cli_build_test.go

@@ -5349,3 +5349,38 @@ RUN cat /proc/self/cgroup
 		c.Fatalf("unexpected failure when running container with --cgroup-parent option - %s\n%v", string(out), err)
 	}
 }
+
+func (s *DockerSuite) TestBuildNoDupOutput(c *check.C) {
+	// Check to make sure our build output prints the Dockerfile cmd
+	// property - there was a bug that caused it to be duplicated on the
+	// Step X  line
+	name := "testbuildnodupoutput"
+
+	_, out, err := buildImageWithOut(name, `
+  FROM busybox
+  RUN env`, false)
+	if err != nil {
+		c.Fatalf("Build should have worked: %q", err)
+	}
+
+	exp := "\nStep 1 : RUN env\n"
+	if !strings.Contains(out, exp) {
+		c.Fatalf("Bad output\nGot:%s\n\nExpected to contain:%s\n", out, exp)
+	}
+}
+
+func (s *DockerSuite) TestBuildBadCmdFlag(c *check.C) {
+	name := "testbuildbadcmdflag"
+
+	_, out, err := buildImageWithOut(name, `
+  FROM busybox
+  MAINTAINER --boo joe@example.com`, false)
+	if err == nil {
+		c.Fatal("Build should have failed")
+	}
+
+	exp := `"Unknown flag: boo"`
+	if !strings.Contains(out, exp) {
+		c.Fatalf("Bad output\nGot:%s\n\nExpected to contain:%s\n", out, exp)
+	}
+}