Junio C Hamano [off-list ref] writes:
quoted hunk
Johan Herland [off-list ref] writes:
quoted
I'm torn about the new remove_everything_inside_dir(). Obviously it's a
copy-paste-modify of dir.c:remove_dir_recursively(), and could instead be
implemented by adding an extra flag to remove_dir_recursively(). However,
adding a "#define REMOVE_DIR_CONTENTS_BUT_NOT_DIR_ITSELF 04" seemed even
uglier to me...
Hmm, what ugliness am I missing when viewing the attached patch? It looks
simple and straightforward enough, at least to me.
dir.c | 14 ++++++++++----
dir.h | 1 +
2 files changed, 11 insertions(+), 4 deletions(-)
diff --git a/dir.c b/dir.c
index 0a78d00..6432728 100644
--- a/dir.c
+++ b/dir.c
@@ -1178,6 +1178,7 @@ int remove_dir_recursively(struct strbuf *path, int flag)
struct dirent *e;
int ret = 0, original_len = path->len, len;
int only_empty = (flag & REMOVE_DIR_EMPTY_ONLY);
+ int keep_toplevel = (flag & REMOVE_DIR_KEEP_TOPLEVEL);
unsigned char submodule_head[20];
if ((flag & REMOVE_DIR_KEEP_NESTED_GIT) &&
@@ -1185,9 +1186,14 @@ int remove_dir_recursively(struct strbuf *path, int flag)
/* Do not descend and nuke a nested git work tree. */
return 0;
+ flag &= ~REMOVE_DIR_KEEP_TOPLEVEL;
Nit. This needs to drop REMOVE_DIR_KEEP_NESTED_GIT as well in order to
preserve the current behaviour.
I actually suspect that the passing of "only_empty" in the original may be
a bug in a0f4afb (clean: require double -f options to nuke nested git
repository and work tree, 2009-06-30), and this patch might be a fix to
the bug, but I didn't think things through, and it is getting late, so...
quoted hunk
dir = opendir(path->buf);
- if (!dir)
- return rmdir(path->buf);
+ if (!dir) {
+ if (!keep_toplevel)
+ return rmdir(path->buf);
+ else
+ return -1;
+ }
if (path->buf[original_len - 1] != '/')
strbuf_addch(path, '/');
@@ -1202,7 +1208,7 @@ int remove_dir_recursively(struct strbuf *path, int flag)
if (lstat(path->buf, &st))
; /* fall thru */
else if (S_ISDIR(st.st_mode)) {
- if (!remove_dir_recursively(path, only_empty))
+ if (!remove_dir_recursively(path, flag))
continue; /* happy */
} else if (!only_empty && !unlink(path->buf))
continue; /* happy, too */
On Thu, Mar 15, 2012 at 08:16, Junio C Hamano [off-list ref] wrote:
Junio C Hamano [off-list ref] writes:
quoted
Johan Herland [off-list ref] writes:
quoted
I'm torn about the new remove_everything_inside_dir(). Obviously it's a
copy-paste-modify of dir.c:remove_dir_recursively(), and could instead be
implemented by adding an extra flag to remove_dir_recursively(). However,
adding a "#define REMOVE_DIR_CONTENTS_BUT_NOT_DIR_ITSELF 04" seemed even
uglier to me...
Hmm, what ugliness am I missing when viewing the attached patch? It looks
simple and straightforward enough, at least to me.
Agreed, you found a much more palatable name than I did. The patch
below looks good to me, and should become patch #3 in this series,
with my "3/2" patch being adjusted accordingly and becoming patch #4.
Do you want me to send the whole series again, or is it easier for you
to simply fix it up yourself?
quoted
dir.c | 14 ++++++++++----
dir.h | 1 +
2 files changed, 11 insertions(+), 4 deletions(-)
diff --git a/dir.c b/dir.c
index 0a78d00..6432728 100644
--- a/dir.c
+++ b/dir.c
@@ -1178,6 +1178,7 @@ int remove_dir_recursively(struct strbuf *path, int flag)
struct dirent *e;
int ret = 0, original_len = path->len, len;
int only_empty = (flag & REMOVE_DIR_EMPTY_ONLY);
+ int keep_toplevel = (flag & REMOVE_DIR_KEEP_TOPLEVEL);
unsigned char submodule_head[20];
if ((flag & REMOVE_DIR_KEEP_NESTED_GIT) &&
@@ -1185,9 +1186,14 @@ int remove_dir_recursively(struct strbuf *path, int flag)
/* Do not descend and nuke a nested git work tree. */
return 0;
+ flag &= ~REMOVE_DIR_KEEP_TOPLEVEL;
Nit. This needs to drop REMOVE_DIR_KEEP_NESTED_GIT as well in order to
preserve the current behaviour.
I actually suspect that the passing of "only_empty" in the original may be
a bug in a0f4afb (clean: require double -f options to nuke nested git
repository and work tree, 2009-06-30), and this patch might be a fix to
the bug, but I didn't think things through, and it is getting late, so...
I noticed the same while looking at this function, and I think your
analysis is correct. As it stands, REMOVE_DIR_KEEP_NESTED_GIT only
applies to .git folders located directly in the toplevel dir, and not
inside a subdirectory. That strikes me as odd given the name of the
flag.
Have fun! :)
...Johan
quoted
dir = opendir(path->buf);
- if (!dir)
- return rmdir(path->buf);
+ if (!dir) {
+ if (!keep_toplevel)
+ return rmdir(path->buf);
+ else
+ return -1;
+ }
if (path->buf[original_len - 1] != '/')
strbuf_addch(path, '/');
@@ -1202,7 +1208,7 @@ int remove_dir_recursively(struct strbuf *path, int flag)
if (lstat(path->buf, &st))
; /* fall thru */
else if (S_ISDIR(st.st_mode)) {
- if (!remove_dir_recursively(path, only_empty))
+ if (!remove_dir_recursively(path, flag))
continue; /* happy */
} else if (!only_empty && !unlink(path->buf))
continue; /* happy, too */
--
Johan Herland, [off-list ref]
www.herland.net