Repository: openllb/hlb
Pull request #348 (2023-03-16 16:16:18)

fix llb.MountOption cast
For some reason llb.ForceNoOutput is not matching [llb.MountOption in the switch statement](https://github.com/openllb/hlb/blob/1e65e28f9153988049f3c36ea9007067ccda8d92/pkg/llbutil/llbutil.go#L70) that applies options to mounts, so just forcing the type.

After more more debuggging it seems the llb.ForceNoOutput is useful for the cases where we mount sources from external locations (http, git), this allows cached results without forcing us to awkwardly use `with readonly`.

Our current builds currently do something like:
```
fs _build() {
  image mybase
  run "go build" with option {
    mount local(".") "/src" as build
  }
}
```
The usage of `local` seems to invalidate the cache, even if the contents are the same, and note we can't apply ForceNoOutput because we bind the mount.   I started a [slack thread](https://dockercommunity.slack.com/archives/C7S7A40MP/p1678908117898669) on why local invalidates the cache, but I think it will take some research to see if there are any efficient viable workarounds.

For now, the only viable workaround I have found is to force a copy, which seems to work pretty well:
```
fs _build() {
  image mybase
  run "go build" with option {
    mount fs {
      copy fs {
        local(".") "/src"
      } "." "."
    } "/src" as build
  }
}
```

The extra overhead of the copy I think is likely a much better trade-off than invaliding the `run` cache.


Patch:
diff --git a/codegen/builtin_fs.go b/codegen/builtin_fs.go
index 91e25a4..fd400ea 100644
--- a/codegen/builtin_fs.go
+++ b/codegen/builtin_fs.go
@@ -245,8 +245,17 @@ func (l Local) Call(ctx context.Context, cln *client.Client, val Value, opts Opt
 		llb.LocalUniqueID(id),
 	)
 
+	st := llb.Local(localPath, localOpts...)
+	if CalleeBinding(ctx).Binds() == "target" {
+		// we are mounting this `local` and the `mount` is bound.  We are
+		// currently seeing cache invalidation even if the `local` contents are
+		// unchanged. A hacky-workaround is to Copy the llb.Local first, then
+		// mount the Copy, which allows for better caching for Run calls.
+		st = llb.Scratch().File(llb.Copy(st, "/", "/"))
+	}
+
 	fs := Filesystem{
-		State:    llb.Local(localPath, localOpts...),
+		State:    st,
 		Platform: DefaultPlatform(ctx),
 	}
 	fs.SessionOpts = append(fs.SessionOpts, llbutil.WithSyncedDir(localPath, filesync.SyncedDir{
diff --git a/codegen/builtin_option.go b/codegen/builtin_option.go
index f7c0fec..d6ee8d9 100644
--- a/codegen/builtin_option.go
+++ b/codegen/builtin_option.go
@@ -708,7 +708,7 @@ func (m Mount) Call(ctx context.Context, cln *client.Client, val Value, opts Opt
 		}
 		retOpts = append(retOpts, &Mount{Bind: mountpoint, Image: input.Image})
 	} else {
-		opts = append(opts, llb.ForceNoOutput)
+		opts = append(opts, llb.MountOption(llb.ForceNoOutput))
 	}
 
 	retOpts = append(retOpts, &llbutil.MountRunOption{
diff --git a/codegen/codegen.go b/codegen/codegen.go
index 15af133..6572b74 100644
--- a/codegen/codegen.go
+++ b/codegen/codegen.go
@@ -647,7 +647,7 @@ func (cg *CodeGen) EmitCallStmt(ctx context.Context, scope *ast.Scope, call *ast
 	}
 
 	// Evaluate args second.
-	args := cg.Evaluate(ctx, scope, call, b)
+	args := cg.Evaluate(WithCalleeBinding(ctx, b), scope, call, b)
 	for i, arg := range call.Arguments() {
 		ctx = WithArg(ctx, i, arg)
 	}
diff --git a/codegen/context.go b/codegen/context.go
index fad507d..b9bf052 100644
--- a/codegen/context.go
+++ b/codegen/context.go
@@ -27,6 +27,7 @@ type (
 	returnTypeKey      struct{}
 	argKey             struct{ n int }
 	bindingKey         struct{}
+	calleeBindingKey   struct{}
 	multiwriterKey     struct{}
 	imageResolverKey   struct{}
 	backtraceKey       struct{}
@@ -89,6 +90,21 @@ func Binding(ctx context.Context) *ast.Binding {
 	return binding
 }
 
+func WithCalleeBinding(ctx context.Context, binding *ast.Binding) context.Context {
+	if binding != nil {
+		return context.WithValue(ctx, calleeBindingKey{}, binding)
+	}
+	return ctx
+}
+
+func CalleeBinding(ctx context.Context) *ast.Binding {
+	binding, ok := ctx.Value(calleeBindingKey{}).(*ast.Binding)
+	if !ok {
+		return &ast.Binding{}
+	}
+	return binding
+}
+
 func WithArg(ctx context.Context, n int, arg ast.Node) context.Context {
 	return context.WithValue(ctx, argKey{n}, arg)
 }


Test patch:
diff --git a/codegen/codegen_test.go b/codegen/codegen_test.go
index cb64025..b9e3b88 100644
--- a/codegen/codegen_test.go
+++ b/codegen/codegen_test.go
@@ -935,6 +935,26 @@ func TestCodeGen(t *testing.T) {
 				llb.AddMount("/foobar", mnt),
 			).Root())
 		},
+	}, {
+		"mount local with bind is copied",
+		[]string{"default"},
+		`
+		fs _default() {
+			image "busybox"
+			run "find" "/foo" with option {
+				mount local(".") "/foo" as default
+			}
+		}
+		`, "",
+		func(ctx context.Context, t *testing.T) solver.Request {
+			return Expect(t, llb.Image("busybox").Run(
+				llb.Args([]string{"find", "/foo"}),
+				llb.AddMount(
+					"/foo",
+					llb.Scratch().File(llb.Copy(LocalState(ctx, t, "."), "/", "/")),
+				),
+			).Root())
+		},
 	}} {
 		tc := tc
 		t.Run(tc.name, func(t *testing.T) {