Re: [PATCH 3/2] notes-merge: Don't remove .git/NOTES_MERGE_WORKTREE; it may be the user's cwd

From: Junio C Hamano <gitster@pobox.com>
Date: Mar 15, 2012, 07:16 UTC
Message-ID: <7v7gyml4g7.fsf@alter.siamese.dyndns.org>
In-Reply-To: <7vipi6l52w.fsf@alter.siamese.dyndns.org>

Junio C Hamano <gitster@pobox.com> writes:

Show 32 quoted lines

> Johan Herland <johan@herland.net> writes:
>
>> 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...

Show 21 quoted lines

>  	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 */