Merge pull request #1692 from tonistiigi/multi-arg

dockerfile: allow multiple values for ARG

frontend/dockerfile/dockerfile2llb/convert.go

@@ -94,11 +94,13 @@ func Dockerfile2LLB(ctx context.Context, dt []byte, opt ConvertOpt) (*llb.State,
 
 	shlex := shell.NewLex(dockerfile.EscapeToken)
 
-	for _, metaArg := range metaArgs {
-		if metaArg.Value != nil {
-			*metaArg.Value, _ = shlex.ProcessWordWithMap(*metaArg.Value, metaArgsToMap(optMetaArgs))
+	for _, cmd := range metaArgs {
+		for _, metaArg := range cmd.Args {
+			if metaArg.Value != nil {
+				*metaArg.Value, _ = shlex.ProcessWordWithMap(*metaArg.Value, metaArgsToMap(optMetaArgs))
+			}
+			optMetaArgs = append(optMetaArgs, setKVValue(metaArg, opt.BuildArgs))
 		}
-		optMetaArgs = append(optMetaArgs, setKVValue(metaArg.KeyValuePairOptional, opt.BuildArgs))
 	}
 
 	metaResolver := opt.MetaResolver
@@ -1072,26 +1074,30 @@ func dispatchShell(d *dispatchState, c *instructions.ShellCommand) error {
 }
 
 func dispatchArg(d *dispatchState, c *instructions.ArgCommand, metaArgs []instructions.KeyValuePairOptional, buildArgValues map[string]string) error {
-	commitStr := "ARG " + c.Key
-	buildArg := setKVValue(c.KeyValuePairOptional, buildArgValues)
+	commitStrs := make([]string, 0, len(c.Args))
+	for _, arg := range c.Args {
+		buildArg := setKVValue(arg, buildArgValues)
 
-	if c.Value != nil {
-		commitStr += "=" + *c.Value
-	}
-	if buildArg.Value == nil {
-		for _, ma := range metaArgs {
-			if ma.Key == buildArg.Key {
-				buildArg.Value = ma.Value
+		commitStr := arg.Key
+		if arg.Value != nil {
+			commitStr += "=" + *arg.Value
+		}
+		commitStrs = append(commitStrs, commitStr)
+		if buildArg.Value == nil {
+			for _, ma := range metaArgs {
+				if ma.Key == buildArg.Key {
+					buildArg.Value = ma.Value
+				}
 			}
 		}
-	}
 
-	if buildArg.Value != nil {
-		d.state = d.state.AddEnv(buildArg.Key, *buildArg.Value)
-	}
+		if buildArg.Value != nil {
+			d.state = d.state.AddEnv(buildArg.Key, *buildArg.Value)
+		}
 
-	d.buildArgs = append(d.buildArgs, buildArg)
-	return commitToHistory(&d.image, commitStr, false, nil)
+		d.buildArgs = append(d.buildArgs, buildArg)
+	}
+	return commitToHistory(&d.image, "ARG "+strings.Join(commitStrs, " "), false, nil)
 }
 
 func pathRelativeToWorkingDir(s llb.State, p string) (string, error) {

frontend/dockerfile/dockerfile_test.go

@@ -101,6 +101,7 @@ var allTests = []integration.Test{
 	testFrontendUseForwardedSolveResults,
 	testFrontendInputs,
 	testErrorsSourceMap,
+	testMultiArgs,
 }
 
 var fileOpTests = []integration.Test{
@@ -1158,6 +1159,52 @@ COPY --from=build /out .
 	require.Equal(t, "bar-box-foo", string(dt))
 }
 
+func testMultiArgs(t *testing.T, sb integration.Sandbox) {
+	f := getFrontend(t, sb)
+
+	dockerfile := []byte(`
+ARG a1="foo bar" a2=box
+ARG a3="$a2-foo"
+FROM busy$a2 AS build
+ARG a3 a4="123 456" a1
+RUN echo -n "$a1:$a3:$a4" > /out
+FROM scratch
+COPY --from=build /out .
+`)
+
+	dir, err := tmpdir(
+		fstest.CreateFile("Dockerfile", dockerfile, 0600),
+	)
+	require.NoError(t, err)
+	defer os.RemoveAll(dir)
+
+	c, err := client.New(context.TODO(), sb.Address())
+	require.NoError(t, err)
+	defer c.Close()
+
+	destDir, err := ioutil.TempDir("", "buildkit")
+	require.NoError(t, err)
+	defer os.RemoveAll(destDir)
+
+	_, err = f.Solve(context.TODO(), c, client.SolveOpt{
+		LocalDirs: map[string]string{
+			builder.DefaultLocalNameDockerfile: dir,
+			builder.DefaultLocalNameContext:    dir,
+		},
+		Exports: []client.ExportEntry{
+			{
+				Type:      client.ExporterLocal,
+				OutputDir: destDir,
+			},
+		},
+	}, nil)
+	require.NoError(t, err)
+
+	dt, err := ioutil.ReadFile(filepath.Join(destDir, "out"))
+	require.NoError(t, err)
+	require.Equal(t, "foo bar:box-foo:123 456", string(dt))
+}
+
 func testExportMultiPlatform(t *testing.T, sb integration.Sandbox) {
 	skipDockerd(t, sb)
 	f := getFrontend(t, sb)

frontend/dockerfile/instructions/commands.go

@@ -380,22 +380,25 @@ func (c *StopSignalCommand) CheckPlatform(platform string) error {
 // Dockerfile author may optionally set a default value of this variable.
 type ArgCommand struct {
 	withNameAndCode
-	KeyValuePairOptional
+	Args []KeyValuePairOptional
 }
 
 // Expand variables
 func (c *ArgCommand) Expand(expander SingleWordExpander) error {
-	p, err := expander(c.Key)
-	if err != nil {
-		return err
-	}
-	c.Key = p
-	if c.Value != nil {
-		p, err = expander(*c.Value)
+	for i, v := range c.Args {
+		p, err := expander(v.Key)
 		if err != nil {
 			return err
 		}
-		c.Value = &p
+		v.Key = p
+		if v.Value != nil {
+			p, err = expander(*v.Value)
+			if err != nil {
+				return err
+			}
+			v.Value = &p
+		}
+		c.Args[i] = v
 	}
 	return nil
 }

frontend/dockerfile/instructions/parse.go

@@ -579,33 +579,37 @@ func parseStopSignal(req parseRequest) (*StopSignalCommand, error) {
 }
 
 func parseArg(req parseRequest) (*ArgCommand, error) {
-	if len(req.args) != 1 {
-		return nil, errExactlyOneArgument("ARG")
+	if len(req.args) < 1 {
+		return nil, errAtLeastOneArgument("ARG")
 	}
 
-	kvpo := KeyValuePairOptional{}
+	pairs := make([]KeyValuePairOptional, len(req.args))
 
-	arg := req.args[0]
-	// 'arg' can just be a name or name-value pair. Note that this is different
-	// from 'env' that handles the split of name and value at the parser level.
-	// The reason for doing it differently for 'arg' is that we support just
-	// defining an arg and not assign it a value (while 'env' always expects a
-	// name-value pair). If possible, it will be good to harmonize the two.
-	if strings.Contains(arg, "=") {
-		parts := strings.SplitN(arg, "=", 2)
-		if len(parts[0]) == 0 {
-			return nil, errBlankCommandNames("ARG")
+	for i, arg := range req.args {
+		kvpo := KeyValuePairOptional{}
+
+		// 'arg' can just be a name or name-value pair. Note that this is different
+		// from 'env' that handles the split of name and value at the parser level.
+		// The reason for doing it differently for 'arg' is that we support just
+		// defining an arg and not assign it a value (while 'env' always expects a
+		// name-value pair). If possible, it will be good to harmonize the two.
+		if strings.Contains(arg, "=") {
+			parts := strings.SplitN(arg, "=", 2)
+			if len(parts[0]) == 0 {
+				return nil, errBlankCommandNames("ARG")
+			}
+
+			kvpo.Key = parts[0]
+			kvpo.Value = &parts[1]
+		} else {
+			kvpo.Key = arg
 		}
-
-		kvpo.Key = parts[0]
-		kvpo.Value = &parts[1]
-	} else {
-		kvpo.Key = arg
+		pairs[i] = kvpo
 	}
 
 	return &ArgCommand{
-		KeyValuePairOptional: kvpo,
-		withNameAndCode:      newWithNameAndCode(req),
+		Args:            pairs,
+		withNameAndCode: newWithNameAndCode(req),
 	}, nil
 }
 

frontend/dockerfile/instructions/parse_test.go

@@ -163,11 +163,6 @@ func TestErrorCases(t *testing.T) {
 			dockerfile:    "ONBUILD MAINTAINER docker.io",
 			expectedError: "MAINTAINER isn't allowed as an ONBUILD trigger",
 		},
-		{
-			name:          "ARG two arguments",
-			dockerfile:    "ARG foo bar",
-			expectedError: "ARG requires exactly one argument",
-		},
 		{
 			name:          "MAINTAINER unknown flag",
 			dockerfile:    "MAINTAINER --boo joe@example.com",

frontend/dockerfile/parser/testfiles/args/Dockerfile

@@ -0,0 +1,3 @@
+ARG foo bar=baz
+FROM ubuntu
+ARG abc="123 456" def

frontend/dockerfile/parser/testfiles/args/result

@@ -0,0 +1,3 @@
+(arg "foo" "bar=baz")
+(from "ubuntu")
+(arg "abc=\"123 456\"" "def")