Merge 8.7 - Have zipfs claim all paths under zipfs root. See [93eb737…

…84a].
tcltk · Oct 8, 2023 · 39b45eb · 39b45eb
2 parents 70a5197 + cacba66
commit 39b45eb
Show file tree

Hide file tree

Showing 3 changed files with 15 additions and 94 deletions.
diff --git a/doc/zipfs.n b/doc/zipfs.n
@@ -141,9 +141,7 @@ on most platforms.
 \fBzipfs unmount \fImountpoint\fR
 .
 Unmounts a previously mounted ZIP archive mounted to \fImountpoint\fR.
-If the current directory is located within the mounted archive,
-the directory that was previously the current directory is restored
-on the unmount. The command will fail with an error exception if
+The command will fail with an error exception if
 there are any files within the mounted archive are open.
 .SS "ZIP CREATION COMMANDS"
 This package also provides several commands to aid the creation of ZIP

diff --git a/generic/tclZipfs.c b/generic/tclZipfs.c
@@ -5619,9 +5619,9 @@ ZipFSMatchInDirectoryProc(
     }
     if (dirOnly) {
 	/*
-	 * Not found in hash. May be a path that is the ancestor of a mount.
-	 * e.g. glob //zipfs:/a/? with mount at //zipfs:/a/b/c. Also have
-	 * to be careful about duplicates, such as when another mount is
+	 * Also check paths that are ancestors of a mount. e.g. glob
+	 * //zipfs:/a/? with mount at //zipfs:/a/b/c. Also have to be
+	 * careful about duplicates, such as when another mount is
 	 * //zipfs:/a/b/d
 	 */
 	Tcl_DString ds;
@@ -5770,8 +5770,6 @@ ZipFSPathInFilesystemProc(
     Tcl_Obj *pathPtr,
     TCL_UNUSED(void **))
 {
-    Tcl_HashEntry *hPtr;
-    Tcl_HashSearch search;
     Tcl_Size len;
     char *path;
 
@@ -5780,92 +5778,13 @@ ZipFSPathInFilesystemProc(
 	return -1;
     }
     path = Tcl_GetStringFromObj(pathPtr, &len);
-    /*
-     * TODO - why not make ZIPFS_VOLUME both necessary AND sufficient?
-     * Currently we only claim ownership if there is a matching mount.
-     */
-    if (strncmp(path, ZIPFS_VOLUME, ZIPFS_VOLUME_LEN) != 0) {
-	return -1;
-    } else if (len == ZIPFS_VOLUME_LEN && ZipFS.zipHash.numEntries != 0) {
-	/* zipfs root and at least one entry */
-	return TCL_OK;
-    }
-
-    int ret = TCL_OK;
-
-    ReadLock();
-    hPtr = Tcl_FindHashEntry(&ZipFS.fileHash, path);
-    if (hPtr) {
-	goto endloop;
-    }
 
     /*
-     * Not in hash table but still could be owned by zipfs in two other cases:
-     * Assuming there is a mount point //zipfs:/a/b/c,
-     *  1. The path is under the mount point, e.g. //zipfs:/a/b/c/f but that
-     *     file does not exist.
-     *  2. The path is an intermediate directory in a mount point, e.g.
-     *     //zipfs:/a/b
+     * Claim any path under ZIPFS_VOLUME as ours. This is both a necessary
+     * and sufficient condition as zipfs mounts at arbitrary paths are
+     * not permitted (unlike Androwish).
      */
-
-    for (hPtr = Tcl_FirstHashEntry(&ZipFS.zipHash, &search); hPtr;
-	    hPtr = Tcl_NextHashEntry(&search)) {
-	ZipFile *zf = (ZipFile *) Tcl_GetHashValue(hPtr);
-
-	if (zf->mountPointLen == 0) {
-	    /*
-	     * Mounted on the root (/)
-	     * TODO - a holdover from androwish? Tcl does not allow mounting
-	     * outside of the //zipfs:/ area.
-	     */
-	    ZipEntry *z;
-
-	    for (z = zf->topEnts; z != NULL; z = z->tnext) {
-		if (strncmp(path, z->name, len) == 0) {
-		    int lenz = (int)strlen(z->name);
-		    if (len == lenz) {
-			/* Would have been in hash table? But nm ... */
-			goto endloop;
-		    } else if (len > lenz) {
-			/* Case 1 above */
-			if (path[lenz] == '/') {
-			    goto endloop;
-			}
-		    } else { /* len < lenz */
-			/* Case 2 above */
-			if (z->name[len] == '/') {
-			    goto endloop;
-			}
-		    }
-		}
-	    }
-	} else {
-	    /* Not mounted on root - the norm in Tcl core */
-
-	    /* Lengths are known so check them before strnmp for efficiency*/
-	    assert(len != ZIPFS_VOLUME_LEN); /* Else already handled at top */
-	    if (len == zf->mountPointLen) {
-		/* A non-root or root mount. */
-		goto endloop;
-	    } else if (len > zf->mountPointLen) {
-		/* Case 1 above */
-		if (path[zf->mountPointLen] == '/' &&
-		strncmp(path, zf->mountPoint, zf->mountPointLen) == 0) {
-		    goto endloop;
-		}
-	    } else { /* len < zf->mountPointLen */
-		if (zf->mountPoint[len] == '/' &&
-		strncmp(path, zf->mountPoint, len) == 0) {
-		    goto endloop;
-		}
-	    }
-	}
-    }
-    ret = -1; /* Not our file */
-
-  endloop:
-    Unlock();
-    return ret;
+    return strncmp(path, ZIPFS_VOLUME, ZIPFS_VOLUME_LEN) ? -1 : TCL_OK;
 }
 
 /*

diff --git a/tests/zipfs.test b/tests/zipfs.test
@@ -636,13 +636,17 @@ namespace eval test_ns_zipfs {
     } -result {filesystem is busy} -returnCodes error
 
     test zipfs-unmount-3 "Unmount mount with current directory" -setup {
+        set cwd [pwd]
         mount [zippath test.zip]
     } -cleanup {
+        cd $cwd
         cleanup
     } -body {
-        set cwd [pwd]
-        cd [file join $defMountPt testdir]
-        list [pwd] [zipfs unmount $defMountPt] [string equal [pwd] $cwd]
+        # Current directory does not change on unmount.
+        # This is the same behavior as when USB pen drive is unmounted
+        set cwd2 [file join $defMountPt testdir]
+        cd $cwd2
+        list [pwd] [zipfs unmount $defMountPt] [string equal [pwd] $cwd2]
     } -result [list [file join $defMountPt testdir] {} 1]
 
     test zipfs-unmount-nested-1 "unmount parent of nested mount on new directory should not affect nested mount" -setup {