From bb429da9a977a9d89121052fe1143528294b8a0c Mon Sep 17 00:00:00 2001 From: Daniel Nephin Date: Tue, 11 Apr 2017 15:07:02 -0400 Subject: [PATCH 1/4] Hide builder.parser.Directive internals Signed-off-by: Daniel Nephin --- builder/dockerfile/builder.go | 11 ++--- builder/dockerfile/builder_test.go | 5 +-- builder/dockerfile/dispatchers.go | 2 +- builder/dockerfile/dispatchers_test.go | 2 + builder/dockerfile/evaluator.go | 2 +- builder/dockerfile/evaluator_test.go | 5 +-- builder/dockerfile/internals.go | 4 +- builder/dockerfile/parser/dumper/main.go | 6 +-- builder/dockerfile/parser/json_test.go | 4 +- builder/dockerfile/parser/line_parsers.go | 4 +- builder/dockerfile/parser/parser.go | 55 ++++++++++++++--------- builder/dockerfile/parser/parser_test.go | 16 ++----- 12 files changed, 58 insertions(+), 58 deletions(-) diff --git a/builder/dockerfile/builder.go b/builder/dockerfile/builder.go index 6f7f20d7d1..d02b3cd7a4 100644 --- a/builder/dockerfile/builder.go +++ b/builder/dockerfile/builder.go @@ -62,7 +62,7 @@ type Builder struct { disableCommit bool cacheBusted bool buildArgs *buildArgs - directive parser.Directive + directive *parser.Directive imageCache builder.ImageCache from builder.Image @@ -122,14 +122,9 @@ func NewBuilder(clientCtx context.Context, config *types.ImageBuildOptions, back runConfig: new(container.Config), tmpContainers: map[string]struct{}{}, buildArgs: newBuildArgs(config.BuildArgs), - directive: parser.Directive{ - EscapeSeen: false, - LookingForDirectives: true, - }, + directive: parser.NewDefaultDirective(), } b.imageContexts = &imageContexts{b: b} - - parser.SetEscapeToken(parser.DefaultEscapeToken, &b.directive) // Assume the default token for escape return b, nil } @@ -325,7 +320,7 @@ func BuildFromConfig(config *container.Config, changes []string) (*container.Con return nil, err } - ast, err := parser.Parse(bytes.NewBufferString(strings.Join(changes, "\n")), &b.directive) + ast, err := parser.Parse(bytes.NewBufferString(strings.Join(changes, "\n")), b.directive) if err != nil { return nil, err } diff --git a/builder/dockerfile/builder_test.go b/builder/dockerfile/builder_test.go index 9a5a17ef4d..cd79af1dfc 100644 --- a/builder/dockerfile/builder_test.go +++ b/builder/dockerfile/builder_test.go @@ -10,9 +10,8 @@ import ( func TestAddNodesForLabelOption(t *testing.T) { dockerfile := "FROM scratch" - d := parser.Directive{} - parser.SetEscapeToken(parser.DefaultEscapeToken, &d) - nodes, err := parser.Parse(strings.NewReader(dockerfile), &d) + d := parser.NewDefaultDirective() + nodes, err := parser.Parse(strings.NewReader(dockerfile), d) assert.NilError(t, err) labels := map[string]string{ diff --git a/builder/dockerfile/dispatchers.go b/builder/dockerfile/dispatchers.go index b674961806..5a467e6285 100644 --- a/builder/dockerfile/dispatchers.go +++ b/builder/dockerfile/dispatchers.go @@ -200,7 +200,7 @@ func from(b *Builder, args []string, attributes map[string]bool, original string substituionArgs = append(substituionArgs, key+"="+value) } - name, err := ProcessWord(args[0], substituionArgs, b.directive.EscapeToken) + name, err := ProcessWord(args[0], substituionArgs, b.directive.EscapeToken()) if err != nil { return err } diff --git a/builder/dockerfile/dispatchers_test.go b/builder/dockerfile/dispatchers_test.go index 644b610825..d721637d13 100644 --- a/builder/dockerfile/dispatchers_test.go +++ b/builder/dockerfile/dispatchers_test.go @@ -10,6 +10,7 @@ import ( "github.com/docker/docker/api/types/container" "github.com/docker/docker/api/types/strslice" "github.com/docker/docker/builder" + "github.com/docker/docker/builder/dockerfile/parser" "github.com/docker/docker/pkg/testutil/assert" "github.com/docker/go-connections/nat" ) @@ -204,6 +205,7 @@ func newBuilderWithMockBackend() *Builder { options: &types.ImageBuildOptions{}, docker: &MockBackend{}, buildArgs: newBuildArgs(make(map[string]*string)), + directive: parser.NewDefaultDirective(), } b.imageContexts = &imageContexts{b: b} return b diff --git a/builder/dockerfile/evaluator.go b/builder/dockerfile/evaluator.go index c456ba41f7..df50ee561d 100644 --- a/builder/dockerfile/evaluator.go +++ b/builder/dockerfile/evaluator.go @@ -180,7 +180,7 @@ func (b *Builder) evaluateEnv(cmd string, str string, envs []string) ([]string, return []string{word}, err } } - return processFunc(str, envs, b.directive.EscapeToken) + return processFunc(str, envs, b.directive.EscapeToken()) } // buildArgsWithoutConfigEnv returns a list of key=value pairs for all the build diff --git a/builder/dockerfile/evaluator_test.go b/builder/dockerfile/evaluator_test.go index 0d4952837f..adc72d3334 100644 --- a/builder/dockerfile/evaluator_test.go +++ b/builder/dockerfile/evaluator_test.go @@ -171,9 +171,7 @@ func executeTestCase(t *testing.T, testCase dispatchTestCase) { }() r := strings.NewReader(testCase.dockerfile) - d := parser.Directive{} - parser.SetEscapeToken(parser.DefaultEscapeToken, &d) - n, err := parser.Parse(r, &d) + n, err := parser.Parse(r, parser.NewDefaultDirective()) if err != nil { t.Fatalf("Error when parsing Dockerfile: %s", err) @@ -190,6 +188,7 @@ func executeTestCase(t *testing.T, testCase dispatchTestCase) { Stdout: ioutil.Discard, context: context, buildArgs: newBuildArgs(options.BuildArgs), + directive: parser.NewDefaultDirective(), } err = b.dispatch(0, len(n.Children), n.Children[0]) diff --git a/builder/dockerfile/internals.go b/builder/dockerfile/internals.go index 32698775d1..5f9b9b0648 100644 --- a/builder/dockerfile/internals.go +++ b/builder/dockerfile/internals.go @@ -462,7 +462,7 @@ func (b *Builder) processImageFrom(img builder.Image) error { // parse the ONBUILD triggers by invoking the parser for _, step := range onBuildTriggers { - ast, err := parser.Parse(strings.NewReader(step), &b.directive) + ast, err := parser.Parse(strings.NewReader(step), b.directive) if err != nil { return err } @@ -702,5 +702,5 @@ func (b *Builder) parseDockerfile() (*parser.Node, error) { return nil, fmt.Errorf("The Dockerfile (%s) cannot be empty", b.options.Dockerfile) } } - return parser.Parse(f, &b.directive) + return parser.Parse(f, b.directive) } diff --git a/builder/dockerfile/parser/dumper/main.go b/builder/dockerfile/parser/dumper/main.go index fff3046fd3..5e9af94121 100644 --- a/builder/dockerfile/parser/dumper/main.go +++ b/builder/dockerfile/parser/dumper/main.go @@ -23,10 +23,8 @@ func main() { } defer f.Close() - d := parser.Directive{LookingForDirectives: true} - parser.SetEscapeToken(parser.DefaultEscapeToken, &d) - - ast, err := parser.Parse(f, &d) + d := parser.NewDefaultDirective() + ast, err := parser.Parse(f, d) if err != nil { panic(err) } else { diff --git a/builder/dockerfile/parser/json_test.go b/builder/dockerfile/parser/json_test.go index 60d74d9c36..26eea97064 100644 --- a/builder/dockerfile/parser/json_test.go +++ b/builder/dockerfile/parser/json_test.go @@ -29,7 +29,7 @@ var validJSONArraysOfStrings = map[string][]string{ func TestJSONArraysOfStrings(t *testing.T) { for json, expected := range validJSONArraysOfStrings { d := Directive{} - SetEscapeToken(DefaultEscapeToken, &d) + d.SetEscapeToken(DefaultEscapeToken) if node, _, err := parseJSON(json, &d); err != nil { t.Fatalf("%q should be a valid JSON array of strings, but wasn't! (err: %q)", json, err) @@ -52,7 +52,7 @@ func TestJSONArraysOfStrings(t *testing.T) { } for _, json := range invalidJSONArraysOfStrings { d := Directive{} - SetEscapeToken(DefaultEscapeToken, &d) + d.SetEscapeToken(DefaultEscapeToken) if _, _, err := parseJSON(json, &d); err != errDockerfileNotStringArray { t.Fatalf("%q should be an invalid JSON array of strings, but wasn't!", json) diff --git a/builder/dockerfile/parser/line_parsers.go b/builder/dockerfile/parser/line_parsers.go index 0334834e62..73cb39a348 100644 --- a/builder/dockerfile/parser/line_parsers.go +++ b/builder/dockerfile/parser/line_parsers.go @@ -103,7 +103,7 @@ func parseWords(rest string, d *Directive) []string { blankOK = true phase = inQuote } - if ch == d.EscapeToken { + if ch == d.escapeToken { if pos+chWidth == len(rest) { continue // just skip an escape token at end of line } @@ -122,7 +122,7 @@ func parseWords(rest string, d *Directive) []string { phase = inWord } // The escape token is special except for ' quotes - can't escape anything for ' - if ch == d.EscapeToken && quote != '\'' { + if ch == d.escapeToken && quote != '\'' { if pos+chWidth == len(rest) { phase = inWord continue // just skip the escape token at end diff --git a/builder/dockerfile/parser/parser.go b/builder/dockerfile/parser/parser.go index adc8a90eb7..e4b3aa61e6 100644 --- a/builder/dockerfile/parser/parser.go +++ b/builder/dockerfile/parser/parser.go @@ -62,15 +62,6 @@ func (node *Node) Dump() string { return strings.TrimSpace(str) } -// Directive is the structure used during a build run to hold the state of -// parsing directives. -type Directive struct { - EscapeToken rune // Current escape token - LineContinuationRegex *regexp.Regexp // Current line continuation regex - LookingForDirectives bool // Whether we are currently looking for directives - EscapeSeen bool // Whether the escape directive has been seen -} - var ( dispatch map[string]func(string, *Directive) (*Node, map[string]bool, error) tokenWhitespace = regexp.MustCompile(`[\t\v\f\r ]+`) @@ -81,16 +72,40 @@ var ( // DefaultEscapeToken is the default escape token const DefaultEscapeToken = "\\" +// Directive is the structure used during a build run to hold the state of +// parsing directives. +type Directive struct { + escapeToken rune // Current escape token + lineContinuationRegex *regexp.Regexp // Current line continuation regex + lookingForDirectives bool // Whether we are currently looking for directives + escapeSeen bool // Whether the escape directive has been seen +} + // SetEscapeToken sets the default token for escaping characters in a Dockerfile. -func SetEscapeToken(s string, d *Directive) error { +func (d *Directive) SetEscapeToken(s string) error { if s != "`" && s != "\\" { return fmt.Errorf("invalid ESCAPE '%s'. Must be ` or \\", s) } - d.EscapeToken = rune(s[0]) - d.LineContinuationRegex = regexp.MustCompile(`\` + s + `[ \t]*$`) + d.escapeToken = rune(s[0]) + d.lineContinuationRegex = regexp.MustCompile(`\` + s + `[ \t]*$`) return nil } +// EscapeToken returns the escape token +func (d *Directive) EscapeToken() rune { + return d.escapeToken +} + +// NewDefaultDirective returns a new Directive with the default escapeToken token +func NewDefaultDirective() *Directive { + directive := Directive{ + escapeSeen: false, + lookingForDirectives: true, + } + directive.SetEscapeToken(DefaultEscapeToken) + return &directive +} + func init() { // Dispatch Table. see line_parsers.go for the parse functions. // The command is parsed and mapped to the line parser. The line parser @@ -123,18 +138,18 @@ func init() { // ParseLine parses a line and returns the remainder. func ParseLine(line string, d *Directive, ignoreCont bool) (string, *Node, error) { if escapeFound, err := handleParserDirective(line, d); err != nil || escapeFound { - d.EscapeSeen = escapeFound + d.escapeSeen = escapeFound return "", nil, err } - d.LookingForDirectives = false + d.lookingForDirectives = false if line = stripComments(line); line == "" { return "", nil, nil } - if !ignoreCont && d.LineContinuationRegex.MatchString(line) { - line = d.LineContinuationRegex.ReplaceAllString(line, "") + if !ignoreCont && d.lineContinuationRegex.MatchString(line) { + line = d.lineContinuationRegex.ReplaceAllString(line, "") return line, nil, nil } @@ -170,22 +185,22 @@ func newNodeFromLine(line string, directive *Directive) (*Node, error) { }, nil } -// Handle the parser directive '# escape=. Parser directives must precede +// Handle the parser directive '# escapeToken=. Parser directives must precede // any builder instruction or other comments, and cannot be repeated. func handleParserDirective(line string, d *Directive) (bool, error) { - if !d.LookingForDirectives { + if !d.lookingForDirectives { return false, nil } tecMatch := tokenEscapeCommand.FindStringSubmatch(strings.ToLower(line)) if len(tecMatch) == 0 { return false, nil } - if d.EscapeSeen == true { + if d.escapeSeen == true { return false, fmt.Errorf("only one escape parser directive can be used") } for i, n := range tokenEscapeCommand.SubexpNames() { if n == "escapechar" { - if err := SetEscapeToken(tecMatch[i], d); err != nil { + if err := d.SetEscapeToken(tecMatch[i]); err != nil { return false, err } return true, nil diff --git a/builder/dockerfile/parser/parser_test.go b/builder/dockerfile/parser/parser_test.go index ee9461c823..82e78e91a9 100644 --- a/builder/dockerfile/parser/parser_test.go +++ b/builder/dockerfile/parser/parser_test.go @@ -40,9 +40,7 @@ func TestTestNegative(t *testing.T) { } defer df.Close() - d := Directive{LookingForDirectives: true} - SetEscapeToken(DefaultEscapeToken, &d) - _, err = Parse(df, &d) + _, err = Parse(df, NewDefaultDirective()) if err == nil { t.Fatalf("No error parsing broken dockerfile for %s", dir) } @@ -60,9 +58,7 @@ func TestTestData(t *testing.T) { } defer df.Close() - d := Directive{LookingForDirectives: true} - SetEscapeToken(DefaultEscapeToken, &d) - ast, err := Parse(df, &d) + ast, err := Parse(df, NewDefaultDirective()) if err != nil { t.Fatalf("Error parsing %s's dockerfile: %v", dir, err) } @@ -122,9 +118,7 @@ func TestParseWords(t *testing.T) { } for _, test := range tests { - d := Directive{LookingForDirectives: true} - SetEscapeToken(DefaultEscapeToken, &d) - words := parseWords(test["input"][0], &d) + words := parseWords(test["input"][0], NewDefaultDirective()) if len(words) != len(test["expect"]) { t.Fatalf("length check failed. input: %v, expect: %q, output: %q", test["input"][0], test["expect"], words) } @@ -143,9 +137,7 @@ func TestLineInformation(t *testing.T) { } defer df.Close() - d := Directive{LookingForDirectives: true} - SetEscapeToken(DefaultEscapeToken, &d) - ast, err := Parse(df, &d) + ast, err := Parse(df, NewDefaultDirective()) if err != nil { t.Fatalf("Error parsing dockerfile %s: %v", testFileLineInfo, err) } From 64c4c1c3d5e0fe364d83db5a8dc99a24fd121754 Mon Sep 17 00:00:00 2001 From: Daniel Nephin Date: Wed, 12 Apr 2017 13:47:19 -0400 Subject: [PATCH 2/4] Keep parser.Directive internal to parser Signed-off-by: Daniel Nephin --- builder/dockerfile/builder.go | 31 ++++++---- builder/dockerfile/builder_test.go | 4 +- builder/dockerfile/dispatchers.go | 2 +- builder/dockerfile/dispatchers_test.go | 2 - builder/dockerfile/evaluator.go | 2 +- builder/dockerfile/evaluator_test.go | 4 +- builder/dockerfile/internals.go | 18 +++--- builder/dockerfile/parser/dumper/main.go | 7 +-- builder/dockerfile/parser/json_test.go | 10 ++-- builder/dockerfile/parser/parser.go | 36 ++++++------ builder/dockerfile/parser/parser_test.go | 75 +++++++----------------- 11 files changed, 82 insertions(+), 109 deletions(-) diff --git a/builder/dockerfile/builder.go b/builder/dockerfile/builder.go index d02b3cd7a4..744f503dd3 100644 --- a/builder/dockerfile/builder.go +++ b/builder/dockerfile/builder.go @@ -62,7 +62,7 @@ type Builder struct { disableCommit bool cacheBusted bool buildArgs *buildArgs - directive *parser.Directive + escapeToken rune imageCache builder.ImageCache from builder.Image @@ -122,7 +122,7 @@ func NewBuilder(clientCtx context.Context, config *types.ImageBuildOptions, back runConfig: new(container.Config), tmpContainers: map[string]struct{}{}, buildArgs: newBuildArgs(config.BuildArgs), - directive: parser.NewDefaultDirective(), + escapeToken: parser.DefaultEscapeToken, } b.imageContexts = &imageContexts{b: b} return b, nil @@ -190,9 +190,9 @@ func (b *Builder) build(stdout io.Writer, stderr io.Writer, out io.Writer) (stri return "", err } - addNodesForLabelOption(dockerfile, b.options.Labels) + addNodesForLabelOption(dockerfile.AST, b.options.Labels) - if err := checkDispatchDockerfile(dockerfile); err != nil { + if err := checkDispatchDockerfile(dockerfile.AST); err != nil { return "", err } @@ -220,10 +220,13 @@ func (b *Builder) build(stdout io.Writer, stderr io.Writer, out io.Writer) (stri return b.image, nil } -func (b *Builder) dispatchDockerfileWithCancellation(dockerfile *parser.Node) (string, error) { - total := len(dockerfile.Children) +func (b *Builder) dispatchDockerfileWithCancellation(dockerfile *parser.Result) (string, error) { + // TODO: pass this to dispatchRequest instead + b.escapeToken = dockerfile.EscapeToken + + total := len(dockerfile.AST.Children) var shortImgID string - for i, n := range dockerfile.Children { + for i, n := range dockerfile.AST.Children { select { case <-b.clientCtx.Done(): logrus.Debug("Builder: build cancelled!") @@ -320,13 +323,13 @@ func BuildFromConfig(config *container.Config, changes []string) (*container.Con return nil, err } - ast, err := parser.Parse(bytes.NewBufferString(strings.Join(changes, "\n")), b.directive) + result, err := parser.Parse(bytes.NewBufferString(strings.Join(changes, "\n"))) if err != nil { return nil, err } // ensure that the commands are valid - for _, n := range ast.Children { + for _, n := range result.AST.Children { if !validCommitCommands[n.Value] { return nil, fmt.Errorf("%s is not a valid change command", n.Value) } @@ -337,11 +340,11 @@ func BuildFromConfig(config *container.Config, changes []string) (*container.Con b.Stderr = ioutil.Discard b.disableCommit = true - if err := checkDispatchDockerfile(ast); err != nil { + if err := checkDispatchDockerfile(result.AST); err != nil { return nil, err } - if err := dispatchFromDockerfile(b, ast); err != nil { + if err := dispatchFromDockerfile(b, result); err != nil { return nil, err } return b.runConfig, nil @@ -356,8 +359,12 @@ func checkDispatchDockerfile(dockerfile *parser.Node) error { return nil } -func dispatchFromDockerfile(b *Builder, ast *parser.Node) error { +func dispatchFromDockerfile(b *Builder, result *parser.Result) error { + // TODO: pass this to dispatchRequest instead + b.escapeToken = result.EscapeToken + ast := result.AST total := len(ast.Children) + for i, n := range ast.Children { if err := b.dispatch(i, total, n); err != nil { return err diff --git a/builder/dockerfile/builder_test.go b/builder/dockerfile/builder_test.go index cd79af1dfc..5c2fc326fe 100644 --- a/builder/dockerfile/builder_test.go +++ b/builder/dockerfile/builder_test.go @@ -10,8 +10,7 @@ import ( func TestAddNodesForLabelOption(t *testing.T) { dockerfile := "FROM scratch" - d := parser.NewDefaultDirective() - nodes, err := parser.Parse(strings.NewReader(dockerfile), d) + result, err := parser.Parse(strings.NewReader(dockerfile)) assert.NilError(t, err) labels := map[string]string{ @@ -21,6 +20,7 @@ func TestAddNodesForLabelOption(t *testing.T) { "org.b": "cli-b", "org.a": "cli-a", } + nodes := result.AST addNodesForLabelOption(nodes, labels) expected := []string{ diff --git a/builder/dockerfile/dispatchers.go b/builder/dockerfile/dispatchers.go index 5a467e6285..3df8070b93 100644 --- a/builder/dockerfile/dispatchers.go +++ b/builder/dockerfile/dispatchers.go @@ -200,7 +200,7 @@ func from(b *Builder, args []string, attributes map[string]bool, original string substituionArgs = append(substituionArgs, key+"="+value) } - name, err := ProcessWord(args[0], substituionArgs, b.directive.EscapeToken()) + name, err := ProcessWord(args[0], substituionArgs, b.escapeToken) if err != nil { return err } diff --git a/builder/dockerfile/dispatchers_test.go b/builder/dockerfile/dispatchers_test.go index d721637d13..644b610825 100644 --- a/builder/dockerfile/dispatchers_test.go +++ b/builder/dockerfile/dispatchers_test.go @@ -10,7 +10,6 @@ import ( "github.com/docker/docker/api/types/container" "github.com/docker/docker/api/types/strslice" "github.com/docker/docker/builder" - "github.com/docker/docker/builder/dockerfile/parser" "github.com/docker/docker/pkg/testutil/assert" "github.com/docker/go-connections/nat" ) @@ -205,7 +204,6 @@ func newBuilderWithMockBackend() *Builder { options: &types.ImageBuildOptions{}, docker: &MockBackend{}, buildArgs: newBuildArgs(make(map[string]*string)), - directive: parser.NewDefaultDirective(), } b.imageContexts = &imageContexts{b: b} return b diff --git a/builder/dockerfile/evaluator.go b/builder/dockerfile/evaluator.go index df50ee561d..472e9ae09a 100644 --- a/builder/dockerfile/evaluator.go +++ b/builder/dockerfile/evaluator.go @@ -180,7 +180,7 @@ func (b *Builder) evaluateEnv(cmd string, str string, envs []string) ([]string, return []string{word}, err } } - return processFunc(str, envs, b.directive.EscapeToken()) + return processFunc(str, envs, b.escapeToken) } // buildArgsWithoutConfigEnv returns a list of key=value pairs for all the build diff --git a/builder/dockerfile/evaluator_test.go b/builder/dockerfile/evaluator_test.go index adc72d3334..419fa0f9ff 100644 --- a/builder/dockerfile/evaluator_test.go +++ b/builder/dockerfile/evaluator_test.go @@ -171,7 +171,7 @@ func executeTestCase(t *testing.T, testCase dispatchTestCase) { }() r := strings.NewReader(testCase.dockerfile) - n, err := parser.Parse(r, parser.NewDefaultDirective()) + result, err := parser.Parse(r) if err != nil { t.Fatalf("Error when parsing Dockerfile: %s", err) @@ -188,9 +188,9 @@ func executeTestCase(t *testing.T, testCase dispatchTestCase) { Stdout: ioutil.Discard, context: context, buildArgs: newBuildArgs(options.BuildArgs), - directive: parser.NewDefaultDirective(), } + n := result.AST err = b.dispatch(0, len(n.Children), n.Children[0]) if err == nil { diff --git a/builder/dockerfile/internals.go b/builder/dockerfile/internals.go index 5f9b9b0648..e4e59e87f8 100644 --- a/builder/dockerfile/internals.go +++ b/builder/dockerfile/internals.go @@ -462,12 +462,12 @@ func (b *Builder) processImageFrom(img builder.Image) error { // parse the ONBUILD triggers by invoking the parser for _, step := range onBuildTriggers { - ast, err := parser.Parse(strings.NewReader(step), b.directive) + result, err := parser.Parse(strings.NewReader(step)) if err != nil { return err } - for _, n := range ast.Children { + for _, n := range result.AST.Children { if err := checkDispatch(n); err != nil { return err } @@ -481,7 +481,7 @@ func (b *Builder) processImageFrom(img builder.Image) error { } } - if err := dispatchFromDockerfile(b, ast); err != nil { + if err := dispatchFromDockerfile(b, result); err != nil { return err } } @@ -650,7 +650,7 @@ func (b *Builder) clearTmp() { } // readAndParseDockerfile reads a Dockerfile from the current context. -func (b *Builder) readAndParseDockerfile() (*parser.Node, error) { +func (b *Builder) readAndParseDockerfile() (*parser.Result, error) { // If no -f was specified then look for 'Dockerfile'. If we can't find // that then look for 'dockerfile'. If neither are found then default // back to 'Dockerfile' and use that in the error message. @@ -664,9 +664,9 @@ func (b *Builder) readAndParseDockerfile() (*parser.Node, error) { } } - nodes, err := b.parseDockerfile() + result, err := b.parseDockerfile() if err != nil { - return nodes, err + return nil, err } // After the Dockerfile has been parsed, we need to check the .dockerignore @@ -680,10 +680,10 @@ func (b *Builder) readAndParseDockerfile() (*parser.Node, error) { if dockerIgnore, ok := b.context.(builder.DockerIgnoreContext); ok { dockerIgnore.Process([]string{b.options.Dockerfile}) } - return nodes, nil + return result, nil } -func (b *Builder) parseDockerfile() (*parser.Node, error) { +func (b *Builder) parseDockerfile() (*parser.Result, error) { f, err := b.context.Open(b.options.Dockerfile) if err != nil { if os.IsNotExist(err) { @@ -702,5 +702,5 @@ func (b *Builder) parseDockerfile() (*parser.Node, error) { return nil, fmt.Errorf("The Dockerfile (%s) cannot be empty", b.options.Dockerfile) } } - return parser.Parse(f, b.directive) + return parser.Parse(f) } diff --git a/builder/dockerfile/parser/dumper/main.go b/builder/dockerfile/parser/dumper/main.go index 5e9af94121..0d98b49270 100644 --- a/builder/dockerfile/parser/dumper/main.go +++ b/builder/dockerfile/parser/dumper/main.go @@ -5,6 +5,7 @@ import ( "os" "github.com/docker/docker/builder/dockerfile/parser" + "go/ast" ) func main() { @@ -23,12 +24,10 @@ func main() { } defer f.Close() - d := parser.NewDefaultDirective() - ast, err := parser.Parse(f, d) + result, err := parser.Parse(f) if err != nil { panic(err) - } else { - fmt.Println(ast.Dump()) } + fmt.Println(result.AST.Dump()) } } diff --git a/builder/dockerfile/parser/json_test.go b/builder/dockerfile/parser/json_test.go index 26eea97064..d4489191da 100644 --- a/builder/dockerfile/parser/json_test.go +++ b/builder/dockerfile/parser/json_test.go @@ -28,10 +28,9 @@ var validJSONArraysOfStrings = map[string][]string{ func TestJSONArraysOfStrings(t *testing.T) { for json, expected := range validJSONArraysOfStrings { - d := Directive{} - d.SetEscapeToken(DefaultEscapeToken) + d := NewDefaultDirective() - if node, _, err := parseJSON(json, &d); err != nil { + if node, _, err := parseJSON(json, d); err != nil { t.Fatalf("%q should be a valid JSON array of strings, but wasn't! (err: %q)", json, err) } else { i := 0 @@ -51,10 +50,9 @@ func TestJSONArraysOfStrings(t *testing.T) { } } for _, json := range invalidJSONArraysOfStrings { - d := Directive{} - d.SetEscapeToken(DefaultEscapeToken) + d := NewDefaultDirective() - if _, _, err := parseJSON(json, &d); err != errDockerfileNotStringArray { + if _, _, err := parseJSON(json, d); err != errDockerfileNotStringArray { t.Fatalf("%q should be an invalid JSON array of strings, but wasn't!", json) } } diff --git a/builder/dockerfile/parser/parser.go b/builder/dockerfile/parser/parser.go index e4b3aa61e6..5864eb9c6c 100644 --- a/builder/dockerfile/parser/parser.go +++ b/builder/dockerfile/parser/parser.go @@ -34,7 +34,7 @@ type Node struct { Original string // original line used before parsing Flags []string // only top Node should have this set StartLine int // the line in the original dockerfile where the node begins - EndLine int // the line in the original dockerfile where the node ends + endLine int // the line in the original dockerfile where the node ends } // Dump dumps the AST defined by `node` as a list of sexps. @@ -70,7 +70,7 @@ var ( ) // DefaultEscapeToken is the default escape token -const DefaultEscapeToken = "\\" +const DefaultEscapeToken = '\\' // Directive is the structure used during a build run to hold the state of // parsing directives. @@ -81,8 +81,8 @@ type Directive struct { escapeSeen bool // Whether the escape directive has been seen } -// SetEscapeToken sets the default token for escaping characters in a Dockerfile. -func (d *Directive) SetEscapeToken(s string) error { +// setEscapeToken sets the default token for escaping characters in a Dockerfile. +func (d *Directive) setEscapeToken(s string) error { if s != "`" && s != "\\" { return fmt.Errorf("invalid ESCAPE '%s'. Must be ` or \\", s) } @@ -91,18 +91,13 @@ func (d *Directive) SetEscapeToken(s string) error { return nil } -// EscapeToken returns the escape token -func (d *Directive) EscapeToken() rune { - return d.escapeToken -} - // NewDefaultDirective returns a new Directive with the default escapeToken token func NewDefaultDirective() *Directive { directive := Directive{ escapeSeen: false, lookingForDirectives: true, } - directive.SetEscapeToken(DefaultEscapeToken) + directive.setEscapeToken(string(DefaultEscapeToken)) return &directive } @@ -200,7 +195,7 @@ func handleParserDirective(line string, d *Directive) (bool, error) { } for i, n := range tokenEscapeCommand.SubexpNames() { if n == "escapechar" { - if err := d.SetEscapeToken(tecMatch[i]); err != nil { + if err := d.setEscapeToken(tecMatch[i]); err != nil { return false, err } return true, nil @@ -209,9 +204,16 @@ func handleParserDirective(line string, d *Directive) (bool, error) { return false, nil } -// Parse is the main parse routine. -// It handles an io.ReadWriteCloser and returns the root of the AST. -func Parse(rwc io.Reader, d *Directive) (*Node, error) { +// Result is the result of parsing a Dockerfile +type Result struct { + AST *Node + EscapeToken rune +} + +// Parse reads lines from a Reader, parses the lines into an AST and returns +// the AST and escape token +func Parse(rwc io.Reader) (*Result, error) { + d := NewDefaultDirective() currentLine := 0 root := &Node{} root.StartLine = -1 @@ -267,18 +269,18 @@ func Parse(rwc io.Reader, d *Directive) (*Node, error) { if child != nil { // Update the line information for the current child. child.StartLine = startLine - child.EndLine = currentLine + child.endLine = currentLine // Update the line information for the root. The starting line of the root is always the // starting line of the first child and the ending line is the ending line of the last child. if root.StartLine < 0 { root.StartLine = currentLine } - root.EndLine = currentLine + root.endLine = currentLine root.Children = append(root.Children, child) } } - return root, nil + return &Result{AST: root, EscapeToken: d.escapeToken}, nil } // covers comments and empty lines. Lines should be trimmed before passing to diff --git a/builder/dockerfile/parser/parser_test.go b/builder/dockerfile/parser/parser_test.go index 82e78e91a9..af54e097bf 100644 --- a/builder/dockerfile/parser/parser_test.go +++ b/builder/dockerfile/parser/parser_test.go @@ -8,6 +8,8 @@ import ( "path/filepath" "runtime" "testing" + + "github.com/docker/docker/pkg/testutil/assert" ) const testDir = "testfiles" @@ -16,17 +18,11 @@ const testFileLineInfo = "testfile-line/Dockerfile" func getDirs(t *testing.T, dir string) []string { f, err := os.Open(dir) - if err != nil { - t.Fatal(err) - } - + assert.NilError(t, err) defer f.Close() dirs, err := f.Readdirnames(0) - if err != nil { - t.Fatal(err) - } - + assert.NilError(t, err) return dirs } @@ -35,15 +31,11 @@ func TestTestNegative(t *testing.T) { dockerfile := filepath.Join(negativeTestDir, dir, "Dockerfile") df, err := os.Open(dockerfile) - if err != nil { - t.Fatalf("Dockerfile missing for %s: %v", dir, err) - } + assert.NilError(t, err) defer df.Close() - _, err = Parse(df, NewDefaultDirective()) - if err == nil { - t.Fatalf("No error parsing broken dockerfile for %s", dir) - } + _, err = Parse(df) + assert.Error(t, err, "") } } @@ -53,31 +45,21 @@ func TestTestData(t *testing.T) { resultfile := filepath.Join(testDir, dir, "result") df, err := os.Open(dockerfile) - if err != nil { - t.Fatalf("Dockerfile missing for %s: %v", dir, err) - } + assert.NilError(t, err) defer df.Close() - ast, err := Parse(df, NewDefaultDirective()) - if err != nil { - t.Fatalf("Error parsing %s's dockerfile: %v", dir, err) - } + result, err := Parse(df) + assert.NilError(t, err) content, err := ioutil.ReadFile(resultfile) - if err != nil { - t.Fatalf("Error reading %s's result file: %v", dir, err) - } + assert.NilError(t, err) if runtime.GOOS == "windows" { // CRLF --> CR to match Unix behavior content = bytes.Replace(content, []byte{'\x0d', '\x0a'}, []byte{'\x0a'}, -1) } - if ast.Dump()+"\n" != string(content) { - fmt.Fprintln(os.Stderr, "Result:\n"+ast.Dump()) - fmt.Fprintln(os.Stderr, "Expected:\n"+string(content)) - t.Fatalf("%s: AST dump of dockerfile does not match result", dir) - } + assert.Equal(t, result.AST.Dump()+"\n", string(content)) } } @@ -119,46 +101,33 @@ func TestParseWords(t *testing.T) { for _, test := range tests { words := parseWords(test["input"][0], NewDefaultDirective()) - if len(words) != len(test["expect"]) { - t.Fatalf("length check failed. input: %v, expect: %q, output: %q", test["input"][0], test["expect"], words) - } - for i, word := range words { - if word != test["expect"][i] { - t.Fatalf("word check failed for word: %q. input: %q, expect: %q, output: %q", word, test["input"][0], test["expect"], words) - } - } + assert.DeepEqual(t, words, test["expect"]) } } func TestLineInformation(t *testing.T) { df, err := os.Open(testFileLineInfo) - if err != nil { - t.Fatalf("Dockerfile missing for %s: %v", testFileLineInfo, err) - } + assert.NilError(t, err) defer df.Close() - ast, err := Parse(df, NewDefaultDirective()) - if err != nil { - t.Fatalf("Error parsing dockerfile %s: %v", testFileLineInfo, err) - } + result, err := Parse(df) + assert.NilError(t, err) - 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) + ast := result.AST + 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.Fatal("Root line information doesn't match result.") } - if len(ast.Children) != 3 { - fmt.Fprintf(os.Stderr, "Wrong number of child: expected(%d), actual(%d)\n", 3, len(ast.Children)) - t.Fatalf("Root line information doesn't match result for %s", testFileLineInfo) - } + assert.Equal(t, len(ast.Children), 3) expected := [][]int{ {5, 5}, {11, 12}, {17, 31}, } for i, child := range ast.Children { - if child.StartLine != expected[i][0] || child.EndLine != expected[i][1] { + if child.StartLine != expected[i][0] || child.endLine != expected[i][1] { t.Logf("Wrong line information for child %d: expected(%d-%d), actual(%d-%d)\n", - i, expected[i][0], expected[i][1], child.StartLine, child.EndLine) + i, expected[i][0], expected[i][1], child.StartLine, child.endLine) t.Fatal("Root line information doesn't match result.") } } From 9c53fa2d0c1796d32aaba2193e7d802cd9b66763 Mon Sep 17 00:00:00 2001 From: Daniel Nephin Date: Wed, 12 Apr 2017 15:40:16 -0400 Subject: [PATCH 3/4] Cleanup processing of directives. Signed-off-by: Daniel Nephin --- builder/dockerfile/parser/dumper/main.go | 1 - builder/dockerfile/parser/parser.go | 75 +++++++++++++----------- builder/dockerfile/parser/parser_test.go | 2 +- 3 files changed, 41 insertions(+), 37 deletions(-) diff --git a/builder/dockerfile/parser/dumper/main.go b/builder/dockerfile/parser/dumper/main.go index 0d98b49270..ea6205073d 100644 --- a/builder/dockerfile/parser/dumper/main.go +++ b/builder/dockerfile/parser/dumper/main.go @@ -5,7 +5,6 @@ import ( "os" "github.com/docker/docker/builder/dockerfile/parser" - "go/ast" ) func main() { diff --git a/builder/dockerfile/parser/parser.go b/builder/dockerfile/parser/parser.go index 5864eb9c6c..1c2b9db2aa 100644 --- a/builder/dockerfile/parser/parser.go +++ b/builder/dockerfile/parser/parser.go @@ -12,6 +12,7 @@ import ( "unicode" "github.com/docker/docker/builder/dockerfile/command" + "github.com/pkg/errors" ) // Node is a structure used to represent a parse tree. @@ -77,7 +78,7 @@ const DefaultEscapeToken = '\\' type Directive struct { escapeToken rune // Current escape token lineContinuationRegex *regexp.Regexp // Current line continuation regex - lookingForDirectives bool // Whether we are currently looking for directives + processingComplete bool // Whether we are done looking for directives escapeSeen bool // Whether the escape directive has been seen } @@ -91,12 +92,35 @@ func (d *Directive) setEscapeToken(s string) error { return nil } +// processLine looks for a parser directive '# escapeToken=. Parser +// directives must precede any builder instruction or other comments, and cannot +// be repeated. +func (d *Directive) processLine(line string) error { + if d.processingComplete { + return nil + } + // Processing is finished after the first call + defer func() { d.processingComplete = true }() + + tecMatch := tokenEscapeCommand.FindStringSubmatch(strings.ToLower(line)) + if len(tecMatch) == 0 { + return nil + } + if d.escapeSeen == true { + return errors.New("only one escape parser directive can be used") + } + for i, n := range tokenEscapeCommand.SubexpNames() { + if n == "escapechar" { + d.escapeSeen = true + return d.setEscapeToken(tecMatch[i]) + } + } + return nil +} + // NewDefaultDirective returns a new Directive with the default escapeToken token func NewDefaultDirective() *Directive { - directive := Directive{ - escapeSeen: false, - lookingForDirectives: true, - } + directive := Directive{} directive.setEscapeToken(string(DefaultEscapeToken)) return &directive } @@ -132,13 +156,10 @@ func init() { // ParseLine parses a line and returns the remainder. func ParseLine(line string, d *Directive, ignoreCont bool) (string, *Node, error) { - if escapeFound, err := handleParserDirective(line, d); err != nil || escapeFound { - d.escapeSeen = escapeFound + if err := d.processLine(line); err != nil { return "", nil, err } - d.lookingForDirectives = false - if line = stripComments(line); line == "" { return "", nil, nil } @@ -180,44 +201,27 @@ func newNodeFromLine(line string, directive *Directive) (*Node, error) { }, nil } -// Handle the parser directive '# escapeToken=. Parser directives must precede -// any builder instruction or other comments, and cannot be repeated. -func handleParserDirective(line string, d *Directive) (bool, error) { - if !d.lookingForDirectives { - return false, nil - } - tecMatch := tokenEscapeCommand.FindStringSubmatch(strings.ToLower(line)) - if len(tecMatch) == 0 { - return false, nil - } - if d.escapeSeen == true { - return false, fmt.Errorf("only one escape parser directive can be used") - } - for i, n := range tokenEscapeCommand.SubexpNames() { - if n == "escapechar" { - if err := d.setEscapeToken(tecMatch[i]); err != nil { - return false, err - } - return true, nil - } - } - return false, nil -} - // Result is the result of parsing a Dockerfile type Result struct { AST *Node EscapeToken rune } +// scanLines is a split function for bufio.Scanner. that augments the default +// line scanner by supporting newline escapes. +func scanLines(data []byte, atEOF bool) (int, []byte, error) { + advance, token, err := bufio.ScanLines(data, atEOF) + return advance, token, err +} + // Parse reads lines from a Reader, parses the lines into an AST and returns // the AST and escape token func Parse(rwc io.Reader) (*Result, error) { d := NewDefaultDirective() currentLine := 0 - root := &Node{} - root.StartLine = -1 + root := &Node{StartLine: -1} scanner := bufio.NewScanner(rwc) + scanner.Split(scanLines) utf8bom := []byte{0xEF, 0xBB, 0xBF} for scanner.Scan() { @@ -226,6 +230,7 @@ func Parse(rwc io.Reader) (*Result, error) { if currentLine == 0 { scannedBytes = bytes.TrimPrefix(scannedBytes, utf8bom) } + scannedLine := strings.TrimLeftFunc(string(scannedBytes), unicode.IsSpace) currentLine++ line, child, err := ParseLine(scannedLine, d, false) diff --git a/builder/dockerfile/parser/parser_test.go b/builder/dockerfile/parser/parser_test.go index af54e097bf..7f273002c7 100644 --- a/builder/dockerfile/parser/parser_test.go +++ b/builder/dockerfile/parser/parser_test.go @@ -59,7 +59,7 @@ func TestTestData(t *testing.T) { content = bytes.Replace(content, []byte{'\x0d', '\x0a'}, []byte{'\x0a'}, -1) } - assert.Equal(t, result.AST.Dump()+"\n", string(content)) + assert.Equal(t, result.AST.Dump()+"\n", string(content), "In "+dockerfile) } } From 4cbc953a5d80c850df7107b28e743e933bbeb1d3 Mon Sep 17 00:00:00 2001 From: Daniel Nephin Date: Wed, 12 Apr 2017 18:00:55 -0400 Subject: [PATCH 4/4] Refactor dockerfile parser Signed-off-by: Daniel Nephin --- builder/dockerfile/parser/line_parsers.go | 2 +- builder/dockerfile/parser/parser.go | 174 +++++++++--------- .../testfiles/continue-at-eof/Dockerfile | 3 + .../parser/testfiles/continue-at-eof/result | 2 + 4 files changed, 90 insertions(+), 91 deletions(-) create mode 100644 builder/dockerfile/parser/testfiles/continue-at-eof/Dockerfile create mode 100644 builder/dockerfile/parser/testfiles/continue-at-eof/result diff --git a/builder/dockerfile/parser/line_parsers.go b/builder/dockerfile/parser/line_parsers.go index 73cb39a348..d0e182e8e3 100644 --- a/builder/dockerfile/parser/line_parsers.go +++ b/builder/dockerfile/parser/line_parsers.go @@ -42,7 +42,7 @@ func parseSubCommand(rest string, d *Directive) (*Node, map[string]bool, error) return nil, nil, nil } - _, child, err := ParseLine(rest, d, false) + child, err := newNodeFromLine(rest, d) if err != nil { return nil, nil, err } diff --git a/builder/dockerfile/parser/parser.go b/builder/dockerfile/parser/parser.go index 1c2b9db2aa..0729fa539d 100644 --- a/builder/dockerfile/parser/parser.go +++ b/builder/dockerfile/parser/parser.go @@ -63,6 +63,21 @@ func (node *Node) Dump() string { return strings.TrimSpace(str) } +func (node *Node) lines(start, end int) { + node.StartLine = start + node.endLine = end +} + +// AddChild adds a new child node, and updates line information +func (node *Node) AddChild(child *Node, startLine, endLine int) { + child.lines(startLine, endLine) + if node.StartLine < 0 { + node.StartLine = startLine + } + node.endLine = endLine + node.Children = append(node.Children, child) +} + var ( dispatch map[string]func(string, *Directive) (*Node, map[string]bool, error) tokenWhitespace = regexp.MustCompile(`[\t\v\f\r ]+`) @@ -154,25 +169,6 @@ func init() { } } -// ParseLine parses a line and returns the remainder. -func ParseLine(line string, d *Directive, ignoreCont bool) (string, *Node, error) { - if err := d.processLine(line); err != nil { - return "", nil, err - } - - if line = stripComments(line); line == "" { - return "", nil, nil - } - - if !ignoreCont && d.lineContinuationRegex.MatchString(line) { - line = d.lineContinuationRegex.ReplaceAllString(line, "") - return line, nil, nil - } - - node, err := newNodeFromLine(line, d) - return "", node, err -} - // newNodeFromLine splits the line into parts, and dispatches to a function // based on the command and command arguments. A Node is created from the // result of the dispatch. @@ -207,13 +203,6 @@ type Result struct { EscapeToken rune } -// scanLines is a split function for bufio.Scanner. that augments the default -// line scanner by supporting newline escapes. -func scanLines(data []byte, atEOF bool) (int, []byte, error) { - advance, token, err := bufio.ScanLines(data, atEOF) - return advance, token, err -} - // Parse reads lines from a Reader, parses the lines into an AST and returns // the AST and escape token func Parse(rwc io.Reader) (*Result, error) { @@ -221,80 +210,85 @@ func Parse(rwc io.Reader) (*Result, error) { currentLine := 0 root := &Node{StartLine: -1} scanner := bufio.NewScanner(rwc) - scanner.Split(scanLines) - utf8bom := []byte{0xEF, 0xBB, 0xBF} + var err error for scanner.Scan() { - scannedBytes := scanner.Bytes() - // We trim UTF8 BOM - if currentLine == 0 { - scannedBytes = bytes.TrimPrefix(scannedBytes, utf8bom) + bytes := scanner.Bytes() + switch currentLine { + case 0: + bytes, err = processFirstLine(d, bytes) + if err != nil { + return nil, err + } + default: + bytes = processLine(bytes, true) + } + currentLine++ + + startLine := currentLine + line, isEndOfLine := trimContinuationCharacter(string(bytes), d) + if isEndOfLine && line == "" { + continue } - scannedLine := strings.TrimLeftFunc(string(scannedBytes), unicode.IsSpace) - currentLine++ - line, child, err := ParseLine(scannedLine, d, false) + for !isEndOfLine && scanner.Scan() { + bytes := processLine(scanner.Bytes(), false) + currentLine++ + + // TODO: warn this is being deprecated/removed + if isEmptyContinuationLine(bytes) { + continue + } + + continuationLine := string(bytes) + continuationLine, isEndOfLine = trimContinuationCharacter(continuationLine, d) + line += continuationLine + } + + child, err := newNodeFromLine(line, d) if err != nil { return nil, err } - startLine := currentLine - - if line != "" && child == nil { - for scanner.Scan() { - newline := scanner.Text() - currentLine++ - - if stripComments(strings.TrimSpace(newline)) == "" { - continue - } - - line, child, err = ParseLine(line+newline, d, false) - if err != nil { - return nil, err - } - - if child != nil { - break - } - } - if child == nil && line != "" { - // When we call ParseLine we'll pass in 'true' for - // the ignoreCont param if we're at the EOF. This will - // prevent the func from returning immediately w/o - // parsing the line thinking that there's more input - // to come. - - _, child, err = ParseLine(line, d, scanner.Err() == nil) - if err != nil { - return nil, err - } - } - } - - if child != nil { - // Update the line information for the current child. - child.StartLine = startLine - child.endLine = currentLine - // Update the line information for the root. The starting line of the root is always the - // starting line of the first child and the ending line is the ending line of the last child. - if root.StartLine < 0 { - root.StartLine = currentLine - } - root.endLine = currentLine - root.Children = append(root.Children, child) - } + root.AddChild(child, startLine, currentLine) } return &Result{AST: root, EscapeToken: d.escapeToken}, nil } -// covers comments and empty lines. Lines should be trimmed before passing to -// this function. -func stripComments(line string) string { - // string is already trimmed at this point - if tokenComment.MatchString(line) { - return tokenComment.ReplaceAllString(line, "") - } - - return line +func trimComments(src []byte) []byte { + return tokenComment.ReplaceAll(src, []byte{}) +} + +func trimWhitespace(src []byte) []byte { + return bytes.TrimLeftFunc(src, unicode.IsSpace) +} + +func isEmptyContinuationLine(line []byte) bool { + return len(trimComments(trimWhitespace(line))) == 0 +} + +var utf8bom = []byte{0xEF, 0xBB, 0xBF} + +func trimContinuationCharacter(line string, d *Directive) (string, bool) { + if d.lineContinuationRegex.MatchString(line) { + line = d.lineContinuationRegex.ReplaceAllString(line, "") + return line, false + } + return line, true +} + +// TODO: remove stripLeftWhitespace after deprecation period. It seems silly +// to preserve whitespace on continuation lines. Why is that done? +func processLine(token []byte, stripLeftWhitespace bool) []byte { + if stripLeftWhitespace { + token = trimWhitespace(token) + } + return trimComments(token) +} + +func processFirstLine(d *Directive, token []byte) ([]byte, error) { + token = bytes.TrimPrefix(token, utf8bom) + token = trimWhitespace(token) + err := d.processLine(string(token)) + return trimComments(token), err } diff --git a/builder/dockerfile/parser/testfiles/continue-at-eof/Dockerfile b/builder/dockerfile/parser/testfiles/continue-at-eof/Dockerfile new file mode 100644 index 0000000000..a8ec369ad1 --- /dev/null +++ b/builder/dockerfile/parser/testfiles/continue-at-eof/Dockerfile @@ -0,0 +1,3 @@ +FROM alpine:3.5 + +RUN something \ \ No newline at end of file diff --git a/builder/dockerfile/parser/testfiles/continue-at-eof/result b/builder/dockerfile/parser/testfiles/continue-at-eof/result new file mode 100644 index 0000000000..14e4f09321 --- /dev/null +++ b/builder/dockerfile/parser/testfiles/continue-at-eof/result @@ -0,0 +1,2 @@ +(from "alpine:3.5") +(run "something")