Skip to content

Commit 8d1ae76

Browse files
thaJeztahthaJeztah
authored
Merge pull request moby#33719 from dnephin/warn-on-empty-continuation-carry
[Builder] Warn on empty continuation lines
2 parents d75eb73 + b47b375 commit 8d1ae76

4 files changed

Lines changed: 72 additions & 32 deletions

File tree

‎builder/dockerfile/builder.go‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -255,6 +255,7 @@ func (b *Builder) build(source builder.Source, dockerfile *parser.Result) (*buil
255255
return nil, errors.Errorf("failed to reach build target %s in Dockerfile", b.options.Target)
256256
}
257257

258+
dockerfile.PrintWarnings(b.Stderr)
258259
b.buildArgs.WarnOnUnusedBuildArgs(b.Stderr)
259260

260261
if dispatchState.imageID == "" {

‎builder/dockerfile/parser/parser.go‎

Lines changed: 28 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -243,6 +243,15 @@ type Result struct {
243243
AST *Node
244244
EscapeToken rune
245245
Platform string
246+
Warnings []string
247+
}
248+
249+
// PrintWarnings to the writer
250+
func (r *Result) PrintWarnings(out io.Writer) {
251+
if len(r.Warnings) == 0 {
252+
return
253+
}
254+
fmt.Fprintf(out, strings.Join(r.Warnings, "\n")+"\n")
246255
}
247256

248257
// Parse reads lines from a Reader, parses the lines into an AST and returns
@@ -252,6 +261,7 @@ func Parse(rwc io.Reader) (*Result, error) {
252261
currentLine := 0
253262
root := &Node{StartLine: -1}
254263
scanner := bufio.NewScanner(rwc)
264+
warnings := []string{}
255265

256266
var err error
257267
for scanner.Scan() {
@@ -272,15 +282,16 @@ func Parse(rwc io.Reader) (*Result, error) {
272282
continue
273283
}
274284

285+
var hasEmptyContinuationLine bool
275286
for !isEndOfLine && scanner.Scan() {
276287
bytesRead, err := processLine(d, scanner.Bytes(), false)
277288
if err != nil {
278289
return nil, err
279290
}
280291
currentLine++
281292

282-
// TODO: warn this is being deprecated/removed
283293
if isEmptyContinuationLine(bytesRead) {
294+
hasEmptyContinuationLine = true
284295
continue
285296
}
286297

@@ -289,13 +300,27 @@ func Parse(rwc io.Reader) (*Result, error) {
289300
line += continuationLine
290301
}
291302

303+
if hasEmptyContinuationLine {
304+
warning := "[WARNING]: Empty continuation line found in:\n " + line
305+
warnings = append(warnings, warning)
306+
}
307+
292308
child, err := newNodeFromLine(line, d)
293309
if err != nil {
294310
return nil, err
295311
}
296312
root.AddChild(child, startLine, currentLine)
297313
}
298-
return &Result{AST: root, EscapeToken: d.escapeToken, Platform: d.platformToken}, nil
314+
315+
if len(warnings) > 0 {
316+
warnings = append(warnings, "[WARNING]: Empty continuation lines will become errors in a future release.")
317+
}
318+
return &Result{
319+
AST: root,
320+
Warnings: warnings,
321+
EscapeToken: d.escapeToken,
322+
Platform: d.platformToken,
323+
}, nil
299324
}
300325

301326
func trimComments(src []byte) []byte {
@@ -326,6 +351,5 @@ func processLine(d *Directive, token []byte, stripLeftWhitespace bool) ([]byte,
326351
if stripLeftWhitespace {
327352
token = trimWhitespace(token)
328353
}
329-
err := d.possibleParserDirective(string(token))
330-
return trimComments(token), err
354+
return trimComments(token), d.possibleParserDirective(string(token))
331355
}

‎builder/dockerfile/parser/parser_test.go‎

Lines changed: 38 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -27,40 +27,39 @@ func getDirs(t *testing.T, dir string) []string {
2727
return dirs
2828
}
2929

30-
func TestTestNegative(t *testing.T) {
30+
func TestParseErrorCases(t *testing.T) {
3131
for _, dir := range getDirs(t, negativeTestDir) {
3232
dockerfile := filepath.Join(negativeTestDir, dir, "Dockerfile")
3333

3434
df, err := os.Open(dockerfile)
35-
require.NoError(t, err)
35+
require.NoError(t, err, dockerfile)
3636
defer df.Close()
3737

3838
_, err = Parse(df)
39-
assert.Error(t, err)
39+
assert.Error(t, err, dockerfile)
4040
}
4141
}
4242

43-
func TestTestData(t *testing.T) {
43+
func TestParseCases(t *testing.T) {
4444
for _, dir := range getDirs(t, testDir) {
4545
dockerfile := filepath.Join(testDir, dir, "Dockerfile")
4646
resultfile := filepath.Join(testDir, dir, "result")
4747

4848
df, err := os.Open(dockerfile)
49-
require.NoError(t, err)
49+
require.NoError(t, err, dockerfile)
5050
defer df.Close()
5151

5252
result, err := Parse(df)
53-
require.NoError(t, err)
53+
require.NoError(t, err, dockerfile)
5454

5555
content, err := ioutil.ReadFile(resultfile)
56-
require.NoError(t, err)
56+
require.NoError(t, err, resultfile)
5757

5858
if runtime.GOOS == "windows" {
5959
// CRLF --> CR to match Unix behavior
6060
content = bytes.Replace(content, []byte{'\x0d', '\x0a'}, []byte{'\x0a'}, -1)
6161
}
62-
63-
assert.Contains(t, result.AST.Dump()+"\n", string(content), "In "+dockerfile)
62+
assert.Equal(t, result.AST.Dump()+"\n", string(content), "In "+dockerfile)
6463
}
6564
}
6665

@@ -106,7 +105,7 @@ func TestParseWords(t *testing.T) {
106105
}
107106
}
108107

109-
func TestLineInformation(t *testing.T) {
108+
func TestParseIncludesLineNumbers(t *testing.T) {
110109
df, err := os.Open(testFileLineInfo)
111110
require.NoError(t, err)
112111
defer df.Close()
@@ -115,21 +114,41 @@ func TestLineInformation(t *testing.T) {
115114
require.NoError(t, err)
116115

117116
ast := result.AST
118-
if ast.StartLine != 5 || ast.endLine != 31 {
119-
fmt.Fprintf(os.Stderr, "Wrong root line information: expected(%d-%d), actual(%d-%d)\n", 5, 31, ast.StartLine, ast.endLine)
120-
t.Fatal("Root line information doesn't match result.")
121-
}
117+
assert.Equal(t, 5, ast.StartLine)
118+
assert.Equal(t, 31, ast.endLine)
122119
assert.Len(t, ast.Children, 3)
123120
expected := [][]int{
124121
{5, 5},
125122
{11, 12},
126123
{17, 31},
127124
}
128125
for i, child := range ast.Children {
129-
if child.StartLine != expected[i][0] || child.endLine != expected[i][1] {
130-
t.Logf("Wrong line information for child %d: expected(%d-%d), actual(%d-%d)\n",
131-
i, expected[i][0], expected[i][1], child.StartLine, child.endLine)
132-
t.Fatal("Root line information doesn't match result.")
133-
}
126+
msg := fmt.Sprintf("Child %d", i)
127+
assert.Equal(t, expected[i], []int{child.StartLine, child.endLine}, msg)
134128
}
135129
}
130+
131+
func TestParseWarnsOnEmptyContinutationLine(t *testing.T) {
132+
dockerfile := bytes.NewBufferString(`
133+
FROM alpine:3.6
134+
135+
RUN something \
136+
137+
following \
138+
139+
more
140+
141+
RUN another \
142+
143+
thing
144+
`)
145+
146+
result, err := Parse(dockerfile)
147+
require.NoError(t, err)
148+
warnings := result.Warnings
149+
assert.Len(t, warnings, 3)
150+
assert.Contains(t, warnings[0], "Empty continuation line found in")
151+
assert.Contains(t, warnings[0], "RUN something following more")
152+
assert.Contains(t, warnings[1], "RUN another thing")
153+
assert.Contains(t, warnings[2], "will become errors in a future release")
154+
}

‎builder/remotecontext/detect.go‎

Lines changed: 5 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -110,17 +110,13 @@ func newURLRemote(url string, dockerfilePath string, progressReader func(in io.R
110110
return progressReader(rc), nil
111111
},
112112
})
113-
if err != nil {
114-
if err == dockerfileFoundErr {
115-
res, err := parser.Parse(dockerfile)
116-
if err != nil {
117-
return nil, nil, err
118-
}
119-
return nil, res, nil
120-
}
113+
switch {
114+
case err == dockerfileFoundErr:
115+
res, err := parser.Parse(dockerfile)
116+
return nil, res, err
117+
case err != nil:
121118
return nil, nil, err
122119
}
123-
124120
return withDockerfileFromContext(c.(modifiableContext), dockerfilePath)
125121
}
126122

0 commit comments

Comments
 (0)