Re: [PATCH v5 5/5] clean: improve performance when removing lots of directories

From: Jeff King <peff@peff.net>
To: "Erik Elfström" <erik.elfstrom@gmail.com>
Cc: git@vger.kernel.org, gitster@pobox.com
Subject: Re: [PATCH v5 5/5] clean: improve performance when removing lots of directories
Date: Tue, 28 Apr 2015 02:24:23 -0400	[thread overview]
Message-ID: <20150428062422.GM24580@peff.net> (raw)
In-Reply-To: <1430030985-14499-6-git-send-email-erik.elfstrom@gmail.com>

On Sun, Apr 26, 2015 at 08:49:45AM +0200, Erik Elfström wrote:

> +/*
> + * Return 1 if the given path is the root of a git repository or
> + * submodule else 0. Will not return 1 for bare repositories with the
> + * exception of creating a bare repository in "foo/.git" and calling
> + * is_git_repository("foo").
> + */
> +static int is_git_repository(struct strbuf *path)
> +{
> +	int ret = 0;
> +	int unused_error_code;
> +	size_t orig_path_len = path->len;
> +	assert(orig_path_len != 0);
> +	if (path->buf[orig_path_len - 1] != '/')
> +		strbuf_addch(path, '/');
> +	strbuf_addstr(path, ".git");
> +	if (read_gitfile_gently(path->buf, &unused_error_code) || is_git_directory(path->buf))
> +		ret = 1;
> +	strbuf_setlen(path, orig_path_len);
> +	return ret;
> +}

This iteration looks reasonable overall to me.

Should this is_git_repository() helper be available to other files? I
think there are other calls to resolve_gitlink_ref() that would want the
same treatment (e.g., I think "git status" may have a similar issue).

-Peff