git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[WIP RFC PATCH 5/7] clone: use dir-iterator to avoid explicit dir traversal

From
Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date
Feb 26, 2019, 00:26 UTC
Message-ID
<20190226002625.13022-6-avarab@gmail.com>
In-Reply-To
<CACsJy8Bw-mXLyT7VaKBjYKLe6uSAQqyLG=_AzjDXykYzSQLkcg@mail.gmail.com>
From: Matheus Tavares <matheus.bernardino@usp.br>

Replace usage of opendir/readdir/closedir API to traverse directories recursively, at copy_or_link_directory function, by the dir-iterator API. This simplifies the code and avoid recursive calls to copy_or_link_directory.

[Ævar: This should be bug-compatible with the existing "clone" behavior. The whole bit here with "iter->relative_path[0] == '.'" is a dirty hack. We don't copy dot-dirs, and then later on just blindly ignore ENOENT errors as we descend into them. That case really wants to be a is_dotdir_or_file_within() test instead]

Now, copy_or_link_directory will call die() in case of an error on openddir, readdir or lstat, inside dir_iterator_advance. That means it will abort in case of an error trying to fetch any iteration entry.

Signed-off-by: Matheus Tavares <matheus.bernardino@usp.br>
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 builtin/clone.c | 55 +++++++++++++++++++++++++++++--------------------
 1 file changed, 33 insertions(+), 22 deletions(-)
diff --git a/builtin/clone.c b/builtin/clone.c
index 862d2ea69c..c32e9022b3 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -23,6 +23,8 @@
 #include "transport.h"
 #include "strbuf.h"
 #include "dir.h"
+#include "dir-iterator.h"
+#include "iterator.h"
 #include "sigchain.h"
 #include "branch.h"
 #include "remote.h"
@@ -411,42 +413,47 @@ static void mkdir_if_missing(const char *pathname, mode_t mode)
 }
 
 static void copy_or_link_directory(struct strbuf *src, struct strbuf *dest,
-				   const char *src_repo, int src_baselen)
+				   const char *src_repo)
 {
-	struct dirent *de;
-	struct stat buf;
 	int src_len, dest_len;
-	DIR *dir;
-
-	dir = opendir(src->buf);
-	if (!dir)
-		die_errno(_("failed to open '%s'"), src->buf);
+	struct dir_iterator *iter;
+	int iter_status;
+	struct stat st;
 
 	mkdir_if_missing(dest->buf, 0777);
 
+	iter = dir_iterator_begin(src->buf, 1);
+
 	strbuf_addch(src, '/');
 	src_len = src->len;
 	strbuf_addch(dest, '/');
 	dest_len = dest->len;
 
-	while ((de = readdir(dir)) != NULL) {
+	while ((iter_status = dir_iterator_advance(iter)) == ITER_OK) {
 		strbuf_setlen(src, src_len);
-		strbuf_addstr(src, de->d_name);
+		strbuf_addstr(src, iter->relative_path);
 		strbuf_setlen(dest, dest_len);
-		strbuf_addstr(dest, de->d_name);
-		if (stat(src->buf, &buf)) {
+		strbuf_addstr(dest, iter->relative_path);
+
+		/*
+		 * dir_iterator_advance already calls lstat to populate iter->st
+		 * but, unlike stat, lstat does not checks for permissions on
+		 * the given path.
+		 */
+		if (stat(src->buf, &st)) {
 			warning (_("failed to stat %s\n"), src->buf);
 			continue;
 		}
-		if (S_ISDIR(buf.st_mode)) {
-			if (de->d_name[0] != '.')
-				copy_or_link_directory(src, dest,
-						       src_repo, src_baselen);
+
+		if (S_ISDIR(iter->st.st_mode)) {
+			if (iter->relative_path[0] == '.')
+				continue;
+			mkdir_if_missing(dest->buf, 0777);
 			continue;
 		}
 
 		/* Files that cannot be copied bit-for-bit... */
-		if (!strcmp(src->buf + src_baselen, "/info/alternates")) {
+		if (!strcmp(iter->relative_path, "info/alternates")) {
 			copy_alternates(src, dest, src_repo);
 			continue;
 		}
@@ -456,14 +463,18 @@ static void copy_or_link_directory(struct strbuf *src, struct strbuf *dest,
 		if (!option_no_hardlinks) {
 			if (!link(src->buf, dest->buf))
 				continue;
-			if (option_local > 0)
-				die_errno(_("failed to create link '%s'"), dest->buf);
+			if (option_local > 0 && errno != ENOENT)
+				warning_errno(_("failed to create link '%s'"), dest->buf);
 			option_no_hardlinks = 1;
 		}
-		if (copy_file_with_time(dest->buf, src->buf, 0666))
+		if (copy_file_with_time(dest->buf, src->buf, 0666) && errno != ENOENT)
 			die_errno(_("failed to copy file to '%s'"), dest->buf);
 	}
-	closedir(dir);
+
+	if (iter_status != ITER_DONE) {
+		strbuf_setlen(src, src_len);
+		die(_("failed to iterate over '%s'"), src->buf);
+	}
 }
 
 static void clone_local(const char *src_repo, const char *dest_repo)
@@ -481,7 +492,7 @@ static void clone_local(const char *src_repo, const char *dest_repo)
 		get_common_dir(&dest, dest_repo);
 		strbuf_addstr(&src, "/objects");
 		strbuf_addstr(&dest, "/objects");
-		copy_or_link_directory(&src, &dest, src_repo, src.len);
+		copy_or_link_directory(&src, &dest, src_repo);
 		strbuf_release(&src);
 		strbuf_release(&dest);
 	}
-- 
2.21.0.rc2.1.g2d5e20a900.dirty
Previous: Ævar Arnfjörð BjarmasonNext: Matheus Tavares Bernardino
Message 22 of 35 in “clone: convert explicit dir traversal to dir-iterator”
  1. Matheus TavaresFeb 23, 2019
  2. [GSoC][PATCH 1/3] dir-iterator: add pedantic option to dir_iterator_beginMatheus Tavares, Feb 23, 2019
  3. Thomas GummererFeb 23, 2019
  4. Christian CouderFeb 24, 2019
  5. Matheus Tavares BernardinoFeb 24, 2019
  6. Thomas GummererFeb 24, 2019
  7. [GSoC][PATCH 2/3] clone: extract function from copy_or_link_directoryMatheus Tavares, Feb 23, 2019
  8. Christian CouderFeb 24, 2019
  9. [GSoC][PATCH 3/3] clone: use dir-iterator to avoid explicit dir traversalMatheus Tavares, Feb 23, 2019
  10. Thomas GummererFeb 23, 2019
  11. Matheus Tavares BernardinoFeb 24, 2019
  12. Ævar Arnfjörð BjarmasonFeb 23, 2019
  13. Christian CouderFeb 24, 2019
  14. Ævar Arnfjörð BjarmasonFeb 24, 2019
  15. Duy NguyenFeb 25, 2019
  16. 0/7 clone: dir iterator refactoring with testsÆvar Arnfjörð Bjarmason, Feb 26, 2019
  17. 1/7 dir-iterator: add pedantic option to dir_iterator_beginÆvar Arnfjörð Bjarmason, Feb 26, 2019
  18. 2/7 dir-iterator: use stat() instead of lstat()Ævar Arnfjörð Bjarmason, Feb 26, 2019
  19. Matheus Tavares BernardinoFeb 26, 2019
  20. 3/7 clone: extract function from copy_or_link_directoryÆvar Arnfjörð Bjarmason, Feb 26, 2019
  21. 4/7 clone: test for our behavior on odd objects/* contentÆvar Arnfjörð Bjarmason, Feb 26, 2019
  22. 5/7 clone: use dir-iterator to avoid explicit dir traversalÆvar Arnfjörð Bjarmason, Feb 26, 2019
  23. Matheus Tavares BernardinoFeb 26, 2019
  24. Ævar Arnfjörð BjarmasonFeb 26, 2019
  25. 7/7 clone: break cloning repos that have symlinks in themÆvar Arnfjörð Bjarmason, Feb 26, 2019
  26. 6/7 clone: stop ignoring dotdirs in --local etc. cloneÆvar Arnfjörð Bjarmason, Feb 26, 2019
  27. Matheus Tavares BernardinoFeb 25, 2019
  28. Ævar Arnfjörð BjarmasonFeb 25, 2019
  29. Christian CouderFeb 25, 2019
  30. Christian CouderFeb 26, 2019
  31. Matheus Tavares BernardinoFeb 23, 2019
  32. Ævar Arnfjörð BjarmasonFeb 23, 2019
  33. Thomas GummererFeb 23, 2019
  34. Matheus Tavares BernardinoFeb 24, 2019
  35. Thomas GummererFeb 24, 2019

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.