Repository: openllb/hlb
Pull request #123 (2020-04-24 16:40:01)

Fix merging user defined option::copy with func lit
- The with options from `llb` upstream only takes `&llb.CopyInfo{}`, this means that merging additional user-defined options overwrites with zero values. I.e. `&llb.CopyInfo{CreateDestPath: true}` + `&llb.CopyInfo{AllowWildcard: true}` = `&{llb.CopyInfo{AllowWildcard: true}`.
- Also adds `option::copy chmod` because I discovered it was also available.
- We should probably get this upstream, I made a issue in BuildKit tracking this here: https://github.com/moby/buildkit/issues/1461


Patch:
diff --git a/builtin/builtin.go b/builtin/builtin.go
index fe411e6..4a571ce 100644
--- a/builtin/builtin.go
+++ b/builtin/builtin.go
@@ -1,4 +1,4 @@
-// Code generated by builtingen language/builtin.hlb builtin/builtin.go; DO NOT EDIT.
+// Code generated by builtingen ../language/builtin.hlb ../builtin/builtin.go; DO NOT EDIT.
 
 package builtin
 
@@ -168,6 +168,11 @@ var (
 							parser.NewField(parser.Str, "owner", false),
 						},
 					},
+					"chmod": FuncLookup{
+						Params: []*parser.Field{
+							parser.NewField(parser.Int, "filemode", false),
+						},
+					},
 					"createdTime": FuncLookup{
 						Params: []*parser.Field{
 							parser.NewField(parser.Str, "created", false),
diff --git a/codegen/chain.go b/codegen/chain.go
index ee9343f..b066308 100644
--- a/codegen/chain.go
+++ b/codegen/chain.go
@@ -469,15 +469,15 @@ func (cg *CodeGen) EmitFilesystemBuiltinChainStmt(ctx context.Context, scope *pa
 			return fc, err
 		}
 
-		var opts []llb.CopyOption
+		info := &llb.CopyInfo{}
 		for _, iopt := range iopts {
-			opt := iopt.(llb.CopyOption)
-			opts = append(opts, opt)
+			opt := iopt.(CopyOption)
+			opt(info)
 		}
 
 		fc = func(st llb.State) (llb.State, error) {
 			return st.File(
-				llb.Copy(input, src, dest, opts...),
+				llb.Copy(input, src, dest, info),
 			), nil
 		}
 	case "dockerPush":
diff --git a/codegen/codegen.go b/codegen/codegen.go
index f59a743..24d4db4 100644
--- a/codegen/codegen.go
+++ b/codegen/codegen.go
@@ -740,55 +740,116 @@ func (cg *CodeGen) EmitRmOptions(ctx context.Context, scope *parser.Scope, op st
 	return
 }
 
-func (cg *CodeGen) EmitCopyOptions(ctx context.Context, scope *parser.Scope, op string, stmts []*parser.Stmt) (opts []interface{}, err error) {
-	cp := &llb.CopyInfo{}
+type CopyOption func(*llb.CopyInfo)
+
+func WithFollowSymlinks(follow bool) CopyOption {
+	return func(info *llb.CopyInfo) {
+		info.FollowSymlinks = follow
+	}
+}
+
+func WithCopyDirContentsOnly(contentsOnly bool) CopyOption {
+	return func(info *llb.CopyInfo) {
+		info.CopyDirContentsOnly = contentsOnly
+	}
+}
+
+func WithAttemptUnpack(unpack bool) CopyOption {
+	return func(info *llb.CopyInfo) {
+		info.AttemptUnpack = unpack
+	}
+}
+
+func WithCreateDestPath(createDest bool) CopyOption {
+	return func(info *llb.CopyInfo) {
+		info.CreateDestPath = createDest
+	}
+}
+
+func WithAllowWildcard(allow bool) CopyOption {
+	return func(info *llb.CopyInfo) {
+		info.AllowWildcard = allow
+	}
+}
+
+func WithAllowEmptyWildcard(allow bool) CopyOption {
+	return func(info *llb.CopyInfo) {
+		info.AllowEmptyWildcard = allow
+	}
+}
+
+func WithChown(owner string) CopyOption {
+	return func(info *llb.CopyInfo) {
+		opt := llb.WithUser(owner).(llb.ChownOpt)
+		info.ChownOpt = &opt
+	}
+}
 
+func WithChmod(mode os.FileMode) CopyOption {
+	return func(info *llb.CopyInfo) {
+		info.Mode = &mode
+	}
+}
+
+func WithCreatedTime(t time.Time) CopyOption {
+	return func(info *llb.CopyInfo) {
+		info.CreatedTime = &t
+	}
+}
+
+func (cg *CodeGen) EmitCopyOptions(ctx context.Context, scope *parser.Scope, op string, stmts []*parser.Stmt) (opts []interface{}, err error) {
 	for _, stmt := range stmts {
 		if stmt.Call != nil {
 			args := stmt.Call.Args
 			switch stmt.Call.Func.Ident.Name {
 			case "followSymlinks":
-				v, err := cg.MaybeEmitBoolExpr(ctx, scope, args)
+				follow, err := cg.MaybeEmitBoolExpr(ctx, scope, args)
 				if err != nil {
 					return opts, err
 				}
-				cp.FollowSymlinks = v
+				opts = append(opts, WithFollowSymlinks(follow))
 			case "contentsOnly":
-				v, err := cg.MaybeEmitBoolExpr(ctx, scope, args)
+				contentsOnly, err := cg.MaybeEmitBoolExpr(ctx, scope, args)
 				if err != nil {
 					return opts, err
 				}
-				cp.CopyDirContentsOnly = v
+				opts = append(opts, WithCopyDirContentsOnly(contentsOnly))
 			case "unpack":
-				v, err := cg.MaybeEmitBoolExpr(ctx, scope, args)
+				unpack, err := cg.MaybeEmitBoolExpr(ctx, scope, args)
 				if err != nil {
 					return opts, err
 				}
-				cp.AttemptUnpack = v
+				opts = append(opts, WithAttemptUnpack(unpack))
 			case "createDestPath":
-				v, err := cg.MaybeEmitBoolExpr(ctx, scope, args)
+				create, err := cg.MaybeEmitBoolExpr(ctx, scope, args)
 				if err != nil {
 					return opts, err
 				}
-				cp.CreateDestPath = v
+				opts = append(opts, WithCreateDestPath(create))
 			case "allowWildcard":
-				v, err := cg.MaybeEmitBoolExpr(ctx, scope, args)
+				allow, err := cg.MaybeEmitBoolExpr(ctx, scope, args)
 				if err != nil {
 					return opts, err
 				}
-				cp.AllowWildcard = v
+				opts = append(opts, WithAllowWildcard(allow))
 			case "allowEmptyWildcard":
-				v, err := cg.MaybeEmitBoolExpr(ctx, scope, args)
+				allow, err := cg.MaybeEmitBoolExpr(ctx, scope, args)
 				if err != nil {
 					return opts, err
 				}
-				cp.AllowEmptyWildcard = v
+				opts = append(opts, WithAllowEmptyWildcard(allow))
 			case "chown":
 				owner, err := cg.EmitStringExpr(ctx, scope, args[0])
 				if err != nil {
 					return opts, err
 				}
-				opts = append(opts, llb.WithUser(owner))
+				opts = append(opts, WithChown(owner))
+			case "chmod":
+				mode, err := cg.EmitIntExpr(ctx, scope, args[0])
+				if err != nil {
+					return opts, err
+				}
+				opts = append(opts, WithChmod(os.FileMode(mode)))
 			case "createdTime":
 				v, err := cg.EmitStringExpr(ctx, scope, args[0])
 				if err != nil {
@@ -799,8 +860,7 @@ func (cg *CodeGen) EmitCopyOptions(ctx context.Context, scope *parser.Scope, op
 				if err != nil {
 					return opts, err
 				}
-
-				opts = append(opts, llb.WithCreatedTime(t))
+				opts = append(opts, WithCreatedTime(t))
 			default:
 				iopts, err := cg.EmitOptionLookup(ctx, scope, stmt.Call.Func, args, op)
 				if err != nil {
@@ -810,8 +870,6 @@ func (cg *CodeGen) EmitCopyOptions(ctx context.Context, scope *parser.Scope, op
 			}
 		}
 	}
-
-	opts = append([]interface{}{cp}, opts...)
 	return
 }
 
diff --git a/language/builtin.hlb b/language/builtin.hlb
index d07c85a..580b001 100644
--- a/language/builtin.hlb
+++ b/language/builtin.hlb
@@ -31,7 +31,7 @@ option::http checksum(string digest)
 
 # Modifies the permissions of the retrieved file.
 #
-# @param filemode the new permissions of the file in int.
+# @param filemode the new permissions of the file.
 # @return an option to chmod the file.
 option::http chmod(int filemode)
 
@@ -445,6 +445,12 @@ option::copy allowEmptyWildcard()
 # @return an option to change the owner of the copy path.
 option::copy chown(string owner)
 
+# Modifies the permissions of the copied files.
+#
+# @param filemode the new permissions of the file.
+# @return an option to chmod the file.
+option::copy chmod(int filemode)
+
 # Sets the created time of the copy path.
 #
 # @param created the created time in the RFC3339 format.


Test patch:
diff --git a/codegen/codegen_test.go b/codegen/codegen_test.go
index 42c4d38..84a68d6 100644
--- a/codegen/codegen_test.go
+++ b/codegen/codegen_test.go
@@ -367,6 +367,33 @@ func TestCodeGen(t *testing.T) {
 				llb.AddSecret("/foo/secret/codegen_test.go", llb.SecretID(sid)),
 			).Root())
 		},
+	}, {
+		"merging user defined option::copy with func lit",
+		[]string{"default"},
+		`
+		fs default() {
+			scratch
+			copy scratch "/" "/foo" with option {
+				createDestPath
+				myOpt
+			}
+		}
+
+		option::copy myOpt() {
+			chown "1001:1001"
+		}
+		`,
+		func(t *testing.T, cg *CodeGen) solver.Request {
+			return Expect(t, llb.Scratch().File(
+				llb.Copy(llb.Scratch(), "/", "/foo", &llb.CopyInfo{
+					CreateDestPath: true,
+					ChownOpt: &llb.ChownOpt{
+						User:  &llb.UserOpt{UID: 1001},
+						Group: &llb.UserOpt{UID: 1001},
+					},
+				}),
+			))
+		},
 	}} {
 		tc := tc
 		t.Run(tc.name, func(t *testing.T) {