Skip to content

Commit

Permalink
Allow continuation after comments
Browse files Browse the repository at this point in the history
This fix is to address moby#29005 where continuation after comments
are not allowed after moby#24725.

This fix made the change so that moby#29005 could be addressed.

An integration test has been added. Other related tests have
been udpated to conform to the behavior changes.

This fix fixes moby#29005, but please feel free for any additional
suggestions.

Signed-off-by: Yong Tang <yong.tang.github@outlook.com>
  • Loading branch information
yongtang committed Dec 1, 2016
1 parent 457d562 commit 26aa42e
Show file tree
Hide file tree
Showing 4 changed files with 29 additions and 26 deletions.
13 changes: 6 additions & 7 deletions builder/dockerfile/parser/parser.go
Original file line number Diff line number Diff line change
Expand Up @@ -176,17 +176,16 @@ func Parse(rwc io.Reader, d *Directive) (*Node, error) {
newline := scanner.Text()
currentLine++

// If escape followed by a comment line then stop
// Note here that comment line starts with `#` at
// the first pos of the line
if stripComments(newline) == "" {
break
}

// If escape followed by an empty line then stop
if strings.TrimSpace(newline) == "" {
break
}

// If escape followed by a comment line then we continue.
if strings.HasPrefix(strings.TrimSpace(newline), "#") {
continue
}

line, child, err = ParseLine(line+newline, d, false)
if err != nil {
return nil, err
Expand Down
5 changes: 3 additions & 2 deletions builder/dockerfile/parser/testfiles/continueIndent/Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -27,5 +27,6 @@ bye\
frog

RUN echo hello \
# this is a comment that breaks escape continuation
RUN echo this is some more useful stuff
# this is a comment
# this is a comment not starting from pos 1
this is some more useful stuff
3 changes: 1 addition & 2 deletions builder/dockerfile/parser/testfiles/continueIndent/result
Original file line number Diff line number Diff line change
Expand Up @@ -6,5 +6,4 @@
(run "echo hi world goodnight")
(run "echo goodbyefrog")
(run "echo goodbyefrog")
(run "echo hello")
(run "echo this is some more useful stuff")
(run "echo hello this is some more useful stuff")
34 changes: 19 additions & 15 deletions integration-cli/docker_cli_build_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7159,21 +7159,6 @@ RUN echo z
c.Assert(out, checker.Contains, "Step 2/4 : RUN echo x")
c.Assert(out, checker.Contains, "Step 3/4 : RUN echo y")
c.Assert(out, checker.Contains, "Step 4/4 : RUN echo z")

// With comment, see #24693
name = "testbuildcommentandemptylineafterescape"
_, out, err = buildImageWithOut(name,
`
FROM busybox
RUN echo grafana && \
echo raintank \
#echo env-load
RUN echo vegeta
`, true)
c.Assert(err, checker.IsNil)
c.Assert(out, checker.Contains, "Step 1/3 : FROM busybox")
c.Assert(out, checker.Contains, "Step 2/3 : RUN echo grafana && echo raintank")
c.Assert(out, checker.Contains, "Step 3/3 : RUN echo vegeta")
}

func (s *DockerSuite) TestBuildSquashParent(c *check.C) {
Expand Down Expand Up @@ -7327,3 +7312,22 @@ func (s *DockerSuite) TestBuildWindowsEnvCaseInsensitive(c *check.C) {
c.Fatalf("Case insensitive environment variables on Windows failed. Got %s", res)
}
}

// Test case for #29005
func (s *DockerSuite) TestBuildRunCommentLineAfterEscape(c *check.C) {
name := "testbuildcommentlineafterescape"

_, out, err := buildImageWithOut(name,
`
FROM busybox
RUN echo hi \
# wut
&& echo no \
# omg
&& echo yes
`, true)
c.Assert(err, checker.IsNil)
c.Assert(out, checker.Contains, "Step 1/2 : FROM busybox")
c.Assert(out, checker.Contains, "Step 2/2 : RUN echo hi && echo no && echo yes")
}

0 comments on commit 26aa42e

Please sign in to comment.