Repository: cockroachdb/pebble
Pull request #4276 (2025-01-21 22:43:42)

pebble-release-1.1: db: fix Open issue with single-item relative paths
Previously Open with a relative path containing a single element would fail on real filesystems, because Open would attempt to open the empty path.

Backport of #3847 for #4268.

cockroach-teamcity: 

<!-- Reviewable:start -->
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/cockroachdb/pebble/4276)
<!-- Reviewable:end -->

RaduBerinde: TFTR!

Patch:
diff --git a/checkpoint.go b/checkpoint.go
index ad70a354..6eb72fdb 100644
--- a/checkpoint.go
+++ b/checkpoint.go
@@ -88,23 +88,20 @@ func mkdirAllAndSyncParents(fs vfs.FS, destDir string) (vfs.File, error) {
 	// Collect paths for all directories between destDir (excluded) and its
 	// closest existing ancestor (included).
 	var parentPaths []string
-	foundExistingAncestor := false
-	for parentPath := fs.PathDir(destDir); parentPath != "."; parentPath = fs.PathDir(parentPath) {
+	for parentPath := fs.PathDir(destDir); ; parentPath = fs.PathDir(parentPath) {
 		parentPaths = append(parentPaths, parentPath)
+		if fs.PathDir(parentPath) == parentPath {
+			break
+		}
 		_, err := fs.Stat(parentPath)
 		if err == nil {
 			// Exit loop at the closest existing ancestor.
-			foundExistingAncestor = true
 			break
 		}
 		if !oserror.IsNotExist(err) {
 			return nil, err
 		}
 	}
-	// Handle empty filesystem edge case.
-	if !foundExistingAncestor {
-		parentPaths = append(parentPaths, "")
-	}
 	// Create destDir and any of its missing parents.
 	if err := fs.MkdirAll(destDir, 0755); err != nil {
 		return nil, err
diff --git a/vfs/mem_fs.go b/vfs/mem_fs.go
index ef6c87d7..b55b01f2 100644
--- a/vfs/mem_fs.go
+++ b/vfs/mem_fs.go
@@ -164,6 +164,9 @@ func (y *MemFS) walk(fullname string, f func(dir *memNode, frag string, final bo
 	for len(fullname) > 0 && fullname[0] == sep[0] {
 		fullname = fullname[1:]
 	}
+	if fullname == "." {
+		fullname = ""
+	}
 	dir := y.root
 
 	for {


Test patch:
diff --git a/open_test.go b/open_test.go
index 11002990..b74a378f 100644
--- a/open_test.go
+++ b/open_test.go
@@ -1388,3 +1388,49 @@ func TestOpenRatchetsNextFileNum(t *testing.T) {
 	require.NoError(t, d.Compact([]byte("a"), []byte("z"), false))
 
 }
+
+func TestMkdirAllAndSyncParents(t *testing.T) {
+	if filepath.Separator != '/' {
+		t.Skip("skipping due to path separator")
+	}
+	pwd, err := os.Getwd()
+	require.NoError(t, err)
+	defer func() { os.Chdir(pwd) }()
+
+	filesystems := map[string]vfs.FS{}
+	rootPaths := map[string]string{}
+	var buf bytes.Buffer
+	datadriven.RunTest(t, "testdata/mkdir_all_and_sync_parents", func(t *testing.T, td *datadriven.TestData) string {
+		buf.Reset()
+		switch td.Cmd {
+		case "mkfs":
+			var fsName string
+			td.ScanArgs(t, "fs", &fsName)
+			if td.HasArg("memfs") {
+				filesystems[fsName] = vfs.NewMem()
+				return "new memfs"
+			}
+			filesystems[fsName] = vfs.Default
+			rootPaths[fsName] = t.TempDir()
+			return "new default fs"
+		case "mkdir-all-and-sync-parents":
+			var fsName, path string
+			td.ScanArgs(t, "fs", &fsName)
+			td.ScanArgs(t, "path", &path)
+			if p, ok := rootPaths[fsName]; ok {
+				require.NoError(t, os.Chdir(p))
+			}
+			fs := vfs.WithLogging(filesystems[fsName], func(format string, args ...interface{}) {
+				fmt.Fprintf(&buf, format+"\n", args...)
+			})
+			f, err := mkdirAllAndSyncParents(fs, path)
+			if err != nil {
+				return err.Error()
+			}
+			require.NoError(t, f.Close())
+			return buf.String()
+		default:
+			return fmt.Sprintf("unrecognized command %q", td.Cmd)
+		}
+	})
+}
diff --git a/testdata/checkpoint b/testdata/checkpoint
index aa412b21..a48e5ece 100644
--- a/testdata/checkpoint
+++ b/testdata/checkpoint
@@ -141,9 +141,9 @@ mkdir-all: checkpoints/checkpoint1 0755
 open-dir: checkpoints
 sync: checkpoints
 close: checkpoints
-open-dir: 
-sync: 
-close: 
+open-dir: .
+sync: .
+close: .
 open-dir: checkpoints/checkpoint1
 link: db/OPTIONS-000003 -> checkpoints/checkpoint1/OPTIONS-000003
 open-dir: checkpoints/checkpoint1
diff --git a/testdata/event_listener b/testdata/event_listener
index 5d9b7916..dbda1472 100644
--- a/testdata/event_listener
+++ b/testdata/event_listener
@@ -409,9 +409,9 @@ sstables
 checkpoint
 ----
 mkdir-all: checkpoint 0755
-open-dir: 
-sync: 
-close: 
+open-dir: .
+sync: .
+close: .
 open-dir: checkpoint
 link: db/OPTIONS-000003 -> checkpoint/OPTIONS-000003
 open-dir: checkpoint
diff --git a/testdata/mkdir_all_and_sync_parents b/testdata/mkdir_all_and_sync_parents
new file mode 100644
index 00000000..b78b974a
--- /dev/null
+++ b/testdata/mkdir_all_and_sync_parents
@@ -0,0 +1,67 @@
+mkfs memfs fs=mem1
+----
+new memfs
+
+mkdir-all-and-sync-parents fs=mem1 path=foo/bar/baz/bax
+----
+mkdir-all: foo/bar/baz/bax 0755
+open-dir: foo/bar/baz
+sync: foo/bar/baz
+close: foo/bar/baz
+open-dir: foo/bar
+sync: foo/bar
+close: foo/bar
+open-dir: foo
+sync: foo
+close: foo
+open-dir: .
+sync: .
+close: .
+open-dir: foo/bar/baz/bax
+close: foo/bar/baz/bax
+
+# Repeating the same command should only sync the parent, and then the new data
+# directory itself.
+
+mkdir-all-and-sync-parents fs=mem1 path=foo/bar/baz/bax
+----
+mkdir-all: foo/bar/baz/bax 0755
+open-dir: foo/bar/baz
+sync: foo/bar/baz
+close: foo/bar/baz
+open-dir: foo/bar/baz/bax
+close: foo/bar/baz/bax
+
+mkfs fs=default1
+----
+new default fs
+
+mkdir-all-and-sync-parents fs=default1 path=foo/bar/baz/bax
+----
+mkdir-all: foo/bar/baz/bax 0755
+open-dir: foo/bar/baz
+sync: foo/bar/baz
+close: foo/bar/baz
+open-dir: foo/bar
+sync: foo/bar
+close: foo/bar
+open-dir: foo
+sync: foo
+close: foo
+open-dir: .
+sync: .
+close: .
+open-dir: foo/bar/baz/bax
+close: foo/bar/baz/bax
+
+# Repeating the same command should only sync the parent, and then the new data
+# directory itself.
+
+mkdir-all-and-sync-parents fs=default1 path=foo/bar/baz/bax
+----
+mkdir-all: foo/bar/baz/bax 0755
+open-dir: foo/bar/baz
+sync: foo/bar/baz
+close: foo/bar/baz
+open-dir: foo/bar/baz/bax
+close: foo/bar/baz/bax