Re: [PATCH] all: new command used for multi-repo operations

From: Junio C Hamano <gitster@pobox.com>
Date: Jan 23, 2013, 06:52 UTC
Message-ID: <7vtxq8ie63.fsf@alter.siamese.dyndns.org>
In-Reply-To: <1358889019-4554-1-git-send-email-hjemli@gmail.com>

Lars Hjemli <hjemli@gmail.com> writes:

Show 5 quoted lines

> +static struct option builtin_all_options[] = {
> +	OPT_BOOLEAN('c', "clean", &only_clean, N_("only show clean repositories")),
> +	OPT_BOOLEAN('d', "dirty", &only_dirty, N_("only show dirty repositories")),
> +	OPT_END(),
> +};

If you were to go in the OPT_SET_INT route, that would give users the usual "last one wins" semantics, e.g.

	$ git for-each-repo --clean --dirty

will look for only dirty repositories. For completeness, we would probably want "all" to defeat either of them, i.e.

	$ git for-each-repo --clean --all

Show 15 quoted lines

> +static int walk(struct strbuf *path, int argc, const char **argv)
> +{
> +	DIR *dir;
> +	struct dirent *ent;
> +	size_t len;
> +
> +	dir = opendir(path->buf);
> +	if (!dir)
> +		return errno;
> +	strbuf_addstr(path, "/");
> +	len = path->len;
> +	while ((ent = readdir(dir))) {
> +		if (!strcmp(ent->d_name, ".") || !strcmp(ent->d_name, ".."))
> +			continue;
> +		if (!strcmp(ent->d_name, ".git")) {

This only looks for the top of working tree. Have you considered if this "iterate over directories and list git repositories in them" may be useful for collection of bare repositories, and if it is, how to go about implementing the discovery process?

> +		if (ent->d_type != DT_DIR)
> +			continue;

I think this is wrong.

On platforms that need a NO_D_TYPE_IN_DIRENT build, your compilation may fail here (you would need to lstat() it yourself). See how dir.c does this without ugly #ifdef's in the code, especially around the use of get_dtype() and DTYPE() macro.