Browse Source

Merge pull request #13004 from duglin/FixBuilderStepMsg

Fix issue where build steps are duplicated in the output
Jessie Frazelle 10 years ago
parent
commit
b5d68944f4
3 changed files with 101 additions and 1 deletions
  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)
+	}
+}