From bb429da9a977a9d89121052fe1143528294b8a0c Mon Sep 17 00:00:00 2001 From: Daniel Nephin Date: Tue, 11 Apr 2017 15:07:02 -0400 Subject: [PATCH] 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) }