Re: [PATCH v3 1/2] for-each-repo: new command used for multi-repo operations

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Lars Hjemli <hjemli@xxxxxxxxx> writes:

> diff --git a/Documentation/git-for-each-repo.txt b/Documentation/git-for-each-repo.txt
> new file mode 100644
> index 0000000..be49e96
> --- /dev/null
> +++ b/Documentation/git-for-each-repo.txt
> @@ -0,0 +1,62 @@
> +git-for-each-repo(1)
> +====================
> +
> +NAME
> +----
> +git-for-each-repo - Execute a git command in multiple repositories

"multiple non-bare repositories", I think.

> +
> +SYNOPSIS
> +--------
> +[verse]
> +'git for-each-repo' [--all|--clean|--dirty] [command]
> +
> +DESCRIPTION
> +-----------
> +The git-for-each-repo command is used to locate all git repositoris

Likewise; "all non-bare Git repositories".

> diff --git a/t/t6400-for-each-repo.sh b/t/t6400-for-each-repo.sh
> new file mode 100755
> index 0000000..4797629
> --- /dev/null
> +++ b/t/t6400-for-each-repo.sh
> @@ -0,0 +1,48 @@
> +#!/bin/sh
> +#
> +# Copyright (c) 2013 Lars Hjemli
> +#
> +
> +test_description='Test the git-for-each-repo command'
> +
> +. ./test-lib.sh
> +
> +test_expect_success "setup" '
> +	test_create_repo clean &&
> +	(cd clean && test_commit foo) &&
> +	git init --separate-git-dir=.cleansub clean/gitfile &&
> +	(cd clean/gitfile && test_commit foo && echo bar >>foo.t) &&
> +	test_create_repo dirty-wt &&
> +	(cd dirty-wt && mv .git .linkedgit && ln -s .linkedgit .git &&
> +	  test_commit foo && rm foo.t) &&
> +	test_create_repo dirty-idx &&
> +	(cd dirty-idx && test_commit foo && git rm foo.t) &&
> +	mkdir fakedir && mkdir fakedir/.git
> +'
> +
> +test_expect_success "without flags, all repos are included" '
> +	echo "." >expect &&
> +	echo "clean" >>expect &&
> +	echo "clean/gitfile" >>expect &&
> +	echo "dirty-idx" >>expect &&
> +	echo "dirty-wt" >>expect &&
> +	git for-each-repo | sort >actual &&
> +	test_cmp expect actual
> +'
> +
> +test_expect_success "--dirty only includes dirty repos" '
> +	echo "clean/gitfile" >expect &&
> +	echo "dirty-idx" >>expect &&
> +	echo "dirty-wt" >>expect &&
> +	git for-each-repo --dirty | sort >actual &&
> +	test_cmp expect actual
> +'
> +
> +test_expect_success "--clean only includes clean repos" '
> +	echo "." >expect &&
> +	echo "clean" >>expect &&
> +	git for-each-repo --clean | sort >actual &&
> +	test_cmp expect actual
> +'

Please add tests to show some command executions (e.g. test output
from "git ls-files", or something).

> +static void handle_repo(char *path, const char **argv)
> +{
> +	if (path[0] == '.' && path[1] == '/')
> +		path += 2;
> +	if (match != ALL && match != get_repo_state())
> +		return;
> +	if (*argv) {
> +		color_fprintf_ln(stdout, GIT_COLOR_YELLOW, "[%s]", path);
> +		run_command_v_opt(argv, RUN_GIT_CMD);

This seems to allow people to run only a single Git subcommand,
which is probably not what most people want to see.  Don't we want
to support something as simple as this?

	git for-each-repository sh -c "ls *.c"

> +	} else
> +		printf("%s\n", path);

Assuming that the non *argv case is for consumption by programs and
scripts (similar to the way "ls-files" output is piped to downstream),
we prefer to (1) support "-z" so that "xargs -0" can read paths with
funny characters, and (2) use quote_c_style() from quote.c when "-z"
is not in effect.

> +}
> + ...
> +			setenv(GIT_DIR_ENVIRONMENT, gitdir, 1);
> +			strbuf_setlen(path, len - 1);
> +			setenv(GIT_WORK_TREE_ENVIRONMENT, path->buf, 1);
> +			handle_repo(path->buf, argv);

When you are only showing the path to a repository, I do not think
you want setenv() or chdir() at all. Shouldn't these be done inside
handle_repo() function?  As you are only dealing with non-bare
repositories (and that is what you print in "listing only" mode
anyway), handle_repo() can borrow path (not path->buf) and append
and strip "/.git" as needed.

Also, while it is a good idea to protect this program from stray
GIT_DIR/GIT_WORK_TREE the user may have in the environment when this
program is started, I think this is not enough, if you allow the
*argv commands to run worktree related operations in each repository
you discover.  You would need to chdir() to the top of the working
tree.

The run-command API lets you specify custom environment only for the
child process without affecting yourself by setting .env member of
the child_process structure, so we may want to use that instead of
doing setenv() on ourselves (and letting it inherited by the child).

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]