git: fix subdir filter on submodule dir

Because subdir filter happened before the submodule update it
resulted in empty directory being filtered and submodule update
being skipped because .gitmodules was already missing.

Signed-off-by: Tonis Tiigi <[email protected]>

Author: tonistiigi

source/git/source.go

@@ -567,6 +567,7 @@ func (gs *gitSourceHandler) Snapshot(ctx context.Context, g session.Group) (out
 		}
 	}
 
+	cd := checkoutDir
 	if gs.src.KeepGitDir && subdir == "." {
 		checkoutDirGit := filepath.Join(checkoutDir, ".git")
 		if err := os.MkdirAll(checkoutDir, 0711); err != nil {
@@ -624,7 +625,6 @@ func (gs *gitSourceHandler) Snapshot(ctx context.Context, g session.Group) (out
 		}
 		gitDir = checkoutDirGit
 	} else {
-		cd := checkoutDir
 		if subdir != "." {
 			cd, err = os.MkdirTemp(cd, "checkout")
 			if err != nil {
@@ -636,41 +636,42 @@ func (gs *gitSourceHandler) Snapshot(ctx context.Context, g session.Group) (out
 		if err != nil {
 			return nil, errors.Wrapf(err, "failed to checkout remote %s", urlutil.RedactCredentials(gs.src.Remote))
 		}
-		if subdir != "." {
-			d, err := os.Open(filepath.Join(cd, subdir))
-			if err != nil {
-				return nil, errors.Wrapf(err, "failed to open subdir %v", subdir)
-			}
-			defer func() {
-				if d != nil {
-					d.Close()
-				}
-			}()
-			names, err := d.Readdirnames(0)
-			if err != nil {
-				return nil, err
-			}
-			for _, n := range names {
-				if err := os.Rename(filepath.Join(cd, subdir, n), filepath.Join(checkoutDir, n)); err != nil {
-					return nil, err
-				}
-			}
-			if err := d.Close(); err != nil {
-				return nil, err
-			}
-			d = nil // reset defer
-			if err := os.RemoveAll(cd); err != nil {
-				return nil, err
-			}
-		}
 	}
 
-	git = git.New(gitutil.WithWorkTree(checkoutDir), gitutil.WithGitDir(gitDir))
+	git = git.New(gitutil.WithWorkTree(cd), gitutil.WithGitDir(gitDir))
 	_, err = git.Run(ctx, "submodule", "update", "--init", "--recursive", "--depth=1")
 	if err != nil {
 		return nil, errors.Wrapf(err, "failed to update submodules for %s", urlutil.RedactCredentials(gs.src.Remote))
 	}
 
+	if subdir != "." {
+		d, err := os.Open(filepath.Join(cd, subdir))
+		if err != nil {
+			return nil, errors.Wrapf(err, "failed to open subdir %v", subdir)
+		}
+		defer func() {
+			if d != nil {
+				d.Close()
+			}
+		}()
+		names, err := d.Readdirnames(0)
+		if err != nil {
+			return nil, err
+		}
+		for _, n := range names {
+			if err := os.Rename(filepath.Join(cd, subdir, n), filepath.Join(checkoutDir, n)); err != nil {
+				return nil, err
+			}
+		}
+		if err := d.Close(); err != nil {
+			return nil, err
+		}
+		d = nil // reset defer
+		if err := os.RemoveAll(cd); err != nil {
+			return nil, err
+		}
+	}
+
 	if idmap := mount.IdentityMapping(); idmap != nil {
 		uid, gid := idmap.RootPair()
 		err := filepath.WalkDir(gitDir, func(p string, _ os.DirEntry, _ error) error {

source/git/source_test.go

@@ -733,6 +733,57 @@ func TestCredentialRedaction(t *testing.T) {
 	require.NotContains(t, err.Error(), "keepthissecret")
 }
 
+func TestSubmoduleSubdir(t *testing.T) {
+	testSubmoduleSubdir(t, false)
+}
+
+func TestSubmoduleSubdirKeepGitDir(t *testing.T) {
+	testSubmoduleSubdir(t, true)
+}
+
+func testSubmoduleSubdir(t *testing.T, keepGitDir bool) {
+	if runtime.GOOS == "windows" {
+		t.Skip("Depends on unimplemented containerd bind-mount support on Windows")
+	}
+	t.Parallel()
+	ctx := namespaces.WithNamespace(context.Background(), "buildkit-test")
+	ctx = logProgressStreams(ctx, t)
+
+	gs := setupGitSource(t, t.TempDir())
+
+	repo := setupGitRepo(t)
+
+	id := &GitIdentifier{Remote: repo.mainURL, KeepGitDir: keepGitDir, Ref: "feature", Subdir: "sub"}
+
+	g, err := gs.Resolve(ctx, id, nil, nil)
+	require.NoError(t, err)
+
+	key1, pin1, _, done, err := g.CacheKey(ctx, nil, 0)
+	require.NoError(t, err)
+	require.True(t, done)
+
+	expLen := 44
+	require.GreaterOrEqual(t, len(key1), expLen)
+	require.Equal(t, 40, len(pin1))
+
+	ref1, err := g.Snapshot(ctx, nil)
+	require.NoError(t, err)
+	defer ref1.Release(context.TODO())
+
+	mount, err := ref1.Mount(ctx, true, nil)
+	require.NoError(t, err)
+
+	lm := snapshot.LocalMounter(mount)
+	dir, err := lm.Mount()
+	require.NoError(t, err)
+	defer lm.Unmount()
+
+	dt, err := os.ReadFile(filepath.Join(dir, "subfile"))
+	require.NoError(t, err)
+
+	require.Equal(t, "subcontents\n", string(dt))
+}
+
 func TestSubdir(t *testing.T) {
 	testSubdir(t, false)
 }