Repository: openllb/hlb
Pull request #280 (2022-01-26 00:24:01)

Refactor usage of llb.Local name and key hint
- Removes `solver.LocalPathDescriptionKey`, it was used only to see what local's digest actually pointed to in the solver tree. Since the path is now part of the local name, this isn't necessary anymore. (https://github.com/openllb/hlb/pull/97#issuecomment-614268142)
- Use `localPath` as name for `llb.Local`, but under the hood use abs path for sync dir
- Use our deterministic ID in `llb.SharedKeyHint` instead, not visible to end user but gives us the same benefits
- Include filename in `localPath` when specifying a file directory with `local "filename"`. Doesn't have any functional difference but makes the output better

Before
```
~/tmp
❯ cat foo.hlb
fs default() {
	local "foo.hlb"
}

~/tmp
❯ hlb run foo.hlb
[+] Building 0.0s (1/1) FINISHED
 => local://sha256:408dc57624f1359d2eaa3b6ef97dcff3b16d16125db02684613a33a8aec2bec0 (foo.hlb)
 => => transferring sha256:408dc57624f1359d2eaa3b6ef97dcff3b16d16125db02684613a33a8aec2bec0: 28B
```

After
```
~/tmp
❯ cat foo.hlb
fs default() {
	local "foo.hlb"
}

~/tmp
❯ hlb run foo.hlb
[+] Building 0.0s (1/1) FINISHED
 => local://foo.hlb (foo.hlb)
 => => transferring foo.hlb: 28B
```


Patch:
diff --git a/codegen/builtin_fs.go b/codegen/builtin_fs.go
index 0df9404..be891c4 100644
--- a/codegen/builtin_fs.go
+++ b/codegen/builtin_fs.go
@@ -173,28 +173,31 @@ func (l Local) Call(ctx context.Context, cln *client.Client, ret Register, opts
 		localOpts = append(localOpts, opt)
 	}
 
+	localDir := localPath
 	if !fi.IsDir() {
 		filename := filepath.Base(localPath)
-		localPath = filepath.Dir(localPath)
+		localDir = filepath.Dir(localPath)
 
 		// When localPath is a filename instead of a directory, include and exclude
 		// patterns should be ignored.
 		localOpts = append(localOpts, llb.IncludePatterns([]string{filename}), llb.ExcludePatterns([]string{}))
 	}
 
-	cwd, err := local.Cwd(ctx)
-	if err != nil {
-		return err
+	absPath := localPath
+	if !filepath.IsAbs(absPath) {
+		cwd, err := local.Cwd(ctx)
+		if err != nil {
+			return err
+		}
+
+		absPath = filepath.Join(cwd, localPath)
 	}
 
-	id, err := llbutil.LocalID(ctx, cwd, localPath, localOpts...)
+	id, err := llbutil.LocalID(ctx, absPath, localOpts...)
 	if err != nil {
 		return err
 	}
-
-	localOpts = append(localOpts, llb.SharedKeyHint(localPath), llb.WithDescription(map[string]string{
-		solver.LocalPathDescriptionKey: fmt.Sprintf("local://%s", localPath),
-	}))
+	localOpts = append(localOpts, llb.SharedKeyHint(id))
 
 	sessionID := SessionID(ctx)
 	if sessionID != "" {
@@ -202,12 +205,12 @@ func (l Local) Call(ctx context.Context, cln *client.Client, ret Register, opts
 	}
 
 	fs := Filesystem{
-		State:    llb.Local(id, localOpts...),
+		State:    llb.Local(localPath, localOpts...),
 		Platform: DefaultPlatform(ctx),
 	}
 	fs.SessionOpts = append(fs.SessionOpts, llbutil.WithSyncedDir(id, filesync.SyncedDir{
-		Name: id,
-		Dir:  localPath,
+		Name: localPath,
+		Dir:  localDir,
 		Map: func(_ string, st *fstypes.Stat) bool {
 			st.Uid = 0
 			st.Gid = 0
diff --git a/pkg/llbutil/id.go b/pkg/llbutil/id.go
index 5282218..e1bf16c 100644
--- a/pkg/llbutil/id.go
+++ b/pkg/llbutil/id.go
@@ -14,13 +14,13 @@ import (
 // LocalID returns a consistent hash for this local (path + options) so that
 // the same content doesn't get transported multiple times when referenced
 // repeatedly.
-func LocalID(ctx context.Context, cwd, path string, opts ...llb.LocalOption) (string, error) {
-	uniqID, err := LocalUniqueID(cwd)
+func LocalID(ctx context.Context, absPath string, opts ...llb.LocalOption) (string, error) {
+	uniqID, err := LocalUniqueID(absPath)
 	if err != nil {
 		return "", err
 	}
 	opts = append(opts, llb.LocalUniqueID(uniqID))
-	st := llb.Local(path, opts...)
+	st := llb.Local("", opts...)
 
 	def, err := st.Marshal(ctx)
 	if err != nil {
diff --git a/pkg/llbutil/session.go b/pkg/llbutil/session.go
index d423718..0083beb 100644
--- a/pkg/llbutil/session.go
+++ b/pkg/llbutil/session.go
@@ -110,6 +110,17 @@ func NewSession(ctx context.Context, opts ...SessionOption) (*session.Session, e
 		attachables = append(attachables, secretsprovider.NewSecretProvider(fileStore))
 	}
 
+	// SharedKey is empty because we already use `llb.SharedKeyHint` for locals.
+	//
+	// Currently, the only use of SharedKey is in the calculation of the cache key
+	// for local immutable ref in BuildKit. There isn't any functional difference
+	// between `llb.SharedKeyHint` and a session's shared key atm. If anything
+	// needs to start leveraging the session's shared key in the future, we
+	// should probably use the codegen.Session(ctx) session id.
+	//
+	// For now, locals also have `llb.LocalUniqueID` that introduces a random
+	// unique ID if a session isn't provided, so regardless of session shared key
+	// being provided or not, we still need to use `llb.SharedKeyHint`.
 	s, err := session.NewSession(ctx, "hlb", "")
 	if err != nil {
 		return s, err
diff --git a/solver/request.go b/solver/request.go
index e092bb4..2944505 100644
--- a/solver/request.go
+++ b/solver/request.go
@@ -11,11 +11,6 @@ import (
 	"golang.org/x/sync/errgroup"
 )
 
-const (
-	// LocalPathDescriptionKey is the key name in the metadata description map for the input path to a local fs.
-	LocalPathDescriptionKey = "hlb.local.path"
-)
-
 // Request is a node in the solve request tree produced by the compiler. The
 // solve request tree has peer nodes that should be executed in parallel, and
 // next nodes that should be executed sequentially. These can be intermingled


Test patch:
diff --git a/codegen/codegen_test.go b/codegen/codegen_test.go
index 7af65ee..974eaf4 100644
--- a/codegen/codegen_test.go
+++ b/codegen/codegen_test.go
@@ -2,8 +2,8 @@ package codegen_test
 
 import (
 	"context"
-	"fmt"
 	"os"
+	"path/filepath"
 	"strings"
 	"testing"
 	"time"
@@ -38,17 +38,18 @@ func Expect(t *testing.T, st llb.State, opts ...solver.SolveOption) solver.Reque
 }
 
 func LocalState(ctx context.Context, t *testing.T, localPath string, opts ...llb.LocalOption) llb.State {
-	cwd, err := local.Cwd(ctx)
-	require.NoError(t, err)
+	absPath := localPath
+	if !filepath.IsAbs(localPath) {
+		cwd, err := local.Cwd(ctx)
+		require.NoError(t, err)
 
-	id, err := llbutil.LocalID(ctx, cwd, ".", opts...)
+		absPath = filepath.Join(cwd, localPath)
+	}
+	id, err := llbutil.LocalID(ctx, absPath, opts...)
 	require.NoError(t, err)
 
 	opts = append([]llb.LocalOption{
-		llb.SharedKeyHint(localPath),
-		llb.WithDescription(map[string]string{
-			solver.LocalPathDescriptionKey: fmt.Sprintf("local://%s", localPath),
-		}),
+		llb.SharedKeyHint(id),
 	}, opts...)
 
 	sessionID := codegen.SessionID(ctx)
@@ -56,7 +57,7 @@ func LocalState(ctx context.Context, t *testing.T, localPath string, opts ...llb
 		opts = append(opts, llb.SessionID(sessionID))
 	}
 
-	return llb.Local(id, opts...)
+	return llb.Local(localPath, opts...)
 }
 
 type testCase struct {
@@ -380,7 +381,7 @@ func TestCodeGen(t *testing.T) {
 		}
 		`, "",
 		func(ctx context.Context, t *testing.T) solver.Request {
-			return Expect(t, LocalState(ctx, t, ".",
+			return Expect(t, LocalState(ctx, t, "codegen_test.go",
 				llb.IncludePatterns([]string{"codegen_test.go"}),
 			))
 		},
@@ -396,7 +397,7 @@ func TestCodeGen(t *testing.T) {
 		}
 		`, "",
 		func(ctx context.Context, t *testing.T) solver.Request {
-			return Expect(t, LocalState(ctx, t, ".",
+			return Expect(t, LocalState(ctx, t, "codegen_test.go",
 				llb.IncludePatterns([]string{"codegen_test.go"}),
 			))
 		},