Revert "Fix dockerfile parser with empty line after escape"

This reverts commit 3e1b539e8d.

Signed-off-by: Vincent Demeester <vincent@sbr.pm>
(cherry picked from commit 6e0666e622)
Signed-off-by: Victor Vieux <victorvieux@gmail.com>

builder/dockerfile/parser/parser.go

@@ -176,17 +176,10 @@ 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 stripComments(strings.TrimSpace(newline)) == "" {
+					continue
 				}
 
-				// If escape followed by an empty line then stop
-				if strings.TrimSpace(newline) == "" {
-					break
-				}
 				line, child, err = ParseLine(line+newline, d, false)
 				if err != nil {
 					return nil, err

builder/dockerfile/parser/parser_test.go

@@ -150,8 +150,8 @@ func TestLineInformation(t *testing.T) {
 		t.Fatalf("Error parsing dockerfile %s: %v", testFileLineInfo, err)
 	}
 
-	if ast.StartLine != 5 || ast.EndLine != 27 {
-		fmt.Fprintf(os.Stderr, "Wrong root line information: expected(%d-%d), actual(%d-%d)\n", 5, 27, ast.StartLine, ast.EndLine)
+	if ast.StartLine != 5 || ast.EndLine != 31 {
+		fmt.Fprintf(os.Stderr, "Wrong root line information: expected(%d-%d), actual(%d-%d)\n", 5, 31, ast.StartLine, ast.EndLine)
 		t.Fatalf("Root line information doesn't match result.")
 	}
 	if len(ast.Children) != 3 {
@@ -161,7 +161,7 @@ func TestLineInformation(t *testing.T) {
 	expected := [][]int{
 		{5, 5},
 		{11, 12},
-		{17, 27},
+		{17, 31},
 	}
 	for i, child := range ast.Children {
 		if child.StartLine != expected[i][0] || child.EndLine != expected[i][1] {

builder/dockerfile/parser/testfile-line/Dockerfile

@@ -16,11 +16,15 @@ ENV GOPATH \
 # Install the packages we need, clean up after them and us
 RUN apt-get update \
 	&& dpkg -l | awk '/^ii/ {print $2}' > /tmp/dpkg.clean \
+
+
     && apt-get install -y --no-install-recommends git golang ca-certificates \
     && apt-get clean \
     && rm -rf /var/lib/apt/lists \
+
 	&& go get -v github.com/brimstone/consuldock \
     && mv $GOPATH/bin/consuldock /usr/local/bin/consuldock \
+
 	&& dpkg -l | awk '/^ii/ {print $2}' > /tmp/dpkg.dirty \
 	&& apt-get remove --purge -y $(diff /tmp/dpkg.clean /tmp/dpkg.dirty | awk '/^>/ {print $2}') \
 	&& rm /tmp/dpkg.* \

builder/dockerfile/parser/testfiles/brimstone-consuldock/Dockerfile

@@ -16,8 +16,10 @@ RUN apt-get update \
     && apt-get install -y --no-install-recommends git golang ca-certificates \
     && apt-get clean \
     && rm -rf /var/lib/apt/lists \
+
 	&& go get -v github.com/brimstone/consuldock \
     && mv $GOPATH/bin/consuldock /usr/local/bin/consuldock \
+
 	&& dpkg -l | awk '/^ii/ {print $2}' > /tmp/dpkg.dirty \
 	&& apt-get remove --purge -y $(diff /tmp/dpkg.clean /tmp/dpkg.dirty | awk '/^>/ {print $2}') \
 	&& rm /tmp/dpkg.* \

builder/dockerfile/parser/testfiles/brimstone-docker-consul/Dockerfile

@@ -23,12 +23,14 @@ RUN apt-get update \
     && apt-get install -y --no-install-recommends unzip wget \
     && apt-get clean \
     && rm -rf /var/lib/apt/lists \
+
     && cd /tmp \
     && wget https://dl.bintray.com/mitchellh/consul/0.3.1_web_ui.zip \
        -O web_ui.zip \
     && unzip web_ui.zip \
     && mv dist /webui \
     && rm web_ui.zip \
+
 	&& dpkg -l | awk '/^ii/ {print $2}' > /tmp/dpkg.dirty \
 	&& apt-get remove --purge -y $(diff /tmp/dpkg.clean /tmp/dpkg.dirty | awk '/^>/ {print $2}') \
 	&& rm /tmp/dpkg.*
@@ -40,8 +42,10 @@ RUN apt-get update \
     && apt-get install -y --no-install-recommends git golang ca-certificates build-essential \
     && apt-get clean \
     && rm -rf /var/lib/apt/lists \
+
 	&& go get -v github.com/hashicorp/consul \
 	&& mv $GOPATH/bin/consul /usr/bin/consul \
+
 	&& dpkg -l | awk '/^ii/ {print $2}' > /tmp/dpkg.dirty \
 	&& apt-get remove --purge -y $(diff /tmp/dpkg.clean /tmp/dpkg.dirty | awk '/^>/ {print $2}') \
 	&& rm /tmp/dpkg.* \

builder/dockerfile/parser/testfiles/continueIndent/Dockerfile

@@ -27,5 +27,8 @@ 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 with a blank line surrounding it
+
+this is some more useful stuff

builder/dockerfile/parser/testfiles/continueIndent/result

@@ -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")

builder/dockerfile/parser/testfiles/empty-line-after-escape/Dockerfile

@@ -1,15 +0,0 @@
-FROM busybox
-
-# The following will create two instructions
-# `Run foo`
-# `bar`
-# because empty line will break the escape.
-# The parser will generate the following:
-# (from "busybox")
-# (run "foo")
-# (bar "")
-# And `bar` will return an error instruction later
-# Note: Parse() will not immediately error out.
-RUN foo \
-
-bar

builder/dockerfile/parser/testfiles/empty-line-after-escape/result

@@ -1,3 +0,0 @@
-(from "busybox")
-(run "foo")
-(bar "")

builder/dockerfile/parser/testfiles/escapes/Dockerfile

@@ -6,7 +6,9 @@ RUN apt-get \update && \
 ADD \conf\\" /.znc
 
 RUN foo \
+
 bar \
+
 baz
 
 CMD [ "\/usr\\\"/bin/znc", "-f", "-r" ]

integration-cli/docker_cli_build_test.go

@@ -3610,8 +3610,8 @@ RUN [ "$(id -u):$(id -g)/$(id -un):$(id -gn)/$(id -G):$(id -Gn)" = '1001:1001/do
 
 # Switch back to root and double check that worked exactly as we might expect it to
 USER root
-# Add a "supplementary" group for our dockerio user
 RUN [ "$(id -u):$(id -g)/$(id -un):$(id -gn)/$(id -G):$(id -Gn)" = '0:0/root:root/0 10:root wheel' ] && \
+        # Add a "supplementary" group for our dockerio user
 	echo 'supplementary:x:1002:dockerio' >> /etc/group
 
 # ... and then go verify that we get it like we expect
@@ -7141,41 +7141,6 @@ func (s *DockerSuite) TestBuildNetContainer(c *check.C) {
 	c.Assert(strings.TrimSpace(host), check.Equals, "foobar")
 }
 
-// Test case for #24693
-func (s *DockerSuite) TestBuildRunEmptyLineAfterEscape(c *check.C) {
-	name := "testbuildemptylineafterescape"
-	_, out, err := buildImageWithOut(name,
-		`
-FROM busybox
-RUN echo x \
-
-RUN echo y
-RUN echo z
-# Comment requires the '#' to start from position 1
-# RUN echo w
-`, true)
-	c.Assert(err, checker.IsNil)
-	c.Assert(out, checker.Contains, "Step 1/4 : FROM busybox")
-	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) {
 	testRequires(c, ExperimentalDaemon)
 	dockerFile := `