* [PATCH v4 1/2] for-each-repo: new command used for multi-repo operations
From: Lars Hjemli @ 2013-01-27 12:46 UTC (permalink / raw)
To: git; +Cc: Lars Hjemli
In-Reply-To: <1359290777-5483-1-git-send-email-hjemli@gmail.com>
When working with multiple, unrelated (or loosly related) git repos,
there is often a need to locate all repos with uncommitted work and
perform some action on them (say, commit and push). Before this patch,
such tasks would require manually visiting all repositories, running
`git status` within each one and then decide what to do next.
This mundane task can now be automated by e.g. `git for-each-repo --dirty
status`, which will find all non-bare git repositories below the current
directory (even nested ones), check if they are dirty (as defined by
`git diff --quiet && git diff --cached --quiet`), and for each dirty repo
print the path to the repo and then execute `git status` within the repo.
The command also honours the option '--clean' which restricts the set of
repos to those which '--dirty' would skip, and '-x' which is used to
execute non-git commands.
Finally, the command to execute within each repo is optional. If none is
given, git-for-each-repo will just print the path to each repo found. And
since the command supports -z, this can be used for more advanced scripting
needs.
Note: since git-for-each-repo can execute both git- and nongit commands, it
must cd into the worktree of each repository before executing the command.
It is then no need for the environment variables $GIT_WORK_TREE and $GIT_DIR
to be specified, so git-for-each-repo will instead unset these variables to
stop them from interfering with the executed commands.
Signed-off-by: Lars Hjemli <hjemli@gmail.com>
---
.gitignore | 1 +
Documentation/git-for-each-repo.txt | 71 +++++++++++++++++
Makefile | 1 +
builtin.h | 1 +
builtin/for-each-repo.c | 145 ++++++++++++++++++++++++++++++++++
git.c | 1 +
t/t6400-for-each-repo.sh | 150 ++++++++++++++++++++++++++++++++++++
7 files changed, 370 insertions(+)
create mode 100644 Documentation/git-for-each-repo.txt
create mode 100644 builtin/for-each-repo.c
create mode 100755 t/t6400-for-each-repo.sh
diff --git a/.gitignore b/.gitignore
index aa258a6..0c27981 100644
--- a/.gitignore
+++ b/.gitignore
@@ -56,6 +56,7 @@
/git-filter-branch
/git-fmt-merge-msg
/git-for-each-ref
+/git-for-each-repo
/git-format-patch
/git-fsck
/git-fsck-objects
diff --git a/Documentation/git-for-each-repo.txt b/Documentation/git-for-each-repo.txt
new file mode 100644
index 0000000..fb12b3f
--- /dev/null
+++ b/Documentation/git-for-each-repo.txt
@@ -0,0 +1,71 @@
+git-for-each-repo(1)
+====================
+
+NAME
+----
+git-for-each-repo - Execute a git command in multiple non-bare repositories
+
+SYNOPSIS
+--------
+[verse]
+'git for-each-repo' [-acdxz] [command]
+
+DESCRIPTION
+-----------
+The git-for-each-repo command is used to locate all non-bare git
+repositories within the current directory tree, and optionally
+execute a git command in each of the found repos.
+
+OPTIONS
+-------
+-a::
+--all::
+ Include both clean and dirty repositories (this is the default
+ behaviour of `git-for-each-repo`).
+
+-c::
+--clean::
+ Only include repositories with a clean worktree.
+
+-d::
+--dirty::
+ Only include repositories with a dirty worktree.
+
+-x::
+ Execute a genric (non-git) command in each repo.
+
+-z::
+ Terminate each path name with the NUL character.
+
+EXAMPLES
+--------
+
+Various ways to exploit this command::
++
+------------
+$ git for-each-repo <1>
+$ git for-each-repo fetch <2>
+$ git for-each-repo -d gui <3>
+$ git for-each-repo -c push <4>
+$ git for-each-repo -x du -sh <5>
+------------
++
+<1> Print the path to all repos found below the current directory.
+
+<2> Fetch updates from default remote in all repos.
+
+<3> Start linkgit:git-gui[1] in each repo containing uncommitted changes.
+
+<4> Push the current branch in each repo with no uncommited changes.
+
+<5> Print disk-usage for each repository.
+
+NOTES
+-----
+
+For the purpose of `git-for-each-repo`, a dirty worktree is defined as a
+worktree with uncommitted changes.
+
+GIT
+---
+Part of the linkgit:git[1] suite
diff --git a/Makefile b/Makefile
index a786d4c..8c42c17 100644
--- a/Makefile
+++ b/Makefile
@@ -870,6 +870,7 @@ BUILTIN_OBJS += builtin/fetch-pack.o
BUILTIN_OBJS += builtin/fetch.o
BUILTIN_OBJS += builtin/fmt-merge-msg.o
BUILTIN_OBJS += builtin/for-each-ref.o
+BUILTIN_OBJS += builtin/for-each-repo.o
BUILTIN_OBJS += builtin/fsck.o
BUILTIN_OBJS += builtin/gc.o
BUILTIN_OBJS += builtin/grep.o
diff --git a/builtin.h b/builtin.h
index 7e7bbd6..02fc712 100644
--- a/builtin.h
+++ b/builtin.h
@@ -73,6 +73,7 @@ extern int cmd_fetch(int argc, const char **argv, const char *prefix);
extern int cmd_fetch_pack(int argc, const char **argv, const char *prefix);
extern int cmd_fmt_merge_msg(int argc, const char **argv, const char *prefix);
extern int cmd_for_each_ref(int argc, const char **argv, const char *prefix);
+extern int cmd_for_each_repo(int argc, const char **argv, const char *prefix);
extern int cmd_format_patch(int argc, const char **argv, const char *prefix);
extern int cmd_fsck(int argc, const char **argv, const char *prefix);
extern int cmd_gc(int argc, const char **argv, const char *prefix);
diff --git a/builtin/for-each-repo.c b/builtin/for-each-repo.c
new file mode 100644
index 0000000..9333ae0
--- /dev/null
+++ b/builtin/for-each-repo.c
@@ -0,0 +1,145 @@
+/*
+ * "git for-each-repo" builtin command.
+ *
+ * Copyright (c) 2013 Lars Hjemli <hjemli@gmail.com>
+ */
+#include "cache.h"
+#include "color.h"
+#include "quote.h"
+#include "builtin.h"
+#include "run-command.h"
+#include "parse-options.h"
+
+#define ALL 0
+#define DIRTY 1
+#define CLEAN 2
+
+static char *color = GIT_COLOR_NORMAL;
+static int eol = '\n';
+static int match;
+static int runopt = RUN_GIT_CMD;
+
+static const char * const builtin_foreachrepo_usage[] = {
+ N_("git for-each-repo [-acdxz] [cmd]"),
+ NULL
+};
+
+static struct option builtin_foreachrepo_options[] = {
+ OPT_SET_INT('a', "all", &match, N_("match both clean and dirty repositories"), ALL),
+ OPT_SET_INT('c', "clean", &match, N_("only show clean repositories"), CLEAN),
+ OPT_SET_INT('d', "dirty", &match, N_("only show dirty repositories"), DIRTY),
+ OPT_SET_INT('x', NULL, &runopt, N_("execute generic (non-git) command"), 0),
+ OPT_SET_INT('z', NULL, &eol, N_("terminate each repo path with NUL character"), 0),
+ OPT_END(),
+};
+
+static int get_repo_state(const char *dir)
+{
+ const char *diffidx[] = {"diff", "--quiet", "--cached", NULL};
+ const char *diffwd[] = {"diff", "--quiet", NULL};
+
+ if (run_command_v_opt_cd_env(diffidx, RUN_GIT_CMD, dir, NULL) != 0)
+ return DIRTY;
+ if (run_command_v_opt_cd_env(diffwd, RUN_GIT_CMD, dir, NULL) != 0)
+ return DIRTY;
+ return CLEAN;
+}
+
+static void print_repo_path(const char *path, unsigned pretty)
+{
+ if (path[0] == '.' && path[1] == '/')
+ path += 2;
+ if (pretty)
+ color_fprintf_ln(stdout, color, "[%s]", path);
+ else
+ write_name_quoted(path, stdout, eol);
+}
+
+static void handle_repo(struct strbuf *path, const char **argv)
+{
+ const char *gitdir;
+ int len;
+
+ len = path->len;
+ strbuf_addstr(path, ".git");
+ gitdir = resolve_gitdir(path->buf);
+ strbuf_setlen(path, len - 1);
+ if (!gitdir)
+ goto done;
+ if (match != ALL && match != get_repo_state(path->buf))
+ goto done;
+ print_repo_path(path->buf, *argv != NULL);
+ if (*argv)
+ run_command_v_opt_cd_env(argv, runopt, path->buf, NULL);
+done:
+ strbuf_addstr(path, "/");
+}
+
+static int walk(struct strbuf *path, int argc, const char **argv)
+{
+ DIR *dir;
+ struct dirent *ent;
+ struct stat st;
+ size_t len;
+ int has_dotgit = 0;
+ struct string_list list = STRING_LIST_INIT_DUP;
+ struct string_list_item *item;
+
+ 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")) {
+ has_dotgit = 1;
+ continue;
+ }
+ switch (DTYPE(ent)) {
+ case DT_UNKNOWN:
+ case DT_LNK:
+ /* Use stat() to figure out if this path leads
+ * to a directory - it's not important if it's
+ * a symlink which gets us there.
+ */
+ strbuf_setlen(path, len);
+ strbuf_addstr(path, ent->d_name);
+ if (stat(path->buf, &st) || !S_ISDIR(st.st_mode))
+ break;
+ /* fallthrough */
+ case DT_DIR:
+ string_list_append(&list, ent->d_name);
+ break;
+ }
+ }
+ closedir(dir);
+ strbuf_setlen(path, len);
+ if (has_dotgit)
+ handle_repo(path, argv);
+ sort_string_list(&list);
+ for_each_string_list_item(item, &list) {
+ strbuf_setlen(path, len);
+ strbuf_addstr(path, item->string);
+ walk(path, argc, argv);
+ }
+ string_list_clear(&list, 0);
+ return 0;
+}
+
+int cmd_for_each_repo(int argc, const char **argv, const char *prefix)
+{
+ struct strbuf path = STRBUF_INIT;
+
+ unsetenv(GIT_DIR_ENVIRONMENT);
+ unsetenv(GIT_WORK_TREE_ENVIRONMENT);
+ argc = parse_options(argc, argv, prefix,
+ builtin_foreachrepo_options,
+ builtin_foreachrepo_usage,
+ PARSE_OPT_STOP_AT_NON_OPTION);
+ if (want_color(GIT_COLOR_AUTO))
+ color = GIT_COLOR_YELLOW;
+ strbuf_addstr(&path, ".");
+ return walk(&path, argc, argv);
+}
diff --git a/git.c b/git.c
index ed66c66..6b53169 100644
--- a/git.c
+++ b/git.c
@@ -337,6 +337,7 @@ static void handle_internal_command(int argc, const char **argv)
{ "fetch-pack", cmd_fetch_pack, RUN_SETUP },
{ "fmt-merge-msg", cmd_fmt_merge_msg, RUN_SETUP },
{ "for-each-ref", cmd_for_each_ref, RUN_SETUP },
+ { "for-each-repo", cmd_for_each_repo },
{ "format-patch", cmd_format_patch, RUN_SETUP },
{ "fsck", cmd_fsck, RUN_SETUP },
{ "fsck-objects", cmd_fsck, RUN_SETUP },
diff --git a/t/t6400-for-each-repo.sh b/t/t6400-for-each-repo.sh
new file mode 100755
index 0000000..af02c0c
--- /dev/null
+++ b/t/t6400-for-each-repo.sh
@@ -0,0 +1,150 @@
+#!/bin/sh
+#
+# Copyright (c) 2013 Lars Hjemli
+#
+
+test_description='Test the git-for-each-repo command'
+
+. ./test-lib.sh
+
+qname="with\"quote"
+qqname="\"with\\\"quote\""
+
+test_expect_success "setup" '
+ test_create_repo clean &&
+ (cd clean && test_commit foo1) &&
+ git init --separate-git-dir=.cleansub clean/gitfile &&
+ (cd clean/gitfile && test_commit foo2 && echo bar >>foo2.t) &&
+ test_create_repo dirty-idx &&
+ (cd dirty-idx && test_commit foo3 && git rm foo3.t) &&
+ test_create_repo dirty-wt &&
+ (cd dirty-wt && mv .git .linkedgit && ln -s .linkedgit .git &&
+ test_commit foo4 && rm foo4.t) &&
+ test_create_repo "$qname" &&
+ (cd "$qname" && test_commit foo5) &&
+ mkdir fakedir && mkdir fakedir/.git
+'
+
+test_expect_success "without filtering, all repos are included" '
+ echo "." >expect &&
+ echo "clean" >>expect &&
+ echo "clean/gitfile" >>expect &&
+ echo "dirty-idx" >>expect &&
+ echo "dirty-wt" >>expect &&
+ echo "$qqname" >>expect &&
+ git for-each-repo >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success "-z NUL-terminates each path" '
+ echo "(.)" >expect &&
+ echo "(clean)" >>expect &&
+ echo "(clean/gitfile)" >>expect &&
+ echo "(dirty-idx)" >>expect &&
+ echo "(dirty-wt)" >>expect &&
+ echo "($qname)" >>expect &&
+ git for-each-repo -z | xargs -0 printf "(%s)\n" >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 >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success "--clean only includes clean repos" '
+ echo "." >expect &&
+ echo "clean" >>expect &&
+ echo "$qqname" >>expect &&
+ git for-each-repo --clean >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success "run a git-command in all repos" '
+ echo "[.]" >expect &&
+ echo "[clean]" >>expect &&
+ echo "[clean/gitfile]" >>expect &&
+ echo " M foo2.t" >>expect &&
+ echo "[dirty-idx]" >>expect &&
+ echo "D foo3.t" >>expect &&
+ echo "[dirty-wt]" >>expect &&
+ echo " D foo4.t" >> expect
+ echo "[$qname]" >>expect &&
+ git for-each-repo status -suno >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success "run a git-command in dirty repos only" '
+ echo "[clean/gitfile]" >expect &&
+ echo " M foo2.t" >>expect &&
+ echo "[dirty-idx]" >>expect &&
+ echo "D foo3.t" >>expect &&
+ echo "[dirty-wt]" >>expect &&
+ echo " D foo4.t" >> expect
+ git for-each-repo -d status -suno >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success "run a git-command in clean repos only" '
+ echo "[.]" >expect &&
+ echo "[clean]" >>expect &&
+ echo "foo1.t" >>expect &&
+ echo "[$qname]" >>expect &&
+ echo "foo5.t" >>expect &&
+ git for-each-repo -c ls-files >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success "-z is disabled when a command is run" '
+ echo "[.]" >expect &&
+ echo "[clean]" >>expect &&
+ echo "foo1.t" >>expect &&
+ echo "[$qname]" >>expect &&
+ echo "foo5.t" >>expect &&
+ git for-each-repo -cz ls-files >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success "-x executes any command in each repo" '
+ echo "[.]" >expect &&
+ echo "$HOME" >>expect &&
+ echo "[clean]" >>expect &&
+ echo "$HOME/clean" >>expect &&
+ echo "[clean/gitfile]" >>expect &&
+ echo "$HOME/clean/gitfile" >>expect &&
+ echo "[dirty-idx]" >>expect &&
+ echo "$HOME/dirty-idx" >>expect &&
+ echo "[dirty-wt]" >>expect &&
+ echo "$HOME/dirty-wt" >> expect
+ echo "[$qname]" >>expect &&
+ echo "$HOME/$qname" >>expect &&
+ git for-each-repo -x pwd >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success "-cx executes any command in clean repos" '
+ echo "[.]" >expect &&
+ echo "$HOME" >>expect &&
+ echo "[clean]" >>expect &&
+ echo "$HOME/clean" >>expect &&
+ echo "[$qname]" >>expect &&
+ echo "$HOME/$qname" >>expect &&
+ git for-each-repo -cx pwd >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success "-dx executes any command in dirty repos" '
+ echo "[clean/gitfile]" >expect &&
+ echo "$HOME/clean/gitfile" >>expect &&
+ echo "[dirty-idx]" >>expect &&
+ echo "$HOME/dirty-idx" >>expect &&
+ echo "[dirty-wt]" >>expect &&
+ echo "$HOME/dirty-wt" >> expect
+ git for-each-repo -dx pwd >actual &&
+ test_cmp expect actual
+'
+
+test_done
--
1.8.1.1.349.g4cdd23e
^ permalink raw reply related
* [PATCH v4 2/2] git: rewrite `git -a` to become a git-for-each-repo command
From: Lars Hjemli @ 2013-01-27 12:46 UTC (permalink / raw)
To: git; +Cc: Lars Hjemli
In-Reply-To: <1359290777-5483-1-git-send-email-hjemli@gmail.com>
With this rewriting, it is now possible to run e.g. `git -ad gui` to
start up git-gui in each repo within the current directory which
contains uncommited work.
Signed-off-by: Lars Hjemli <hjemli@gmail.com>
---
git.c | 36 +++++++++++++++++++++++++++
t/t6400-for-each-repo.sh | 63 ++++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 99 insertions(+)
diff --git a/git.c b/git.c
index 6b53169..f933b5d 100644
--- a/git.c
+++ b/git.c
@@ -31,8 +31,42 @@ static void commit_pager_choice(void) {
}
}
+/*
+ * Rewrite 'git -ad status' to 'git for-each-repo -d status'
+ */
+static int rewrite_foreach_repo(const char ***orig_argv,
+ const char **curr_argv,
+ int *curr_argc)
+{
+ const char **new_argv;
+ char *tmp;
+ int new_argc, curr_pos, i, j;
+
+ curr_pos = curr_argv - *orig_argv;
+ if (strlen(curr_argv[0]) == 2) {
+ curr_argv[0] = "for-each-repo";
+ return curr_pos - 1;
+ }
+
+ new_argc = curr_pos + *curr_argc + 1;
+ new_argv = xmalloc(new_argc * sizeof(void *));
+ for (i = j = 0; j < new_argc; i++, j++) {
+ if (i == curr_pos) {
+ asprintf(&tmp, "-%s", (*orig_argv)[i] + 2);
+ new_argv[j] = "for-each-repo";
+ new_argv[++j] = tmp;
+ } else {
+ new_argv[j] = (*orig_argv)[i];
+ }
+ }
+ *orig_argv = new_argv;
+ (*curr_argc)++;
+ return curr_pos;
+}
+
static int handle_options(const char ***argv, int *argc, int *envchanged)
{
+ const char ***pargv = argv;
const char **orig_argv = *argv;
while (*argc > 0) {
@@ -143,6 +177,8 @@ static int handle_options(const char ***argv, int *argc, int *envchanged)
setenv(GIT_LITERAL_PATHSPECS_ENVIRONMENT, "0", 1);
if (envchanged)
*envchanged = 1;
+ } else if (!strncmp(cmd, "-a", 2)) {
+ return rewrite_foreach_repo(pargv, *argv, argc);
} else {
fprintf(stderr, "Unknown option: %s\n", cmd);
usage(git_usage_string);
diff --git a/t/t6400-for-each-repo.sh b/t/t6400-for-each-repo.sh
index af02c0c..eaa4518 100755
--- a/t/t6400-for-each-repo.sh
+++ b/t/t6400-for-each-repo.sh
@@ -147,4 +147,67 @@ test_expect_success "-dx executes any command in dirty repos" '
test_cmp expect actual
'
+test_expect_success "rewrite 'git -a'" '
+ echo "." >expect &&
+ echo "clean" >>expect &&
+ echo "clean/gitfile" >>expect &&
+ echo "dirty-idx" >>expect &&
+ echo "dirty-wt" >>expect &&
+ echo "$qqname" >>expect &&
+ git -a >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success "rewrite 'git -az'" '
+ echo "(.)" >expect &&
+ echo "(clean)" >>expect &&
+ echo "(clean/gitfile)" >>expect &&
+ echo "(dirty-idx)" >>expect &&
+ echo "(dirty-wt)" >>expect &&
+ echo "($qname)" >>expect &&
+ git -az | xargs -0 printf "(%s)\n" >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success "rewrite 'git -ad'" '
+ echo "clean/gitfile" >expect &&
+ echo "dirty-idx" >>expect &&
+ echo "dirty-wt" >>expect &&
+ git -ad >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success "rewrite 'git -ac'" '
+ echo "." >expect &&
+ echo "clean" >>expect &&
+ echo "$qqname" >>expect &&
+ git -ac >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success "rewrite 'git -a status -suno'" '
+ echo "[.]" >expect &&
+ echo "[clean]" >>expect &&
+ echo "[clean/gitfile]" >>expect &&
+ echo " M foo2.t" >>expect &&
+ echo "[dirty-idx]" >>expect &&
+ echo "D foo3.t" >>expect &&
+ echo "[dirty-wt]" >>expect &&
+ echo " D foo4.t" >> expect
+ echo "[$qname]" >>expect &&
+ git -a status -suno >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success "rewrite 'git -acx pwd'" '
+ echo "[.]" >expect &&
+ echo "$HOME" >>expect &&
+ echo "[clean]" >>expect &&
+ echo "$HOME/clean" >>expect &&
+ echo "[$qname]" >>expect &&
+ echo "$HOME/$qname" >>expect &&
+ git -acx pwd >actual &&
+ test_cmp expect actual
+'
+
test_done
--
1.8.1.1.349.g4cdd23e
^ permalink raw reply related
* Re: [PATCH] tests: turn on test-lint-shell-syntax by default
From: Torsten Bögershausen @ 2013-01-27 13:13 UTC (permalink / raw)
To: Jonathan Nieder; +Cc: Torsten Bögershausen, Junio C Hamano, git, kraai
In-Reply-To: <20130127093121.GA4228@elie.Belkin>
On 27.01.13 10:31, Jonathan Nieder wrote:
> Hi,
>
> Torsten Bögershausen wrote:
>> On 15.01.13 21:38, Junio C Hamano wrote:
>>> Torsten Bögershausen <tboegi@web.de> writes:
>
>>>> What do we think about something like this for fishing for which:
> [...]
>>>> +which () {
>>>> + echo >&2 "which is not portable (please use type)"
>>>> + exit 1
>>>> +}
> [...]
>>> if (
>>> which frotz &&
>>> test $(frobonitz --version" -le 2.0
>>> )
>
> With the above definition of "which", the only sign of a mistake would
> be some extra output to stderr (which is quelled when running tests in
> the normal way). The "exit" is caught by the subshell and just makes
> the "if" condition false.
>
> That's not so terrible --- it could still dissuade new test authors
> from using "which". The downside I'd worry about is that it provides
> a false sense of security despite not catching problems like
>
> write_script x <<-EOF &&
> # Use "foo" if possible. Otherwise use "bar".
> if which foo && test $(foo --version) -le 2.0
> then
> ...
> ...
> EOF
> ./x
>
> That's not a great tradeoff relative to the impact of the problem
> being solved.
>
> Don't get me wrong. I really do want to see more static or dynamic
> analysis of git's shell scripts in the future. I fear that for the
> tradeoffs to make sense, though, the analysis needs to be more
> sophisticated:
>
> * A very common error in test scripts is leaving out the "&&"
> connecting adjacent statements, which causes early errors
> in a test assertion to be missed and tests to pass by mistake.
> Unfortunately the grammar of the dialect of shell used in tests is
> not regular enough to make this easily detectable using regexps.
>
> * Another common mistake is using "cd" without entering a subshell.
> Detecting this requires counting nested parentheses and noticing
> when a parenthesis is quoted.
>
> * Another common mistake is relying on the semantics of variable
> assignments in front of function calls. Detecting this requires
> recognizing which commands are function calls.
>
> In the end the analysis that works best would probably involve a
> full-fledged shell script parser. Something like "sparse", except for
> shell command language.
>
> Sorry I don't have more practical advice in the short term.
>
> My two cents,
> Jonathan
Jonathan,
thanks for the review.
My ambition is to get the check-non-portable-shell.pl into a shape
that we can enable it by default.
This may be with or without checking for "which".
As a first step we will hopefully see less breakage for e.g. Mac OS
caused by "echo -n" or "sed -i" usage.
On the longer run, we may be able to have something more advanced.
Back to the which:
Writing a t0009-no-which.sh like this:
--------------------
#!/bin/sh
test_description='Test the which'
. ./test-lib.sh
which () {
echo >&2 "which is not portable (please use type)"
exit 1
}
test_expect_success 'which is not portable' '
if which frotz
then
say "frotz does not exist"
else
say "frotz does exist"
fi
'
test_done
--------------
and running "make test" gives the following, at least in my system:
[snip]
*** t0009-no-which.sh ***
FATAL: Unexpected exit with code 1
make[2]: *** [t0009-no-which.sh] Error 1
make[1]: *** [test] Error 2
make: *** [test] Error 2
-----------------------
running inside t/
./t0009-no-which.sh --verbose
Initialized empty Git repository in /Users/tb/projects/git/tb/t/trash directory.t0009-no-which/.git/
expecting success:
if which frotz
then
say "frotz does not exist"
else
say "frotz does exist"
fi
which is not portable (please use type)
FATAL: Unexpected exit with code 1
/Torsten
^ permalink raw reply
* Re: git-svn problems with white-space in tag names
From: Hans-Juergen Euler @ 2013-01-27 14:12 UTC (permalink / raw)
To: git
In-Reply-To: <CAK3CF+4GPKBfAmgsHYnf_6nCCOCe-1d31cGsWp4jkKC28cZr0g@mail.gmail.com>
This seems to be a problem of the windows version. At least with its
complete severity. Installed git on Ubuntu in a virtual machine was
able to clone the subversion repos past the tag with the white-space
at the end. I am not sure but apparently this tag has not been
converted.
The git repos I could copy from Ubuntu to windows. So far no problems
seen in this copy on windows.
2013/1/23 Hans-Juergen Euler <waas.nett@gmail.com>:
> I have discussed already the problem a bit more in this thread
> groups.google.com/d/topic/git-users/kfMFZ3uEFsM/discussion
>
> -----Operating system (specifically which version)
> windows 7 64 bit
>
> ------Git version (git --version)
> Git version 1.8.0 for windows obviously.
> git bash and git gui installed and using
>
> ------Git configuration (system, home, repository)
> hmm guess is covered with git bash and git gui. Using the standard config stuff
>
> using subversion
> TortoiseSVN 1.7.11
> Subversion 1.7.8
> Was typically always up-to-date (within 2 months or so) with previous versions
>
> using an external subversion provider for storing the information
> externally. guess the version there is older but do not know
>
>
> I have tried to convert some of my external subversion data bases with
> git-svn clone
>
> I have encountered a problem with one of my subversion repos. I have
> obviously introduced about 2 years ago a problem with an additional
> white space at the end of tag name.
>
> So there is an entry "tags/blabla "
>
> in the subversion repos. The sequential handling of the svn repos with
> git-svn gets stuck there. I could not find a way around this. My guess
> is that the white-space was introduced by accident on windows by
> Tortoise-SVN.
> Unfortunately this occurs at revision 90 something and I have almost
> 1000 revisions stored.
>
> Let me know if you need more details.Thanks.
^ permalink raw reply
* Re: [PATCH v3 6/8] git-remote-testpy: hash bytes explicitly
From: John Keeping @ 2013-01-27 14:13 UTC (permalink / raw)
To: Michael Haggerty; +Cc: Junio C Hamano, git, Sverre Rabbelier
In-Reply-To: <5104B0B5.1030501@alum.mit.edu>
On Sun, Jan 27, 2013 at 05:44:37AM +0100, Michael Haggerty wrote:
> On 01/26/2013 10:44 PM, Junio C Hamano wrote:
> > John Keeping <john@keeping.me.uk> writes:
> >> @@ -45,7 +45,7 @@ def get_repo(alias, url):
> >> repo.get_head()
> >>
> >> hasher = _digest()
> >> - hasher.update(repo.path)
> >> + hasher.update(repo.path.encode('utf-8'))
> >> repo.hash = hasher.hexdigest()
> >>
> >> repo.get_base_path = lambda base: os.path.join(
>
> This will still fail under Python 2.x if repo.path is a byte string that
> contains non-ASCII characters.
I had forgotten about Python 2 while doing this.
> And it will fail under Python 3.1 and
> later if repo.path contains characters using the surrogateescape
> encoding option [1], as it will if the original command-line argument
> contained bytes that cannot be decoded into Unicode using the user's
> default encoding:
Interesting. I wasn't aware of the "surrogateescape" error handler.
> 'surrogateescape' is not supported in Python 3.0, but I think it would
> be quite acceptable only to support Python 3.x for x >= 1.
I agree.
> But 'surrogateescape' doesn't seem to be supported at all in Python 2.x
> (I tested 2.7.3 and it's not there).
>
> Here you don't really need byte-for-byte correctness; it would be enough
> to get *some* byte string that is unique for a given input (ideally,
> consistent with ASCII or UTF-8 for backwards compatibility). So you
> could use
>
> b = s.encode('utf-8', 'backslashreplace')
>
> Unfortunately, this doesn't work under Python 2.x:
>
> $ python2 -c "
> import sys
> print(repr(sys.argv[1]))
> print(repr(sys.argv[1].encode('utf-8', 'backslashreplace')))
> " $(echo français|iconv -t latin1)
> 'fran\xe7ais'
> Traceback (most recent call last):
> File "<string>", line 4, in <module>
> UnicodeDecodeError: 'ascii' codec can't decode byte 0xe7 in position
> 4: ordinal not in range(128)
>
> Apparently when you call bytestring.encode(), Python first tries to
> decode the string to Unicode using the 'ascii' encoding.
Actually it appears to use sys.getdefaultencoding() to do this initial
decode. Not that it makes much difference here since the failure is the
same.
> So to handle all of the cases across Python versions as closely as
> possible to the old 2.x code, it might be necessary to make the code
> explicitly depend on the Python version number, like:
>
> hasher = _digest()
> if sys.hexversion < 0x03000000:
> pathbytes = repo.path
> elif sys.hexversion < 0x03010000:
> # If support for Python 3.0.x is desired (note: result can
> # be different in this case than under 2.x or 3.1+):
> pathbytes = repo.path.encode(sys.getfilesystemencoding(),
> 'backslashreplace')
> else
> pathbytes = repo.path.encode(sys.getfilesystemencoding(),
> 'surrogateescape')
> hasher.update(pathbytes)
> repo.hash = hasher.hexdigest()
If we don't want to put a version check in it probably wants to look
like this (ignoring Python 3.0 since I don't think we need to support
it):
hasher = _digest()
try:
codecs.lookup_error('surrogateescape')
except LookupError:
pathbytes = repo.path
else:
pathbytes = repo.path.encode(sys.getfilesystemencoding(),
'surrogateescape')
hasher.update(pathbytes)
repo.hash = hasher.hexdigest()
The version with a version check seems better to me, although this
should probably be a utility function.
John
^ permalink raw reply
* Re: [PATCH v3 6/8] git-remote-testpy: hash bytes explicitly
From: John Keeping @ 2013-01-27 14:21 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Michael Haggerty, git, Sverre Rabbelier
In-Reply-To: <7vy5ffxkfb.fsf@alter.siamese.dyndns.org>
On Sat, Jan 26, 2013 at 09:30:00PM -0800, Junio C Hamano wrote:
> Michael Haggerty <mhagger@alum.mit.edu> writes:
>
> > This will still fail under Python 2.x if repo.path is a byte string that
> > contains non-ASCII characters. And it will fail under Python 3.1 and
> > later if repo.path contains characters using the surrogateescape
> > encoding option [1],...
> > Here you don't really need byte-for-byte correctness; it would be enough
> > to get *some* byte string that is unique for a given input ...
>
> Yeek.
>
> As we do not care about the actual value at all, how about doing
> something like this instead?
>
> + hasher.update(".".join([str(ord(c)) for c in repo.path]))
This doesn't solve the original problem since we're still ending up with
a Unicode string. If we wanted something like this it would need to be:
hasher.update(b'.'.join([b'%X' % ord(c) for c in repo.path]))
which limits us to Python 2.6 and later and seems to me to be less clear
than introducing an "encode_filepath" helper function using Michael's
suggestion.
John
^ permalink raw reply
* Re: [PATCH 0/7] guilt patches, including git 1.8 support
From: Josef 'Jeff' Sipek @ 2013-01-27 14:38 UTC (permalink / raw)
To: Jonathan Nieder
Cc: git, Per Cederqvist, Theodore Ts'o, Iulian Udrea,
Axel Beckert
In-Reply-To: <20130116022606.GI12524@google.com>
On Tue, Jan 15, 2013 at 06:26:06PM -0800, Jonathan Nieder wrote:
> Hi Jeff and other guilty parties,
>
> I collected all the guilt patches I could find on-list and added one
> of my own. Completely untested, except for running the regression
> tests. These are also available via git protocol from
>
> git://repo.or.cz/guilt/mob.git mob
>
> Thoughts?
Sorry for taking so long. I finally reclaimed access to
git://repo.or.cz/guilt.git. I pulled from mob, and merged it with what I
had locally.
http://repo.or.cz/w/guilt.git
Thanks for collecting all these in one place!
Jeff.
> Jonathan Nieder (1):
> Drop unneeded git version check.
>
> Per Cederqvist (6):
> get rid of "cat: write error: Broken pipe" error message
> The tests should not fail if log.date or log.decorate are set.
> Testsuite: get rid of "Broken pipe" errors from yes.
> Handle empty patches and patches with only a header.
> Fix fatal "guilt graph" error in sha1sum invocation.
> Change git branch when patches are applied.
>
> Documentation/guilt.7 | 4 +
> guilt | 73 +++++---
> guilt-branch | 12 +-
> guilt-commit | 7 +
> guilt-import-commit | 4 +-
> guilt-repair | 7 +-
> os.Darwin | 7 +-
> os.Linux | 7 +-
> os.SunOS | 7 +-
> regression/scaffold | 7 +-
> regression/t-029.sh | 6 +-
> regression/t-052.out | 24 +--
> regression/t-052.sh | 7 +-
> regression/t-061.out | 468 ++++++++++++++++++++++++++++++++++++++++++++++++++
> regression/t-061.sh | 148 ++++++++++++++++
> 15 files changed, 743 insertions(+), 45 deletions(-)
> create mode 100644 regression/t-061.out
> create mode 100755 regression/t-061.sh
--
I have always wished for my computer to be as easy to use as my telephone;
my wish has come true because I can no longer figure out how to use my
telephone.
- Bjarne Stroustrup
^ permalink raw reply
* [PATCH] git-remote-testpy: fix patch hashing on Python 3
From: John Keeping @ 2013-01-27 14:50 UTC (permalink / raw)
To: Michael Haggerty; +Cc: Junio C Hamano, git, Sverre Rabbelier
In-Reply-To: <20130127141329.GN7498@serenity.lan>
When this change was originally made (0846b0c - git-remote-testpy: hash
bytes explicitly , I didn't realised that the "hex" encoding we chose is
a "bytes to bytes" encoding so it just fails with an error on Python 3
in the same way as the original code.
It is not possible to provide a single code path that works on Python 2
and Python 3 since Python 2.x will attempt to decode the string before
encoding it, which fails for strings that are not valid in the default
encoding. Python 3.1 introduced the "surrogateescape" error handler
which handles this correctly and permits a bytes -> unicode -> bytes
round-trip to be lossless.
At this point Python 3.0 is unsupported so we don't go out of our way to
try to support it.
Helped-by: Michael Haggerty <mhagger@alum.mit.edu>
Signed-off-by: John Keeping <john@keeping.me.uk>
---
On Sun, Jan 27, 2013 at 02:13:29PM +0000, John Keeping wrote:
> On Sun, Jan 27, 2013 at 05:44:37AM +0100, Michael Haggerty wrote:
> > So to handle all of the cases across Python versions as closely as
> > possible to the old 2.x code, it might be necessary to make the code
> > explicitly depend on the Python version number, like:
> >
> > hasher = _digest()
> > if sys.hexversion < 0x03000000:
> > pathbytes = repo.path
> > elif sys.hexversion < 0x03010000:
> > # If support for Python 3.0.x is desired (note: result can
> > # be different in this case than under 2.x or 3.1+):
> > pathbytes = repo.path.encode(sys.getfilesystemencoding(),
> > 'backslashreplace')
> > else
> > pathbytes = repo.path.encode(sys.getfilesystemencoding(),
> > 'surrogateescape')
> > hasher.update(pathbytes)
> > repo.hash = hasher.hexdigest()
How about this?
git-remote-testpy.py | 18 +++++++++++++++++-
1 file changed, 17 insertions(+), 1 deletion(-)
diff --git a/git-remote-testpy.py b/git-remote-testpy.py
index c7a04ec..16b0c52 100644
--- a/git-remote-testpy.py
+++ b/git-remote-testpy.py
@@ -36,6 +36,22 @@ if sys.hexversion < 0x02000000:
sys.stderr.write("git-remote-testgit: requires Python 2.0 or later.\n")
sys.exit(1)
+
+def _encode_filepath(path):
+ """Encodes a Unicode file path to a byte string.
+
+ On Python 2 this is a no-op; on Python 3 we encode the string as
+ suggested by [1] which allows an exact round-trip from the command line
+ to the filesystem.
+
+ [1] http://docs.python.org/3/c-api/unicode.html#file-system-encoding
+
+ """
+ if sys.hexversion < 0x03000000:
+ return path
+ return path.encode('utf-8', 'surrogateescape')
+
+
def get_repo(alias, url):
"""Returns a git repository object initialized for usage.
"""
@@ -45,7 +61,7 @@ def get_repo(alias, url):
repo.get_head()
hasher = _digest()
- hasher.update(repo.path.encode('hex'))
+ hasher.update(_encode_filepath(repo.path))
repo.hash = hasher.hexdigest()
repo.get_base_path = lambda base: os.path.join(
--
1.8.1.1
^ permalink raw reply related
* [PATCH v2] Reduce false positive in check-non-portable-shell.pl
From: Torsten Bögershausen @ 2013-01-27 7:47 UTC (permalink / raw)
To: git; +Cc: tboegi
check-non-portable-shell.pl is using simple regular expressions to
find illegal shell syntax.
Improve the expressions and reduce the chance for false positves:
"sed -i" must be followed by 1..n whitespace and 1 non whitespace
"declare" must be followed by 1..n whitespace and 1 non whitespace
"echo -n" must be followed by 1..n whitespace and 1 non whitespace
"which": catch lines like "if which foo"
Signed-off-by: Torsten Bögershausen <tboegi@web.de>
---
t/check-non-portable-shell.pl | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/t/check-non-portable-shell.pl b/t/check-non-portable-shell.pl
index 8b5a71d..d9ddcdf 100755
--- a/t/check-non-portable-shell.pl
+++ b/t/check-non-portable-shell.pl
@@ -16,10 +16,10 @@ sub err {
while (<>) {
chomp;
- /^\s*sed\s+-i/ and err 'sed -i is not portable';
- /^\s*echo\s+-n/ and err 'echo -n is not portable (please use printf)';
- /^\s*declare\s+/ and err 'arrays/declare not portable';
- /^\s*[^#]\s*which\s/ and err 'which is not portable (please use type)';
+ /^\s*sed\s+-i\s+\S/ and err 'sed -i is not portable';
+ /^\s*echo\s+-n\s+\S/ and err 'echo -n is not portable (please use printf)';
+ /^\s*declare\s+\S/ and err 'arrays/declare not portable';
+ /^\s*if\s+which\s+\S/ and err 'which is not portable (please use type)';
/test\s+[^=]*==/ and err '"test a == b" is not portable (please use =)';
# this resets our $. for each file
close ARGV if eof;
--
1.8.1.1
^ permalink raw reply related
* [RFC] test-lib.sh: No POSIXPERM for cygwin
From: Torsten Bögershausen @ 2013-01-27 14:57 UTC (permalink / raw)
To: ramsay, git; +Cc: j6t, tboegi
t0070 and t1301 fail when running the test suite under cygwin.
Skip the failing tests by unsetting POSIXPERM.
Signed-off-by: Torsten Bögershausen <tboegi@web.de>
---
t/test-lib.sh | 1 -
1 file changed, 1 deletion(-)
diff --git a/t/test-lib.sh b/t/test-lib.sh
index 1a6c4ab..94b097e 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -669,7 +669,6 @@ case $(uname -s) in
test_set_prereq SED_STRIPS_CR
;;
*CYGWIN*)
- test_set_prereq POSIXPERM
test_set_prereq EXECKEEPSPID
test_set_prereq NOT_MINGW
test_set_prereq SED_STRIPS_CR
--
1.8.1.1
^ permalink raw reply related
* Re: [PATCH v2] add: warn when -u or -A is used without filepattern
From: Matthieu Moy @ 2013-01-27 16:10 UTC (permalink / raw)
To: Junio C Hamano
Cc: git, Jonathan Nieder, Robin Rosenberg, Piotr Krukowiecki,
Eric James Michael Ritz, Tomas Carnecky
In-Reply-To: <7v8v7h3vx8.fsf@alter.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> writes:
> Matthieu Moy <Matthieu.Moy@imag.fr> writes:
>
>> Most git commands that can be used with our without a filepattern are
>> tree-wide by default, the filepattern being used to restrict their scope.
>> A few exceptions are: 'git grep', 'git clean', 'git add -u' and 'git add -A'.
>>
>> The inconsistancy of 'git add -u' and 'git add -A' are particularly
>
> s/consistan/consisten/;
Thanks, will fix.
> I wonder if we want to say in the message
>
> The behaviour of 'git add --all (or -A)'...
>
> otherwise people who typed "git add -A" and got this message with
> just "--all" may go "Huh?" for a brief moment. I however do not
> think replacing these strings to
>
> option_with_implicit_dot = "--all (-A)";
>
> is a solution, given they are goven to _("l10n template %s").
Plus, option_with_implicit_dot is used in cut-and-paste ready commands
below. I can easily add another variable short_option or so to display
both. Ideally, we should use whatever the user had typed, but that does
not seem easy to do with parse-option so I'd say it's overkill.
--
Matthieu Moy
http://www-verimag.imag.fr/~moy/
^ permalink raw reply
* mergetool: include custom tools in '--tool-help'
From: John Keeping @ 2013-01-27 16:34 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, David Aguilar
'git mergetool --tool-help' only lists builtin tools, not those that the
user has configured via a 'mergetool.<tool>.cmd' config value. Fix this
by inspecting the tools configured in this way and adding them to the
available and unavailable lists before displaying them.
Signed-off-by: John Keeping <john@keeping.me.uk>
---
After the recent changes to mergetool, do we want to do something like
this as well, so that 'git mergetool --tool-help' will display any tools
configured by the user/system administrator?
This is on top of jk/mergetool.
git-mergetool--lib.sh | 29 +++++++++++++++++++++++++++++
1 file changed, 29 insertions(+)
diff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh
index 1d0fb12..f9a617c 100644
--- a/git-mergetool--lib.sh
+++ b/git-mergetool--lib.sh
@@ -206,6 +206,29 @@ list_merge_tool_candidates () {
esac
}
+# Adds tools from git-config to the available and unavailable lists.
+# The tools are found in "$1.<tool>.cmd".
+add_config_tools() {
+ section=$1
+
+ eval $(git config --get-regexp $section'\..*\.cmd' |
+ while read -r key value
+ do
+ tool=${key#mergetool.}
+ tool=${tool%.cmd}
+
+ tool=$(echo "$tool" |sed -e 's/'\''/'\''\\'\'\''/g')
+
+ cmd=$(eval -- "set -- $value"; echo "$1")
+ if type "$cmd" >/dev/null 2>&1
+ then
+ echo "available=\"\${available}\"'$tool'\"\$LF\""
+ else
+ echo "unavailable=\"\${unavailable}\"'$tool'\"\$LF\""
+ fi
+ done)
+}
+
show_tool_help () {
unavailable= available= LF='
'
@@ -223,6 +246,12 @@ show_tool_help () {
fi
done
+ add_config_tools mergetool
+ if diff_mode
+ then
+ add_config_tools difftool
+ fi
+
cmd_name=${TOOL_MODE}tool
if test -n "$available"
then
--
1.8.1.1
^ permalink raw reply related
* Re: [PATCH] tests: turn on test-lint-shell-syntax by default
From: Junio C Hamano @ 2013-01-27 17:15 UTC (permalink / raw)
To: Jonathan Nieder; +Cc: Torsten Bögershausen, git, kraai
In-Reply-To: <20130127093121.GA4228@elie.Belkin>
Jonathan Nieder <jrnieder@gmail.com> writes:
> ...
> With the above definition of "which", the only sign of a mistake would
> be some extra output to stderr (which is quelled when running tests in
> the normal way). The "exit" is caught by the subshell and just makes
> the "if" condition false.
>
> That's not so terrible --- it could still dissuade new test authors
> from using "which". The downside I'd worry about is that it provides
> a false sense of security despite not catching problems ...
> ...
> In the end the analysis that works best would probably involve a
> full-fledged shell script parser. Something like "sparse", except for
> shell command language.
Exactly.
That is why I keep saying that whole test-lint-shell-syntax should
stay outside the default until it gets more robust by becoming a
reasonable shell parser; it may not necessarily have to become
"full" parser though.
As we discourage the use of tricky features of the language like
eval in individual test scripts to implement their own mini test
framework, the "something like sparse" parser may initialy start
small and still be useful; for example it can learn to exclude
anything inside <<HERE_DOCUMENT from getting inspected.
^ permalink raw reply
* Re: [PATCH] tests: turn on test-lint-shell-syntax by default
From: Junio C Hamano @ 2013-01-27 17:34 UTC (permalink / raw)
To: Torsten Bögershausen; +Cc: Jonathan Nieder, git, kraai
In-Reply-To: <5105280A.80002@web.de>
Torsten Bögershausen <tboegi@web.de> writes:
> Back to the which:
> ...
> and running "make test" gives the following, at least in my system:
> ...
I think everybody involved in this discussion already knows that;
the point is that it can easily give false negative, without the
scripts working very hard to do so.
If we did not care about incurring runtime performance cost, we
could arrange:
- the test framework to define a variable $TEST_ABORT that has a
full path to a file that is in somewhere test authors cannot
touch unless they really try hard to (i.e. preferrably outside
$TRASH_DIRECTORY, as it is not uncommon for to tests to do "rm *"
there). This location should be per $(basename "$0" .sh) to allow
running multiple tests in paralell;
- the test framework to "rm -f $TEST_ABORT" at the beginning of
test_expect_success/failure;
- test_expect_success/failure to check $TEST_ABORT and if it
exists, abort the execution, showing the contents of the file as
an error message.
Then you can wrap commands whose use we want to limit, perhaps like
this, in the test framework:
which () {
cat >"$TEST_ABORT" <<-\EOF
Do not use unportable 'which' in the test script.
"if type $cmd" is a good way to see if $cmd exists.
EOF
}
sed () {
saw_wantarg= must_abort=
for arg
do
if test -n "$saw_wantarg"
then
saw_wantarg=
continue
fi
case "$arg" in
--) break ;; # end of options
-i) echo >"$TEST_ABORT" "Do not use 'sed -i'"
must_abort=yes
break ;;
-e) saw_wantarg=yes ;; # skip next arg
-*) continue ;; # options without arg
*) break ;; # filename
esac
done
if test -z "$must_abort"
sed "$@"
fi
}
Then you can check that TEST_ABORT does not appear in test scripts
(ensuring that they do not attempt to circumvent the mechanis) and
catch use of unwanted commands or unwanted extended features of
commands at runtime.
But this will incur runtime performace hit, so I am not sure it
would be worth it.
^ permalink raw reply
* Re: [PATCH 2/2] mergetools: Make tortoisemerge work with
From: Junio C Hamano @ 2013-01-27 17:48 UTC (permalink / raw)
To: Sven Strickroth; +Cc: git, David Aguilar, Sebastian Schuberth, Jeff King
In-Reply-To: <5104F009.5020606@tu-clausthal.de>
Sven Strickroth <sven.strickroth@tu-clausthal.de> writes:
> Am 26.01.2013 08:10 schrieb David Aguilar:
>> These patches look correct (I do not have the tool to test)
>> but I think we should fixup this commit message.
>>
>> How about something like...
>>
>> mergetools: Teach tortoisemerge about TortoiseGitMerge
>>
>> TortoiseGitMerge improved its syntax to allow for file paths
>> with spaces. Detect when it is installed and prefer it over
>> TortoiseMerge.
>
> This message implies, that I have to combine two patches again?!
We can see that [1/2] teaches mergetool to use tortoisegitmerge when
the user tells it to use tortoisemerge and the former is available.
The change in [2/2] is to use -base "$BASE" instead of -base:"$BASE"
when the real tool is tortoisegitmerge (when it is tortoisemerge,
nothing changes).
By reading these two patches, I would imagine that tortoisegitmerge
can accept both forms, i.e. -base:"$BASE" and -base "$BASE", but the
patch [2/2] considers that the latter form is preferrable in some
way. As you talked about "paths with SPs in them", I would imagine
that is the difference? That is -base:"$BASE" form will not work if
the varilable $BASE has a SP in it (even though it is encolosed in
dq, which does not make much sense from my POSIXy point of view, but
perhaps command line argument processing in the Windows land may
have different rules) but if you write -base "$BASE" then "$BASE"
will be taken as a single thing even it has a SP in it? Also I
would guess that the reason why patch [2/2] does this only for
tortoisegitmerge is either because tortoisemerge will break paths
with SPs even if it is given -base "$BASE" form, or because it only
accepts -base:"$BASE" form? I cannot read it from your description,
but let's assume that is the reason.
If that is the case, then the log message for the second patch would
be easier to understand if it says so in a more explicit way,
perhaps like this:
TortoiseGitMerge, unlike TortoiseMerge, can be told to
handle paths with SPs in them by using -option "$FILE" (not
-option:"$FILE", which does not work for such paths) syntax.
Use it to allow such paths to be handled correctly.
But I cannot read exactly why the patch [2/2] considers -base "$BASE"
is preferrable over -base:"$BASE" from your original description, so
this may well be way off the mark.
In short, I think proposed log message for [2/2] was not clear what
is being fixed and how.
^ permalink raw reply
* Re: Behavior of stash apply vs merge
From: Junio C Hamano @ 2013-01-27 17:57 UTC (permalink / raw)
To: Robin Rosenberg; +Cc: Git List
In-Reply-To: <61173190.1700449.1359286556865.JavaMail.root@dewire.com>
Robin Rosenberg <robin.rosenberg@dewire.com> writes:
> What good reason is it that 'git stash apply' gives hairy conflict
> markers, while 'git merge stash' does not. No renames involved.
"git merge stash" is nonsensical and would do a vastly different
thing compared to "git stash apply" depending on where you created
the stash and where you are attempting to run that operation.
Imagine you started fixing a bug while on the 'master' branch,
realized that the fix equally well applies to your 'maint' branch.
You would do "git stash" followed by "git checkout maint".
A sane person would do "git stash apply" at this point. It applies
the difference between the 'master' you were working on and your WIP
on top of your 'maint'.
"git merge stash" is entirely different. The history leading to a
stash looks like this:
I
/ \
---o---o---B---W
where
B is the commit you were working on (i.e. 'master');
I records the state of the index;
W records the state of the working tree.
and "stash" refers to W.
Think what commit B is in this example and the reason why you should
never ever do "git merge stash" will become apparent. By merging W
into 'maint', you would be pulling the entire history between
'maint' and 'master' to the result.
^ permalink raw reply
* Re: mergetool: include custom tools in '--tool-help'
From: Junio C Hamano @ 2013-01-27 18:03 UTC (permalink / raw)
To: John Keeping; +Cc: git, David Aguilar
In-Reply-To: <20130127163442.GQ7498@serenity.lan>
John Keeping <john@keeping.me.uk> writes:
> 'git mergetool --tool-help' only lists builtin tools, not those that the
> user has configured via a 'mergetool.<tool>.cmd' config value. Fix this
> by inspecting the tools configured in this way and adding them to the
> available and unavailable lists before displaying them.
Although I am not a mergetool user, I would imagine that it would
make sense to show it as available.
Just like "git help -a" lists subcommands in a way that can be easy
to tell which ones are the standard ones and which ones are user
customizations, this may want to give a similar distinction, though.
I dunno.
>
> Signed-off-by: John Keeping <john@keeping.me.uk>
> ---
> After the recent changes to mergetool, do we want to do something like
> this as well, so that 'git mergetool --tool-help' will display any tools
> configured by the user/system administrator?
>
> This is on top of jk/mergetool.
>
> git-mergetool--lib.sh | 29 +++++++++++++++++++++++++++++
> 1 file changed, 29 insertions(+)
>
> diff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh
> index 1d0fb12..f9a617c 100644
> --- a/git-mergetool--lib.sh
> +++ b/git-mergetool--lib.sh
> @@ -206,6 +206,29 @@ list_merge_tool_candidates () {
> esac
> }
>
> +# Adds tools from git-config to the available and unavailable lists.
> +# The tools are found in "$1.<tool>.cmd".
> +add_config_tools() {
> + section=$1
> +
> + eval $(git config --get-regexp $section'\..*\.cmd' |
> + while read -r key value
> + do
> + tool=${key#mergetool.}
> + tool=${tool%.cmd}
> +
> + tool=$(echo "$tool" |sed -e 's/'\''/'\''\\'\'\''/g')
> +
> + cmd=$(eval -- "set -- $value"; echo "$1")
> + if type "$cmd" >/dev/null 2>&1
> + then
> + echo "available=\"\${available}\"'$tool'\"\$LF\""
> + else
> + echo "unavailable=\"\${unavailable}\"'$tool'\"\$LF\""
> + fi
> + done)
> +}
> +
> show_tool_help () {
> unavailable= available= LF='
> '
> @@ -223,6 +246,12 @@ show_tool_help () {
> fi
> done
>
> + add_config_tools mergetool
> + if diff_mode
> + then
> + add_config_tools difftool
> + fi
> +
> cmd_name=${TOOL_MODE}tool
> if test -n "$available"
> then
^ permalink raw reply
* Re: [git-multimail] License unknown (#1)
From: Michael Haggerty @ 2013-01-27 18:52 UTC (permalink / raw)
To: git discussion list, Andy Parkins; +Cc: mhagger/git-multimail, Michiel Holtkamp
In-Reply-To: <mhagger/git-multimail/issues/1/12754195@github.com>
I have a question about the license of contrib/hooks/post-commit-email.
I had assumed that since it is in the git project, which is GPLv2, and
since it contains no contrary information, it would by implication also
fall under GPLv2. But the file itself contains no explicit license
information, and it is not clear to me that the "signed-off-by" line
implies a particular license, either. (The signed-off-by *does* seem to
imply that the source code is under some kind of open source license,
but not which one.)
If somebody can explain what license the code is under and how they come
to that conclusion, I would be very grateful.
And if Andy Parkins (the original author) is listening, please indicate
whether you had any intent *other* than GPLv2.
For anybody who is interested, the file was first committed in
4557e0de5b and has been modified by several authors since then.
Given the pretty clear open-sourciness of the script, I don't think this
has to be made into a big issue. But it would be nice to state the
license explicitly for future users' information.
Thanks,
Michael
On 01/27/2013 02:38 PM, Michiel Holtkamp wrote:
> Actually, I'm not sure that it is GPLv2 for the original script. The
> COPYING file in the main project declares the project as GPLv2, but it
> also says that people contributing should make their preferences (for
> licensing) known. Maybe we can assume it's GPLv2, (as the original
> writer might have assumed it was GPLv2), but it's not explicitly stated
> so I'm not sure (IANAL).
--
Michael Haggerty
mhagger@alum.mit.edu
http://softwareswirl.blogspot.com/
^ permalink raw reply
* Re: [PATCH v4 1/2] for-each-repo: new command used for multi-repo operations
From: Junio C Hamano @ 2013-01-27 19:04 UTC (permalink / raw)
To: Lars Hjemli; +Cc: git
In-Reply-To: <1359290777-5483-2-git-send-email-hjemli@gmail.com>
Lars Hjemli <hjemli@gmail.com> writes:
> When working with multiple, unrelated (or loosly related) git repos,
> there is often a need to locate all repos with uncommitted work and
> perform some action on them (say, commit and push). Before this patch,
> such tasks would require manually visiting all repositories, running
> `git status` within each one and then decide what to do next.
>
> This mundane task can now be automated by e.g. `git for-each-repo --dirty
> status`, which will find all non-bare git repositories below the current
> directory (even nested ones), check if they are dirty (as defined by
> `git diff --quiet && git diff --cached --quiet`), and for each dirty repo
> print the path to the repo and then execute `git status` within the repo.
>
> The command also honours the option '--clean' which restricts the set of
> repos to those which '--dirty' would skip, and '-x' which is used to
> execute non-git commands.
It might make sense to internally use RUN_GIT_CMD flag when the
first word of the command line is 'git' as an optimization, but
I am not sure it is a good idea to force the end users to think
when to use -x and when not to is a good idea.
In other words, I think
git for-each-repo -d diff --name-only
git for-each-repo -d -x ls '*.c'
is less nice than letting the user say
git for-each-repo -d git diff --name-only
git for-each-repo -d ls '*.c'
> Finally, the command to execute within each repo is optional. If none is
> given, git-for-each-repo will just print the path to each repo found. And
> since the command supports -z, this can be used for more advanced scripting
> needs.
It amounts to the same thing, but I would rather describe it as:
To allow scripts to handle paths with shell-unsafe characters,
support "-z" to show paths with NUL termination. Otherwise,
such paths are shown with the usual c-quoting.
One more thing that nobody brought up during the previous reviews is
if we want to support subset of repositories by allowing the
standard pathspec match mechanism. For example,
git for-each-repo -d git diff --name-only -- foo/ bar/b\*z
might be a way to ask "please find repositories match the given
pathspecs (i.e. foo/ bar/b\*z) and run the command in the ones that
are dirty". We would need to think about how to mark the end of the
command though---we could borrow \; from find(1), even though find
is not the best example of the UI design. I.e.
git for-each-repo -d git diff --name-only \; [--] foo/ bar/b\*z
with or without "--".
> diff --git a/Documentation/git-for-each-repo.txt b/Documentation/git-for-each-repo.txt
> new file mode 100644
> index 0000000..fb12b3f
> --- /dev/null
> +++ b/Documentation/git-for-each-repo.txt
> @@ -0,0 +1,71 @@
> +git-for-each-repo(1)
> +====================
> +
> +NAME
> +----
> +git-for-each-repo - Execute a git command in multiple non-bare repositories
There is a separate topic in flight that turns s/git/Git/ when we
refer to the system as a whole. In any case, this is no longer
limited to "execute a Git command".
Find non-bare Git repositories in subdirectories
or
Find or execute a command in non-bare Git repositories in subdirectories
perhaps?
> +SYNOPSIS
> +--------
> +[verse]
> +'git for-each-repo' [-acdxz] [command]
> +
> +DESCRIPTION
> +-----------
> +The git-for-each-repo command is used to locate all non-bare git
Should be sufficient to say s/is used to locate/locates/.
> +repositories within the current directory tree, and optionally
> +execute a git command in each of the found repos.
s/a git command/a command/;
> +OPTIONS
> +-------
> ...
> +-x::
> + Execute a genric (non-git) command in each repo.
Drop this option.
> +NOTES
> +-----
> +
> +For the purpose of `git-for-each-repo`, a dirty worktree is defined as a
> +worktree with uncommitted changes.
Is it a definition that is different from usual? If so why does it
need to be inconsistent with the rest of the system?
> diff --git a/builtin/for-each-repo.c b/builtin/for-each-repo.c
> new file mode 100644
> index 0000000..9333ae0
> --- /dev/null
> +++ b/builtin/for-each-repo.c
> @@ -0,0 +1,145 @@
> +/*
> + * "git for-each-repo" builtin command.
> + *
> + * Copyright (c) 2013 Lars Hjemli <hjemli@gmail.com>
> + */
> +#include "cache.h"
> +#include "color.h"
> +#include "quote.h"
> +#include "builtin.h"
> +#include "run-command.h"
> +#include "parse-options.h"
> +
> +#define ALL 0
> +#define DIRTY 1
> +#define CLEAN 2
> +
> +static char *color = GIT_COLOR_NORMAL;
> +static int eol = '\n';
> +static int match;
> +static int runopt = RUN_GIT_CMD;
> +
> +static const char * const builtin_foreachrepo_usage[] = {
> + N_("git for-each-repo [-acdxz] [cmd]"),
> + NULL
> +};
> +
> +static struct option builtin_foreachrepo_options[] = {
> + OPT_SET_INT('a', "all", &match, N_("match both clean and dirty repositories"), ALL),
> + OPT_SET_INT('c', "clean", &match, N_("only show clean repositories"), CLEAN),
> + OPT_SET_INT('d', "dirty", &match, N_("only show dirty repositories"), DIRTY),
> + OPT_SET_INT('x', NULL, &runopt, N_("execute generic (non-git) command"), 0),
> + OPT_SET_INT('z', NULL, &eol, N_("terminate each repo path with NUL character"), 0),
> + OPT_END(),
> +};
> +
> +static int get_repo_state(const char *dir)
> +{
> + const char *diffidx[] = {"diff", "--quiet", "--cached", NULL};
> + const char *diffwd[] = {"diff", "--quiet", NULL};
> +
> + if (run_command_v_opt_cd_env(diffidx, RUN_GIT_CMD, dir, NULL) != 0)
> + return DIRTY;
> + if (run_command_v_opt_cd_env(diffwd, RUN_GIT_CMD, dir, NULL) != 0)
> + return DIRTY;
> + return CLEAN;
> +}
> +
> +static void print_repo_path(const char *path, unsigned pretty)
> +{
> + if (path[0] == '.' && path[1] == '/')
> + path += 2;
> + if (pretty)
> + color_fprintf_ln(stdout, color, "[%s]", path);
This is shown before running a command in that repository. I am of
two minds. It certainly is nice to be able to tell which repository
each block of output lines comes from, and not requiring the command
to do this themselves is a good default. However, I wonder if people
would want to do something like this:
git for-each-repo sh -c '
git diff --name-only |
sed -e "s|^|$path/|"
'
to get a consolidated view, in a way similar to how "submodule
foreach" can be used. This unconditional output will get in the way
for such a use case.
Oh, that reminds me of another thing. Perhaps we would want to
export the (relative) path to the found repository in some way to
allow the commands to do this kind of thing in the first place?
"submodule foreach" does this with $path, I think.
> + else
> + write_name_quoted(path, stdout, eol);
> +}
Nice. Doubly nice that you do not hardcode "color" at this point
but made it into a separate variable.
> +static void handle_repo(struct strbuf *path, const char **argv)
> +{
> + const char *gitdir;
> + int len;
> +
> + len = path->len;
> + strbuf_addstr(path, ".git");
> + gitdir = resolve_gitdir(path->buf);
> + strbuf_setlen(path, len - 1);
> + if (!gitdir)
> + goto done;
> + if (match != ALL && match != get_repo_state(path->buf))
> + goto done;
> + print_repo_path(path->buf, *argv != NULL);
> + if (*argv)
> + run_command_v_opt_cd_env(argv, runopt, path->buf, NULL);
> +done:
> + strbuf_addstr(path, "/");
OK, you get "$D/" from the caller, make it "$D/.git" to call
resolve_gitdir() with, turn it to "$D" before printing and runnning,
and then add "/" back. Slightly tricky but correct.
> +static int walk(struct strbuf *path, int argc, const char **argv)
> +{
> + DIR *dir;
> + struct dirent *ent;
> + struct stat st;
> + size_t len;
> + int has_dotgit = 0;
> + struct string_list list = STRING_LIST_INIT_DUP;
> + struct string_list_item *item;
> +
> + 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")) {
> + has_dotgit = 1;
> + continue;
> + }
> + switch (DTYPE(ent)) {
> + case DT_UNKNOWN:
> + case DT_LNK:
> + /* Use stat() to figure out if this path leads
> + * to a directory - it's not important if it's
> + * a symlink which gets us there.
> + */
> + strbuf_setlen(path, len);
> + strbuf_addstr(path, ent->d_name);
> + if (stat(path->buf, &st) || !S_ISDIR(st.st_mode))
> + break;
> + /* fallthrough */
> + case DT_DIR:
> + string_list_append(&list, ent->d_name);
> + break;
> + }
> + }
> + closedir(dir);
> + strbuf_setlen(path, len);
> + if (has_dotgit)
> + handle_repo(path, argv);
> + sort_string_list(&list);
> + for_each_string_list_item(item, &list) {
> + strbuf_setlen(path, len);
> + strbuf_addstr(path, item->string);
> + walk(path, argc, argv);
> + }
> + string_list_clear(&list, 0);
> + return 0;
> +}
Is the "collect-first-and-then-sort" done so that the repositories
are shown in a stable order regardless of the order in which
readdir() returns he entries? I am not complaining, but being
curious.
> diff --git a/t/t6400-for-each-repo.sh b/t/t6400-for-each-repo.sh
This command does not look like "6 - the revision tree commands" to
me. "7 - the porcelainish commands concerning the working tree" or
"9 - the git tools" may be a better match?
> new file mode 100755
> index 0000000..af02c0c
> --- /dev/null
> +++ b/t/t6400-for-each-repo.sh
> @@ -0,0 +1,150 @@
> +#!/bin/sh
> +#
> +# Copyright (c) 2013 Lars Hjemli
> +#
> +
> +test_description='Test the git-for-each-repo command'
> +
> +. ./test-lib.sh
> +
> +qname="with\"quote"
> +qqname="\"with\\\"quote\""
If Windows does not have problems with paths with dq in it, then
this is fine, but I dunno. Otherwise, you may want to exclude the
c-quote testing from the main part of the test, and have a single
test that has prerequisite for filesystems that can do this at the
end of the script.
> +test_expect_success "setup" '
> + test_create_repo clean &&
> + (cd clean && test_commit foo1) &&
> + git init --separate-git-dir=.cleansub clean/gitfile &&
> + (cd clean/gitfile && test_commit foo2 && echo bar >>foo2.t) &&
> + test_create_repo dirty-idx &&
> + (cd dirty-idx && test_commit foo3 && git rm foo3.t) &&
> + test_create_repo dirty-wt &&
> + (cd dirty-wt && mv .git .linkedgit && ln -s .linkedgit .git &&
Some platforms are symlink-challenged. Can we do this test without
"ln -s"? SYMLINKS prereq wouldn't be very useful for the setup
step, as all the remaining tests won't work without setting up the
test scenario.
> + test_commit foo4 && rm foo4.t) &&
> + test_create_repo "$qname" &&
> + (cd "$qname" && test_commit foo5) &&
> + mkdir fakedir && mkdir fakedir/.git
> +'
> +
> +test_expect_success "without filtering, all repos are included" '
> + echo "." >expect &&
> + echo "clean" >>expect &&
> + echo "clean/gitfile" >>expect &&
> + echo "dirty-idx" >>expect &&
> + echo "dirty-wt" >>expect &&
> + echo "$qqname" >>expect &&
A single
cat >expect <<-EOF
.
clean
clean/gitfile
...
$qqname
EOF
may be a lot easier to read (likewise for all the "expect"
preparation in the rest of the script).
> +test_expect_success "-z NUL-terminates each path" '
> + echo "(.)" >expect &&
> + echo "(clean)" >>expect &&
> + echo "(clean/gitfile)" >>expect &&
> + echo "(dirty-idx)" >>expect &&
> + echo "(dirty-wt)" >>expect &&
> + echo "($qname)" >>expect &&
> + git for-each-repo -z | xargs -0 printf "(%s)\n" >actual &&
This needs prereq on "xargs -0", but because we know we do not have
any string with Q in it in the expected list of repositories, it may
be simpler to do something like this:
echo ".QcleanQclean/gitfileQ...$qname" >expect &&
git for-each-repo -z | tr "\0" Q >actual &&
test_cmp expect actual
Thanks.
^ permalink raw reply
* Re: Behavior of stash apply vs merge
From: Robin Rosenberg @ 2013-01-27 19:09 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Git List
In-Reply-To: <7vvcaiwltj.fsf@alter.siamese.dyndns.org>
Thanks. Feeling a bit studid now.
I was actually thinking about using merge to implement stash apply
in JGit. What we have is broken so I tried using merge to implement
it and them compared to git merge --no-commit.. FAIL.
The main difference is of course that I set the merge base to
stash^1, which is obviously not what my question was about.
-- robin
^ permalink raw reply
* Re: Behavior of stash apply vs merge
From: Junio C Hamano @ 2013-01-27 19:12 UTC (permalink / raw)
To: Robin Rosenberg; +Cc: Git List
In-Reply-To: <2043716001.1806075.1359313796808.JavaMail.root@dewire.com>
Robin Rosenberg <robin.rosenberg@dewire.com> writes:
> Thanks. Feeling a bit studid now.
>
> I was actually thinking about using merge to implement stash apply
> in JGit. What we have is broken so I tried using merge to implement
> it and them compared to git merge --no-commit.. FAIL.
Do you have "cherry-pick"?
In short, "stash apply" is a "cherry-pick" in disguise.
^ permalink raw reply
* Re: [PATCH v4 1/2] for-each-repo: new command used for multi-repo operations
From: John Keeping @ 2013-01-27 19:42 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Lars Hjemli, git
In-Reply-To: <7vk3qywiqf.fsf@alter.siamese.dyndns.org>
On Sun, Jan 27, 2013 at 11:04:08AM -0800, Junio C Hamano wrote:
> One more thing that nobody brought up during the previous reviews is
> if we want to support subset of repositories by allowing the
> standard pathspec match mechanism. For example,
>
> git for-each-repo -d git diff --name-only -- foo/ bar/b\*z
>
> might be a way to ask "please find repositories match the given
> pathspecs (i.e. foo/ bar/b\*z) and run the command in the ones that
> are dirty". We would need to think about how to mark the end of the
> command though---we could borrow \; from find(1), even though find
> is not the best example of the UI design. I.e.
>
> git for-each-repo -d git diff --name-only \; [--] foo/ bar/b\*z
>
> with or without "--".
Would it be better to make this a (multi-valued) option?
git for-each-repo -d --filter=foo/ --filter=bar/b\*z git diff --name-only
It seems a lot simpler than trying to figure out how the command is
going to handle '--' arguments.
> Oh, that reminds me of another thing. Perhaps we would want to
> export the (relative) path to the found repository in some way to
> allow the commands to do this kind of thing in the first place?
> "submodule foreach" does this with $path, I think.
I think $path is the only variable exported by "submodule foreach" which
is applicable here, but it doesn't work on Windows, where environment
variables are case-insensitive.
Commit 64394e3 (git-submodule.sh: Don't use $path variable in
eval_gettext string) changed "submodule foreach" to use $sm_path
internally although I notice that the documentation still uses $path.
Perhaps $repo_path in this case?
John
^ permalink raw reply
* Re: [PATCH v4 1/2] for-each-repo: new command used for multi-repo operations
From: Junio C Hamano @ 2013-01-27 19:45 UTC (permalink / raw)
To: John Keeping; +Cc: Lars Hjemli, git
In-Reply-To: <20130127194223.GR7498@serenity.lan>
John Keeping <john@keeping.me.uk> writes:
> On Sun, Jan 27, 2013 at 11:04:08AM -0800, Junio C Hamano wrote:
>> One more thing that nobody brought up during the previous reviews is
>> if we want to support subset of repositories by allowing the
>> standard pathspec match mechanism. For example,
>>
>> git for-each-repo -d git diff --name-only -- foo/ bar/b\*z
>>
>> might be a way to ask "please find repositories match the given
>> pathspecs (i.e. foo/ bar/b\*z) and run the command in the ones that
>> are dirty". We would need to think about how to mark the end of the
>> command though---we could borrow \; from find(1), even though find
>> is not the best example of the UI design. I.e.
>>
>> git for-each-repo -d git diff --name-only \; [--] foo/ bar/b\*z
>>
>> with or without "--".
>
> Would it be better to make this a (multi-valued) option?
>
> git for-each-repo -d --filter=foo/ --filter=bar/b\*z git diff --name-only
The standard way to use filtering based on paths we have is to use
the pathspec parameters at the end of the commmand line.
I see no reason for such an inconsistency with an option like --filter.
>> Oh, that reminds me of another thing. Perhaps we would want to
>> export the (relative) path to the found repository in some way to
>> allow the commands to do this kind of thing in the first place?
>> "submodule foreach" does this with $path, I think.
>
> I think $path is the only variable exported by "submodule foreach" which
> is applicable here, but it doesn't work on Windows, where environment
> variables are case-insensitive.
>
> Commit 64394e3 (git-submodule.sh: Don't use $path variable in
> eval_gettext string) changed "submodule foreach" to use $sm_path
> internally although I notice that the documentation still uses $path.
>
> Perhaps $repo_path in this case?
I do not care too deeply about the name, as long as the names used
by both mechanisms are the same.
^ permalink raw reply
* Re: [PATCH] git-remote-testpy: fix patch hashing on Python 3
From: Junio C Hamano @ 2013-01-27 19:49 UTC (permalink / raw)
To: John Keeping; +Cc: Michael Haggerty, git, Sverre Rabbelier
In-Reply-To: <20130127145056.GP7498@serenity.lan>
John Keeping <john@keeping.me.uk> writes:
> When this change was originally made (0846b0c - git-remote-testpy: hash
> bytes explicitly , I didn't realised that the "hex" encoding we chose is
> a "bytes to bytes" encoding so it just fails with an error on Python 3
> in the same way as the original code.
>
> It is not possible to provide a single code path that works on Python 2
> and Python 3 since Python 2.x will attempt to decode the string before
> encoding it, which fails for strings that are not valid in the default
> encoding. Python 3.1 introduced the "surrogateescape" error handler
> which handles this correctly and permits a bytes -> unicode -> bytes
> round-trip to be lossless.
>
> At this point Python 3.0 is unsupported so we don't go out of our way to
> try to support it.
>
> Helped-by: Michael Haggerty <mhagger@alum.mit.edu>
> Signed-off-by: John Keeping <john@keeping.me.uk>
> ---
Thanks; will queue and wait for an Ack from Michael.
Does the helper function need to be named with leading underscore,
though?
> On Sun, Jan 27, 2013 at 02:13:29PM +0000, John Keeping wrote:
>> On Sun, Jan 27, 2013 at 05:44:37AM +0100, Michael Haggerty wrote:
>> > So to handle all of the cases across Python versions as closely as
>> > possible to the old 2.x code, it might be necessary to make the code
>> > explicitly depend on the Python version number, like:
>> >
>> > hasher = _digest()
>> > if sys.hexversion < 0x03000000:
>> > pathbytes = repo.path
>> > elif sys.hexversion < 0x03010000:
>> > # If support for Python 3.0.x is desired (note: result can
>> > # be different in this case than under 2.x or 3.1+):
>> > pathbytes = repo.path.encode(sys.getfilesystemencoding(),
>> > 'backslashreplace')
>> > else
>> > pathbytes = repo.path.encode(sys.getfilesystemencoding(),
>> > 'surrogateescape')
>> > hasher.update(pathbytes)
>> > repo.hash = hasher.hexdigest()
>
> How about this?
>
> git-remote-testpy.py | 18 +++++++++++++++++-
> 1 file changed, 17 insertions(+), 1 deletion(-)
>
> diff --git a/git-remote-testpy.py b/git-remote-testpy.py
> index c7a04ec..16b0c52 100644
> --- a/git-remote-testpy.py
> +++ b/git-remote-testpy.py
> @@ -36,6 +36,22 @@ if sys.hexversion < 0x02000000:
> sys.stderr.write("git-remote-testgit: requires Python 2.0 or later.\n")
> sys.exit(1)
>
> +
> +def _encode_filepath(path):
> + """Encodes a Unicode file path to a byte string.
> +
> + On Python 2 this is a no-op; on Python 3 we encode the string as
> + suggested by [1] which allows an exact round-trip from the command line
> + to the filesystem.
> +
> + [1] http://docs.python.org/3/c-api/unicode.html#file-system-encoding
> +
> + """
> + if sys.hexversion < 0x03000000:
> + return path
> + return path.encode('utf-8', 'surrogateescape')
> +
> +
> def get_repo(alias, url):
> """Returns a git repository object initialized for usage.
> """
> @@ -45,7 +61,7 @@ def get_repo(alias, url):
> repo.get_head()
>
> hasher = _digest()
> - hasher.update(repo.path.encode('hex'))
> + hasher.update(_encode_filepath(repo.path))
> repo.hash = hasher.hexdigest()
>
> repo.get_base_path = lambda base: os.path.join(
^ permalink raw reply
* Re: Behavior of stash apply vs merge
From: Robin Rosenberg @ 2013-01-27 19:51 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Git List
In-Reply-To: <7vfw1mwibu.fsf@alter.siamese.dyndns.org>
----- Ursprungligt meddelande -----
> Robin Rosenberg <robin.rosenberg@dewire.com> writes:
>
> > Thanks. Feeling a bit studid now.
> >
> > I was actually thinking about using merge to implement stash apply
> > in JGit. What we have is broken so I tried using merge to implement
> > it and them compared to git merge --no-commit.. FAIL.
>
> Do you have "cherry-pick"?
>
> In short, "stash apply" is a "cherry-pick" in disguise.
Yes, that's what I did. Thanks for confirming this. One for the working
tree and if that succeeds I do another one to restore the index if requested.
-- robin
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox