"GOT", but the "O" is a cute, smiling pufferfish. Index | Thread | Search

From:
Stefan Sperling <stsp@stsp.name>
Subject:
ugly hack for repositories on NFS
To:
gameoftrees@openbsd.org
Date:
Tue, 21 Jul 2020 15:04:22 +0200

Download raw body.

Thread
While viewing a repository mounted read-only over NFS with tog(1)
I noticed that tog wasn't showing any branch labels next to commits.

It turned out that gather_on_disk_refs() failed to traverse the refs/
hierarchy because on NFS readdir(3) returns entries with type DT_UNKNOWN
rather than DT_DIR, DT_REG, etc.

The problem can be prevented by passing -l to mount_nfs(8) which enables
"readdir plus" RPC. But I think it would be good if Got worked out of the
box on filesystems which return DT_UNKNOWN.

After some experimentation I found that open(2) with O_DIRECTORY|O_NOFOLLOW
will tell me whether a path contains a directory. This is probably the best
solution in case an open file descriptor is needed. But it's not ideal
for traversing a tree hierarchy. Using O_NOFOLLOW and ELOOP would perhaps
allow detection of symlinks. But I'm not sure how to tell all the other
file types (e.g. block, fifo, socket) apart from regular files with open(2).

Then I found out that lstat(2) seems to be returning proper types over NFS.
I ended up with this hack which maps the type returned by lstat(2) back
to DT_ dirent style types. Is this an acceptable workaround?
It is racy, but then again this is about NFS... ;-)

Any better ideas?

diff 4da1bbe9cccd432c3afb675e07eedc58bc12ae31 /home/stsp/src/got
blob - 57b823a4f04403e0dfd1d1e79cc8fe80f6dbc79c
file + include/got_path.h
--- include/got_path.h
+++ include/got_path.h
@@ -21,6 +21,8 @@
 #define GOT_DEFAULT_DIR_MODE	(S_IFDIR | \
 	S_IRWXU | S_IRGRP|S_IXGRP | S_IROTH|S_IXOTH)
 
+struct dirent;
+
 /* Determine whether a path is an absolute path. */
 int got_path_is_absolute(const char *);
 
@@ -101,6 +103,20 @@ int got_path_dir_is_empty(const char *);
 
 /* dirname(3) with error handling and dynamically allocated result. */
 const struct got_error *got_path_dirname(char **, const char *);
+
+/*
+ * Obtain the file type of a given directory entry.
+ *
+ * If the entry has some type other than DT_UNKNOWN, resolve to this type.
+ *
+ * Otherwise, attempt to resolve the type of a DT_UNKNOWN directory entry
+ * with lstat(2). If this fails, the result may still be DT_UNKNOWN.
+ * This is a racy fallback to accommodate filesystems which do not provide
+ * directory entry type information.
+ * DT_UNKNOWN directory entries occur on NFS mounts without "readdir plus" RPC.
+ */
+const struct got_error *got_path_dirent_type(int *, const char *,
+    struct dirent *);
 
 /* basename(3) with dynamically allocated result. */
 const struct got_error *got_path_basename(char **, const char *);
blob - 656685591bdfde81e5346507229c0bd87d2fa18c
file + lib/fileindex.c
--- lib/fileindex.c
+++ lib/fileindex.c
@@ -933,10 +933,23 @@ walk_dir(struct got_pathlist_entry **next, struct got_
 	struct dirent *de = dle->data;
 	DIR *subdir = NULL;
 	int subdirfd = -1;
+	int type;
 
 	*next = NULL;
 
-	if (de->d_type == DT_DIR) {
+	if (de->d_type == DT_UNKNOWN) {
+		/* Occurs on NFS mounts without "readdir plus" RPC. */
+		char *dir_path;
+		if (asprintf(&dir_path, "%s/%s", rootpath, path) == -1)
+			return got_error_from_errno("asprintf");
+		err = got_path_dirent_type(&type, dir_path, de);
+		free(dir_path);
+		if (err)
+			return err;
+	} else
+		type = de->d_type;
+
+	if (type == DT_DIR) {
 		char *subpath;
 		char *subdirpath;
 		struct got_pathlist_head subdirlist;
blob - be971e697a2b76f0479272ad328aa87e1fe8450d
file + lib/path.c
--- lib/path.c
+++ lib/path.c
@@ -375,6 +375,53 @@ got_path_dirname(char **parent, const char *path)
 }
 
 const struct got_error *
+got_path_dirent_type(int *type, const char *path_parent, struct dirent *dent)
+{
+	const struct got_error *err = NULL;
+	char *path_child;
+	struct stat sb;
+
+	if (dent->d_type != DT_UNKNOWN) {
+		*type = dent->d_type;
+		return NULL;
+	}
+
+	*type = DT_UNKNOWN;
+
+	/*
+	 * This is a racy fallback to accommodate filesystems which do not
+	 * provide directory entry type information. DT_UNKNOWN directory
+	 * entries occur on NFS mounts without "readdir plus" RPC.
+	 */
+
+	if (asprintf(&path_child, "%s/%s", path_parent, dent->d_name) == -1)
+		return got_error_from_errno("asprintf");
+
+	if (lstat(path_child, &sb) == -1) {
+		err = got_error_from_errno2("lstat", path_child);
+		goto done;
+	}
+
+	if (S_ISFIFO(sb.st_mode))
+		*type = DT_FIFO;
+	else if (S_ISCHR(sb.st_mode))
+		*type = DT_CHR;
+	else if (S_ISDIR(sb.st_mode))
+		*type = DT_DIR;
+	else if (S_ISBLK(sb.st_mode))
+		*type = DT_BLK;
+	else if (S_ISLNK(sb.st_mode))
+		*type = DT_LNK;
+	else if (S_ISREG(sb.st_mode))
+		*type = DT_REG;
+	else if (S_ISSOCK(sb.st_mode))
+		*type = DT_SOCK;
+done:
+	free(path_child);
+	return err;
+}
+
+const struct got_error *
 got_path_basename(char **s, const char *path)
 {
 	char *base;
blob - a4aa416aa740d7f102a9b038776457ef3096c1e4
file + lib/reference.c
--- lib/reference.c
+++ lib/reference.c
@@ -826,6 +826,7 @@ gather_on_disk_refs(struct got_reflist_head *refs, con
 		struct dirent *dent;
 		struct got_reference *ref;
 		char *child;
+		int type;
 
 		dent = readdir(d);
 		if (dent == NULL)
@@ -835,7 +836,11 @@ gather_on_disk_refs(struct got_reflist_head *refs, con
 		    strcmp(dent->d_name, "..") == 0)
 			continue;
 
-		switch (dent->d_type) {
+		err = got_path_dirent_type(&type, path_subdir, dent);
+		if (err)
+			break;
+
+		switch (type) {
 		case DT_REG:
 			err = open_ref(&ref, path_refs, subdir, dent->d_name,
 			    0);
blob - 7531c15dc53560bdff8f322c2b1f6f3c61d849f4
file + lib/repository.c
--- lib/repository.c
+++ lib/repository.c
@@ -1579,6 +1579,7 @@ write_tree(struct got_object_id **new_tree_id, const c
 	nentries = 0;
 	while ((de = readdir(dir)) != NULL) {
 		int ignore = 0;
+		int type;
 
 		if (strcmp(de->d_name, ".") == 0 ||
 		    strcmp(de->d_name, "..") == 0)
@@ -1592,7 +1593,12 @@ write_tree(struct got_object_id **new_tree_id, const c
 		}
 		if (ignore)
 			continue;
-		if (de->d_type == DT_DIR) {
+
+		err = got_path_dirent_type(&type, path_dir, de);
+		if (err)
+			goto done;
+
+		if (type == DT_DIR) {
 			err = import_subdir(&new_te, de, path_dir,
 			    ignores, repo, progress_cb, progress_arg);
 			if (err) {
@@ -1601,7 +1607,7 @@ write_tree(struct got_object_id **new_tree_id, const c
 				err = NULL;
 				continue;
 			}
-		} else if (de->d_type == DT_REG) {
+		} else if (type == DT_REG) {
 			err = import_file(&new_te, de, path_dir, repo);
 			if (err)
 				goto done;