Repository: openllb/hlb
Pull request #115 (2020-04-21 19:07:58)

Coerce fs func lit into group func lit
- Coerce fs func lit into group func lit
- Add `treeprint` branches for solve options so that they are testable
- Add codegen test

```
group default() {
	parallel group {
		image "alpine"
	} fs {
		scratch
		mkfile "foo" 0o644 "hello world"
		download "."
	}
}
```


Patch:
diff --git a/checker/checker.go b/checker/checker.go
index 896f8ef..e45b1cf 100644
--- a/checker/checker.go
+++ b/checker/checker.go
@@ -319,12 +319,7 @@ func (c *checker) checkBlockStmt(scope *parser.Scope, typ parser.ObjType, block
 		// If the function is not a builtin, retrieve it from the scope and then
 		// type check it.
 		_, ok := builtin.Lookup.ByType[typ].Func[name]
-		if !ok && typ == parser.Group {
-			_, ok = builtin.Lookup.ByType[parser.Filesystem].Func[name]
-		}
-
 		if !ok {
-
 			obj := scope.Lookup(name)
 			if obj == nil {
 				return ErrIdentNotDefined{Ident: call.Func.Ident}
@@ -536,6 +531,10 @@ func (c *checker) checkBasicLitArg(typ parser.ObjType, lit *parser.BasicLit) err
 }
 
 func (c *checker) checkFuncLitArg(scope *parser.Scope, typ parser.ObjType, lit *parser.FuncLit) error {
+	if typ == parser.Group && lit.Type.ObjType == parser.Filesystem {
+		typ = lit.Type.ObjType
+	}
+
 	err := c.checkType(lit, typ, lit.Type)
 	if err != nil {
 		return err
diff --git a/codegen/chain.go b/codegen/chain.go
index 9aa1c6e..993edaa 100644
--- a/codegen/chain.go
+++ b/codegen/chain.go
@@ -696,7 +696,7 @@ func (cg *CodeGen) EmitGroupChainStmt(ctx context.Context, scope *parser.Scope,
 	case "parallel":
 		var peerRequests []solver.Request
 		for _, arg := range args {
-			request, err := cg.EmitGroupExpr(ctx, scope, arg, ac, nil)
+			request, err := cg.EmitGroupExpr(ctx, scope, arg, ac)
 			if err != nil {
 				return gc, err
 			}
@@ -705,38 +705,14 @@ func (cg *CodeGen) EmitGroupChainStmt(ctx context.Context, scope *parser.Scope,
 		}
 
 		gc = func(requests []solver.Request) ([]solver.Request, error) {
-			requests = append(requests, solver.Parallel(peerRequests...))
+			if len(peerRequests) == 1 {
+				requests = append(requests, peerRequests[0])
+			} else {
+				requests = append(requests, solver.Parallel(peerRequests...))
+			}
 			return requests, nil
 		}
 	default:
-		so, err := cg.EmitFilesystemBuiltinChainStmt(ctx, scope, expr, args, with, ac, nil)
-		if err != nil {
-			return gc, err
-		}
-
-		if so != nil {
-			return func(requests []solver.Request) ([]solver.Request, error) {
-				st, err := so(llb.Scratch())
-				if err != nil {
-					return requests, err
-				}
-
-				request, err := cg.outputRequest(ctx, st, Output{})
-				if err != nil {
-					return requests, err
-				}
-
-				if len(cg.requests) > 0 {
-					request = solver.Parallel(append([]solver.Request{request}, cg.requests...)...)
-				}
-
-				cg.reset()
-
-				requests = append(requests, request)
-				return requests, nil
-			}, nil
-		}
-
 		// Must be a named reference.
 		obj := scope.Lookup(expr.Name())
 		if obj == nil {
diff --git a/codegen/decl.go b/codegen/decl.go
index 9561f05..f32004c 100644
--- a/codegen/decl.go
+++ b/codegen/decl.go
@@ -176,7 +176,7 @@ func (cg *CodeGen) ParameterizedScope(ctx context.Context, scope *parser.Scope,
 			data = v
 		case parser.Group:
 			var v solver.Request
-			v, err = cg.EmitGroupExpr(ctx, scope, args[i], ac, nil)
+			v, err = cg.EmitGroupExpr(ctx, scope, args[i], ac)
 			data = v
 		}
 		if err != nil {
diff --git a/codegen/expr.go b/codegen/expr.go
index 1a229a8..1694f4e 100644
--- a/codegen/expr.go
+++ b/codegen/expr.go
@@ -127,10 +127,10 @@ func (cg *CodeGen) EmitOptionExpr(ctx context.Context, scope *parser.Scope, expr
 	}
 }
 
-func (cg *CodeGen) EmitGroupExpr(ctx context.Context, scope *parser.Scope, expr *parser.Expr, ac aliasCallback, chainStart interface{}) (solver.Request, error) {
+func (cg *CodeGen) EmitGroupExpr(ctx context.Context, scope *parser.Scope, expr *parser.Expr, ac aliasCallback) (solver.Request, error) {
 	switch {
 	case expr.Ident != nil, expr.Selector != nil:
-		gc, err := cg.EmitGroupChainStmt(ctx, scope, expr, nil, nil, ac, chainStart)
+		gc, err := cg.EmitGroupChainStmt(ctx, scope, expr, nil, nil, ac, nil)
 		if err != nil {
 			return nil, err
 		}
@@ -148,7 +148,29 @@ func (cg *CodeGen) EmitGroupExpr(ctx context.Context, scope *parser.Scope, expr
 	case expr.BasicLit != nil:
 		return nil, errors.WithStack(ErrCodeGen{expr, errors.Errorf("group expr cannot be basic lit")})
 	case expr.FuncLit != nil:
-		return cg.EmitGroupBlock(ctx, scope, expr.FuncLit.Body.NonEmptyStmts(), ac, chainStart)
+		switch expr.FuncLit.Type.Primary() {
+		case parser.Group:
+			return cg.EmitGroupBlock(ctx, scope, expr.FuncLit.Body.NonEmptyStmts(), ac, nil)
+		case parser.Filesystem:
+			st, err := cg.EmitFilesystemBlock(ctx, scope, expr.FuncLit.Body.NonEmptyStmts(), ac, nil)
+			if err != nil {
+				return nil, err
+			}
+
+			request, err := cg.outputRequest(ctx, st, Output{})
+			if err != nil {
+				return nil, err
+			}
+
+			if len(cg.requests) > 0 {
+				request = solver.Parallel(append([]solver.Request{request}, cg.requests...)...)
+			}
+			cg.reset()
+
+			return request, nil
+		default:
+			return nil, errors.WithStack(ErrCodeGen{expr, errors.Errorf("invalid group func lit")})
+		}
 	default:
 		return nil, errors.WithStack(ErrCodeGen{expr, errors.Errorf("unknown fs expr")})
 	}
diff --git a/parser/cst.go b/parser/cst.go
index bbc456e..083e950 100644
--- a/parser/cst.go
+++ b/parser/cst.go
@@ -403,9 +403,6 @@ func (t *Type) Equals(typ ObjType) bool {
 		parts := typeParts(typ)
 		return ObjType(parts[0]) == Option
 	}
-	if typ == Group {
-		return t.ObjType == Filesystem || t.ObjType == Group
-	}
 	return t.ObjType == typ
 }
 
diff --git a/solver/request.go b/solver/request.go
index 25123e3..92dec91 100644
--- a/solver/request.go
+++ b/solver/request.go
@@ -69,10 +69,18 @@ func (r *singleRequest) Solve(ctx context.Context, cln *client.Client, mw *progr
 }
 
 func (r *singleRequest) Tree(tree treeprint.Tree) error {
-	return treeFromDefinition(tree, r.params.Def)
+	return treeFromDefinition(tree, r.params.Def, r.params.SolveOpts)
 }
 
-func treeFromDefinition(tree treeprint.Tree, def *llb.Definition) error {
+func treeFromDefinition(tree treeprint.Tree, def *llb.Definition, opts []SolveOption) error {
+	var info SolveInfo
+	for _, opt := range opts {
+		err := opt(&info)
+		if err != nil {
+			return err
+		}
+	}
+
 	ops := make(map[digest.Digest]*pb.Op)
 
 	var dgst digest.Digest
@@ -90,7 +98,7 @@ func treeFromDefinition(tree treeprint.Tree, def *llb.Definition) error {
 	}
 
 	terminal := ops[dgst]
-	child := op{dgst: terminal.Inputs[0].Digest, ops: ops, meta: def.Metadata}
+	child := op{dgst: terminal.Inputs[0].Digest, ops: ops, meta: def.Metadata, info: info}
 	return child.Tree(tree)
 }
 
@@ -98,6 +106,7 @@ type op struct {
 	dgst digest.Digest
 	ops  map[digest.Digest]*pb.Op
 	meta map[digest.Digest]pb.OpMetadata
+	info SolveInfo
 }
 
 func (o op) Tree(tree treeprint.Tree) error {
@@ -191,6 +200,23 @@ func (o op) Tree(tree treeprint.Tree) error {
 		branch = tree.AddMetaBranch("build", v.Build)
 	}
 
+	solve := branch.AddBranch("solve options")
+	if o.info.OutputDockerRef != "" {
+		solve.AddMetaNode("dockerRef", o.info.OutputDockerRef)
+	}
+	if o.info.OutputPushImage != "" {
+		solve.AddMetaNode("pushImage", o.info.OutputPushImage)
+	}
+	if o.info.OutputLocal != "" {
+		solve.AddMetaNode("download", o.info.OutputLocal)
+	}
+	if o.info.OutputLocalTarball {
+		solve.AddNode("downloadTarball")
+	}
+	if o.info.OutputLocalOCITarball {
+		solve.AddNode("downloadOCITarball")
+	}
+
 	for _, input := range pbOp.Inputs {
 		if _, ok := reportedInputs[input.Digest]; ok {
 			continue


Test patch:
diff --git a/checker/checker_test.go b/checker/checker_test.go
index 1747cdd..8952382 100644
--- a/checker/checker_test.go
+++ b/checker/checker_test.go
@@ -297,6 +297,40 @@ func TestChecker_Check(t *testing.T) {
 				Name: "myFunction",
 			},
 		},
+	}, {
+		// Until we support func literals as stmts, we have to use a parallel
+		// group of one element to coerce fs to group.
+		"basic group support",
+		`
+		group default() {
+			parallel groupA fs { image "b"; }
+			groupC
+		}
+		group groupA() {
+			parallel fs { image "a"; }
+		}
+		group groupC() {
+			parallel fs { image "c"; }
+		}
+		`,
+		nil,
+	}, {
+		"errors when fs statement is called in a group block",
+		`
+		group badGroup() {
+			image "alpine"
+		}
+		`,
+		ErrIdentNotDefined{
+			Ident: &parser.Ident{
+				Pos: lexer.Position{
+					Filename: "<stdin>",
+					Line:     2,
+					Column:   1,
+				},
+				Name: "image",
+			},
+		},
 	}} {
 		tc := tc
 		t.Run(tc.name, func(t *testing.T) {
diff --git a/codegen/codegen_test.go b/codegen/codegen_test.go
index 94afc27..8553899 100644
--- a/codegen/codegen_test.go
+++ b/codegen/codegen_test.go
@@ -14,12 +14,13 @@ import (
 	"github.com/xlab/treeprint"
 )
 
-func Expect(t *testing.T, st llb.State) solver.Request {
+func Expect(t *testing.T, st llb.State, opts ...solver.SolveOption) solver.Request {
 	def, err := st.Marshal(context.Background(), llb.LinuxAmd64)
 	require.NoError(t, err)
 
 	return solver.Single(&solver.Params{
-		Def: def,
+		Def:       def,
+		SolveOpts: opts,
 	})
 }
 
@@ -139,8 +140,8 @@ func TestCodeGen(t *testing.T) {
 		[]string{"default"},
 		`
 		group default() {
-			image "alpine"
-			image "busybox"
+			parallel fs { image "alpine"; }
+			parallel fs { image "busybox"; }
 		}
 		`,
 		func(t *testing.T, cg *CodeGen) solver.Request {
@@ -154,9 +155,9 @@ func TestCodeGen(t *testing.T) {
 		[]string{"default"},
 		`
 		group default() {
-			parallel group {
+			parallel fs {
 				image "alpine"
-			} group {
+			} fs {
 				image "busybox"
 			}
 		}
@@ -172,13 +173,13 @@ func TestCodeGen(t *testing.T) {
 		[]string{"default"},
 		`
 		group default() {
-			image "golang:alpine"
-			parallel group {
+			parallel fs { image "golang:alpine"; }
+			parallel fs {
 				image "alpine"
-			} group {
+			} fs {
 				image "busybox"
 			}
-			image "node:alpine"
+			parallel fs { image "node:alpine"; }
 		}
 		`,
 		func(t *testing.T, cg *CodeGen) solver.Request {
@@ -200,8 +201,12 @@ func TestCodeGen(t *testing.T) {
 		}
 
 		group foo(string ref) {
-			image string { format "alpine:%s" ref; }
-			image string { format "busybox:%s" ref; }
+			parallel fs {
+				image string { format "alpine:%s" ref; }
+			}
+			parallel fs {
+				image string { format "busybox:%s" ref; }
+			}
 		}
 		`,
 		func(t *testing.T, cg *CodeGen) solver.Request {
@@ -210,6 +215,29 @@ func TestCodeGen(t *testing.T) {
 				Expect(t, llb.Image("busybox:stable")),
 			)
 		},
+	}, {
+		"parallel coercing fs to group",
+		[]string{"default"},
+		`
+		group default() {
+			parallel fs {
+				image "alpine"
+			} fs {
+				scratch
+				mkfile "foo" 0o644 "hello world"
+				download "."
+			}
+		}
+		`,
+		func(t *testing.T, cg *CodeGen) solver.Request {
+			return solver.Parallel(
+				Expect(t, llb.Image("alpine")),
+				solver.Parallel(
+					Expect(t, llb.Scratch().File(llb.Mkfile("foo", 0644, []byte("hello world")))),
+					Expect(t, llb.Scratch().File(llb.Mkfile("foo", 0644, []byte("hello world"))), solver.WithDownload(".")),
+				),
+			)
+		},
 	}, {
 		"here doc processing",
 		[]string{"default"},