Fix lakectl local diff (#7563)

treeverse · Mar 19, 2024 · 839f374 · 839f374
1 parent 6bea5f8
commit 839f374
Show file tree

Hide file tree

Showing 4 changed files with 36 additions and 8 deletions.
diff --git a/esti/lakectl_local_test.go b/esti/lakectl_local_test.go
@@ -437,18 +437,23 @@ func TestLakectlLocal_commit(t *testing.T) {
 			presign := fmt.Sprintf(" --pre-sign=%v ", tt.presign)
 			RunCmdAndVerifyContainsText(t, Lakectl()+" local clone lakefs://"+repoName+"/"+vars["BRANCH"]+"/"+vars["PREFIX"]+presign+dataDir, false, "Successfully cloned lakefs://${REPO}/${REF}/${PREFIX} to ${LOCAL_DIR}.", vars)
 
-			//relPath, err := filepath.Rel(tmpDir, dataDir)
-			//require.NoError(t, err)
-
 			RunCmdAndVerifyContainsText(t, Lakectl()+" local status "+dataDir, false, "No diff found", vars)
 
 			// Modify local folder - add and remove files
+			os.MkdirAll(filepath.Join(dataDir, "subdir"), os.ModePerm)
+			os.MkdirAll(filepath.Join(dataDir, "subdir-a"), os.ModePerm)
+			fd, err = os.Create(filepath.Join(dataDir, "subdir", "test.txt"))
+			require.NoError(t, err)
+			fd, err = os.Create(filepath.Join(dataDir, "subdir-a", "test.txt"))
+			require.NoError(t, err)
 			fd, err = os.Create(filepath.Join(dataDir, "test.txt"))
 			require.NoError(t, err)
 			require.NoError(t, fd.Close())
 			require.NoError(t, os.Remove(filepath.Join(dataDir, deleted)))
 
 			RunCmdAndVerifyContainsText(t, Lakectl()+" local status "+dataDir, false, "local  ║ added   ║ test.txt", vars)
+			RunCmdAndVerifyContainsText(t, Lakectl()+" local status "+dataDir, false, "local  ║ added   ║ subdir/test.txt", vars)
+			RunCmdAndVerifyContainsText(t, Lakectl()+" local status "+dataDir, false, "local  ║ added   ║ subdir-a/test.txt", vars)
 
 			// Commit changes to branch
 			RunCmdAndVerifyContainsText(t, Lakectl()+" local commit -m test"+presign+dataDir, false, "Commit for branch \"${BRANCH}\" completed", vars)

diff --git a/esti/lakectl_test.go b/esti/lakectl_test.go
@@ -557,7 +557,13 @@ func TestLakectlFsUpload(t *testing.T) {
 	})
 	t.Run("single_file_with_recursive", func(t *testing.T) {
 		vars["FILE_PATH"] = "data/ro/ro_1k.0"
-		RunCmdAndVerifyFailure(t, Lakectl()+" fs upload --recursive -s files/ro_1k lakefs://"+repoName+"/"+mainBranch+"/"+vars["FILE_PATH"], false, "\ndiff 'local://files/ro_1k/' <--> 'lakefs://${REPO}/${BRANCH}/${FILE_PATH}'...\nlstat files/ro_1k/: not a directory\nError executing command.\n", vars)
+		sanitizedResult := runCmd(t, Lakectl()+" fs upload --recursive -s files/ro_1k lakefs://"+repoName+"/"+mainBranch+"/"+vars["FILE_PATH"], false, false, vars)
+		require.Contains(t, sanitizedResult, "diff 'local://files/ro_1k/' <--> 'lakefs://"+repoName+"/"+mainBranch+"/"+vars["FILE_PATH"]+"'...")
+		require.Contains(t, sanitizedResult, "upload .")
+		require.Contains(t, sanitizedResult, "Upload Summary:")
+		require.Contains(t, sanitizedResult, "Downloaded: 0")
+		require.Contains(t, sanitizedResult, "Uploaded: 1")
+		require.Contains(t, sanitizedResult, "Removed: 0")
 	})
 	t.Run("dir", func(t *testing.T) {
 		vars["FILE_PATH"] = "data/ro/"

diff --git a/pkg/block/local/adapter.go b/pkg/block/local/adapter.go
@@ -558,7 +558,7 @@ func (l *Adapter) RuntimeStats() map[string]string {
 
 func VerifyAbsPath(absPath, adapterPath string, allowedPrefixes []string) error {
 	// check we have a valid abs path
-	if !path.IsAbs(absPath) || path.Clean(absPath) != absPath {
+	if !filepath.IsAbs(absPath) || filepath.Clean(absPath) != absPath {
 		return ErrBadPath
 	}
 	// point to storage namespace

diff --git a/pkg/local/diff.go b/pkg/local/diff.go
@@ -3,13 +3,16 @@ package local
 import (
 	"context"
 	"fmt"
-	"io/fs"
 	"net/http"
+	"net/url"
+	"os"
 	"path/filepath"
 	"strings"
 
 	"github.com/go-openapi/swag"
 	"github.com/treeverse/lakefs/pkg/api/apigen"
+	"github.com/treeverse/lakefs/pkg/block"
+	"github.com/treeverse/lakefs/pkg/block/local"
 	"github.com/treeverse/lakefs/pkg/uri"
 )
 
@@ -194,14 +197,28 @@ func DiffLocalWithHead(left <-chan apigen.ObjectStats, rightPath string) (Change
 		currentRemoteFile apigen.ObjectStats
 		hasMore           bool
 	)
-	err := filepath.Walk(rightPath, func(path string, info fs.FileInfo, err error) error {
+	absPath, err := filepath.Abs(rightPath)
+	if err != nil {
+		return nil, err
+	}
+	uri := url.URL{Scheme: "local", Path: absPath}
+	adapter, err := local.NewAdapter("", local.WithRemoveEmptyDir(false), local.WithAllowedExternalPrefixes([]string{absPath}))
+	if err != nil {
+		return nil, err
+	}
+	reader, err := adapter.GetWalker(&uri)
+	if err != nil {
+		return nil, err
+	}
+	err = reader.Walk(context.Background(), &uri, block.WalkOptions{}, func(e block.ObjectStoreEntry) error {
+		info, err := os.Stat(e.FullKey)
 		if err != nil {
 			return err
 		}
 		if info.IsDir() || diffShouldIgnore(info.Name()) {
 			return nil
 		}
-		localPath := strings.TrimPrefix(path, rightPath)
+		localPath := e.RelativeKey
 		localPath = strings.TrimPrefix(localPath, string(filepath.Separator))
 		localPath = filepath.ToSlash(localPath) // normalize to use "/" always