Re: [PATCH 8/8] dir: avoid removing the current working directory

[Glen Choo <chooglen@google.com>]

> @@ -3259,9 +3259,12 @@ static int remove_dir_recurse(struct strbuf *path, int flag, int *kept_up)
>  	closedir(dir);
>  
>  	strbuf_setlen(path, original_len);
> -	if (!ret && !keep_toplevel && !kept_down)
> -		ret = (!rmdir(path->buf) || errno == ENOENT) ? 0 : -1;
> -	else if (kept_up)
> +	if (!ret && !keep_toplevel && !kept_down) {
> +		if (the_cwd && !strcmp(the_cwd, path->buf))
> +			ret = -1; /* Do not remove current working directory */
> +		else
> +			ret = (!rmdir(path->buf) || errno == ENOENT) ? 0 : -1;
> +	} else if (kept_up)
>  		/*
>  		 * report the uplevel that it is not an error that we
>  		 * did not rmdir() our directory.
> @@ -3327,6 +3330,8 @@ int remove_path(const char *name)
>  		slash = dirs + (slash - name);
>  		do {
>  			*slash = '\0';
> +			if (the_cwd && !strcmp(the_cwd, dirs))
> +				break;
>  		} while (rmdir(dirs) == 0 && (slash = strrchr(dirs, '/')));
>  		free(dirs);
>  	}

I don't think it's appropriate to implement user-facing concern (don't remove 
cwd because this will confuse users) in library functions like remove_path().
remove_path() has other callers e.g. midx.c, and possible future callers e.g. 
we're working on adding a command to delete corrupted commit-graphs and this 
library function would be extremely handy.

It seems more appropriate to check the_cwd from builtin/add.c and builtin/rm.c 
instead.