Re: [PATCH/RFC] git.c: support "!!" aliases that do not move cwd

[Duy Nguyen]

On Fri, Oct 7, 2016 at 10:55 PM, Jakub Narębski  wrote:
> W dniu 07.10.2016 o 16:19, Johannes Schindelin pisze:
>> On Fri, 7 Oct 2016, Jakub Narębski wrote:
>
>>> Note that we would have to teach git completion about new syntax;
>>> or new configuration variable if we go that route.
>>
>> Why would we? Git's completion does not expand aliases, it only completes
>> the aliases' names, not their values.
>
> Because Git completion finds out which _options_ and _arguments_
> to complete for an alias based on its expansion.
>
> Yes, this is nice bit of trickery...

It's c63437c (bash: improve aliased command recognition - 2010-02-23)
isn't it? This may favor j6t's approach [1] because retrieving alias
attributes is much easier.

[1] 
https://public-inbox.org/git/20161006190720.4ecf3ptl6mszt...@sigill.intra.peff.net/T/#mb1d7b8f31d595b05105b8ea2137756761e13dbf4
-- 
Duy

[PATCH] remote.c: free previous results when looking for a ref match

[Stefan Beller]

Signed-off-by: Stefan Beller 
---
 remote.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/remote.c b/remote.c
index ad6c542..5f9afb4 100644
--- a/remote.c
+++ b/remote.c
@@ -833,6 +833,8 @@ static int match_name_with_pattern(const char *key, const 
char *name,
strbuf_add(, value, vstar - value);
strbuf_add(, name + klen, namelen - klen - ksuffixlen);
strbuf_addstr(, vstar + 1);
+   if (*result)
+   free(*result);
*result = strbuf_detach(, NULL);
}
return ret;
@@ -1262,6 +1264,8 @@ static char *get_ref_match(const struct refspec *rs, int 
rs_nr, const struct ref
 */
if (!send_mirror && !starts_with(ref->name, "refs/heads/"))
return NULL;
+   if (name)
+   free(name);
name = xstrdup(ref->name);
}
if (ret_pat)
-- 
2.10.1.353.g1629400

[PATCH v4 4/4] mergetool: honor -O

[David Aguilar]

Teach mergetool to pass "-O" down to `git diff` when
specified on the command-line.

Helped-by: Johannes Sixt 
Signed-off-by: David Aguilar 
---
Since v3:

I missed one last piped invocation of "git mergetool" in the tests,
which has been fixed.

 Documentation/git-mergetool.txt | 10 ++
 git-mergetool.sh|  9 +++--
 t/t7610-mergetool.sh| 27 +++
 3 files changed, 40 insertions(+), 6 deletions(-)

diff --git a/Documentation/git-mergetool.txt b/Documentation/git-mergetool.txt
index c4c1a9b..3622d66 100644
--- a/Documentation/git-mergetool.txt
+++ b/Documentation/git-mergetool.txt
@@ -79,10 +79,12 @@ success of the resolution after the custom tool has exited.
Prompt before each invocation of the merge resolution program
to give the user a chance to skip the path.
 
-DIFF ORDER FILES
-
-`git mergetool` honors the `diff.orderFile` configuration variable
-used by `git diff`.  See linkgit:git-config[1] for more details.
+-O::
+   Process files in the order specified in the
+   , which has one shell glob pattern per line.
+   This overrides the `diff.orderFile` configuration variable
+   (see linkgit:git-config[1]).  To cancel `diff.orderFile`,
+   use `-O/dev/null`.
 
 TEMPORARY FILES
 ---
diff --git a/git-mergetool.sh b/git-mergetool.sh
index 65696d8..e52b4e4 100755
--- a/git-mergetool.sh
+++ b/git-mergetool.sh
@@ -9,7 +9,7 @@
 # at the discretion of Junio C Hamano.
 #
 
-USAGE='[--tool=tool] [--tool-help] [-y|--no-prompt|--prompt] [file to merge] 
...'
+USAGE='[--tool=tool] [--tool-help] [-y|--no-prompt|--prompt] [-O] 
[file to merge] ...'
 SUBDIRECTORY_OK=Yes
 NONGIT_OK=Yes
 OPTIONS_SPEC=
@@ -390,6 +390,7 @@ print_noop_and_exit () {
 main () {
prompt=$(git config --bool mergetool.prompt)
guessed_merge_tool=false
+   orderfile=
 
while test $# != 0
do
@@ -419,6 +420,9 @@ main () {
--prompt)
prompt=true
;;
+   -O*)
+   orderfile="$1"
+   ;;
--)
shift
break
@@ -460,7 +464,8 @@ main () {
fi
 
files=$(git -c core.quotePath=false \
-   diff --name-only --diff-filter=U -- "$@")
+   diff --name-only --diff-filter=U \
+   ${orderfile:+"$orderfile"} -- "$@")
 
cd_to_toplevel
 
diff --git a/t/t7610-mergetool.sh b/t/t7610-mergetool.sh
index 38c1e4d..6d9f215 100755
--- a/t/t7610-mergetool.sh
+++ b/t/t7610-mergetool.sh
@@ -638,5 +638,32 @@ test_expect_success 'diff.orderFile configuration is 
honored' '
test_cmp expect actual &&
git reset --hard >/dev/null
 '
+test_expect_success 'mergetool -Oorder-file is honored' '
+   test_config diff.orderFile order-file &&
+   test_config mergetool.myecho.cmd "echo \"\$LOCAL\"" &&
+   test_config mergetool.myecho.trustExitCode true &&
+   test_must_fail git merge order-file-side1 &&
+   cat >expect <<-\EOF &&
+   Merging:
+   a
+   b
+   EOF
+   git mergetool -O/dev/null --no-prompt --tool myecho >output &&
+   git grep --no-index -h -A2 Merging: output >actual &&
+   test_cmp expect actual &&
+   git reset --hard >/dev/null 2>&1 &&
+
+   git config --unset diff.orderFile &&
+   test_must_fail git merge order-file-side1 &&
+   cat >expect <<-\EOF &&
+   Merging:
+   b
+   a
+   EOF
+   git mergetool -Oorder-file --no-prompt --tool myecho >output &&
+   git grep --no-index -h -A2 Merging: output >actual &&
+   test_cmp expect actual &&
+   git reset --hard >/dev/null 2>&1
+'
 
 test_done
-- 
2.10.1.386.g8ee99a0

[PATCH v3 2/4] mergetool: move main program flow into a main() function

[David Aguilar]

Make it easier to follow the program's flow by isolating all
logic into functions.  Isolate the main execution code path into
a single unit instead of having prompt_after_failed_merge()
interrupt it partyway through.

The use of a main() function is borrowing a convention from C,
Python, Perl, and many other languages.  This helps readers more
familiar with other languages understand the purpose of each
function when diving into the codebase with fresh eyes.

Signed-off-by: David Aguilar 
---
Unchanged since v2; included for completeness.

 git-mergetool.sh | 180 ---
 1 file changed, 93 insertions(+), 87 deletions(-)

diff --git a/git-mergetool.sh b/git-mergetool.sh
index 300ce7f..b2cd0a4 100755
--- a/git-mergetool.sh
+++ b/git-mergetool.sh
@@ -366,51 +366,6 @@ merge_file () {
return 0
 }
 
-prompt=$(git config --bool mergetool.prompt)
-guessed_merge_tool=false
-
-while test $# != 0
-do
-   case "$1" in
-   --tool-help=*)
-   TOOL_MODE=${1#--tool-help=}
-   show_tool_help
-   ;;
-   --tool-help)
-   show_tool_help
-   ;;
-   -t|--tool*)
-   case "$#,$1" in
-   *,*=*)
-   merge_tool=$(expr "z$1" : 'z-[^=]*=\(.*\)')
-   ;;
-   1,*)
-   usage ;;
-   *)
-   merge_tool="$2"
-   shift ;;
-   esac
-   ;;
-   -y|--no-prompt)
-   prompt=false
-   ;;
-   --prompt)
-   prompt=true
-   ;;
-   --)
-   shift
-   break
-   ;;
-   -*)
-   usage
-   ;;
-   *)
-   break
-   ;;
-   esac
-   shift
-done
-
 prompt_after_failed_merge () {
while true
do
@@ -427,57 +382,108 @@ prompt_after_failed_merge () {
done
 }
 
-git_dir_init
-require_work_tree
+main () {
+   prompt=$(git config --bool mergetool.prompt)
+   guessed_merge_tool=false
+
+   while test $# != 0
+   do
+   case "$1" in
+   --tool-help=*)
+   TOOL_MODE=${1#--tool-help=}
+   show_tool_help
+   ;;
+   --tool-help)
+   show_tool_help
+   ;;
+   -t|--tool*)
+   case "$#,$1" in
+   *,*=*)
+   merge_tool=$(expr "z$1" : 'z-[^=]*=\(.*\)')
+   ;;
+   1,*)
+   usage ;;
+   *)
+   merge_tool="$2"
+   shift ;;
+   esac
+   ;;
+   -y|--no-prompt)
+   prompt=false
+   ;;
+   --prompt)
+   prompt=true
+   ;;
+   --)
+   shift
+   break
+   ;;
+   -*)
+   usage
+   ;;
+   *)
+   break
+   ;;
+   esac
+   shift
+   done
+
+   git_dir_init
+   require_work_tree
 
-if test -z "$merge_tool"
-then
-   # Check if a merge tool has been configured
-   merge_tool=$(get_configured_merge_tool)
-   # Try to guess an appropriate merge tool if no tool has been set.
if test -z "$merge_tool"
then
-   merge_tool=$(guess_merge_tool) || exit
-   guessed_merge_tool=true
+   # Check if a merge tool has been configured
+   merge_tool=$(get_configured_merge_tool)
+   # Try to guess an appropriate merge tool if no tool has been 
set.
+   if test -z "$merge_tool"
+   then
+   merge_tool=$(guess_merge_tool) || exit
+   guessed_merge_tool=true
+   fi
fi
-fi
-merge_keep_backup="$(git config --bool mergetool.keepBackup || echo true)"
-merge_keep_temporaries="$(git config --bool mergetool.keepTemporaries || echo 
false)"
-
-files=
+   merge_keep_backup="$(git config --bool mergetool.keepBackup || echo 
true)"
+   merge_keep_temporaries="$(git config --bool mergetool.keepTemporaries 
|| echo false)"
 
-if test $# -eq 0
-then
-   cd_to_toplevel
+   files=
 
-   if test -e "$GIT_DIR/MERGE_RR"
+   if test $# -eq 0
then
-   files=$(git rerere remaining)
+   cd_to_toplevel
+
+   if test -e "$GIT_DIR/MERGE_RR"
+   then
+   files=$(git rerere remaining)
+   else
+   files=$(git ls-files -u |
+ 

[PATCH v3 3/4] mergetool: honor diff.orderFile

[David Aguilar]

Teach mergetool to get the list of files to edit via `diff` so that we
gain support for diff.orderFile.

Suggested-by: Luis Gutierrez 
Helped-by: Johannes Sixt 
Signed-off-by: David Aguilar 
---
Changes since v2:

The tests no longer rely on "grep -A" and instead use "git grep"
for portability.  The mergetool output in the test is redirected
to a file so that we can catch its exit code.

 Documentation/git-mergetool.txt |  5 +
 git-mergetool.sh| 30 +++---
 t/t7610-mergetool.sh| 33 +
 3 files changed, 53 insertions(+), 15 deletions(-)

diff --git a/Documentation/git-mergetool.txt b/Documentation/git-mergetool.txt
index e846c2e..c4c1a9b 100644
--- a/Documentation/git-mergetool.txt
+++ b/Documentation/git-mergetool.txt
@@ -79,6 +79,11 @@ success of the resolution after the custom tool has exited.
Prompt before each invocation of the merge resolution program
to give the user a chance to skip the path.
 
+DIFF ORDER FILES
+
+`git mergetool` honors the `diff.orderFile` configuration variable
+used by `git diff`.  See linkgit:git-config[1] for more details.
+
 TEMPORARY FILES
 ---
 `git mergetool` creates `*.orig` backup files while resolving merges.
diff --git a/git-mergetool.sh b/git-mergetool.sh
index b2cd0a4..65696d8 100755
--- a/git-mergetool.sh
+++ b/git-mergetool.sh
@@ -382,6 +382,11 @@ prompt_after_failed_merge () {
done
 }
 
+print_noop_and_exit () {
+   echo "No files need merging"
+   exit 0
+}
+
 main () {
prompt=$(git config --bool mergetool.prompt)
guessed_merge_tool=false
@@ -445,28 +450,23 @@ main () {
merge_keep_backup="$(git config --bool mergetool.keepBackup || echo 
true)"
merge_keep_temporaries="$(git config --bool mergetool.keepTemporaries 
|| echo false)"
 
-   files=
-
-   if test $# -eq 0
+   if test $# -eq 0 && test -e "$GIT_DIR/MERGE_RR"
then
-   cd_to_toplevel
-
-   if test -e "$GIT_DIR/MERGE_RR"
+   set -- $(git rerere remaining)
+   if test $# -eq 0
then
-   files=$(git rerere remaining)
-   else
-   files=$(git ls-files -u |
-   sed -e 's/^[^   ]*  //' | sort -u)
+   print_noop_and_exit
fi
-   else
-   files=$(git ls-files -u -- "$@" |
-   sed -e 's/^[^   ]*  //' | sort -u)
fi
 
+   files=$(git -c core.quotePath=false \
+   diff --name-only --diff-filter=U -- "$@")
+
+   cd_to_toplevel
+
if test -z "$files"
then
-   echo "No files need merging"
-   exit 0
+   print_noop_and_exit
fi
 
printf "Merging:\n"
diff --git a/t/t7610-mergetool.sh b/t/t7610-mergetool.sh
index 7217f39..38c1e4d 100755
--- a/t/t7610-mergetool.sh
+++ b/t/t7610-mergetool.sh
@@ -606,4 +606,37 @@ test_expect_success MKTEMP 'temporary filenames are used 
with mergetool.writeToT
git reset --hard master >/dev/null 2>&1
 '
 
+test_expect_success 'diff.orderFile configuration is honored' '
+   test_config diff.orderFile order-file &&
+   test_config mergetool.myecho.cmd "echo \"\$LOCAL\"" &&
+   test_config mergetool.myecho.trustExitCode true &&
+   echo b >order-file &&
+   echo a >>order-file &&
+   git checkout -b order-file-start master &&
+   echo start >a &&
+   echo start >b &&
+   git add a b &&
+   git commit -m start &&
+   git checkout -b order-file-side1 order-file-start &&
+   echo side1 >a &&
+   echo side1 >b &&
+   git add a b &&
+   git commit -m side1 &&
+   git checkout -b order-file-side2 order-file-start &&
+   echo side2 >a &&
+   echo side2 >b &&
+   git add a b &&
+   git commit -m side2 &&
+   test_must_fail git merge order-file-side1 &&
+   cat >expect <<-\EOF &&
+   Merging:
+   b
+   a
+   EOF
+   git mergetool --no-prompt --tool myecho >output &&
+   git grep --no-index -h -A2 Merging: output >actual &&
+   test_cmp expect actual &&
+   git reset --hard >/dev/null
+'
+
 test_done
-- 
2.10.1.386.g8ee99a0

[PATCH v3 4/4] mergetool: honor -O

[David Aguilar]

Teach mergetool to pass "-O" down to `git diff` when
specified on the command-line.

Helped-by: Johannes Sixt 
Signed-off-by: David Aguilar 
---
Changes since v2:

The tests no longer rely on "grep -A" and instead use "git grep"
for portability.  The mergetool output in the test is redirected
to a file so that we can catch its exit code.

The $orderfile variable is now passed using ${xxx:+"$xxx"}
to avoid conditional logic.

 Documentation/git-mergetool.txt | 10 ++
 git-mergetool.sh|  9 +++--
 t/t7610-mergetool.sh| 27 +++
 3 files changed, 40 insertions(+), 6 deletions(-)

diff --git a/Documentation/git-mergetool.txt b/Documentation/git-mergetool.txt
index c4c1a9b..3622d66 100644
--- a/Documentation/git-mergetool.txt
+++ b/Documentation/git-mergetool.txt
@@ -79,10 +79,12 @@ success of the resolution after the custom tool has exited.
Prompt before each invocation of the merge resolution program
to give the user a chance to skip the path.
 
-DIFF ORDER FILES
-
-`git mergetool` honors the `diff.orderFile` configuration variable
-used by `git diff`.  See linkgit:git-config[1] for more details.
+-O::
+   Process files in the order specified in the
+   , which has one shell glob pattern per line.
+   This overrides the `diff.orderFile` configuration variable
+   (see linkgit:git-config[1]).  To cancel `diff.orderFile`,
+   use `-O/dev/null`.
 
 TEMPORARY FILES
 ---
diff --git a/git-mergetool.sh b/git-mergetool.sh
index 65696d8..e52b4e4 100755
--- a/git-mergetool.sh
+++ b/git-mergetool.sh
@@ -9,7 +9,7 @@
 # at the discretion of Junio C Hamano.
 #
 
-USAGE='[--tool=tool] [--tool-help] [-y|--no-prompt|--prompt] [file to merge] 
...'
+USAGE='[--tool=tool] [--tool-help] [-y|--no-prompt|--prompt] [-O] 
[file to merge] ...'
 SUBDIRECTORY_OK=Yes
 NONGIT_OK=Yes
 OPTIONS_SPEC=
@@ -390,6 +390,7 @@ print_noop_and_exit () {
 main () {
prompt=$(git config --bool mergetool.prompt)
guessed_merge_tool=false
+   orderfile=
 
while test $# != 0
do
@@ -419,6 +420,9 @@ main () {
--prompt)
prompt=true
;;
+   -O*)
+   orderfile="$1"
+   ;;
--)
shift
break
@@ -460,7 +464,8 @@ main () {
fi
 
files=$(git -c core.quotePath=false \
-   diff --name-only --diff-filter=U -- "$@")
+   diff --name-only --diff-filter=U \
+   ${orderfile:+"$orderfile"} -- "$@")
 
cd_to_toplevel
 
diff --git a/t/t7610-mergetool.sh b/t/t7610-mergetool.sh
index 38c1e4d..5cfad76 100755
--- a/t/t7610-mergetool.sh
+++ b/t/t7610-mergetool.sh
@@ -638,5 +638,32 @@ test_expect_success 'diff.orderFile configuration is 
honored' '
test_cmp expect actual &&
git reset --hard >/dev/null
 '
+test_expect_success 'mergetool -Oorder-file is honored' '
+   test_config diff.orderFile order-file &&
+   test_config mergetool.myecho.cmd "echo \"\$LOCAL\"" &&
+   test_config mergetool.myecho.trustExitCode true &&
+   test_must_fail git merge order-file-side1 &&
+   cat >expect <<-\EOF &&
+   Merging:
+   a
+   b
+   EOF
+   git mergetool -O/dev/null --no-prompt --tool myecho |
+   grep -A 2 Merging: >actual &&
+   test_cmp expect actual &&
+   git reset --hard >/dev/null 2>&1 &&
+
+   git config --unset diff.orderFile &&
+   test_must_fail git merge order-file-side1 &&
+   cat >expect <<-\EOF &&
+   Merging:
+   b
+   a
+   EOF
+   git mergetool -Oorder-file --no-prompt --tool myecho >output &&
+   git grep --no-index -h -A2 Merging: output >actual &&
+   test_cmp expect actual &&
+   git reset --hard >/dev/null 2>&1
+'
 
 test_done
-- 
2.10.1.386.g8ee99a0

[PATCH v3 1/4] mergetool: add copyright

[David Aguilar]

Signed-off-by: David Aguilar 
---
Unchanged since v1; included for completeness.

 git-mergetool.sh | 1 +
 1 file changed, 1 insertion(+)

diff --git a/git-mergetool.sh b/git-mergetool.sh
index bf86270..300ce7f 100755
--- a/git-mergetool.sh
+++ b/git-mergetool.sh
@@ -3,6 +3,7 @@
 # This program resolves merge conflicts in git
 #
 # Copyright (c) 2006 Theodore Y. Ts'o
+# Copyright (c) 2009-2016 David Aguilar
 #
 # This file is licensed under the GPL v2, or a later version
 # at the discretion of Junio C Hamano.
-- 
2.10.1.386.g8ee99a0

[PATCH v4 5/7] builtin/tag: add --format argument for tag -v

[santiago]

From: Lukas Puehringer 

Adding --format to git tag -v mutes the default output of the GPG
verification and instead prints the formatted tag object.
This allows callers to cross-check the tagname from refs/tags with
the tagname from the tag object header upon GPG verification.

The callback function for for_each_tag_name() didn't allow callers to
pass custom data to their callback functions. Add a new opaque pointer
to each_tag_name_fn's parameter to allow this.

Signed-off-by: Lukas Puehringer 
---
 Documentation/git-tag.txt |  2 +-
 builtin/tag.c | 34 +++---
 builtin/verify-tag.c  |  2 +-
 3 files changed, 25 insertions(+), 13 deletions(-)

diff --git a/Documentation/git-tag.txt b/Documentation/git-tag.txt
index 7ecca8e..3bb5e3c 100644
--- a/Documentation/git-tag.txt
+++ b/Documentation/git-tag.txt
@@ -15,7 +15,7 @@ SYNOPSIS
 'git tag' [-n[]] -l [--contains ] [--points-at ]
[--column[=] | --no-column] [--create-reflog] [--sort=]
[--format=] [--[no-]merged []] [...]
-'git tag' -v ...
+'git tag' -v [--format=] ...
 
 DESCRIPTION
 ---
diff --git a/builtin/tag.c b/builtin/tag.c
index 14f3b48..7730fd0 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -24,7 +24,7 @@ static const char * const git_tag_usage[] = {
N_("git tag -d ..."),
N_("git tag -l [-n[]] [--contains ] [--points-at ]"
"\n\t\t[--format=] [--[no-]merged []] 
[...]"),
-   N_("git tag -v ..."),
+   N_("git tag -v [--format=] ..."),
NULL
 };
 
@@ -66,15 +66,17 @@ static int list_tags(struct ref_filter *filter, struct 
ref_sorting *sorting, con
 }
 
 typedef int (*each_tag_name_fn)(const char *name, const char *ref,
-   const unsigned char *sha1);
+   const unsigned char *sha1, void *cb_data);
 
-static int for_each_tag_name(const char **argv, each_tag_name_fn fn)
+static int for_each_tag_name(const char **argv, each_tag_name_fn fn,
+   void *cb_data)
 {
const char **p;
char ref[PATH_MAX];
int had_error = 0;
unsigned char sha1[20];
 
+
for (p = argv; *p; p++) {
if (snprintf(ref, sizeof(ref), "refs/tags/%s", *p)
>= sizeof(ref)) {
@@ -87,14 +89,14 @@ static int for_each_tag_name(const char **argv, 
each_tag_name_fn fn)
had_error = 1;
continue;
}
-   if (fn(*p, ref, sha1))
+   if (fn(*p, ref, sha1, cb_data))
had_error = 1;
}
return had_error;
 }
 
 static int delete_tag(const char *name, const char *ref,
-   const unsigned char *sha1)
+   const unsigned char *sha1, void *cb_data)
 {
if (delete_ref(ref, sha1, 0))
return 1;
@@ -103,9 +105,16 @@ static int delete_tag(const char *name, const char *ref,
 }
 
 static int verify_tag(const char *name, const char *ref,
-   const unsigned char *sha1)
+   const unsigned char *sha1, void *cb_data)
 {
-   return verify_and_format_tag(sha1, name, NULL, GPG_VERIFY_VERBOSE);
+   int flags;
+   char *fmt_pretty = cb_data;
+   flags = GPG_VERIFY_VERBOSE;
+
+   if (fmt_pretty)
+   flags = GPG_VERIFY_QUIET;
+
+   return verify_and_format_tag(sha1, name, fmt_pretty, flags);
 }
 
 static int do_sign(struct strbuf *buffer)
@@ -334,7 +343,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
struct strbuf err = STRBUF_INIT;
struct ref_filter filter;
static struct ref_sorting *sorting = NULL, **sorting_tail = 
-   const char *format = NULL;
+   char *format = NULL;
struct option options[] = {
OPT_CMDMODE('l', "list", , N_("list tag names"), 'l'),
{ OPTION_INTEGER, 'n', NULL, , N_("n"),
@@ -424,9 +433,12 @@ int cmd_tag(int argc, const char **argv, const char 
*prefix)
if (filter.merge_commit)
die(_("--merged and --no-merged option are only allowed with 
-l"));
if (cmdmode == 'd')
-   return for_each_tag_name(argv, delete_tag);
-   if (cmdmode == 'v')
-   return for_each_tag_name(argv, verify_tag);
+   return for_each_tag_name(argv, delete_tag, NULL);
+   if (cmdmode == 'v') {
+   if (format)
+   verify_ref_format(format);
+   return for_each_tag_name(argv, verify_tag, format);
+   }
 
if (msg.given || msgfile) {
if (msg.given && msgfile)
-- 
2.10.0

[PATCH v4 7/7] t/t7004-tag: Add --format specifier tests

[santiago]

From: Santiago Torres 

tag -v now supports --format specifiers to inspect the contents of a tag
upon verification. Add two tests to ensure this behavior is respected in
future changes.

Signed-off-by: Santiago Torres 
---
 t/t7004-tag.sh | 16 
 1 file changed, 16 insertions(+)

diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh
index 8b0f71a..633b089 100755
--- a/t/t7004-tag.sh
+++ b/t/t7004-tag.sh
@@ -847,6 +847,22 @@ test_expect_success GPG 'verifying a forged tag should 
fail' '
test_must_fail git tag -v forged-tag
 '
 
+test_expect_success 'verifying a proper tag with --format pass and format 
accordingly' '
+   cat >expect <<-\EOF &&
+   tagname : signed-tag
+   EOF
+   git tag -v --format="tagname : %(tag)" "signed-tag" >actual &&
+   test_cmp expect actual
+'
+
+test_expect_success 'verifying a forged tag with --format fail and format 
accordingly' '
+   cat >expect <<-\EOF &&
+   tagname : forged-tag
+   EOF
+   test_must_fail git tag -v --format="tagname : %(tag)" "forged-tag" 
>actual &&
+   test_cmp expect actual
+'
+
 # blank and empty messages for signed tags:
 
 get_tag_header empty-signed-tag $commit commit $time >expect
-- 
2.10.0

[PATCH v4 6/7] t/t7030-verify-tag: Add --format specifier tests

[santiago]

From: Santiago Torres 

Verify-tag now provides --format specifiers to inspect and ensure the
contents of the tag are proper. We add two tests to ensure this
functionality works as expected: the return value should indicate if
verification passed, and the format specifiers must be respected.

Signed-off-by: Santiago Torres 
---
 t/t7030-verify-tag.sh | 16 
 1 file changed, 16 insertions(+)

diff --git a/t/t7030-verify-tag.sh b/t/t7030-verify-tag.sh
index 07079a4..ce37fd9 100755
--- a/t/t7030-verify-tag.sh
+++ b/t/t7030-verify-tag.sh
@@ -125,4 +125,20 @@ test_expect_success GPG 'verify multiple tags' '
test_cmp expect.stderr actual.stderr
 '
 
+test_expect_success 'verifying tag with --format' '
+   cat >expect <<-\EOF &&
+   tagname : fourth-signed
+   EOF
+   git verify-tag --format="tagname : %(tag)" "fourth-signed" >actual &&
+   test_cmp expect actual
+'
+
+test_expect_success 'verifying a forged tag with --format fail and format 
accordingly' '
+   cat >expect <<-\EOF &&
+   tagname : 7th forged-signed
+   EOF
+   test_must_fail git verify-tag --format="tagname : %(tag)" $(cat 
forged1.tag) >actual-forged &&
+   test_cmp expect actual-forged
+'
+
 test_done
-- 
2.10.0

[PATCH v4 0/7] Add --format to tag verification

[santiago]

From: Santiago Torres 

This is the fourth iteration of the series in [1][2][3], which comes as a
result of the discussion in [4]. The main goal of this patch series is to bring
--format to git tag verification so that upper-layer tools can inspect the
content of a tag and make decisions based on those contents.

In this re-woll we:

* Fixed the author fields and signed off by's throughout the patch
  series

* Added two more patches with unit tests to ensure the format specifier
  behaves as expected

* Fixed a missing initialization for the format specifier in verify-tag which
  caused a crash.

* Fixed an outdated git commit message that had the previous name of a
  function definition.

Thanks,
-Santiago

[1] http://public-inbox.org/git/20160930221806.3398-1-santi...@nyu.edu/
[2] http://public-inbox.org/git/20160922185317.349-1-santi...@nyu.edu/
[3] http://public-inbox.org/git/20160926224233.32702-1-santi...@nyu.edu/
[4] http://public-inbox.org/git/20160607195608.16643-1-santi...@nyu.edu/


Lukas Puehringer (4):
  tag: add format specifier to gpg_verify_tag
  gpg-interface, tag: add GPG_VERIFY_QUIET flag
  ref-filter: add function to print single ref_array_item
  builtin/tag: add --format argument for tag -v

Santiago Torres (3):
  builtin/verify-tag: add --format to verify-tag
  t/t7030-verify-tag: Add --format specifier tests
  t/t7004-tag: Add --format specifier tests

 Documentation/git-tag.txt|  2 +-
 Documentation/git-verify-tag.txt |  2 +-
 builtin/tag.c| 34 +++---
 builtin/verify-tag.c | 13 +++--
 gpg-interface.h  |  1 +
 ref-filter.c | 10 ++
 ref-filter.h |  3 +++
 t/t7004-tag.sh   | 16 
 t/t7030-verify-tag.sh| 16 
 tag.c| 22 +++---
 tag.h|  4 ++--
 11 files changed, 99 insertions(+), 24 deletions(-)

-- 
2.10.0

[PATCH v4 1/7] gpg-interface, tag: add GPG_VERIFY_QUIET flag

[santiago]

From: Lukas Puehringer 

Functions that print git object information may require that the
gpg-interface functions be silent. Add GPG_VERIFY_QUIET flag and prevent
print_signature_buffer from being called if flag is set.

Signed-off-by: Lukas Puehringer 
---
 gpg-interface.h | 1 +
 tag.c   | 5 -
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/gpg-interface.h b/gpg-interface.h
index ea68885..85dc982 100644
--- a/gpg-interface.h
+++ b/gpg-interface.h
@@ -3,6 +3,7 @@
 
 #define GPG_VERIFY_VERBOSE 1
 #define GPG_VERIFY_RAW 2
+#define GPG_VERIFY_QUIET   4
 
 struct signature_check {
char *payload;
diff --git a/tag.c b/tag.c
index d1dcd18..291073f 100644
--- a/tag.c
+++ b/tag.c
@@ -3,6 +3,7 @@
 #include "commit.h"
 #include "tree.h"
 #include "blob.h"
+#include "gpg-interface.h"
 
 const char *tag_type = "tag";
 
@@ -24,7 +25,9 @@ static int run_gpg_verify(const char *buf, unsigned long 
size, unsigned flags)
 
ret = check_signature(buf, payload_size, buf + payload_size,
size - payload_size, );
-   print_signature_buffer(, flags);
+
+   if (!(flags & GPG_VERIFY_QUIET))
+   print_signature_buffer(, flags);
 
signature_check_clear();
return ret;
-- 
2.10.0

[PATCH v4 2/7] ref-filter: add function to print single ref_array_item

[santiago]

From: Lukas Puehringer 

ref-filter functions are useful for printing git object information
using a format specifier. However, some other modules may not want to use
this functionality on a ref-array but only print a single item.

Expose a pretty_print_ref function to create, pretty print and free
individual ref-items.

Signed-off-by: Lukas Puehringer 
---
 ref-filter.c | 10 ++
 ref-filter.h |  3 +++
 2 files changed, 13 insertions(+)

diff --git a/ref-filter.c b/ref-filter.c
index 9adbb8a..cfbcd73 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -1637,6 +1637,16 @@ void show_ref_array_item(struct ref_array_item *info, 
const char *format, int qu
putchar('\n');
 }
 
+void pretty_print_ref(const char *name, const unsigned char *sha1,
+   const char *format, unsigned kind)
+{
+   struct ref_array_item *ref_item;
+   ref_item = new_ref_array_item(name, sha1, 0);
+   ref_item->kind = kind;
+   show_ref_array_item(ref_item, format, 0);
+   free_array_item(ref_item);
+}
+
 /*  If no sorting option is given, use refname to sort as default */
 struct ref_sorting *ref_default_sorting(void)
 {
diff --git a/ref-filter.h b/ref-filter.h
index 14d435e..3d23090 100644
--- a/ref-filter.h
+++ b/ref-filter.h
@@ -107,4 +107,7 @@ struct ref_sorting *ref_default_sorting(void);
 /*  Function to parse --merged and --no-merged options */
 int parse_opt_merge_filter(const struct option *opt, const char *arg, int 
unset);
 
+void pretty_print_ref(const char *name, const unsigned char *sha1,
+   const char *format, unsigned kind);
+
 #endif /*  REF_FILTER_H  */
-- 
2.10.0

[PATCH v4 3/7] tag: add format specifier to gpg_verify_tag

[santiago]

From: Lukas Puehringer 

Calling functions for gpg_verify_tag() may desire to print relevant
information about the header for further verification. Add an optional
format argument to print any desired information after GPG verification.

Signed-off-by: Lukas Puehringer 
---
 builtin/tag.c|  2 +-
 builtin/verify-tag.c |  2 +-
 tag.c| 17 +++--
 tag.h|  4 ++--
 4 files changed, 15 insertions(+), 10 deletions(-)

diff --git a/builtin/tag.c b/builtin/tag.c
index 50e4ae5..14f3b48 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -105,7 +105,7 @@ static int delete_tag(const char *name, const char *ref,
 static int verify_tag(const char *name, const char *ref,
const unsigned char *sha1)
 {
-   return gpg_verify_tag(sha1, name, GPG_VERIFY_VERBOSE);
+   return verify_and_format_tag(sha1, name, NULL, GPG_VERIFY_VERBOSE);
 }
 
 static int do_sign(struct strbuf *buffer)
diff --git a/builtin/verify-tag.c b/builtin/verify-tag.c
index 99f8148..de10198 100644
--- a/builtin/verify-tag.c
+++ b/builtin/verify-tag.c
@@ -51,7 +51,7 @@ int cmd_verify_tag(int argc, const char **argv, const char 
*prefix)
const char *name = argv[i++];
if (get_sha1(name, sha1))
had_error = !!error("tag '%s' not found.", name);
-   else if (gpg_verify_tag(sha1, name, flags))
+   else if (verify_and_format_tag(sha1, name, NULL, flags))
had_error = 1;
}
return had_error;
diff --git a/tag.c b/tag.c
index 291073f..d3512c0 100644
--- a/tag.c
+++ b/tag.c
@@ -4,6 +4,7 @@
 #include "tree.h"
 #include "blob.h"
 #include "gpg-interface.h"
+#include "ref-filter.h"
 
 const char *tag_type = "tag";
 
@@ -33,8 +34,8 @@ static int run_gpg_verify(const char *buf, unsigned long 
size, unsigned flags)
return ret;
 }
 
-int gpg_verify_tag(const unsigned char *sha1, const char *name_to_report,
-   unsigned flags)
+int verify_and_format_tag(const unsigned char *sha1, const char *name,
+   const char *fmt_pretty, unsigned flags)
 {
enum object_type type;
char *buf;
@@ -44,21 +45,25 @@ int gpg_verify_tag(const unsigned char *sha1, const char 
*name_to_report,
type = sha1_object_info(sha1, NULL);
if (type != OBJ_TAG)
return error("%s: cannot verify a non-tag object of type %s.",
-   name_to_report ?
-   name_to_report :
+   name ?
+   name :
find_unique_abbrev(sha1, DEFAULT_ABBREV),
typename(type));
 
buf = read_sha1_file(sha1, , );
if (!buf)
return error("%s: unable to read file.",
-   name_to_report ?
-   name_to_report :
+   name ?
+   name :
find_unique_abbrev(sha1, DEFAULT_ABBREV));
 
ret = run_gpg_verify(buf, size, flags);
 
free(buf);
+
+   if (fmt_pretty)
+   pretty_print_ref(name, sha1, fmt_pretty, FILTER_REFS_TAGS);
+
return ret;
 }
 
diff --git a/tag.h b/tag.h
index a5721b6..896b9c2 100644
--- a/tag.h
+++ b/tag.h
@@ -17,7 +17,7 @@ extern int parse_tag_buffer(struct tag *item, const void 
*data, unsigned long si
 extern int parse_tag(struct tag *item);
 extern struct object *deref_tag(struct object *, const char *, int);
 extern struct object *deref_tag_noverify(struct object *);
-extern int gpg_verify_tag(const unsigned char *sha1,
-   const char *name_to_report, unsigned flags);
+extern int verify_and_format_tag(const unsigned char *sha1, const char *name,
+   const char *fmt_pretty, unsigned flags);
 
 #endif /* TAG_H */
-- 
2.10.0

[PATCH v4 4/7] builtin/verify-tag: add --format to verify-tag

[santiago]

From: Santiago Torres 

Callers of verify-tag may want to cross-check the tagname from refs/tags
with the tagname from the tag object header upon GPG verification. This
is to avoid tag refs that point to an incorrect object.

Add a --format parameter to git verify-tag to print the formatted tag
object header in addition to or instead of the --verbose or --raw GPG
verification output.

Signed-off-by: Santiago Torres 
---
 Documentation/git-verify-tag.txt |  2 +-
 builtin/verify-tag.c | 13 +++--
 2 files changed, 12 insertions(+), 3 deletions(-)

diff --git a/Documentation/git-verify-tag.txt b/Documentation/git-verify-tag.txt
index d590edc..0b8075d 100644
--- a/Documentation/git-verify-tag.txt
+++ b/Documentation/git-verify-tag.txt
@@ -8,7 +8,7 @@ git-verify-tag - Check the GPG signature of tags
 SYNOPSIS
 
 [verse]
-'git verify-tag' ...
+'git verify-tag' [--format=] ...
 
 DESCRIPTION
 ---
diff --git a/builtin/verify-tag.c b/builtin/verify-tag.c
index de10198..745b6a6 100644
--- a/builtin/verify-tag.c
+++ b/builtin/verify-tag.c
@@ -12,12 +12,14 @@
 #include 
 #include "parse-options.h"
 #include "gpg-interface.h"
+#include "ref-filter.h"
 
 static const char * const verify_tag_usage[] = {
-   N_("git verify-tag [-v | --verbose] ..."),
+   N_("git verify-tag [-v | --verbose] [--format=] 
..."),
NULL
 };
 
+
 static int git_verify_tag_config(const char *var, const char *value, void *cb)
 {
int status = git_gpg_config(var, value, cb);
@@ -30,9 +32,11 @@ int cmd_verify_tag(int argc, const char **argv, const char 
*prefix)
 {
int i = 1, verbose = 0, had_error = 0;
unsigned flags = 0;
+   char *fmt_pretty = NULL;
const struct option verify_tag_options[] = {
OPT__VERBOSE(, N_("print tag contents")),
OPT_BIT(0, "raw", , N_("print raw gpg status output"), 
GPG_VERIFY_RAW),
+   OPT_STRING(  0 , "format", _pretty, N_("format"), 
N_("format to use for the output")),
OPT_END()
};
 
@@ -46,12 +50,17 @@ int cmd_verify_tag(int argc, const char **argv, const char 
*prefix)
if (verbose)
flags |= GPG_VERIFY_VERBOSE;
 
+   if (fmt_pretty) {
+   verify_ref_format(fmt_pretty);
+   flags |= GPG_VERIFY_QUIET;
+   }
+
while (i < argc) {
unsigned char sha1[20];
const char *name = argv[i++];
if (get_sha1(name, sha1))
had_error = !!error("tag '%s' not found.", name);
-   else if (verify_and_format_tag(sha1, name, NULL, flags))
+   else if (verify_and_format_tag(sha1, name, fmt_pretty, flags))
had_error = 1;
}
return had_error;
-- 
2.10.0

Re: [PATCH 2/2] use strbuf_add_unique_abbrev() for adding short hashes, part 2

[René Scharfe]

Am 07.10.2016 um 02:46 schrieb Jeff King:

On Tue, Sep 27, 2016 at 09:11:58PM +0200, René Scharfe wrote:


Call strbuf_add_unique_abbrev() to add abbreviated hashes to strbufs
instead of taking detours through find_unique_abbrev() and its static
buffer.  This is shorter and a bit more efficient.
[...]
diff --git a/diff.c b/diff.c
index a178ed3..be11e4e 100644
--- a/diff.c
+++ b/diff.c
@@ -3109,7 +3109,7 @@ static void fill_metainfo(struct strbuf *msg,
}
strbuf_addf(msg, "%s%sindex %s..", line_prefix, set,
find_unique_abbrev(one->oid.hash, abbrev));
-   strbuf_addstr(msg, find_unique_abbrev(two->oid.hash, abbrev));
+   strbuf_add_unique_abbrev(msg, two->oid.hash, abbrev);
if (one->mode == two->mode)
strbuf_addf(msg, " %06o", one->mode);
strbuf_addf(msg, "%s\n", reset);


This one is an interesting case, and maybe a good example of why blind
coccinelle usage can have some pitfalls. :)


Thank you for paying attention. :)  In general I agree that the 
surrounding code of such changes should be checked; the issue at hand 
could be part of a bigger problem.



We get rid of the strbuf_addstr(), but notice that we leave untouched
the find_unique_abbrev() call immediately above. There was a symmetry to
the two that has been lost.

Probably either:

  strbuf_addf(msg, "%s%sindex %s..%s", line_prefix, set
find_unique_abbrev(one->oid.hash, abbrev),
find_unique_abbrev(two->oid.hash, abbrev));

or:

  strbuf_addf(msg, "%s%sindex ", line_prefix, set);
  strbuf_add_unique_abbrev(msg, one->oid.hash, abbrev);
  strbuf_addstr(msg, "..");
  strbuf_add_unique_abbrev(msg, two->oid.hash, abbrev);

would be a more appropriate refactoring. The problem is in the original
patch (which also lacks symmetry; either this predates the multi-buffer
find_unique_abbrev, or the original author didn't know about it), but I
think your refactor makes it slightly worse.


I still think the automatically generated patch is a net win, but we 
shouldn't stop there.



I noticed because I have another series which touches these lines, and
it wants to symmetrically swap out find_unique_abbrev for something
else. :) I don't think it's a big enough deal to switch now (and I've
already rebased my series which will touch these lines), but I wanted to
mention it as a thing to watch out for as we do more of these kinds of
automated transformations.


OK, then I'll wait for that series to land.


--- a/submodule.c
+++ b/submodule.c
@@ -396,7 +396,7 @@ static void show_submodule_header(FILE *f, const char *path,
find_unique_abbrev(one->hash, DEFAULT_ABBREV));
if (!fast_backward && !fast_forward)
strbuf_addch(, '.');
-   strbuf_addstr(, find_unique_abbrev(two->hash, DEFAULT_ABBREV));
+   strbuf_add_unique_abbrev(>hash, two, DEFAULT_ABBREV);


This one is a similar situation, I think.


Yes, and there are some more.  Will take a look.

I don't know how to crack printf-style formats using semantic patches. 
It's easy for fixed formats (silly example):


- strbuf_addf(sb, "%s%s", a, b);
+ strbuf_addf(sb, "%s", a);
+ strbuf_addf(sb, "%s", b);

But how to do that for arbitrary formats?  Probably not worth it..

René

RE: Uninitialized submodules as symlinks

[David Turner]

> -Original Message-
> From: Stefan Beller [mailto:sbel...@google.com]
> Sent: Friday, October 07, 2016 2:56 PM
> To: David Turner
> Cc: git@vger.kernel.org
> Subject: Re: Uninitialized submodules as symlinks
> 
> On Fri, Oct 7, 2016 at 11:17 AM, David Turner 
> wrote:
> > Presently, uninitialized submodules are materialized in the working tree
> > as empty directories.
> 
> Right, there has to be something, to hint at the user that creating a file
> with that path is probably not what they want.
> 
> >  We would like to consider having them be symlinks.  Specifically, we'd
> > like them to be symlinks into a FUSE filesystem which retrieves files on
> > demand.
> >
> > We've actually already got a FUSE filesystem written, but we use a
> > different (semi-manual) means to connect it to the initialized submodules.
> 
> So you currently do a
> 
> git submodule init 
> custom-submodule make-symlink 
> 
> ?

We do something like

For each initialized submodule: symlink it into the right place in .../somedir
For each uninitialized submodule: symlink from the FUSE into the right place in 
.../somedir

So .../somedir has the structure of the git main repo, but is all symlinks -- 
some into FUSE, some into the git repo.

This means that when we initialize (or deinitialize) a submodule, we need to 
re-run the linking script.  

> > We hope to release this FUSE filesystem as free software at some point
> > soon, but we do not yet have a fixed schedule for doing so.  Having to run
> > a command to create the symlink-based "union" filesystem is not optimal
> > (since we have to re-run it every time we initialize or deinitialize a
> > submodule).
> >
> > But if the uninitialized submodules could be symlinks into the FUSE
> > filesystem, we wouldn't have this problem.  This solution isn't
> > necessarily FUSE-specific -- perhaps someone would want copies of the same
> > submodule in multiple repos, and would want to save disk space by having
> > all copies point to the same place.  So the symlinks would be configured
> > by a per-submodule config variable.
> 
> I'd imagine that you want both a per-submodule config variable as well as
> a global variable that is a default for all submodules?
> 
> git config submodule.trySymlinkDefault /mounted/fuse/
> # any (new) submodule tries to be linked to /mounted/fuse/
> git config submodule..symlinked ~/my/private/symlinked
> # The  submodule goes into another path.
> 
> As you propose the FUSE filesystem fetches files on demand, you probably
> want to disable things that scan the whole submodule, e.g. look at
> submodule..ignore to suppress status looking at all files.

I would actually expect that git would detect that the symlink is unmodified 
from the configured symlink and automatically decide not to look there.
 
> When looking through the options, you could add the value "symlink" to
> submodule..update, which then respects the
> submodule.trySymlinkDefault if present, such that
> 
> git clone --recurse-submodules ...
> 
> works and sets up the FUSE thing correctly.
> 
> How does the FUSE system handle different versions, i.e.
> `git submodule update` to checkout another version of the submodule?
> (btw, I plan on working on integrating submodules to "git checkout", so
> "submodule update" would not need to be run there, but we'd hook it into
> checkout instead)

The fuse has a (virtual) directory for each SHA of the main repo, with each 
submodule mapped to the then-current version of the submodule's code. Actually, 
it's a bit more complicated because the uninitialized modules point to 
already-built binaries -- that is, the symlink is to something like 
$fuse/$SHA/built/$submodule. 

If you check out a new version of the main module, in our current setup, you 
need to again update all of the submodule symlinks (as described above). 

Under my proposal, I guess this would still need to happen.  A post-checkout 
hook could handle it either way.  Despite this flaw, switching a submodule 
between an initialized and deinitialized state would still be more seamless 
with the symlinks.

> > Naturally, this would require some changes to code that examines the
> working tree -- git status, git diff, etc.  They would have to report
> "unchanged" for submodules which were still symlinks to the configured
> location.  I have not yet looked at the implementation details beyond
> this.
> >
> > Does this idea make any sense?  If I were to implement it (probably in a
> few months, but no official timeline yet), would patches be considered?
> 
> I am happy to review patches.

Thanks.

Re: "Purposes, Concepts,Misfits, and a Redesign of Git" (a research paper)

[Jakub Narębski]

W dniu 07.10.2016 o 19:52, Konstantin Khomoutov pisze:
> On Fri, 30 Sep 2016 19:14:13 +0300
> Konstantin Khomoutov  wrote:
> 
>> The "It Will Never Work in Theory" blog has just posted a summary of a
>> study which tried to identify shortcomings in the design of Git.
>>
>> In the hope it might be interesting, I post this summary here.
>> URL: http://neverworkintheory.org/2016/09/30/rethinking-git.html
> 
> I think it would be cool if someone among subscribers could post a link
> to our discussion thread [2] back onto that page.  Unfortunatrly that
> requires a Disqus login which I don't have, so may be someone in
> possession of such login could do that? ;-)
> 
> 2. 
> https://public-inbox.org/git/ce42f934-4a94-fa29-cff0-5ebb0f004...@gmail.com/T/#e95875b7940512b432ab2e29b3dd50ca448df9720

Posted.  Thanks for the idea.

-- 
Jakub Narębski

Re: [PATCH v7 0/4] recursive support for ls-files

[Brandon Williams]

On 10/07, Stefan Beller wrote:
> On Fri, Oct 7, 2016 at 11:34 AM, Stefan Beller  wrote:
> > On Fri, Oct 7, 2016 at 11:18 AM, Brandon Williams  wrote:
> >> Few minor fixes pointed out Stefan
> >
> 
> The series itself looks good though. :)
> 
> Thanks,
> Stefan

Thanks! And I'll keep that in mind for the future.  Still lots to learn
about contributing to the project.

-- 
Brandon Williams

Re: [PATCH v7 0/4] recursive support for ls-files

[Stefan Beller]

On Fri, Oct 7, 2016 at 11:34 AM, Stefan Beller  wrote:
> On Fri, Oct 7, 2016 at 11:18 AM, Brandon Williams  wrote:
>> Few minor fixes pointed out Stefan
>

The series itself looks good though. :)

Thanks,
Stefan

Re: [PATCH v7 0/4] recursive support for ls-files

[Stefan Beller]

On Fri, Oct 7, 2016 at 11:18 AM, Brandon Williams  wrote:
> Few minor fixes pointed out Stefan

Nit: This is not very informative, so you could provide an "interdiff"
to the last patch, (see https://github.com/trast/tbdiff for a
sophisticated tool,
I usually do a git diff origin/sb/)
That helps reviewers as they do not need to review it all again, but
they may remember what they annotated and see how you addressed it.

Thanks,
Stefan

Re: [PATCH v2 3/3] batch check whether submodule needs pushing into one call

[Stefan Beller]

On Fri, Oct 7, 2016 at 8:06 AM, Heiko Voigt  wrote:
> We run a command for each sha1 change in a submodule. This is
> unnecessary since we can simply batch all sha1's we want to check into
> one command. Lets do it so we can speedup the check when many submodule
> changes are in need of checking.
>
> Signed-off-by: Heiko Voigt 
> ---
>  submodule.c | 63 
> +
>  1 file changed, 34 insertions(+), 29 deletions(-)
>
> diff --git a/submodule.c b/submodule.c
> index 5044afc2f8..a05c2a34b1 100644
> --- a/submodule.c
> +++ b/submodule.c
> @@ -529,27 +529,49 @@ static int append_hash_to_argv(const unsigned char 
> sha1[20], void *data)
> return 0;
>  }
>
> -static int submodule_needs_pushing(const char *path, const unsigned char 
> sha1[20])
> +static int check_has_hash(const unsigned char sha1[20], void *data)
>  {
> -   if (add_submodule_odb(path) || !lookup_commit_reference(sha1))
> +   int *has_hash = (int *) data;
> +
> +   if (!lookup_commit_reference(sha1))
> +   *has_hash = 0;
> +
> +   return 0;
> +}
> +
> +static int submodule_has_hashes(const char *path, struct sha1_array *hashes)
> +{
> +   int has_hash = 1;
> +
> +   if (add_submodule_odb(path))
> +   return 0;
> +
> +   sha1_array_for_each_unique(hashes, check_has_hash, _hash);
> +   return has_hash;
> +}
> +
> +static int submodule_needs_pushing(const char *path, struct sha1_array 
> *hashes)
> +{
> +   if (!submodule_has_hashes(path, hashes))

So the above is an implicit lookup already, but we did that before,
too, so it's fine.

> @@ -658,13 +665,11 @@ int find_unpushed_submodules(struct sha1_array *hashes,
> argv_array_clear();
>
> for (i = 0; i < submodules.nr; i++) {
> -   struct string_list_item *item = [i];
> -   struct collect_submodule_from_sha1s_data data;
> -   data.submodule_path = item->string;
> -   data.needs_pushing = needs_pushing;
> -   sha1_array_for_each_unique((struct sha1_array *) item->util,
> -   collect_submodules_from_sha1s,
> -   );
> +   struct string_list_item *submodule = [i];
> +   struct sha1_array *hashes = (struct sha1_array *) 
> submodule->util;
> +
> +   if (submodule_needs_pushing(submodule->string, hashes))
> +   string_list_insert(needs_pushing, submodule->string);

That makes sense.

Thanks!
Stefan

[PATCH v7 3/4] ls-files: pass through safe options for --recurse-submodules

[Brandon Williams]

Pass through some known-safe options when recursing into submodules.
(--cached, -v, -t, -z, --debug, --eol)

Signed-off-by: Brandon Williams 
---
 builtin/ls-files.c | 30 +++---
 t/t3007-ls-files-recurse-submodules.sh | 16 
 2 files changed, 39 insertions(+), 7 deletions(-)

diff --git a/builtin/ls-files.c b/builtin/ls-files.c
index 63befed..b6144a5 100644
--- a/builtin/ls-files.c
+++ b/builtin/ls-files.c
@@ -30,6 +30,7 @@ static int line_terminator = '\n';
 static int debug_mode;
 static int show_eol;
 static int recurse_submodules;
+static struct argv_array submodules_options = ARGV_ARRAY_INIT;
 
 static const char *prefix;
 static const char *super_prefix;
@@ -168,6 +169,25 @@ static void show_killed_files(struct dir_struct *dir)
}
 }
 
+/*
+ * Compile an argv_array with all of the options supported by 
--recurse_submodules
+ */
+static void compile_submodule_options(const struct dir_struct *dir, int 
show_tag)
+{
+   if (line_terminator == '\0')
+   argv_array_push(_options, "-z");
+   if (show_tag)
+   argv_array_push(_options, "-t");
+   if (show_valid_bit)
+   argv_array_push(_options, "-v");
+   if (show_cached)
+   argv_array_push(_options, "--cached");
+   if (show_eol)
+   argv_array_push(_options, "--eol");
+   if (debug_mode)
+   argv_array_push(_options, "--debug");
+}
+
 /**
  * Recursively call ls-files on a submodule
  */
@@ -182,6 +202,9 @@ static void show_gitlink(const struct cache_entry *ce)
argv_array_push(, "ls-files");
argv_array_push(, "--recurse-submodules");
 
+   /* add supported options */
+   argv_array_pushv(, submodules_options.argv);
+
cp.git_cmd = 1;
cp.dir = ce->name;
status = run_command();
@@ -567,11 +590,12 @@ int cmd_ls_files(int argc, const char **argv, const char 
*cmd_prefix)
if (require_work_tree && !is_inside_work_tree())
setup_work_tree();
 
+   if (recurse_submodules)
+   compile_submodule_options(, show_tag);
+
if (recurse_submodules &&
(show_stage || show_deleted || show_others || show_unmerged ||
-show_killed || show_modified || show_resolve_undo ||
-show_valid_bit || show_tag || show_eol || with_tree ||
-(line_terminator == '\0')))
+show_killed || show_modified || show_resolve_undo || with_tree))
die("ls-files --recurse-submodules unsupported mode");
 
if (recurse_submodules && error_unmatch)
diff --git a/t/t3007-ls-files-recurse-submodules.sh 
b/t/t3007-ls-files-recurse-submodules.sh
index b5a53c3..33a2ea7 100755
--- a/t/t3007-ls-files-recurse-submodules.sh
+++ b/t/t3007-ls-files-recurse-submodules.sh
@@ -34,6 +34,18 @@ test_expect_success 'ls-files correctly outputs files in 
submodule' '
test_cmp expect actual
 '
 
+test_expect_success 'ls-files correctly outputs files in submodule with -z' '
+   lf_to_nul >expect <<-\EOF &&
+   .gitmodules
+   a
+   b/b
+   submodule/c
+   EOF
+
+   git ls-files --recurse-submodules -z >actual &&
+   test_cmp expect actual
+'
+
 test_expect_success 'ls-files does not output files not added to a repo' '
cat >expect <<-\EOF &&
.gitmodules
@@ -86,15 +98,11 @@ test_incompatible_with_recurse_submodules () {
"
 }
 
-test_incompatible_with_recurse_submodules -z
-test_incompatible_with_recurse_submodules -v
-test_incompatible_with_recurse_submodules -t
 test_incompatible_with_recurse_submodules --deleted
 test_incompatible_with_recurse_submodules --modified
 test_incompatible_with_recurse_submodules --others
 test_incompatible_with_recurse_submodules --stage
 test_incompatible_with_recurse_submodules --killed
 test_incompatible_with_recurse_submodules --unmerged
-test_incompatible_with_recurse_submodules --eol
 
 test_done
-- 
2.10.0

[PATCH v7 4/4] ls-files: add pathspec matching for submodules

[Brandon Williams]

Pathspecs can be a bit tricky when trying to apply them to submodules.
The main challenge is that the pathspecs will be with respect to the
superproject and not with respect to paths in the submodule.  The
approach this patch takes is to pass in the identical pathspec from the
superproject to the submodule in addition to the submodule-prefix, which
is the path from the root of the superproject to the submodule, and then
we can compare an entry in the submodule prepended with the
submodule-prefix to the pathspec in order to determine if there is a
match.

This patch also permits the pathspec logic to perform a prefix match against
submodules since a pathspec could refer to a file inside of a submodule.
Due to limitations in the wildmatch logic, a prefix match is only done
literally.  If any wildcard character is encountered we'll simply punt
and produce a false positive match.  More accurate matching will be done
once inside the submodule.  This is due to the superproject not knowing
what files could exist in the submodule.

Signed-off-by: Brandon Williams 
---
 Documentation/git-ls-files.txt |   3 +-
 builtin/ls-files.c |  27 +++--
 dir.c  |  46 +-
 dir.h  |   4 ++
 t/t3007-ls-files-recurse-submodules.sh | 108 -
 5 files changed, 175 insertions(+), 13 deletions(-)

diff --git a/Documentation/git-ls-files.txt b/Documentation/git-ls-files.txt
index ea01d45..446209e 100644
--- a/Documentation/git-ls-files.txt
+++ b/Documentation/git-ls-files.txt
@@ -140,8 +140,7 @@ a space) at the start of each line:
 
 --recurse-submodules::
Recursively calls ls-files on each submodule in the repository.
-   Currently there is only support for the --cached mode without a
-   pathspec.
+   Currently there is only support for the --cached mode.
 
 --abbrev[=]::
Instead of showing the full 40-byte hexadecimal object
diff --git a/builtin/ls-files.c b/builtin/ls-files.c
index b6144a5..0f25914 100644
--- a/builtin/ls-files.c
+++ b/builtin/ls-files.c
@@ -195,6 +195,7 @@ static void show_gitlink(const struct cache_entry *ce)
 {
struct child_process cp = CHILD_PROCESS_INIT;
int status;
+   int i;
 
argv_array_pushf(, "--super-prefix=%s%s/",
 super_prefix ? super_prefix : "",
@@ -205,6 +206,15 @@ static void show_gitlink(const struct cache_entry *ce)
/* add supported options */
argv_array_pushv(, submodules_options.argv);
 
+   /*
+* Pass in the original pathspec args.  The submodule will be
+* responsible for prepending the 'submodule_prefix' prior to comparing
+* against the pathspec for matches.
+*/
+   argv_array_push(, "--");
+   for (i = 0; i < pathspec.nr; i++)
+   argv_array_push(, pathspec.items[i].original);
+
cp.git_cmd = 1;
cp.dir = ce->name;
status = run_command();
@@ -223,7 +233,8 @@ static void show_ce_entry(const char *tag, const struct 
cache_entry *ce)
if (len >= ce_namelen(ce))
die("git ls-files: internal error - cache entry not superset of 
prefix");
 
-   if (recurse_submodules && S_ISGITLINK(ce->ce_mode)) {
+   if (recurse_submodules && S_ISGITLINK(ce->ce_mode) &&
+   submodule_path_match(, name.buf, ps_matched)) {
show_gitlink(ce);
} else if (match_pathspec(, name.buf, name.len,
  len, ps_matched,
@@ -602,16 +613,20 @@ int cmd_ls_files(int argc, const char **argv, const char 
*cmd_prefix)
die("ls-files --recurse-submodules does not support "
"--error-unmatch");
 
-   if (recurse_submodules && argc)
-   die("ls-files --recurse-submodules does not support pathspec");
-
parse_pathspec(, 0,
   PATHSPEC_PREFER_CWD |
   PATHSPEC_STRIP_SUBMODULE_SLASH_CHEAP,
   prefix, argv);
 
-   /* Find common prefix for all pathspec's */
-   max_prefix = common_prefix();
+   /*
+* Find common prefix for all pathspec's
+* This is used as a performance optimization which unfortunately cannot
+* be done when recursing into submodules
+*/
+   if (recurse_submodules)
+   max_prefix = NULL;
+   else
+   max_prefix = common_prefix();
max_prefix_len = max_prefix ? strlen(max_prefix) : 0;
 
/* Treat unmatching pathspec elements as errors */
diff --git a/dir.c b/dir.c
index 0ea235f..28e9736 100644
--- a/dir.c
+++ b/dir.c
@@ -207,8 +207,9 @@ int within_depth(const char *name, int namelen,
return 1;
 }
 
-#define DO_MATCH_EXCLUDE   1
-#define DO_MATCH_DIRECTORY 2
+#define DO_MATCH_EXCLUDE   (1<<0)
+#define DO_MATCH_DIRECTORY (1<<1)
+#define DO_MATCH_SUBMODULE (1<<2)
 
 /*
  * Does 'match' match the given 

[PATCH v7 2/4] ls-files: optionally recurse into submodules

[Brandon Williams]

Allow ls-files to recognize submodules in order to retrieve a list of
files from a repository's submodules.  This is done by forking off a
process to recursively call ls-files on all submodules. Use top-level
--super-prefix option to pass a path to the submodule which it can
use to prepend to output or pathspec matching logic.

Signed-off-by: Brandon Williams 
---
 Documentation/git-ls-files.txt |   8 +-
 builtin/ls-files.c | 138 -
 git.c  |   2 +-
 t/t3007-ls-files-recurse-submodules.sh | 100 
 4 files changed, 208 insertions(+), 40 deletions(-)
 create mode 100755 t/t3007-ls-files-recurse-submodules.sh

diff --git a/Documentation/git-ls-files.txt b/Documentation/git-ls-files.txt
index 0d933ac..ea01d45 100644
--- a/Documentation/git-ls-files.txt
+++ b/Documentation/git-ls-files.txt
@@ -18,7 +18,8 @@ SYNOPSIS
[--exclude-per-directory=]
[--exclude-standard]
[--error-unmatch] [--with-tree=]
-   [--full-name] [--abbrev] [--] [...]
+   [--full-name] [--recurse-submodules]
+   [--abbrev] [--] [...]
 
 DESCRIPTION
 ---
@@ -137,6 +138,11 @@ a space) at the start of each line:
option forces paths to be output relative to the project
top directory.
 
+--recurse-submodules::
+   Recursively calls ls-files on each submodule in the repository.
+   Currently there is only support for the --cached mode without a
+   pathspec.
+
 --abbrev[=]::
Instead of showing the full 40-byte hexadecimal object
lines, show only a partial prefix.
diff --git a/builtin/ls-files.c b/builtin/ls-files.c
index 00ea91a..63befed 100644
--- a/builtin/ls-files.c
+++ b/builtin/ls-files.c
@@ -14,6 +14,7 @@
 #include "resolve-undo.h"
 #include "string-list.h"
 #include "pathspec.h"
+#include "run-command.h"
 
 static int abbrev;
 static int show_deleted;
@@ -28,8 +29,10 @@ static int show_valid_bit;
 static int line_terminator = '\n';
 static int debug_mode;
 static int show_eol;
+static int recurse_submodules;
 
 static const char *prefix;
+static const char *super_prefix;
 static int max_prefix_len;
 static int prefix_len;
 static struct pathspec pathspec;
@@ -68,11 +71,24 @@ static void write_eolinfo(const struct cache_entry *ce, 
const char *path)
 static void write_name(const char *name)
 {
/*
+* Prepend the super_prefix to name to construct the full_name to be
+* written.
+*/
+   struct strbuf full_name = STRBUF_INIT;
+   if (super_prefix) {
+   strbuf_addstr(_name, super_prefix);
+   strbuf_addstr(_name, name);
+   name = full_name.buf;
+   }
+
+   /*
 * With "--full-name", prefix_len=0; this caller needs to pass
 * an empty string in that case (a NULL is good for "").
 */
write_name_quoted_relative(name, prefix_len ? prefix : NULL,
   stdout, line_terminator);
+
+   strbuf_release(_name);
 }
 
 static void show_dir_entry(const char *tag, struct dir_entry *ent)
@@ -152,55 +168,84 @@ static void show_killed_files(struct dir_struct *dir)
}
 }
 
+/**
+ * Recursively call ls-files on a submodule
+ */
+static void show_gitlink(const struct cache_entry *ce)
+{
+   struct child_process cp = CHILD_PROCESS_INIT;
+   int status;
+
+   argv_array_pushf(, "--super-prefix=%s%s/",
+super_prefix ? super_prefix : "",
+ce->name);
+   argv_array_push(, "ls-files");
+   argv_array_push(, "--recurse-submodules");
+
+   cp.git_cmd = 1;
+   cp.dir = ce->name;
+   status = run_command();
+   if (status)
+   exit(status);
+}
+
 static void show_ce_entry(const char *tag, const struct cache_entry *ce)
 {
+   struct strbuf name = STRBUF_INIT;
int len = max_prefix_len;
+   if (super_prefix)
+   strbuf_addstr(, super_prefix);
+   strbuf_addstr(, ce->name);
 
if (len >= ce_namelen(ce))
die("git ls-files: internal error - cache entry not superset of 
prefix");
 
-   if (!match_pathspec(, ce->name, ce_namelen(ce),
-   len, ps_matched,
-   S_ISDIR(ce->ce_mode) || S_ISGITLINK(ce->ce_mode)))
-   return;
+   if (recurse_submodules && S_ISGITLINK(ce->ce_mode)) {
+   show_gitlink(ce);
+   } else if (match_pathspec(, name.buf, name.len,
+ len, ps_matched,
+ S_ISDIR(ce->ce_mode) ||
+ S_ISGITLINK(ce->ce_mode))) {
+   if (tag && *tag && show_valid_bit &&
+   (ce->ce_flags & CE_VALID)) {
+   static char alttag[4];
+   memcpy(alttag, tag, 3);
+   if 

[PATCH v7 1/4] git: make super-prefix option

[Brandon Williams]

Add a super-prefix environment variable 'GIT_INTERNAL_SUPER_PREFIX'
which can be used to specify a path from above a repository down to its
root.  When such a super-prefix is specified, the paths reported by Git
are prefixed with it to make them relative to that directory "above".
The paths given by the user on the command line
(e.g. "git subcmd --output-file=path/to/a/file" and pathspecs) are taken
relative to the directory "above" to match.

The immediate use of this option is by commands which have a
--recurse-submodule option in order to give context to submodules about
how they were invoked.  This option is currently only allowed for
builtins which support a super-prefix.

Signed-off-by: Brandon Williams 
---
 Documentation/git.txt |  6 ++
 cache.h   |  2 ++
 environment.c | 13 +
 git.c | 25 +
 4 files changed, 46 insertions(+)

diff --git a/Documentation/git.txt b/Documentation/git.txt
index 7913fc2..2188ae6 100644
--- a/Documentation/git.txt
+++ b/Documentation/git.txt
@@ -13,6 +13,7 @@ SYNOPSIS
 [--exec-path[=]] [--html-path] [--man-path] [--info-path]
 [-p|--paginate|--no-pager] [--no-replace-objects] [--bare]
 [--git-dir=] [--work-tree=] [--namespace=]
+[--super-prefix=]
  []
 
 DESCRIPTION
@@ -601,6 +602,11 @@ foo.bar= ...`) sets `foo.bar` to the empty string.
details.  Equivalent to setting the `GIT_NAMESPACE` environment
variable.
 
+--super-prefix=::
+   Currently for internal use only.  Set a prefix which gives a path from
+   above a repository down to its root.  One use is to give submodules
+   context about the superproject that invoked it.
+
 --bare::
Treat the repository as a bare repository.  If GIT_DIR
environment is not set, it is set to the current working
diff --git a/cache.h b/cache.h
index 3556326..8cf495d 100644
--- a/cache.h
+++ b/cache.h
@@ -408,6 +408,7 @@ static inline enum object_type object_type(unsigned int 
mode)
 #define GIT_NAMESPACE_ENVIRONMENT "GIT_NAMESPACE"
 #define GIT_WORK_TREE_ENVIRONMENT "GIT_WORK_TREE"
 #define GIT_PREFIX_ENVIRONMENT "GIT_PREFIX"
+#define GIT_SUPER_PREFIX_ENVIRONMENT "GIT_INTERNAL_SUPER_PREFIX"
 #define DEFAULT_GIT_DIR_ENVIRONMENT ".git"
 #define DB_ENVIRONMENT "GIT_OBJECT_DIRECTORY"
 #define INDEX_ENVIRONMENT "GIT_INDEX_FILE"
@@ -468,6 +469,7 @@ extern int get_common_dir_noenv(struct strbuf *sb, const 
char *gitdir);
 extern int get_common_dir(struct strbuf *sb, const char *gitdir);
 extern const char *get_git_namespace(void);
 extern const char *strip_namespace(const char *namespaced_ref);
+extern const char *get_super_prefix(void);
 extern const char *get_git_work_tree(void);
 
 /*
diff --git a/environment.c b/environment.c
index ca72464..d12d7db 100644
--- a/environment.c
+++ b/environment.c
@@ -100,6 +100,8 @@ static char *work_tree;
 static const char *namespace;
 static size_t namespace_len;
 
+static const char *super_prefix;
+
 static const char *git_dir, *git_common_dir;
 static char *git_object_dir, *git_index_file, *git_graft_file;
 int git_db_env, git_index_env, git_graft_env, git_common_dir_env;
@@ -120,6 +122,7 @@ const char * const local_repo_env[] = {
NO_REPLACE_OBJECTS_ENVIRONMENT,
GIT_REPLACE_REF_BASE_ENVIRONMENT,
GIT_PREFIX_ENVIRONMENT,
+   GIT_SUPER_PREFIX_ENVIRONMENT,
GIT_SHALLOW_FILE_ENVIRONMENT,
GIT_COMMON_DIR_ENVIRONMENT,
NULL
@@ -222,6 +225,16 @@ const char *strip_namespace(const char *namespaced_ref)
return namespaced_ref + namespace_len;
 }
 
+const char *get_super_prefix(void)
+{
+   static int initialized;
+   if (!initialized) {
+   super_prefix = getenv(GIT_SUPER_PREFIX_ENVIRONMENT);
+   initialized = 1;
+   }
+   return super_prefix;
+}
+
 static int git_work_tree_initialized;
 
 /*
diff --git a/git.c b/git.c
index 1c61151..469a83f 100644
--- a/git.c
+++ b/git.c
@@ -164,6 +164,20 @@ static int handle_options(const char ***argv, int *argc, 
int *envchanged)
setenv(GIT_WORK_TREE_ENVIRONMENT, cmd, 1);
if (envchanged)
*envchanged = 1;
+   } else if (!strcmp(cmd, "--super-prefix")) {
+   if (*argc < 2) {
+   fprintf(stderr, "No prefix given for 
--super-prefix.\n" );
+   usage(git_usage_string);
+   }
+   setenv(GIT_SUPER_PREFIX_ENVIRONMENT, (*argv)[1], 1);
+   if (envchanged)
+   *envchanged = 1;
+   (*argv)++;
+   (*argc)--;
+   } else if (skip_prefix(cmd, "--super-prefix=", )) {
+   setenv(GIT_SUPER_PREFIX_ENVIRONMENT, cmd, 1);
+   if (envchanged)
+   *envchanged = 1;
} 

[PATCH v7 0/4] recursive support for ls-files

[Brandon Williams]

Few minor fixes pointed out Stefan

Brandon Williams (4):
  git: make super-prefix option
  ls-files: optionally recurse into submodules
  ls-files: pass through safe options for --recurse-submodules
  ls-files: add pathspec matching for submodules

 Documentation/git-ls-files.txt |   7 +-
 Documentation/git.txt  |   6 +
 builtin/ls-files.c | 181 +---
 cache.h|   2 +
 dir.c  |  46 +++-
 dir.h  |   4 +
 environment.c  |  13 ++
 git.c  |  27 -
 t/t3007-ls-files-recurse-submodules.sh | 210 +
 9 files changed, 452 insertions(+), 44 deletions(-)
 create mode 100755 t/t3007-ls-files-recurse-submodules.sh

-- 
2.10.0

Re: [PATCH v2 2/3] serialize collection of refs that contain submodule changes

[Stefan Beller]

On Fri, Oct 7, 2016 at 8:06 AM, Heiko Voigt  wrote:
> We are iterating over each pushed ref and want to check whether it
> contains changes to submodules. Instead of immediately checking each ref
> lets first collect them and then do the check for all of them in one
> revision walk.
>
> Signed-off-by: Heiko Voigt 
> ---
>  submodule.c | 36 +---
>  submodule.h |  5 +++--
>  transport.c | 29 +
>  3 files changed, 45 insertions(+), 25 deletions(-)
>
> diff --git a/submodule.c b/submodule.c
> index 59c9d15905..5044afc2f8 100644
> --- a/submodule.c
> +++ b/submodule.c
> @@ -522,6 +522,13 @@ static int has_remote(const char *refname, const struct 
> object_id *oid,
> return 1;
>  }
>
> +static int append_hash_to_argv(const unsigned char sha1[20], void *data)
> +{
> +   struct argv_array *argv = (struct argv_array *) data;
> +   argv_array_push(argv, sha1_to_hex(sha1));

Nit of the day:
When using the struct child-process, we have the oldstyle argv NULL
terminated array as
well as the new style args argv_array. So in that context we'd prefer
`args` as a name for
argv_array as that helps to distinguish from the old array type.
Here however `argv` seems to be a reasonable name, in fact whenever we
do not deal with
child processes, we seem to not like the `args` name:

$ git grep argv_array |wc -l
577
$ git grep argv_array |grep args |wc -l
293

The rest looks good to me. :)

Uninitialized submodules as symlinks

[David Turner]

Presently, uninitialized submodules are materialized in the working tree as 
empty directories.  We would like to consider having them be symlinks.  
Specifically, we'd like them to be symlinks into a FUSE filesystem which 
retrieves files on demand.

We've actually already got a FUSE filesystem written, but we use a different 
(semi-manual) means to connect it to the initialized submodules.  We hope to 
release this FUSE filesystem as free software at some point soon, but we do not 
yet have a fixed schedule for doing so.  Having to run a command to create the 
symlink-based "union" filesystem is not optimal (since we have to re-run it 
every time we initialize or deinitialize a submodule).

But if the uninitialized submodules could be symlinks into the FUSE filesystem, 
we wouldn't have this problem.  This solution isn't necessarily FUSE-specific 
-- perhaps someone would want copies of the same submodule in multiple repos, 
and would want to save disk space by having all copies point to the same place. 
 So the symlinks would be configured by a per-submodule config variable.

Naturally, this would require some changes to code that examines the working 
tree -- git status, git diff, etc.  They would have to report "unchanged" for 
submodules which were still symlinks to the configured location.  I have not 
yet looked at the implementation details beyond this.

Does this idea make any sense?  If I were to implement it (probably in a few 
months, but no official timeline yet), would patches be considered?

Re: [PATCH v2 1/3] serialize collection of changed submodules

[Stefan Beller]

On Fri, Oct 7, 2016 at 8:06 AM, Heiko Voigt  wrote:
> To check whether a submodule needs to be pushed we need to collect all
> changed submodules. Lets collect them first and then execute the
> possibly expensive test whether certain revisions are already pushed
> only once per submodule.
>
> There is further potential for optimization since we can assemble one
> command and only issued that instead of one call for each remote ref in
> the submodule.
>
> Signed-off-by: Heiko Voigt 
> ---
>  submodule.c | 63 
> -
>  1 file changed, 58 insertions(+), 5 deletions(-)
>
> diff --git a/submodule.c b/submodule.c
> index 2de06a3351..59c9d15905 100644
> --- a/submodule.c
> +++ b/submodule.c
> @@ -554,19 +554,34 @@ static int submodule_needs_pushing(const char *path, 
> const unsigned char sha1[20
> return 0;
>  }
>
> +static struct sha1_array *get_sha1s_from_list(struct string_list *submodules,
> +   const char *path)

So this will take the stringlist `submodules` and insert the path into it,
if it wasn't already in there. In case it is newly inserted, add a sha1_array
as util, so each inserted path has it's own empty array.

So it is both init of the data structures as well as retrieving them. I was
initially confused by the name as I assumed it would give you sha1s out
of a string list (e.g. transform strings to internal sha1 things).
Maybe it's just
me having a hard time to understand that, but I feel like the name could be
improved.

lookup_sha1_list_by_path,
insert_path_and_return_sha1_list ?

> +{
> +   struct string_list_item *item;
> +
> +   item = string_list_insert(submodules, path);
> +   if (item->util)
> +   return (struct sha1_array *) item->util;
> +
> +   /* NEEDSWORK: should we have sha1_array_init()? */
> +   item->util = xcalloc(1, sizeof(struct sha1_array));
> +   return (struct sha1_array *) item->util;
> +}
> +
>  static void collect_submodules_from_diff(struct diff_queue_struct *q,
>  struct diff_options *options,
>  void *data)
>  {
> int i;
> -   struct string_list *needs_pushing = data;
> +   struct string_list *submodules = data;
>
> for (i = 0; i < q->nr; i++) {
> struct diff_filepair *p = q->queue[i];
> +   struct sha1_array *hashes;
> if (!S_ISGITLINK(p->two->mode))
> continue;
> -   if (submodule_needs_pushing(p->two->path, p->two->oid.hash))
> -   string_list_insert(needs_pushing, p->two->path);
> +   hashes = get_sha1s_from_list(submodules, p->two->path);
> +   sha1_array_append(hashes, p->two->oid.hash);
> }
>  }
>
> @@ -582,14 +597,41 @@ static void find_unpushed_submodule_commits(struct 
> commit *commit,
> diff_tree_combined_merge(commit, 1, );
>  }
>
> +struct collect_submodule_from_sha1s_data {
> +   char *submodule_path;
> +   struct string_list *needs_pushing;
> +};
> +
> +static void collect_submodules_from_sha1s(const unsigned char sha1[20],
> +   void *data)
> +{
> +   struct collect_submodule_from_sha1s_data *me =
> +   (struct collect_submodule_from_sha1s_data *) data;
> +
> +   if (submodule_needs_pushing(me->submodule_path, sha1))
> +   string_list_insert(me->needs_pushing, me->submodule_path);
> +}
> +
> +static void free_submodules_sha1s(struct string_list *submodules)
> +{
> +   int i;
> +   for (i = 0; i < submodules->nr; i++) {
> +   struct string_list_item *item = >items[i];

You do not seem to make use of `i` explicitely, so
for_each_string_list_item might be more readable here?


> +   struct sha1_array *hashes = (struct sha1_array *) item->util;
> +   sha1_array_clear(hashes);
> +   }
> +   string_list_clear(submodules, 1);
> +}
> +
>  int find_unpushed_submodules(unsigned char new_sha1[20],
> const char *remotes_name, struct string_list *needs_pushing)
>  {
> struct rev_info rev;
> struct commit *commit;
> const char *argv[] = {NULL, NULL, "--not", "NULL", NULL};
> -   int argc = ARRAY_SIZE(argv) - 1;
> +   int argc = ARRAY_SIZE(argv) - 1, i;
> char *sha1_copy;
> +   struct string_list submodules = STRING_LIST_INIT_DUP;
>
> struct strbuf remotes_arg = STRBUF_INIT;
>
> @@ -603,12 +645,23 @@ int find_unpushed_submodules(unsigned char new_sha1[20],
> die("revision walk setup failed");
>
> while ((commit = get_revision()) != NULL)
> -   find_unpushed_submodule_commits(commit, needs_pushing);
> +   find_unpushed_submodule_commits(commit, );
>
> reset_revision_walk();
> free(sha1_copy);
> strbuf_release(_arg);
>
> +   for 

Re: "Purposes, Concepts,Misfits, and a Redesign of Git" (a research paper)

[Jakub Narębski]

On 7 October 2016 at 18:55, Santiago Perez De Rosso
 wrote:
> On Wed, Oct 5, 2016 at 6:15 AM Jakub Narębski  wrote:
>> On 5 October 2016 at 04:55, Santiago Perez De Rosso
>>  wrote:
>>> On Fri, Sep 30, 2016 at 6:25 PM Jakub Narębski  wrote:
 W dniu 30.09.2016 o 18:14, Konstantin Khomoutov pisze:

> The "It Will Never Work in Theory" blog has just posted a summary of a
> study which tried to identify shortcomings in the design of Git.
>
> In the hope it might be interesting, I post this summary here.
> URL: http://neverworkintheory.org/2016/09/30/rethinking-git.html

 I will comment on the article itself, not just on the summary.

 | 2.2 Git
 [...]
 | But tracked files cannot be ignored; to ignore a tracked file
 | one has to mark it as “assume unchanged.” This “assume
 | unchanged” file will not be recognized by add; to make it
 | tracked again this marking has to be removed.
[...]
>> Yes, this is true that users may want to be able to ignore changes to
>> tracked files (commit with dirty tree), but using `assume-unchanged` is
>> wrong and dangerous solution.  Unfortunately the advice to use it is
>> surprisingly pervasive.  I would thank you to not further this error.
>
> Ok, I added a footnote in the paper when we first mention assume unchanged
> that says:
>
> Assume unchanged was intended to be used as a performance optimization but
> has since been appropriated by users as a way to ignore tracked files. The
> current advice is to use the “skip worktree” marking instead
>
> This should prompt readers to look into skip worktree next time they want to
> ignore tracked files. I don't think people reading the paper are doing so to
> learn Git but at least it should contribute to not furthering the error.

Thank you very much.

The problem with "assume-unchanged" is that by using it to ignore
changes to tracked files you are lying to Git (telling it 'assume this
is unchanged' while changing it), and can lead to DATA LOSS, that
is to losing those changes.

[...]
 [...]
 | The problem
 | is the lack of connection between this purpose and the highlevel
 | purposes for version control, which suggests that the
 | introduction of stashing might be to patch flaws in the design
 | of Git and not to satisfy a requirement of version control.

 Or the problem might be that you are missing some (maybe minor)
 requirement of version control system. Just saying...
>>>
>>> What would that purpose be? and why would you say that's a
>>> high-level purpose for version control and not one that's
>>> git-specific?
>>
>> The stash (or rather its equivalent) is not something Git specific.
>> It is present also in other version control systems, among others:
>>
>> * Mercurial: as 'shelve' extension (in core since 1.8)
>> * Bazaar: as 'bzr shelve' command
>> * Fossil: as 'fossil stash' command (with subcommands)
>> * Subversion: Shelve planned for 1.10 (2017?)
>
> Do these other VCSs have the same "Switching branches" misfit? Do you
> usually need to stash if you want to switch with uncommitted changes? I know
> Mercurial has the same problem since ``bookmarks'' are like Git branches, so
> it makes sense for them to have added something like stashing (if otherwise
> switching with uncommitted changes would be very difficult).

I suspect that all those are inspired by each other, and that
they all use 'uncommitted changes are not tied to a branch'
paradigm, which allows for creating a branch for changes
after a fact (when it turns out that it would take more than
one commit to implement the feature) quite easy.

>> I would say that 'stash' could be considered about isolating work on
>> different features, different sub-branch sized parallel work.

Note that 'isolating work' is missing from your list of purposes
of a version control system; though it is fairly obvious.

>
> That sounds a lot like having independent lines of development, which is
> what branches are supposed to be for

Those are sub-commit changes. Branches are composed of
commits. But I agree that this may be a bit of a stretch in
trying to find a high-level purpose for stash (rather than it being
a convenience feature). As I said below...

>> But it might be that stash doesn't have connection with highlevel
>> purposes for version control, and that it is purely convenience
>> feature.  Just playing the role of Advocatus Diaboli (important in
>> scientific works, isn' it?)...

[...]

 | 7. Gitless
[...]
 |  Also, there
 | is no possible way of getting in a “detached head” state; at
 | any time, the user is always working on some branch (the
 | “current” branch). Head is a per-branch reference to the last
 | commit of the branch.
[...]
 [...] during some long lived multi-step operations, like bisect
 or interactive rebase, you are not really on any 

Re: "Purposes, Concepts,Misfits, and a Redesign of Git" (a research paper)

[Konstantin Khomoutov]

On Fri, 30 Sep 2016 19:14:13 +0300
Konstantin Khomoutov  wrote:

> The "It Will Never Work in Theory" blog has just posted a summary of a
> study which tried to identify shortcomings in the design of Git.
> 
> In the hope it might be interesting, I post this summary here.
> URL: http://neverworkintheory.org/2016/09/30/rethinking-git.html

I think it would be cool if someone among subscribers could post a link
to our discussion thread [2] back onto that page.  Unfortunatrly that
requires a Disqus login which I don't have, so may be someone in
possession of such login could do that? ;-)

2. 
https://public-inbox.org/git/ce42f934-4a94-fa29-cff0-5ebb0f004...@gmail.com/T/#e95875b7940512b432ab2e29b3dd50ca448df9720

Re: What's cooking in git.git (Oct 2016, #02; Thu, 6)

[Kevin Daudt]

On Fri, Oct 07, 2016 at 09:40:01AM -0700, Junio C Hamano wrote:
> Kevin Daudt  writes:
> 
> > Just wondering why the topic "kd/mailinfo-quoted-string (2016-09-28) 2
> > commits" is not listed anymore. Previous what's cooking said it was
> > merged to "next".
> 
> Thanks for your contribution.
> 
> The topic was listed as graduated to 'master' in issue #1 of this
> month:
> 
> http://public-inbox.org/git/
> 
> After a topic graduates, it will not be included in the report.

Ok, was confused then about what the state was. Thanks.

Re: [PATCH/RFC] git.c: support "!!" aliases that do not move cwd

[Jeff King]

On Fri, Oct 07, 2016 at 07:42:35PM +0200, Johannes Sixt wrote:

> Maybe it's time to aim for
> 
>   git config alias.d2u.shell \
>'f() { git ls-files "$@" | xargs dos2unix; }; f'
>   git config alias.d2u.cdup false
>   git d2u *.c   # yada!

That would be nice. It would also allow "alias.foo_bar.shell"; right now
"alias.foo_bar" is forbidden because of the config syntax, which does
not allow underscores outside of the "subsection" name.

-Peff

Re: [PATCH/RFC] git.c: support "!!" aliases that do not move cwd

[Johannes Sixt]

Am 07.10.2016 um 14:27 schrieb Duy Nguyen:

On Fri, Oct 7, 2016 at 6:20 PM, Johannes Schindelin
 wrote:

Hi Junio,

On Thu, 6 Oct 2016, Junio C Hamano wrote:


Nguyễn Thái Ngọc Duy   writes:


Throwing something at the mailing list to see if anybody is
interested.

Current '!' aliases move cwd to $GIT_WORK_TREE first, which could make
handling path arguments hard because they are relative to the original
cwd. We set GIT_PREFIX to work around it, but I still think it's more
natural to keep cwd where it is.

We have a way to do that now after 441981b (git: simplify environment
save/restore logic - 2016-01-26). It's just a matter of choosing the
right syntax. I'm going with '!!'. I'm not very happy with it. But I
do like this type of alias.


I do not know why you are not happy with the syntax, but I
personally think it brilliant, both the idea and the preliminary
clean-up that made this possible with a simple patch like this.


I guess he is not happy with it because "!!" is quite unintuitive a
construct. I know that *I* would have been puzzled by it, asking "What the
heck does this do?".


Yep. And I wouldn't want to set a tradition for the next alias type
'!!!'. There's no good choice to represent a new alias type with a
leading symbol. This just occurred to me, however, what do you think
about a new config group for it? With can have something like
externalAlias.* (or some other name) that lives in parallel with
alias.*. Then we don't need '!' (or '!!') at all.


Maybe it's time to aim for

  git config alias.d2u.shell \
   'f() { git ls-files "$@" | xargs dos2unix; }; f'
  git config alias.d2u.cdup false
  git d2u *.c   # yada!

-- Hannes

Re: [PATCH 1/2] submodule add: extend force flag to add existing repos

[Stefan Beller]

On Fri, Oct 7, 2016 at 5:52 AM, Heiko Voigt  wrote:
> On Thu, Oct 06, 2016 at 01:11:20PM -0700, Junio C Hamano wrote:
>> Stefan Beller  writes:
>>
>> > Currently the force flag in `git submodule add` takes care of possibly
>> > ignored files or when a name collision occurs.
>> >
>> > However there is another situation where submodule add comes in handy:
>> > When you already have a gitlink recorded, but no configuration was
>> > done (i.e. no .gitmodules file nor any entry in .git/config) and you
>> > want to generate these config entries. For this situation allow
>> > `git submodule add` to proceed if there is already a submodule at the
>> > given path in the index.
>
> Is it important that the submodule is in the index?

If it is not in the index, it already works.

> How about worktree?
> From the index entry alone we can not deduce the values anyway.

Right, but as of now this is the only show stopper, i.e.
* you have an existing repo? -> fine, it works with --force
* you even ignored that repo -> --force knows how to do it.
* you already have a gitlink -> Sorry, you're out of luck.

So that is why I stressed index in this commit message, as it is only about this
case.

>
> [1] http://public-inbox.org/git/%3c20160916141143.ga47...@book.hvoigt.net%3E/

Current situation:

> clone the submodule into a directory
> git submodule add --force
> git commit everything

works fine, but:

> clone the submodule into a directory
> git add 
> git commit  -m "Add submodule"
> # me: "Oh crap! I did forget the configuration."
> git submodule add --force  
> # Git: "It already exists in the index, I am not going to produce the config 
> for you."

The last step is changed with this patch, as
it will just work fine then.

Re: [PATCH 4/4] mergetool: honor -O

[Johannes Sixt]

Am 07.10.2016 um 01:47 schrieb David Aguilar:

diff --git a/git-mergetool.sh b/git-mergetool.sh
index 65696d8..9dca58b 100755
--- a/git-mergetool.sh
+++ b/git-mergetool.sh
@@ -9,7 +9,7 @@
 # at the discretion of Junio C Hamano.
 #

-USAGE='[--tool=tool] [--tool-help] [-y|--no-prompt|--prompt] [file to merge] 
...'
+USAGE='[--tool=tool] [--tool-help] [-y|--no-prompt|--prompt] [-O] 
[file to merge] ...'
 SUBDIRECTORY_OK=Yes
 NONGIT_OK=Yes
 OPTIONS_SPEC=
@@ -390,6 +390,7 @@ print_noop_and_exit () {
 main () {
prompt=$(git config --bool mergetool.prompt)
guessed_merge_tool=false
+   order_file=

while test $# != 0
do
@@ -419,6 +420,9 @@ main () {
--prompt)
prompt=true
;;
+   -O*)
+   order_file="$1"
+   ;;
--)
shift
break
@@ -459,8 +463,14 @@ main () {
fi
fi

+   if test -n "$order_file"
+   then
+   set -- "$order_file" -- "$@"
+   else
+   set -- -- "$@"
+   fi
files=$(git -c core.quotePath=false \
-   diff --name-only --diff-filter=U -- "$@")
+   diff --name-only --diff-filter=U "$@")


You can write this as

files=$(git -c core.quotePath=false \
diff --name-only --diff-filter=U \
${order_file:+"$order_file"} -- "$@")

without the case distinction earlier. This construct is usually used to 
attach the option (-O) in front of the argument, but it is already 
included in the value; so, we use the construct only to avoid an empty 
argument when the option is unset and to get a quoted string when it is set.




cd_to_toplevel

diff --git a/t/t7610-mergetool.sh b/t/t7610-mergetool.sh
index fb2aeef..0f9869a 100755
--- a/t/t7610-mergetool.sh
+++ b/t/t7610-mergetool.sh
@@ -637,6 +637,33 @@ test_expect_success 'diff.orderFile configuration is 
honored' '
test_cmp expect actual &&
git reset --hard >/dev/null
 '
+
+test_expect_success 'mergetool -Oorder-file is honored' '
+   test_config diff.orderFile order-file &&
+   test_config mergetool.myecho.cmd "echo \"\$LOCAL\"" &&
+   test_config mergetool.myecho.trustExitCode true &&
+   test_must_fail git merge order-file-side1 &&
+   cat >expect <<-\EOF &&
+   Merging:
+   a
+   b
+   EOF
+   git mergetool -O/dev/null --no-prompt --tool myecho |
+   grep -A 2 Merging: >actual &&
+   test_cmp expect actual &&
+   git reset --hard >/dev/null 2>&1 &&
+
+   git config --unset diff.orderFile &&
+   test_must_fail git merge order-file-side1 &&
+   cat >expect <<-\EOF &&
+   Merging:
+   b
+   a
+   EOF
+   git mergetool -Oorder-file --no-prompt --tool myecho |
+   grep -A 2 Merging: >actual &&


grep -A again. Furthermore, you call git mergetool in the upstream of a 
pipe. This will ignore the exit status.



+   test_cmp expect actual &&
+   git reset --hard >/dev/null 2>&1
 '


Ah, the earlier lone quote should have been added in this patch.

-- Hannes

Re: [PATCH 3/4] mergetool: honor diff.orderFile

[Johannes Sixt]

Am 07.10.2016 um 01:47 schrieb David Aguilar:

@@ -606,4 +606,37 @@ test_expect_success MKTEMP 'temporary filenames are used 
with mergetool.writeToT
git reset --hard master >/dev/null 2>&1
 '

+test_expect_success 'diff.orderFile configuration is honored' '
+   test_config diff.orderFile order-file &&
+   test_config mergetool.myecho.cmd "echo \"\$LOCAL\"" &&
+   test_config mergetool.myecho.trustExitCode true &&
+   echo b >order-file &&
+   echo a >>order-file &&
+   git checkout -b order-file-start master &&
+   echo start >a &&
+   echo start >b &&
+   git add a b &&
+   git commit -m start &&
+   git checkout -b order-file-side1 order-file-start &&
+   echo side1 >a &&
+   echo side1 >b &&
+   git add a b &&
+   git commit -m side1 &&
+   git checkout -b order-file-side2 order-file-start &&
+   echo side2 >a &&
+   echo side2 >b &&
+   git add a b &&
+   git commit -m side2 &&
+   test_must_fail git merge order-file-side1 &&
+   cat >expect <<-\EOF &&
+   Merging:
+   b
+   a
+   EOF
+   git mergetool --no-prompt --tool myecho | grep -A 2 Merging: >actual &&


Is grep -A universally available?


+   test_cmp expect actual &&
+   git reset --hard >/dev/null
+'
+'


Two single-quotes?


+
 test_done



Otherwise the patch looks good.

-- Hannes

[PATCH v2] l10n: de.po: translate 260 new messages

[Ralf Thielow]

Translate 260 new message came from git.pot updates in 9fa976f (l10n:
git.pot: v2.10.0 round 1 (248 new, 56 removed)) and 5bd166d (l10n:
git.pot: v2.10.0 round 2 (12 new, 44 removed)).

Signed-off-by: Ralf Thielow 
---
 po/de.po | 783 ++-
 1 file changed, 421 insertions(+), 362 deletions(-)

diff --git a/po/de.po b/po/de.po
index e1865c6ca..0755cdf6c 100644
--- a/po/de.po
+++ b/po/de.po
@@ -14,72 +14,60 @@ msgstr ""
 "Language: de\n"
 "MIME-Version: 1.0\n"
 "Content-Type: text/plain; charset=UTF-8\n"
 "Content-Transfer-Encoding: 8bit\n"
 "Plural-Forms: nplurals=2; plural=(n!=1);\n"
 
 #: advice.c:55
 #, c-format
 msgid "hint: %.*s\n"
 msgstr "Hinweis: %.*s\n"
 
 #: advice.c:83
-#, fuzzy
 msgid "Cherry-picking is not possible because you have unmerged files."
-msgstr ""
-"\"pull\" ist nicht möglich, weil Sie nicht zusammengeführte Dateien haben."
+msgstr "Cherry-Picken ist nicht möglich, weil Sie nicht zusammengeführte 
Dateien haben."
 
 #: advice.c:85
-#, fuzzy
 msgid "Committing is not possible because you have unmerged files."
-msgstr ""
-"\"pull\" ist nicht möglich, weil Sie nicht zusammengeführte Dateien haben."
+msgstr "Committen ist nicht möglich, weil Sie nicht zusammengeführte Dateien 
haben."
 
 #: advice.c:87
-#, fuzzy
 msgid "Merging is not possible because you have unmerged files."
-msgstr ""
-"\"pull\" ist nicht möglich, weil Sie nicht zusammengeführte Dateien haben."
+msgstr "Mergen ist nicht möglich, weil Sie nicht zusammengeführte Dateien 
haben."
 
 #: advice.c:89
-#, fuzzy
 msgid "Pulling is not possible because you have unmerged files."
-msgstr ""
-"\"pull\" ist nicht möglich, weil Sie nicht zusammengeführte Dateien haben."
+msgstr "Pullen ist nicht möglich, weil Sie nicht zusammengeführte Dateien 
haben."
 
 #: advice.c:91
-#, fuzzy
 msgid "Reverting is not possible because you have unmerged files."
-msgstr ""
-"\"pull\" ist nicht möglich, weil Sie nicht zusammengeführte Dateien haben."
+msgstr "Reverten ist nicht möglich, weil Sie nicht zusammengeführte Dateien 
haben."
 
 #: advice.c:93
-#, fuzzy, c-format
+#, c-format
 msgid "It is not possible to %s because you have unmerged files."
-msgstr ""
-"\"pull\" ist nicht möglich, weil Sie nicht zusammengeführte Dateien haben."
+msgstr "%s ist nicht möglich, weil Sie nicht zusammengeführte Dateien haben."
 
 #: advice.c:101
 msgid ""
 "Fix them up in the work tree, and then use 'git add/rm '\n"
 "as appropriate to mark resolution and make a commit."
 msgstr ""
 "Korrigieren Sie dies im Arbeitsverzeichnis, und benutzen Sie\n"
 "dann 'git add/rm ', um die Auflösung entsprechend zu markieren\n"
 "und zu committen."
 
 #: advice.c:109
-#, fuzzy
 msgid "Exiting because of an unresolved conflict."
-msgstr "Beende wegen nicht abgeschlossenem Merge."
+msgstr "Beende wegen unaufgelöstem Konflikt."
 
 #: advice.c:114 builtin/merge.c:1181
 msgid "You have not concluded your merge (MERGE_HEAD exists)."
 msgstr "Sie haben Ihren Merge nicht abgeschlossen (MERGE_HEAD existiert)."
 
 #: advice.c:116
 msgid "Please, commit your changes before merging."
 msgstr "Bitte committen Sie Ihre Änderungen, bevor Sie mergen."
 
 #: advice.c:117
 msgid "Exiting because of unfinished merge."
 msgstr "Beende wegen nicht abgeschlossenem Merge."
@@ -90,24 +78,38 @@ msgid ""
 "Note: checking out '%s'.\n"
 "\n"
 "You are in 'detached HEAD' state. You can look around, make experimental\n"
 "changes and commit them, and you can discard any commits you make in this\n"
 "state without impacting any branches by performing another checkout.\n"
 "\n"
 "If you want to create a new branch to retain commits you create, you may\n"
 "do so (now or later) by using -b with the checkout command again. Example:\n"
 "\n"
 "  git checkout -b \n"
 "\n"
 msgstr ""
+"Hinweis: Checke '%s' aus.\n"
+"\n"
+"Sie befinden sich im Zustand eines 'lösgelösten HEAD'. Sie können sich\n"
+"umschauen, experimentelle Änderungen vornehmen und diese committen, und\n"
+"Sie können alle möglichen Commits, die Sie in diesem Zustand machen,\n"
+"ohne Auswirkungen auf irgendeinen Branch verwerfen, indem Sie einen\n"
+"weiteren Checkout durchführen.\n"
+"\n"
+"Wenn Sie einen neuen Branch erstellen möchten, um Ihre erstellten Commits\n"
+"zu behalten, können Sie das (jetzt oder später) durch einen weiteren 
Checkout\n"
+"mit der Option -b tun. Beispiel:\n"
+"\n"
+"  git checkout -b \n"
+"\n"
 
 #: archive.c:12
 msgid "git archive []  [...]"
 msgstr "git archive []  [...]"
 
 #: archive.c:13
 msgid "git archive --list"
 msgstr "git archive --list"
 
 #: archive.c:14
 msgid ""
 "git archive --remote  [--exec ] []  [...]"
@@ -185,165 +187,176 @@ msgstr "Repository"
 msgid "retrieve the archive from remote repository "
 msgstr "Archiv vom Remote-Repository  abrufen"
 
 #: archive.c:453 builtin/archive.c:92 builtin/notes.c:483
 msgid "command"
 msgstr "Programm"
 
 #: archive.c:454 builtin/archive.c:93
 msgid "path to the remote 

Re: What's cooking in git.git (Oct 2016, #02; Thu, 6)

[Junio C Hamano]

Kevin Daudt  writes:

> Just wondering why the topic "kd/mailinfo-quoted-string (2016-09-28) 2
> commits" is not listed anymore. Previous what's cooking said it was
> merged to "next".

Thanks for your contribution.

The topic was listed as graduated to 'master' in issue #1 of this
month:

http://public-inbox.org/git/

After a topic graduates, it will not be included in the report.

Re: [PATCH v4 0/6] Pull out require_clean_work_tree() functionality from builtin/pull.c

[Jakub Narębski]

W dniu 07.10.2016 o 18:08, Johannes Schindelin pisze:

> - marked the hint "please commit or stash them" (reintroduced from the
>   original git-pull.sh script) as translatable.

I wonder if we can make automatic check if everything introduced is
translatable, for example with something akin to "English (XT)"
autogenerated pseudo-localization that Android uses?

Just food for thought.
-- 
Jakub Narębski

Re: [PATCH v4 0/6] Pull out require_clean_work_tree() functionality from builtin/pull.c

[Junio C Hamano]

Johannes Schindelin  writes:

> - changed the exit code to 128 (emulating a die()) if
>   require_clean_work-tree() was asked to be non-gentle.

Ah, I didn't spot this the last round.  Good change.

Thanks, will replace.

Re: [PATCH/RFC] git.c: support "!!" aliases that do not move cwd

[Jakub Narębski]

W dniu 07.10.2016 o 16:19, Johannes Schindelin pisze: 
> On Fri, 7 Oct 2016, Jakub Narębski wrote:

>> Note that we would have to teach git completion about new syntax;
>> or new configuration variable if we go that route.
> 
> Why would we? Git's completion does not expand aliases, it only completes
> the aliases' names, not their values.

Because Git completion finds out which _options_ and _arguments_
to complete for an alias based on its expansion.

Yes, this is nice bit of trickery...
-- 
Jakub Narębski

[PATCH v4 4/6] wt-status: export also the has_un{staged,committed}_changes() functions

[Johannes Schindelin]

They will be used in the upcoming rebase helper.

Signed-off-by: Johannes Schindelin 
---
 wt-status.c | 4 ++--
 wt-status.h | 4 +++-
 2 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/wt-status.c b/wt-status.c
index 89475f1..8fe7d0a 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -2213,7 +2213,7 @@ void wt_status_print(struct wt_status *s)
 /**
  * Returns 1 if there are unstaged changes, 0 otherwise.
  */
-static int has_unstaged_changes(void)
+int has_unstaged_changes(void)
 {
struct rev_info rev_info;
int result;
@@ -2229,7 +2229,7 @@ static int has_unstaged_changes(void)
 /**
  * Returns 1 if there are uncommitted changes, 0 otherwise.
  */
-static int has_uncommitted_changes(void)
+int has_uncommitted_changes(void)
 {
struct rev_info rev_info;
int result;
diff --git a/wt-status.h b/wt-status.h
index 68b4709..68e367a 100644
--- a/wt-status.h
+++ b/wt-status.h
@@ -128,7 +128,9 @@ void status_printf_ln(struct wt_status *s, const char 
*color, const char *fmt, .
 __attribute__((format (printf, 3, 4)))
 void status_printf(struct wt_status *s, const char *color, const char *fmt, 
...);
 
-/* The following function expects that the caller took care of reading the 
index. */
+/* The following functions expect that the caller took care of reading the 
index. */
+int has_unstaged_changes(void);
+int has_uncommitted_changes(void);
 int require_clean_work_tree(const char *action, const char *hint, int gently);
 
 #endif /* STATUS_H */
-- 
2.10.0.windows.1.325.ge6089c1

[PATCH v4 1/6] pull: drop confusing prefix parameter of die_on_unclean_work_tree()

[Johannes Schindelin]

In cmd_pull(), when verifying that there are no changes preventing a
rebasing pull, we diligently pass the prefix parameter to the
die_on_unclean_work_tree() function which in turn diligently passes it
to the has_unstaged_changes() and has_uncommitted_changes() functions.

The casual reader might now be curious (as this developer was) whether
that means that calling `git pull --rebase` in a subdirectory will
ignore unstaged changes in other parts of the working directory. And be
puzzled that `git pull --rebase` (correctly) complains about those
changes outside of the current directory.

The puzzle is easily resolved: while we take pains to pass around the
prefix and even pass it to init_revisions(), the fact that no paths are
passed to init_revisions() ensures that the prefix is simply ignored.

That, combined with the fact that we will *always* want a *full* working
directory check before running a rebasing pull, is reason enough to
simply do away with the actual prefix parameter and to pass NULL
instead, as if we were running this from the top-level working directory
anyway.

Signed-off-by: Johannes Schindelin 
---
 builtin/pull.c | 16 
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/builtin/pull.c b/builtin/pull.c
index 398aae1..d4bd635 100644
--- a/builtin/pull.c
+++ b/builtin/pull.c
@@ -328,12 +328,12 @@ static int git_pull_config(const char *var, const char 
*value, void *cb)
 /**
  * Returns 1 if there are unstaged changes, 0 otherwise.
  */
-static int has_unstaged_changes(const char *prefix)
+static int has_unstaged_changes(void)
 {
struct rev_info rev_info;
int result;
 
-   init_revisions(_info, prefix);
+   init_revisions(_info, NULL);
DIFF_OPT_SET(_info.diffopt, IGNORE_SUBMODULES);
DIFF_OPT_SET(_info.diffopt, QUICK);
diff_setup_done(_info.diffopt);
@@ -344,7 +344,7 @@ static int has_unstaged_changes(const char *prefix)
 /**
  * Returns 1 if there are uncommitted changes, 0 otherwise.
  */
-static int has_uncommitted_changes(const char *prefix)
+static int has_uncommitted_changes(void)
 {
struct rev_info rev_info;
int result;
@@ -352,7 +352,7 @@ static int has_uncommitted_changes(const char *prefix)
if (is_cache_unborn())
return 0;
 
-   init_revisions(_info, prefix);
+   init_revisions(_info, NULL);
DIFF_OPT_SET(_info.diffopt, IGNORE_SUBMODULES);
DIFF_OPT_SET(_info.diffopt, QUICK);
add_head_to_pending(_info);
@@ -365,7 +365,7 @@ static int has_uncommitted_changes(const char *prefix)
  * If the work tree has unstaged or uncommitted changes, dies with the
  * appropriate message.
  */
-static void die_on_unclean_work_tree(const char *prefix)
+static void die_on_unclean_work_tree(void)
 {
struct lock_file *lock_file = xcalloc(1, sizeof(*lock_file));
int do_die = 0;
@@ -375,12 +375,12 @@ static void die_on_unclean_work_tree(const char *prefix)
update_index_if_able(_index, lock_file);
rollback_lock_file(lock_file);
 
-   if (has_unstaged_changes(prefix)) {
+   if (has_unstaged_changes()) {
error(_("Cannot pull with rebase: You have unstaged changes."));
do_die = 1;
}
 
-   if (has_uncommitted_changes(prefix)) {
+   if (has_uncommitted_changes()) {
if (do_die)
error(_("Additionally, your index contains uncommitted 
changes."));
else
@@ -875,7 +875,7 @@ int cmd_pull(int argc, const char **argv, const char 
*prefix)
die(_("Updating an unborn branch with changes added to 
the index."));
 
if (!autostash)
-   die_on_unclean_work_tree(prefix);
+   die_on_unclean_work_tree();
 
if (get_rebase_fork_point(rebase_fork_point, repo, *refspecs))
hashclr(rebase_fork_point);
-- 
2.10.0.windows.1.325.ge6089c1

[PATCH v4 3/6] wt-status: make the require_clean_work_tree() function reusable

[Johannes Schindelin]

The function used by "git pull" to stop the user when the working
tree has changes is useful in other places.

Let's move it into a more prominent (and into an actually reusable)
spot: wt-status.[ch].

Signed-off-by: Johannes Schindelin 
---
 builtin/pull.c | 77 +-
 wt-status.c| 76 +
 wt-status.h|  3 +++
 3 files changed, 80 insertions(+), 76 deletions(-)

diff --git a/builtin/pull.c b/builtin/pull.c
index 58fc176..01b6465 100644
--- a/builtin/pull.c
+++ b/builtin/pull.c
@@ -17,6 +17,7 @@
 #include "revision.h"
 #include "tempfile.h"
 #include "lockfile.h"
+#include "wt-status.h"
 
 enum rebase_type {
REBASE_INVALID = -1,
@@ -326,82 +327,6 @@ static int git_pull_config(const char *var, const char 
*value, void *cb)
 }
 
 /**
- * Returns 1 if there are unstaged changes, 0 otherwise.
- */
-static int has_unstaged_changes(void)
-{
-   struct rev_info rev_info;
-   int result;
-
-   init_revisions(_info, NULL);
-   DIFF_OPT_SET(_info.diffopt, IGNORE_SUBMODULES);
-   DIFF_OPT_SET(_info.diffopt, QUICK);
-   diff_setup_done(_info.diffopt);
-   result = run_diff_files(_info, 0);
-   return diff_result_code(_info.diffopt, result);
-}
-
-/**
- * Returns 1 if there are uncommitted changes, 0 otherwise.
- */
-static int has_uncommitted_changes(void)
-{
-   struct rev_info rev_info;
-   int result;
-
-   if (is_cache_unborn())
-   return 0;
-
-   init_revisions(_info, NULL);
-   DIFF_OPT_SET(_info.diffopt, IGNORE_SUBMODULES);
-   DIFF_OPT_SET(_info.diffopt, QUICK);
-   add_head_to_pending(_info);
-   diff_setup_done(_info.diffopt);
-   result = run_diff_index(_info, 1);
-   return diff_result_code(_info.diffopt, result);
-}
-
-/**
- * If the work tree has unstaged or uncommitted changes, dies with the
- * appropriate message.
- */
-static int require_clean_work_tree(const char *action, const char *hint,
-   int gently)
-{
-   struct lock_file *lock_file = xcalloc(1, sizeof(*lock_file));
-   int err = 0;
-
-   hold_locked_index(lock_file, 0);
-   refresh_cache(REFRESH_QUIET);
-   update_index_if_able(_index, lock_file);
-   rollback_lock_file(lock_file);
-
-   if (has_unstaged_changes()) {
-   /* TRANSLATORS: the action is e.g. "pull with rebase" */
-   error(_("Cannot %s: You have unstaged changes."), _(action));
-   err = 1;
-   }
-
-   if (has_uncommitted_changes()) {
-   if (err)
-   error(_("Additionally, your index contains uncommitted 
changes."));
-   else
-   error(_("Cannot %s: Your index contains uncommitted 
changes."),
- _(action));
-   err = 1;
-   }
-
-   if (err) {
-   if (hint)
-   error("%s", hint);
-   if (!gently)
-   exit(128);
-   }
-
-   return err;
-}
-
-/**
  * Appends merge candidates from FETCH_HEAD that are not marked not-for-merge
  * into merge_heads.
  */
diff --git a/wt-status.c b/wt-status.c
index 99d1b0a..89475f1 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -16,6 +16,7 @@
 #include "strbuf.h"
 #include "utf8.h"
 #include "worktree.h"
+#include "lockfile.h"
 
 static const char cut_line[] =
 " >8 \n";
@@ -2208,3 +2209,78 @@ void wt_status_print(struct wt_status *s)
break;
}
 }
+
+/**
+ * Returns 1 if there are unstaged changes, 0 otherwise.
+ */
+static int has_unstaged_changes(void)
+{
+   struct rev_info rev_info;
+   int result;
+
+   init_revisions(_info, NULL);
+   DIFF_OPT_SET(_info.diffopt, IGNORE_SUBMODULES);
+   DIFF_OPT_SET(_info.diffopt, QUICK);
+   diff_setup_done(_info.diffopt);
+   result = run_diff_files(_info, 0);
+   return diff_result_code(_info.diffopt, result);
+}
+
+/**
+ * Returns 1 if there are uncommitted changes, 0 otherwise.
+ */
+static int has_uncommitted_changes(void)
+{
+   struct rev_info rev_info;
+   int result;
+
+   if (is_cache_unborn())
+   return 0;
+
+   init_revisions(_info, NULL);
+   DIFF_OPT_SET(_info.diffopt, IGNORE_SUBMODULES);
+   DIFF_OPT_SET(_info.diffopt, QUICK);
+   add_head_to_pending(_info);
+   diff_setup_done(_info.diffopt);
+   result = run_diff_index(_info, 1);
+   return diff_result_code(_info.diffopt, result);
+}
+
+/**
+ * If the work tree has unstaged or uncommitted changes, dies with the
+ * appropriate message.
+ */
+int require_clean_work_tree(const char *action, const char *hint, int gently)
+{
+   struct lock_file *lock_file = xcalloc(1, sizeof(*lock_file));
+   int err = 0;
+
+   hold_locked_index(lock_file, 0);
+   refresh_cache(REFRESH_QUIET);
+   

[PATCH v4 2/6] pull: make code more similar to the shell script again

[Johannes Schindelin]

When converting the pull command to a builtin, the
require_clean_work_tree() function was renamed and the pull-specific
parts hard-coded.

This makes it impossible to reuse the code, so let's modify the code to
make it more similar to the original shell script again.

Note: when the hint "Please commit or stash them" was introduced first,
Git did not have the convention of continuing error messages in lower
case, but now we do have that convention, therefore we reintroduce this
hint down-cased, obeying said convention.

Signed-off-by: Johannes Schindelin 
---
 builtin/pull.c | 30 --
 1 file changed, 20 insertions(+), 10 deletions(-)

diff --git a/builtin/pull.c b/builtin/pull.c
index d4bd635..58fc176 100644
--- a/builtin/pull.c
+++ b/builtin/pull.c
@@ -365,10 +365,11 @@ static int has_uncommitted_changes(void)
  * If the work tree has unstaged or uncommitted changes, dies with the
  * appropriate message.
  */
-static void die_on_unclean_work_tree(void)
+static int require_clean_work_tree(const char *action, const char *hint,
+   int gently)
 {
struct lock_file *lock_file = xcalloc(1, sizeof(*lock_file));
-   int do_die = 0;
+   int err = 0;
 
hold_locked_index(lock_file, 0);
refresh_cache(REFRESH_QUIET);
@@ -376,20 +377,28 @@ static void die_on_unclean_work_tree(void)
rollback_lock_file(lock_file);
 
if (has_unstaged_changes()) {
-   error(_("Cannot pull with rebase: You have unstaged changes."));
-   do_die = 1;
+   /* TRANSLATORS: the action is e.g. "pull with rebase" */
+   error(_("Cannot %s: You have unstaged changes."), _(action));
+   err = 1;
}
 
if (has_uncommitted_changes()) {
-   if (do_die)
+   if (err)
error(_("Additionally, your index contains uncommitted 
changes."));
else
-   error(_("Cannot pull with rebase: Your index contains 
uncommitted changes."));
-   do_die = 1;
+   error(_("Cannot %s: Your index contains uncommitted 
changes."),
+ _(action));
+   err = 1;
}
 
-   if (do_die)
-   exit(1);
+   if (err) {
+   if (hint)
+   error("%s", hint);
+   if (!gently)
+   exit(128);
+   }
+
+   return err;
 }
 
 /**
@@ -875,7 +884,8 @@ int cmd_pull(int argc, const char **argv, const char 
*prefix)
die(_("Updating an unborn branch with changes added to 
the index."));
 
if (!autostash)
-   die_on_unclean_work_tree();
+   require_clean_work_tree(N_("pull with rebase"),
+   _("please commit or stash them."), 0);
 
if (get_rebase_fork_point(rebase_fork_point, repo, *refspecs))
hashclr(rebase_fork_point);
-- 
2.10.0.windows.1.325.ge6089c1

[PATCH v4 5/6] wt-status: teach has_{unstaged,uncommitted}_changes() about submodules

[Johannes Schindelin]

Sometimes we are *actually* interested in those changes... For
example when an interactive rebase wants to continue with a staged
submodule update.

Signed-off-by: Johannes Schindelin 
---
 builtin/pull.c |  2 +-
 wt-status.c| 16 +---
 wt-status.h|  7 ---
 3 files changed, 14 insertions(+), 11 deletions(-)

diff --git a/builtin/pull.c b/builtin/pull.c
index 01b6465..d6e46ee 100644
--- a/builtin/pull.c
+++ b/builtin/pull.c
@@ -810,7 +810,7 @@ int cmd_pull(int argc, const char **argv, const char 
*prefix)
 
if (!autostash)
require_clean_work_tree(N_("pull with rebase"),
-   _("please commit or stash them."), 0);
+   _("please commit or stash them."), 1, 0);
 
if (get_rebase_fork_point(rebase_fork_point, repo, *refspecs))
hashclr(rebase_fork_point);
diff --git a/wt-status.c b/wt-status.c
index 8fe7d0a..0020140 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -2213,13 +2213,14 @@ void wt_status_print(struct wt_status *s)
 /**
  * Returns 1 if there are unstaged changes, 0 otherwise.
  */
-int has_unstaged_changes(void)
+int has_unstaged_changes(int ignore_submodules)
 {
struct rev_info rev_info;
int result;
 
init_revisions(_info, NULL);
-   DIFF_OPT_SET(_info.diffopt, IGNORE_SUBMODULES);
+   if (ignore_submodules)
+   DIFF_OPT_SET(_info.diffopt, IGNORE_SUBMODULES);
DIFF_OPT_SET(_info.diffopt, QUICK);
diff_setup_done(_info.diffopt);
result = run_diff_files(_info, 0);
@@ -2229,7 +2230,7 @@ int has_unstaged_changes(void)
 /**
  * Returns 1 if there are uncommitted changes, 0 otherwise.
  */
-int has_uncommitted_changes(void)
+int has_uncommitted_changes(int ignore_submodules)
 {
struct rev_info rev_info;
int result;
@@ -2238,7 +2239,8 @@ int has_uncommitted_changes(void)
return 0;
 
init_revisions(_info, NULL);
-   DIFF_OPT_SET(_info.diffopt, IGNORE_SUBMODULES);
+   if (ignore_submodules)
+   DIFF_OPT_SET(_info.diffopt, IGNORE_SUBMODULES);
DIFF_OPT_SET(_info.diffopt, QUICK);
add_head_to_pending(_info);
diff_setup_done(_info.diffopt);
@@ -2250,7 +2252,7 @@ int has_uncommitted_changes(void)
  * If the work tree has unstaged or uncommitted changes, dies with the
  * appropriate message.
  */
-int require_clean_work_tree(const char *action, const char *hint, int gently)
+int require_clean_work_tree(const char *action, const char *hint, int 
ignore_submodules, int gently)
 {
struct lock_file *lock_file = xcalloc(1, sizeof(*lock_file));
int err = 0;
@@ -2260,13 +2262,13 @@ int require_clean_work_tree(const char *action, const 
char *hint, int gently)
update_index_if_able(_index, lock_file);
rollback_lock_file(lock_file);
 
-   if (has_unstaged_changes()) {
+   if (has_unstaged_changes(ignore_submodules)) {
/* TRANSLATORS: the action is e.g. "pull with rebase" */
error(_("Cannot %s: You have unstaged changes."), _(action));
err = 1;
}
 
-   if (has_uncommitted_changes()) {
+   if (has_uncommitted_changes(ignore_submodules)) {
if (err)
error(_("Additionally, your index contains uncommitted 
changes."));
else
diff --git a/wt-status.h b/wt-status.h
index 68e367a..54fec77 100644
--- a/wt-status.h
+++ b/wt-status.h
@@ -129,8 +129,9 @@ __attribute__((format (printf, 3, 4)))
 void status_printf(struct wt_status *s, const char *color, const char *fmt, 
...);
 
 /* The following functions expect that the caller took care of reading the 
index. */
-int has_unstaged_changes(void);
-int has_uncommitted_changes(void);
-int require_clean_work_tree(const char *action, const char *hint, int gently);
+int has_unstaged_changes(int ignore_submodules);
+int has_uncommitted_changes(int ignore_submodules);
+int require_clean_work_tree(const char *action, const char *hint,
+   int ignore_submodules, int gently);
 
 #endif /* STATUS_H */
-- 
2.10.0.windows.1.325.ge6089c1

[PATCH v4 0/6] Pull out require_clean_work_tree() functionality from builtin/pull.c

[Johannes Schindelin]

This is the 5th last patch series of my work to accelerate interactive
rebases in particular on Windows.

Basically, all it does is to make reusable some functions that were
ported over from git-pull.sh but made private to builtin/pull.c.

Changes since v3:

- reworded 3/5's commit message according to Junio's suggestion.

- fixed a tyop in 4/5's commit message, pointed out by Jakub.

- marked the hint "please commit or stash them" (reintroduced from the
  original git-pull.sh script) as translatable.

- changed the exit code to 128 (emulating a die()) if
  require_clean_work-tree() was asked to be non-gentle.

- fixed a tyop in 3/6 (which was replaced in 4/6, but it is good not to
  introduce bugs only to fix them right away).

- prefixed the commit message of 4/6 with the "wt-status:" prefix,
  replicating Junio's commit message in the `pu` branch.


Johannes Schindelin (6):
  pull: drop confusing prefix parameter of die_on_unclean_work_tree()
  pull: make code more similar to the shell script again
  wt-status: make the require_clean_work_tree() function reusable
  wt-status: export also the has_un{staged,committed}_changes()
functions
  wt-status: teach has_{unstaged,uncommitted}_changes() about submodules
  wt-status: begin error messages with lower-case

 builtin/pull.c | 71 +++-
 wt-status.c| 78 ++
 wt-status.h|  6 +
 3 files changed, 87 insertions(+), 68 deletions(-)

Published-As: 
https://github.com/dscho/git/releases/tag/require-clean-work-tree-v4
Fetch-It-Via: git fetch https://github.com/dscho/git require-clean-work-tree-v4

Interdiff vs v3:

 diff --git a/builtin/pull.c b/builtin/pull.c
 index 0bf9802..d6e46ee 100644
 --- a/builtin/pull.c
 +++ b/builtin/pull.c
 @@ -810,7 +810,7 @@ int cmd_pull(int argc, const char **argv, const char 
*prefix)
  
if (!autostash)
require_clean_work_tree(N_("pull with rebase"),
 -  "please commit or stash them.", 1, 0);
 +  _("please commit or stash them."), 1, 0);
  
if (get_rebase_fork_point(rebase_fork_point, repo, *refspecs))
hashclr(rebase_fork_point);
 diff --git a/wt-status.c b/wt-status.c
 index ef67593..e8e5de4 100644
 --- a/wt-status.c
 +++ b/wt-status.c
 @@ -2281,7 +2281,7 @@ int require_clean_work_tree(const char *action, const 
char *hint, int ignore_sub
if (hint)
error("%s", hint);
if (!gently)
 -  exit(err);
 +  exit(128);
}
  
return err;

-- 
2.10.0.windows.1.325.ge6089c1

base-commit: a23ca1b8dc42ffd4de2ef30d67ce1e21ded29886

[PATCH v4 6/6] wt-status: begin error messages with lower-case

[Johannes Schindelin]

The previous code still followed the old git-pull.sh code which did not
adhere to our new convention.

Signed-off-by: Johannes Schindelin 
---
 wt-status.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/wt-status.c b/wt-status.c
index 0020140..e8e5de4 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -2264,15 +2264,15 @@ int require_clean_work_tree(const char *action, const 
char *hint, int ignore_sub
 
if (has_unstaged_changes(ignore_submodules)) {
/* TRANSLATORS: the action is e.g. "pull with rebase" */
-   error(_("Cannot %s: You have unstaged changes."), _(action));
+   error(_("cannot %s: You have unstaged changes."), _(action));
err = 1;
}
 
if (has_uncommitted_changes(ignore_submodules)) {
if (err)
-   error(_("Additionally, your index contains uncommitted 
changes."));
+   error(_("additionally, your index contains uncommitted 
changes."));
else
-   error(_("Cannot %s: Your index contains uncommitted 
changes."),
+   error(_("cannot %s: Your index contains uncommitted 
changes."),
  _(action));
err = 1;
}
-- 
2.10.0.windows.1.325.ge6089c1

Re: Systems with old regex system headers/libraries don't pick up git's compat/regex header file

[Jeff King]

On Fri, Oct 07, 2016 at 04:45:08PM +0100, Richard Lloyd wrote:

> On 06/10/16 20:11, Jeff King wrote:
> > Junio mentioned the NO_REGEX knob in the Makefile. If that works for
> > you, the next step is probably to add a line to the HP-UX section of
> > config.mak.uname, so that it just works out of the box.
> 
> This doesn't work because the check in git-compat-util.h only looks
> for REG_STARTEND being defined (if it isn't, it #error's out).
> 
> That define is not mentioned anywhere else other than in the
> compat/regex tree, which is why I used -Icompat/regex to pick up
>  from there - this was the "easiest" solution for me on
> HP-UX 11.

I'm confused. Setting NO_REGEX in the Makefile will add -Icompat/regex
to your compiler invocation. So git-compat-util.h should pick up our
compat regex routines, which _do_ have REG_STARTEND.

How are you building? Doing:

  make NO_REGEX=1

is supposed to work, and if it doesn't, there's a bug.

-Peff

Re: Systems with old regex system headers/libraries don't pick up git's compat/regex header file

[Richard Lloyd]

On 06/10/16 20:11, Jeff King wrote:

Junio mentioned the NO_REGEX knob in the Makefile. If that works for
you, the next step is probably to add a line to the HP-UX section of
config.mak.uname, so that it just works out of the box.


This doesn't work because the check in git-compat-util.h only looks
for REG_STARTEND being defined (if it isn't, it #error's out).

That define is not mentioned anywhere else other than in the
compat/regex tree, which is why I used -Icompat/regex to pick up
 from there - this was the "easiest" solution for me on
HP-UX 11.

Note that with this inclusion change, the source compiled and linked
fine on HP-UX 11 and git passed its tests, including the regex-based ones.

Richard K. Lloyd,   E-mail: richard.ll...@connectinternetsolutions.com
Connect Internet Solutions,WWW: https://www.connectinternetsolutions.com/
4th Floor, New Barratt House,
47, North John Street,
Liverpool,
Merseyside, UK. L2 6SG






-Peff






--
This e-mail (and any attachments) is private and confidential. If you have 
received it in error, please notify the sender immediately and delete it 
from your system. Do not use, copy or disclose the information in any way 
nor act in reliance on it.


Any views expressed in this message are those of the individual sender,
except where the sender specifically states them to be the views of Connect
Internet Solutions Ltd. This e-mail and any attachments are believed to be
virus free but it is the recipient's responsibility to ensure that they are.

Connect Internet Solutions Ltd
(A company registered in England No: 04424350)
Registered Office: 4th Floor, New Barratt House, 47 North John Street,
Liverpool, L2 6SG
Telephone: +44 (0) 151 282 4321
VAT registration number: 758 2838 85

[PATCH v2 3/3] batch check whether submodule needs pushing into one call

[Heiko Voigt]

We run a command for each sha1 change in a submodule. This is
unnecessary since we can simply batch all sha1's we want to check into
one command. Lets do it so we can speedup the check when many submodule
changes are in need of checking.

Signed-off-by: Heiko Voigt 
---
 submodule.c | 63 +
 1 file changed, 34 insertions(+), 29 deletions(-)

diff --git a/submodule.c b/submodule.c
index 5044afc2f8..a05c2a34b1 100644
--- a/submodule.c
+++ b/submodule.c
@@ -529,27 +529,49 @@ static int append_hash_to_argv(const unsigned char 
sha1[20], void *data)
return 0;
 }
 
-static int submodule_needs_pushing(const char *path, const unsigned char 
sha1[20])
+static int check_has_hash(const unsigned char sha1[20], void *data)
 {
-   if (add_submodule_odb(path) || !lookup_commit_reference(sha1))
+   int *has_hash = (int *) data;
+
+   if (!lookup_commit_reference(sha1))
+   *has_hash = 0;
+
+   return 0;
+}
+
+static int submodule_has_hashes(const char *path, struct sha1_array *hashes)
+{
+   int has_hash = 1;
+
+   if (add_submodule_odb(path))
+   return 0;
+
+   sha1_array_for_each_unique(hashes, check_has_hash, _hash);
+   return has_hash;
+}
+
+static int submodule_needs_pushing(const char *path, struct sha1_array *hashes)
+{
+   if (!submodule_has_hashes(path, hashes))
return 0;
 
if (for_each_remote_ref_submodule(path, has_remote, NULL) > 0) {
struct child_process cp = CHILD_PROCESS_INIT;
-   const char *argv[] = {"rev-list", NULL, "--not", "--remotes", 
"-n", "1" , NULL};
struct strbuf buf = STRBUF_INIT;
int needs_pushing = 0;
 
-   argv[1] = sha1_to_hex(sha1);
-   cp.argv = argv;
+   argv_array_push(, "rev-list");
+   sha1_array_for_each_unique(hashes, append_hash_to_argv, 
);
+   argv_array_pushl(, "--not", "--remotes", "-n", "1" , 
NULL);
+
prepare_submodule_repo_env(_array);
cp.git_cmd = 1;
cp.no_stdin = 1;
cp.out = -1;
cp.dir = path;
if (start_command())
-   die("Could not run 'git rev-list %s --not --remotes -n 
1' command in submodule %s",
-   sha1_to_hex(sha1), path);
+   die("Could not run 'git rev-list  --not 
--remotes -n 1' command in submodule %s",
+   path);
if (strbuf_read(, cp.out, 41))
needs_pushing = 1;
finish_command();
@@ -604,21 +626,6 @@ static void find_unpushed_submodule_commits(struct commit 
*commit,
diff_tree_combined_merge(commit, 1, );
 }
 
-struct collect_submodule_from_sha1s_data {
-   char *submodule_path;
-   struct string_list *needs_pushing;
-};
-
-static void collect_submodules_from_sha1s(const unsigned char sha1[20],
-   void *data)
-{
-   struct collect_submodule_from_sha1s_data *me =
-   (struct collect_submodule_from_sha1s_data *) data;
-
-   if (submodule_needs_pushing(me->submodule_path, sha1))
-   string_list_insert(me->needs_pushing, me->submodule_path);
-}
-
 static void free_submodules_sha1s(struct string_list *submodules)
 {
int i;
@@ -658,13 +665,11 @@ int find_unpushed_submodules(struct sha1_array *hashes,
argv_array_clear();
 
for (i = 0; i < submodules.nr; i++) {
-   struct string_list_item *item = [i];
-   struct collect_submodule_from_sha1s_data data;
-   data.submodule_path = item->string;
-   data.needs_pushing = needs_pushing;
-   sha1_array_for_each_unique((struct sha1_array *) item->util,
-   collect_submodules_from_sha1s,
-   );
+   struct string_list_item *submodule = [i];
+   struct sha1_array *hashes = (struct sha1_array *) 
submodule->util;
+
+   if (submodule_needs_pushing(submodule->string, hashes))
+   string_list_insert(needs_pushing, submodule->string);
}
free_submodules_sha1s();
 
-- 
2.10.1.637.g09b28c5

[PATCH v2 2/3] serialize collection of refs that contain submodule changes

[Heiko Voigt]

We are iterating over each pushed ref and want to check whether it
contains changes to submodules. Instead of immediately checking each ref
lets first collect them and then do the check for all of them in one
revision walk.

Signed-off-by: Heiko Voigt 
---
 submodule.c | 36 +---
 submodule.h |  5 +++--
 transport.c | 29 +
 3 files changed, 45 insertions(+), 25 deletions(-)

diff --git a/submodule.c b/submodule.c
index 59c9d15905..5044afc2f8 100644
--- a/submodule.c
+++ b/submodule.c
@@ -522,6 +522,13 @@ static int has_remote(const char *refname, const struct 
object_id *oid,
return 1;
 }
 
+static int append_hash_to_argv(const unsigned char sha1[20], void *data)
+{
+   struct argv_array *argv = (struct argv_array *) data;
+   argv_array_push(argv, sha1_to_hex(sha1));
+   return 0;
+}
+
 static int submodule_needs_pushing(const char *path, const unsigned char 
sha1[20])
 {
if (add_submodule_odb(path) || !lookup_commit_reference(sha1))
@@ -623,24 +630,24 @@ static void free_submodules_sha1s(struct string_list 
*submodules)
string_list_clear(submodules, 1);
 }
 
-int find_unpushed_submodules(unsigned char new_sha1[20],
+int find_unpushed_submodules(struct sha1_array *hashes,
const char *remotes_name, struct string_list *needs_pushing)
 {
struct rev_info rev;
struct commit *commit;
-   const char *argv[] = {NULL, NULL, "--not", "NULL", NULL};
-   int argc = ARRAY_SIZE(argv) - 1, i;
-   char *sha1_copy;
+   int i;
struct string_list submodules = STRING_LIST_INIT_DUP;
+   struct argv_array argv = ARGV_ARRAY_INIT;
 
-   struct strbuf remotes_arg = STRBUF_INIT;
-
-   strbuf_addf(_arg, "--remotes=%s", remotes_name);
init_revisions(, NULL);
-   sha1_copy = xstrdup(sha1_to_hex(new_sha1));
-   argv[1] = sha1_copy;
-   argv[3] = remotes_arg.buf;
-   setup_revisions(argc, argv, , NULL);
+
+   /* argv.argv[0] will be ignored by setup_revisions */
+   argv_array_push(, "find_unpushed_submodules");
+   sha1_array_for_each_unique(hashes, append_hash_to_argv, );
+   argv_array_push(, "--not");
+   argv_array_pushf(, "--remotes=%s", remotes_name);
+
+   setup_revisions(argv.argc, argv.argv, , NULL);
if (prepare_revision_walk())
die("revision walk setup failed");
 
@@ -648,8 +655,7 @@ int find_unpushed_submodules(unsigned char new_sha1[20],
find_unpushed_submodule_commits(commit, );
 
reset_revision_walk();
-   free(sha1_copy);
-   strbuf_release(_arg);
+   argv_array_clear();
 
for (i = 0; i < submodules.nr; i++) {
struct string_list_item *item = [i];
@@ -687,12 +693,12 @@ static int push_submodule(const char *path)
return 1;
 }
 
-int push_unpushed_submodules(unsigned char new_sha1[20], const char 
*remotes_name)
+int push_unpushed_submodules(struct sha1_array *hashes, const char 
*remotes_name)
 {
int i, ret = 1;
struct string_list needs_pushing = STRING_LIST_INIT_DUP;
 
-   if (!find_unpushed_submodules(new_sha1, remotes_name, _pushing))
+   if (!find_unpushed_submodules(hashes, remotes_name, _pushing))
return 1;
 
for (i = 0; i < needs_pushing.nr; i++) {
diff --git a/submodule.h b/submodule.h
index d9e197a948..065b2f0a2a 100644
--- a/submodule.h
+++ b/submodule.h
@@ -3,6 +3,7 @@
 
 struct diff_options;
 struct argv_array;
+struct sha1_array;
 
 enum {
RECURSE_SUBMODULES_CHECK = -4,
@@ -62,9 +63,9 @@ int submodule_uses_gitfile(const char *path);
 int ok_to_remove_submodule(const char *path);
 int merge_submodule(unsigned char result[20], const char *path, const unsigned 
char base[20],
const unsigned char a[20], const unsigned char b[20], int 
search);
-int find_unpushed_submodules(unsigned char new_sha1[20], const char 
*remotes_name,
+int find_unpushed_submodules(struct sha1_array *hashes, const char 
*remotes_name,
struct string_list *needs_pushing);
-int push_unpushed_submodules(unsigned char new_sha1[20], const char 
*remotes_name);
+int push_unpushed_submodules(struct sha1_array *hashes, const char 
*remotes_name);
 void connect_work_tree_and_git_dir(const char *work_tree, const char *git_dir);
 int parallel_submodules(void);
 
diff --git a/transport.c b/transport.c
index 94d6dc3725..05f2ce83f1 100644
--- a/transport.c
+++ b/transport.c
@@ -903,23 +903,36 @@ int transport_push(struct transport *transport,
 
if ((flags & TRANSPORT_RECURSE_SUBMODULES_ON_DEMAND) && 
!is_bare_repository()) {
struct ref *ref = remote_refs;
+   struct sha1_array hashes = SHA1_ARRAY_INIT;
+
for (; ref; ref = ref->next)
-   if (!is_null_oid(>new_oid) &&
-   

[PATCH v2 1/3] serialize collection of changed submodules

[Heiko Voigt]

To check whether a submodule needs to be pushed we need to collect all
changed submodules. Lets collect them first and then execute the
possibly expensive test whether certain revisions are already pushed
only once per submodule.

There is further potential for optimization since we can assemble one
command and only issued that instead of one call for each remote ref in
the submodule.

Signed-off-by: Heiko Voigt 
---
 submodule.c | 63 -
 1 file changed, 58 insertions(+), 5 deletions(-)

diff --git a/submodule.c b/submodule.c
index 2de06a3351..59c9d15905 100644
--- a/submodule.c
+++ b/submodule.c
@@ -554,19 +554,34 @@ static int submodule_needs_pushing(const char *path, 
const unsigned char sha1[20
return 0;
 }
 
+static struct sha1_array *get_sha1s_from_list(struct string_list *submodules,
+   const char *path)
+{
+   struct string_list_item *item;
+
+   item = string_list_insert(submodules, path);
+   if (item->util)
+   return (struct sha1_array *) item->util;
+
+   /* NEEDSWORK: should we have sha1_array_init()? */
+   item->util = xcalloc(1, sizeof(struct sha1_array));
+   return (struct sha1_array *) item->util;
+}
+
 static void collect_submodules_from_diff(struct diff_queue_struct *q,
 struct diff_options *options,
 void *data)
 {
int i;
-   struct string_list *needs_pushing = data;
+   struct string_list *submodules = data;
 
for (i = 0; i < q->nr; i++) {
struct diff_filepair *p = q->queue[i];
+   struct sha1_array *hashes;
if (!S_ISGITLINK(p->two->mode))
continue;
-   if (submodule_needs_pushing(p->two->path, p->two->oid.hash))
-   string_list_insert(needs_pushing, p->two->path);
+   hashes = get_sha1s_from_list(submodules, p->two->path);
+   sha1_array_append(hashes, p->two->oid.hash);
}
 }
 
@@ -582,14 +597,41 @@ static void find_unpushed_submodule_commits(struct commit 
*commit,
diff_tree_combined_merge(commit, 1, );
 }
 
+struct collect_submodule_from_sha1s_data {
+   char *submodule_path;
+   struct string_list *needs_pushing;
+};
+
+static void collect_submodules_from_sha1s(const unsigned char sha1[20],
+   void *data)
+{
+   struct collect_submodule_from_sha1s_data *me =
+   (struct collect_submodule_from_sha1s_data *) data;
+
+   if (submodule_needs_pushing(me->submodule_path, sha1))
+   string_list_insert(me->needs_pushing, me->submodule_path);
+}
+
+static void free_submodules_sha1s(struct string_list *submodules)
+{
+   int i;
+   for (i = 0; i < submodules->nr; i++) {
+   struct string_list_item *item = >items[i];
+   struct sha1_array *hashes = (struct sha1_array *) item->util;
+   sha1_array_clear(hashes);
+   }
+   string_list_clear(submodules, 1);
+}
+
 int find_unpushed_submodules(unsigned char new_sha1[20],
const char *remotes_name, struct string_list *needs_pushing)
 {
struct rev_info rev;
struct commit *commit;
const char *argv[] = {NULL, NULL, "--not", "NULL", NULL};
-   int argc = ARRAY_SIZE(argv) - 1;
+   int argc = ARRAY_SIZE(argv) - 1, i;
char *sha1_copy;
+   struct string_list submodules = STRING_LIST_INIT_DUP;
 
struct strbuf remotes_arg = STRBUF_INIT;
 
@@ -603,12 +645,23 @@ int find_unpushed_submodules(unsigned char new_sha1[20],
die("revision walk setup failed");
 
while ((commit = get_revision()) != NULL)
-   find_unpushed_submodule_commits(commit, needs_pushing);
+   find_unpushed_submodule_commits(commit, );
 
reset_revision_walk();
free(sha1_copy);
strbuf_release(_arg);
 
+   for (i = 0; i < submodules.nr; i++) {
+   struct string_list_item *item = [i];
+   struct collect_submodule_from_sha1s_data data;
+   data.submodule_path = item->string;
+   data.needs_pushing = needs_pushing;
+   sha1_array_for_each_unique((struct sha1_array *) item->util,
+   collect_submodules_from_sha1s,
+   );
+   }
+   free_submodules_sha1s();
+
return needs_pushing->nr;
 }
 
-- 
2.10.1.637.g09b28c5

[PATCH v2 0/3] Speedup finding of unpushed submodules

[Heiko Voigt]

You can find the first iteration of this series as part of this thread:

http://public-inbox.org/git/%3C20160914173124.GA7613@sandbox%3E/

All mentioned issues should be fixed. I dropped the last patch which was
the cause of the broken tests.

This should optimize every part of this test to a nice speed if you are
pushing to a remote. The only case that is still broken/slow as hell is
when calling push with a direct url.

I am thinking whether we should maybe error out with a "not implemented"
message or something and mention that --recurse-submoules does not work
with direct urls? But we might want to have another look at performance
with this patch included. Maybe it is actually useable with the last
patch included which was not yet on pu.

Cheers Heiko

Heiko Voigt (3):
  serialize collection of changed submodules
  serialize collection of refs that contain submodule changes
  batch check whether submodule needs pushing into one call

 submodule.c | 116 ++--
 submodule.h |   5 +--
 transport.c |  29 ++-
 3 files changed, 114 insertions(+), 36 deletions(-)

-- 
2.10.1.637.g09b28c5

Re: [PATCH/RFC] git.c: support "!!" aliases that do not move cwd

[Matthieu Moy]

Johannes Schindelin  writes:

> Hi Matthieu,
>
> On Fri, 7 Oct 2016, Matthieu Moy wrote:
>
>> Another possibility: !(nocd), which leaves room
>> for !(keyword1,keyword2,...) if needed later. Also, it is consistent
>> with the :(word) syntax of pathspecs.
>
> But is this backwards-compatible? Don't we execute everything that comes
> after the exclamation mark as a command-line via shell, where the
> parentheses mean "open a subshell"?

Good point. I can imagine someone already having

[alias]
foo = !(cd bar && $something) && $something_else

Your proposed (keyword)! is better.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

Re: [PATCH] clean up confusing suggestion for commit references

[Jeff King]

On Fri, Oct 07, 2016 at 11:56:38AM +0200, Heiko Voigt wrote:

> The description for referencing commits looks as if it is contradicting
> the example, since it is itself enclosed in double quotes. Lets use
> single quotes around the description and include the double quotes in
> the description so it matches the example.
> ---
> Sorry for opening this up again but I just looked up the format and was
> like: "Umm, which one is now the correct one..."
> 
> For this makes more sense. What do others think?

Looking over the threads, I wasn't sure there was consensus[1,2]. So it would
be equally correct to drop the quotes from the example.

I dunno. I am in favor of no-quotes, myself, so maybe I am just
manufacturing dissent in my mind. :)

-Peff

[1] http://public-inbox.org/git/a9731f60-5c30-0bc6-f73a-f7ffb7bd4...@kdbg.org/
[2] 
http://public-inbox.org/git/20160829183015.2uqnfezekjfa3...@sigill.intra.peff.net/

Re: [PATCH/RFC] git.c: support "!!" aliases that do not move cwd

[Jeff King]

On Fri, Oct 07, 2016 at 04:11:34PM +0200, Johannes Schindelin wrote:

> Possibly a better idea would be to use *another* special symbol, one that
> makes intuitive sense as a modifier, such as:
> 
>   [alias]
>   # This works as before
>   xyz = !pwd
>   # As does this
>   stat = -p status
>   # This, however, is different:
>   duy = (nocd)!pwd
> 
> This is backwards compatible as "(" is not a part of any Git command, nor
> of a valid alias, nor is it commonly used as part of a git-*
> executable/script.
> 
> It is also kind of a bit more intuitive, I'd wager, and it is also
> extensible to future options we may want to introduce.

I like this much better (like you, I am concerned about things like
"!(foo)" as conflicting with the shell). And I think your "(nocd)!pwd"
example is quite readable.

-Peff

Re: [PATCH/RFC] git.c: support "!!" aliases that do not move cwd

[Johannes Schindelin]

Hi Kuba,

On Fri, 7 Oct 2016, Jakub Narębski wrote:

> W dniu 07.10.2016 o 13:20, Johannes Schindelin pisze:
> > On Thu, 6 Oct 2016, Junio C Hamano wrote:
> >> Nguyễn Thái Ngọc Duy   writes:
> >>
> >>> Throwing something at the mailing list to see if anybody is
> >>> interested.
> >>>
> >>> Current '!' aliases move cwd to $GIT_WORK_TREE first, which could make
> >>> handling path arguments hard because they are relative to the original
> >>> cwd. We set GIT_PREFIX to work around it, but I still think it's more
> >>> natural to keep cwd where it is.
> >>>
> >>> We have a way to do that now after 441981b (git: simplify environment
> >>> save/restore logic - 2016-01-26). It's just a matter of choosing the
> >>> right syntax. I'm going with '!!'. I'm not very happy with it. But I
> >>> do like this type of alias.
> >>
> >> I do not know why you are not happy with the syntax, but I
> >> personally think it brilliant, both the idea and the preliminary
> >> clean-up that made this possible with a simple patch like this.
> > 
> > I guess he is not happy with it because "!!" is quite unintuitive a
> > construct. I know that *I* would have been puzzled by it, asking "What the
> > heck does this do?".
> 
> Well, "!" as a prefix is not intuitive either.

You do not use vi, do you? :-P

In vi, if you enter command mode (typing a colon) and then want to
execute, say, `pwd`, you type !pwd

> Perhaps "!.", because "." is current directory, and the "." command
> (that is, alias to "source") doesn't make sense in git aliases.

If you want to execute, say, `pwd` in the current directory, that would
mean you want to write

!.pwd

But that already means "execute `.pwd`"...

> Note that we would have to teach git completion about new syntax;
> or new configuration variable if we go that route.

Why would we? Git's completion does not expand aliases, it only completes
the aliases' names, not their values.

Ciao,
Dscho

Re: [PATCH/RFC] git.c: support "!!" aliases that do not move cwd

[Johannes Schindelin]

Hi Matthieu,

On Fri, 7 Oct 2016, Matthieu Moy wrote:

> Duy Nguyen  writes:
> 
> > On Fri, Oct 7, 2016 at 6:20 PM, Johannes Schindelin
> >  wrote:
> >> Hi Junio,
> >>
> >> On Thu, 6 Oct 2016, Junio C Hamano wrote:
> >>
> >>> Nguyễn Thái Ngọc Duy   writes:
> >>>
> >>> > Throwing something at the mailing list to see if anybody is
> >>> > interested.
> >>> >
> >>> > Current '!' aliases move cwd to $GIT_WORK_TREE first, which could
> >>> > make
> >>> > handling path arguments hard because they are relative to the
> >>> > original
> >>> > cwd. We set GIT_PREFIX to work around it, but I still think it's
> >>> > more
> >>> > natural to keep cwd where it is.
> >>> >
> >>> > We have a way to do that now after 441981b (git: simplify
> >>> > environment
> >>> > save/restore logic - 2016-01-26). It's just a matter of choosing
> >>> > the
> >>> > right syntax. I'm going with '!!'. I'm not very happy with it.
> >>> > But I
> >>> > do like this type of alias.
> >>>
> >>> I do not know why you are not happy with the syntax, but I
> >>> personally think it brilliant, both the idea and the preliminary
> >>> clean-up that made this possible with a simple patch like this.
> >>
> >> I guess he is not happy with it because "!!" is quite unintuitive a
> >> construct. I know that *I* would have been puzzled by it, asking
> >> "What the
> >> heck does this do?".
> >
> > Yep. And I wouldn't want to set a tradition for the next alias type
> > '!!!'. There's no good choice to represent a new alias type with a
> > leading symbol. This just occurred to me, however, what do you think
> > about a new config group for it? With can have something like
> > externalAlias.* (or some other name) that lives in parallel with
> > alias.*. Then we don't need '!' (or '!!') at all.
> 
> Another possibility: !(nocd), which leaves room
> for !(keyword1,keyword2,...) if needed later. Also, it is consistent
> with the :(word) syntax of pathspecs.

But is this backwards-compatible? Don't we execute everything that comes
after the exclamation mark as a command-line via shell, where the
parentheses mean "open a subshell"?

Ciao,
Dscho

Re: [PATCH/RFC] git.c: support "!!" aliases that do not move cwd

[Johannes Schindelin]

Hi Duy,

On Fri, 7 Oct 2016, Duy Nguyen wrote:

> On Fri, Oct 7, 2016 at 6:20 PM, Johannes Schindelin
>  wrote:
> >
> > On Thu, 6 Oct 2016, Junio C Hamano wrote:
> >
> >> Nguyễn Thái Ngọc Duy   writes:
> >>
> >> > Throwing something at the mailing list to see if anybody is
> >> > interested.
> >> >
> >> > Current '!' aliases move cwd to $GIT_WORK_TREE first, which could make
> >> > handling path arguments hard because they are relative to the original
> >> > cwd. We set GIT_PREFIX to work around it, but I still think it's more
> >> > natural to keep cwd where it is.
> >> >
> >> > We have a way to do that now after 441981b (git: simplify environment
> >> > save/restore logic - 2016-01-26). It's just a matter of choosing the
> >> > right syntax. I'm going with '!!'. I'm not very happy with it. But I
> >> > do like this type of alias.
> >>
> >> I do not know why you are not happy with the syntax, but I
> >> personally think it brilliant, both the idea and the preliminary
> >> clean-up that made this possible with a simple patch like this.
> >
> > I guess he is not happy with it because "!!" is quite unintuitive a
> > construct. I know that *I* would have been puzzled by it, asking "What the
> > heck does this do?".
> 
> Yep. And I wouldn't want to set a tradition for the next alias type
> '!!!'. There's no good choice to represent a new alias type with a
> leading symbol. This just occurred to me, however, what do you think
> about a new config group for it? With can have something like
> externalAlias.* (or some other name) that lives in parallel with
> alias.*. Then we don't need '!' (or '!!') at all.

But what would the precedence be? externalAlias.xyz wins over alias.xyz?

And we still would need '!' support: tons of people (including myself)
rely on it.

Possibly a better idea would be to use *another* special symbol, one that
makes intuitive sense as a modifier, such as:

[alias]
# This works as before
xyz = !pwd
# As does this
stat = -p status
# This, however, is different:
duy = (nocd)!pwd

This is backwards compatible as "(" is not a part of any Git command, nor
of a valid alias, nor is it commonly used as part of a git-*
executable/script.

It is also kind of a bit more intuitive, I'd wager, and it is also
extensible to future options we may want to introduce.

Ciao,
Dscho

Re: [PATCH/RFC] git.c: support "!!" aliases that do not move cwd

[Jakub Narębski]

Hello, Johannes

W dniu 07.10.2016 o 13:20, Johannes Schindelin pisze:
> On Thu, 6 Oct 2016, Junio C Hamano wrote:
>> Nguyễn Thái Ngọc Duy   writes:
>>
>>> Throwing something at the mailing list to see if anybody is
>>> interested.
>>>
>>> Current '!' aliases move cwd to $GIT_WORK_TREE first, which could make
>>> handling path arguments hard because they are relative to the original
>>> cwd. We set GIT_PREFIX to work around it, but I still think it's more
>>> natural to keep cwd where it is.
>>>
>>> We have a way to do that now after 441981b (git: simplify environment
>>> save/restore logic - 2016-01-26). It's just a matter of choosing the
>>> right syntax. I'm going with '!!'. I'm not very happy with it. But I
>>> do like this type of alias.
>>
>> I do not know why you are not happy with the syntax, but I
>> personally think it brilliant, both the idea and the preliminary
>> clean-up that made this possible with a simple patch like this.
> 
> I guess he is not happy with it because "!!" is quite unintuitive a
> construct. I know that *I* would have been puzzled by it, asking "What the
> heck does this do?".

Well, "!" as a prefix is not intuitive either.

Perhaps "!.", because "." is current directory, and the "." command
(that is, alias to "source") doesn't make sense in git aliases.

Note that we would have to teach git completion about new syntax;
or new configuration variable if we go that route.
-- 
Jakub Narębski

Re: [PATCH/RFC] git.c: support "!!" aliases that do not move cwd

[Matthieu Moy]

Duy Nguyen  writes:

> On Fri, Oct 7, 2016 at 6:20 PM, Johannes Schindelin
>  wrote:
>> Hi Junio,
>>
>> On Thu, 6 Oct 2016, Junio C Hamano wrote:
>>
>>> Nguyễn Thái Ngọc Duy   writes:
>>>
>>> > Throwing something at the mailing list to see if anybody is
>>> > interested.
>>> >
>>> > Current '!' aliases move cwd to $GIT_WORK_TREE first, which could
>>> > make
>>> > handling path arguments hard because they are relative to the
>>> > original
>>> > cwd. We set GIT_PREFIX to work around it, but I still think it's
>>> > more
>>> > natural to keep cwd where it is.
>>> >
>>> > We have a way to do that now after 441981b (git: simplify
>>> > environment
>>> > save/restore logic - 2016-01-26). It's just a matter of choosing
>>> > the
>>> > right syntax. I'm going with '!!'. I'm not very happy with it.
>>> > But I
>>> > do like this type of alias.
>>>
>>> I do not know why you are not happy with the syntax, but I
>>> personally think it brilliant, both the idea and the preliminary
>>> clean-up that made this possible with a simple patch like this.
>>
>> I guess he is not happy with it because "!!" is quite unintuitive a
>> construct. I know that *I* would have been puzzled by it, asking
>> "What the
>> heck does this do?".
>
> Yep. And I wouldn't want to set a tradition for the next alias type
> '!!!'. There's no good choice to represent a new alias type with a
> leading symbol. This just occurred to me, however, what do you think
> about a new config group for it? With can have something like
> externalAlias.* (or some other name) that lives in parallel with
> alias.*. Then we don't need '!' (or '!!') at all.

Another possibility: !(nocd), which leaves room
for !(keyword1,keyword2,...) if needed later. Also, it is consistent
with the :(word) syntax of pathspecs.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

Re: [PATCH 5/6] Documentation/git-merge.txt: improve short description in DESCRIPTION

[Sergey Organov]

Junio C Hamano  writes:

> Sergey Organov  writes:
>
 Last, if "reference" is not good enough and we get to internals anyway,
 why not say SHA1 then?
>>>
>>> Because that is still colloquial? I think s/name/object name/ is a
>>> sensible change, but not s/name/reference/.
>>
>> No, "reference" is more sensible here than any of "name", "object name",
>> or "SHA-1", the same way as here:
>>
>> $ git help glossary
>> [...]
>> chain
>> A list of objects, where each object in the list contains a
>> reference to its successor (for example, the successor of a
>> commit could be one of its parents).
>> [...]
>
> The entry for "chain" and the description under discussion have
> stress on different aspect, though.  The description of "chain" is
> more general: an object refers to another object by referring to it,
> by unspecified means.  The reason why it is left unspecified is
> because the way a tree object refers to blobs and trees is different
> from the way a commit object refers to its parents (the former has
> object names of blobs and trees in the tree entries; the latter uses
> "parent" entries in the object header part to record object names of
> parent commits).  It wants to stress more on the fact that there is
> some mechanism to associate one object to others, than how that
> association/linkage is expressed.
>
> The way the resulting commit is described in the original text of
> "git merge" description stresses more on "how" by being a lot more
> specific to commit objects.  It does not just say "refers to parents
> (by unspecified means)"; instead it tries to say what exactly are
> recorded, i.e. the parents are referred to by recording the object
> names of them in a new commit object.  It stresses more on "how"
> (because it can afford to be more specific, unlike the description
> of more general concept of a "chain").

That's were our disagreement actually is, and that's what I've tried to
fix with s/name/reference/, and that's why I'm against s/name/object
name/.

Rather than being more (and more) specific at every opportunity, one
needs a good reason to get more specific. In this particular case,
general DAG terminology seems to be enough to describe git-merge
semantics, thus using GIT specifics is unfounded.

> It may be debatable if we want to give the description of what is
> exactly recorded at that point of the document,

Exactly. My point in this particular discussion is that details of
recording of references to parents don't belong here, even though to
tell the truth I think they don't belong to git _user_ documentation at
all.

> but I personally
> think that the users deserve a chance to learn how a merge is
> recorded in "git merge" documentation.

I doubt a user will gain anything from this sacred knowledge suddenly
being thrown on him when what she is looking for is understanding of
basic merge semantics in GIT.

That said, if you still disagree, please feel free to just drop the
patch.

-- Sergey

Re: [PATCH 1/3] Resurrect "diff-lib.c: adjust position of i-t-a entries in diff"

[Duy Nguyen]

On Fri, Oct 7, 2016 at 2:15 AM, Junio C Hamano  wrote:
> Duy Nguyen  writes:
>
>> On Tue, Oct 4, 2016 at 11:15 PM, Junio C Hamano  wrote:
>>> Duy Nguyen  writes:
>>>
 We don't use it internally _yet_. I need to go through all the
 external diff code and see --shift-ita should be there. The end goal
 is still changing the default behavior and getting rid of --shift-ita,
>>>
>>> I do not agree with that endgame, and quite honestly I do not want
>>> to waste time reviewing such a series.
>
> I definitely shouldn't have said that, especially "waste".  Many
> issues around i-t-a and diff make my head hurt when I think about
> them [*1*], but not wanting to spend time that gets my
> head hurt and not wanting to waste time are totally different
> things.  My apologies.

No problem. I do appreciate a straight shoot down though. Many of my
topics have been going on for months (ones not in 'pu') and seeing it
rejected near the end is worse than stopping working on them early.

> I missed something curious in your statement above, i.e. "external
> diff".  I thought we have pretty much got rid of all the invocation
> of "git diff" via the run_command() interface and we do not need the
> command line option (we only need the options->shift_ita so that
> callers like "git status" can seletively ask for it when making
> internal calls), and that is why I didn't want to see it.

I don't know if we have had any external diff calls in our shell-based
commands. I don't read them often. Regardless, people do use "git
diff" and it should show the "right thing" (I know it's subjective).
Or at least be consistent with both git-commit and git-status.

> [Footnote]
>
> Here is one of the things around i-t-a and diff.  If you make "git
> diff" (between the index and the working tree) report "new" file, it
> would imply that "git apply" run without "--index" should create an

Off topic. This reminds me of an old patch about apply and ita [1] but
that one is not the same here

> ita entry in the index for symmetry, wouldn't it?  That by itself
> can be seen as an improvement (we no longer would have to say that
> "git apply patchfile && git commit -a" that is run in a clean state
> will forget new files the patchfile creates), but it also means we
> now need a repository in order to run "git apply" (without "--index"),
> which is a problem, as "git apply" is often used as a better "patch".

We could detect "no repo available" and ignore the index, I guess.

> "git apply --cached" may also become "interesting".  A patch that
> would apply cleanly to HEAD should apply cleanly if you did this:
>
> $ git read-tree HEAD
> $ git apply --cached 
> no matter what the working tree state is.  Should a patch that
> creates a "new" file add contents to the index, or just an i-t-a
> entry?  I could argue it both ways, but either is quite satisfactory
> and makes my head hurt.

--cached tells you to put new contents in the index. I-ta entries,
being a reminder to add stuff, don't really fit in here because you
want to add contents _now_, i think. After a successful "git apply
--cached", a "git commit" should contain exactly what the applied
patch has. If new files are i-t-a entries instead, then the new commit
would not be the same as the patch.

[1] 
https://public-inbox.org/git/1451181092-26054-4-git-send-email-pclo...@gmail.com/
-- 
Duy

Re: [PATCH/RFC] git.c: support "!!" aliases that do not move cwd

[Duy Nguyen]

On Fri, Oct 7, 2016 at 7:47 PM, Matthieu Moy
 wrote:
> Duy Nguyen  writes:
>
>> On Fri, Oct 7, 2016 at 6:20 PM, Johannes Schindelin
>>  wrote:
>>> Hi Junio,
>>>
>>> On Thu, 6 Oct 2016, Junio C Hamano wrote:
>>>
 Nguyễn Thái Ngọc Duy   writes:

 > Throwing something at the mailing list to see if anybody is
 > interested.
 >
 > Current '!' aliases move cwd to $GIT_WORK_TREE first, which could
 > make
 > handling path arguments hard because they are relative to the
 > original
 > cwd. We set GIT_PREFIX to work around it, but I still think it's
 > more
 > natural to keep cwd where it is.
 >
 > We have a way to do that now after 441981b (git: simplify
 > environment
 > save/restore logic - 2016-01-26). It's just a matter of choosing
 > the
 > right syntax. I'm going with '!!'. I'm not very happy with it.
 > But I
 > do like this type of alias.

 I do not know why you are not happy with the syntax, but I
 personally think it brilliant, both the idea and the preliminary
 clean-up that made this possible with a simple patch like this.
>>>
>>> I guess he is not happy with it because "!!" is quite unintuitive a
>>> construct. I know that *I* would have been puzzled by it, asking
>>> "What the
>>> heck does this do?".
>>
>> Yep. And I wouldn't want to set a tradition for the next alias type
>> '!!!'. There's no good choice to represent a new alias type with a
>> leading symbol. This just occurred to me, however, what do you think
>> about a new config group for it? With can have something like
>> externalAlias.* (or some other name) that lives in parallel with
>> alias.*. Then we don't need '!' (or '!!') at all.
>
> Another possibility: !(nocd), which leaves room
> for !(keyword1,keyword2,...) if needed later. Also, it is consistent
> with the :(word) syntax of pathspecs.

This seems to solve my problem nicely.
-- 
Duy

Re: [PATCH 1/2] submodule add: extend force flag to add existing repos

[Heiko Voigt]

On Thu, Oct 06, 2016 at 01:11:20PM -0700, Junio C Hamano wrote:
> Stefan Beller  writes:
> 
> > Currently the force flag in `git submodule add` takes care of possibly
> > ignored files or when a name collision occurs.
> >
> > However there is another situation where submodule add comes in handy:
> > When you already have a gitlink recorded, but no configuration was
> > done (i.e. no .gitmodules file nor any entry in .git/config) and you
> > want to generate these config entries. For this situation allow
> > `git submodule add` to proceed if there is already a submodule at the
> > given path in the index.

Is it important that the submodule is in the index? How about worktree?
>From the index entry alone we can not deduce the values anyway. So I
would say the submodule has to be in the worktree, no matter what is in
the index. If its not in the index we can also add it.

BTW, that is the way I imagined submodules would work in the first
place: just clone and add them, like I described here[1].

> > Signed-off-by: Stefan Beller 
> > ---
> 
> Yup, the goal makes perfect sense.  
> 
> I vaguely recall discussing this exact issue of "git submodule add"
> that refuses to add a path that already is a gitlink (via "git add"
> that has previously been run) elsewhere on this list some time ago.

Yes there was a discussion, see the link.

Cheers Heiko

[1] http://public-inbox.org/git/%3c20160916141143.ga47...@book.hvoigt.net%3E/

Re: [PATCHv3 1/2] push: change submodule default to check when submodules exist

[Heiko Voigt]

On Thu, Oct 06, 2016 at 10:20:16AM -0700, Stefan Beller wrote:
> On Thu, Oct 6, 2016 at 2:23 AM, Heiko Voigt  wrote:
> > On Wed, Oct 05, 2016 at 03:53:25PM +0200, Heiko Voigt wrote:
> >> On Tue, Oct 04, 2016 at 02:03:58PM -0700, Stefan Beller wrote:
> >> > Jeff,
> >> > thanks for the suggestions, both git_path(..) as well as checking the 
> >> > config,
> >> > this seems quite readable to me:
> >>
> >> When reading the discussion I thought the same: What about the
> >> "old-style" repositories. I like this one. Checking both locations
> >> is nice.
> >
> > BTW, since it seems we all agree on the direction. Should we add some
> > tests?
> >
> 
> Good call. What do we want to test for?
> * Correctness in case of submodules? (just push and get rejected)
>   I think that is easy to do.
> * Performance with [no, lots of] submodules? That seems harder to me.
> 
> I'll add a test for the correctness part and resend.

Well I though about the following tests:

 * Without submodules: Make sure submodule processing is disabled by
   default
 * With new-style submodules: Make sure submodules are processed by
   default
 * With old-style submodules: Make sure submodules are processed by
   default

But I have to admit that I did not think about the "how can we do that".
But performance seems to be the only thing that is exposing the
processing when we have no submodules, so it seems we can only easily do
the tests with submodules.

Cheers Heiko

Re: [PATCH/RFC] git.c: support "!!" aliases that do not move cwd

[Duy Nguyen]

On Fri, Oct 7, 2016 at 6:20 PM, Johannes Schindelin
 wrote:
> Hi Junio,
>
> On Thu, 6 Oct 2016, Junio C Hamano wrote:
>
>> Nguyễn Thái Ngọc Duy   writes:
>>
>> > Throwing something at the mailing list to see if anybody is
>> > interested.
>> >
>> > Current '!' aliases move cwd to $GIT_WORK_TREE first, which could make
>> > handling path arguments hard because they are relative to the original
>> > cwd. We set GIT_PREFIX to work around it, but I still think it's more
>> > natural to keep cwd where it is.
>> >
>> > We have a way to do that now after 441981b (git: simplify environment
>> > save/restore logic - 2016-01-26). It's just a matter of choosing the
>> > right syntax. I'm going with '!!'. I'm not very happy with it. But I
>> > do like this type of alias.
>>
>> I do not know why you are not happy with the syntax, but I
>> personally think it brilliant, both the idea and the preliminary
>> clean-up that made this possible with a simple patch like this.
>
> I guess he is not happy with it because "!!" is quite unintuitive a
> construct. I know that *I* would have been puzzled by it, asking "What the
> heck does this do?".

Yep. And I wouldn't want to set a tradition for the next alias type
'!!!'. There's no good choice to represent a new alias type with a
leading symbol. This just occurred to me, however, what do you think
about a new config group for it? With can have something like
externalAlias.* (or some other name) that lives in parallel with
alias.*. Then we don't need '!' (or '!!') at all.
-- 
Duy

Re: [PATCH 6/6] Documentation/git-merge.txt: get rid of irrelevant references to git-pull

[Sergey Organov]

Junio C Hamano  writes:

> Sergey Organov  writes:
>
>> Ah, I now see. I tried to keep the text intact as much as possible, and
>> only split it into description and a note. Well, how about this then:
>
> Much better than your earlier patch, but I am not sure if the
> updated one is that much better compared to the original.

It's not intended to be much better. It is aimed at single simple
target: get rid of git-pull from descriptions of operations of
git-merge.

I'd just remove those git-pull reference, the only one that is left
after the patch, but it looks like git-merge needs an excuse to have
fast-forward on by default, and that excuse is the common git-pull case.

[I'd prefer 'git-merge --ff' were called from 'git-pull' and --no-ff be
the default for git-merge, but that's not the case, so I left the
reference to git-pull intact.]

>
> The pre- and post- state of this "how about this" patch essentially
> say the same thing, and I suspect that the primary reason why you
> think the post- state is easier to read is because you wrote it,
> while the reason why I do not see much difference is because I
> didn't write the updated one ;-).
>
> I do find "In this case, ... store the combined history" in the
> original a bit awkward to read, but most of that awkardness is
> inherited by the updated text.  It may benefit from hinting why a
> new commit is not needed a bit stronger.  Here is my attempt:
>
> When the commit we are merging is a descendant of the current
> HEAD, the history leading to the named commit can be, and by
> default is, taken as the combined history of the two.  Our
> history is "fast forwarded" to their history by updating `HEAD`
> along with the index to point at the named commit.
>
> This often happens when you are following along somebody else's
> work via "git pull" without doing your own development.
>
> I think the awkwardness I felt in the original and your version is
> gone from the above attempt, but I doubt that it is better over
> either of them in any other way.

This is entirely different matter, and should be a subject of another
patch, if any. My patch meant to only address git-pull references, with
as few changes as possible.

-- Sergey

Re: Regression: git no longer works with musl libc's regex impl

[Jakub Narębski]

W dniu 07.10.2016 o 00:42, Ramsay Jones pisze: 
> On 06/10/16 20:18, Ævar Arnfjörð Bjarmason wrote:
[...]
>> But just to clarify, does anyone have any objection to making our
>> configure.ac compile a C program to check for this sort of thing?
>> Because that seems like the easiest solution to this class of problem.
> 
> Err, you do know that we already do that, right?
> 
> [see commit a1e3b669 ("autoconf: don't use platform regex if it lacks 
> REG_STARTEND", 17-08-2010)]
> 
> In fact, if you run the auto tools on cygwin, you get a different setting
> for NO_REGEX than via config.mak.uname. Which is why I don't run configure
> on cygwin. :-D
> 
> [The issue is exposed by t7008-grep-binary.sh, where the cygwin native
> regex library matches '.' in a pattern with the NUL character. ie the
> test_expect_failure test passes.]

Huh.  So we have NO_REGEX support in ./configure, and people using
Git on untypical architectures and systems *can* make use of it.

It was just described wrongly, so in turn to have the more neutral
description, the same as in Makefile, let's do this:

 >8 -- >8 - >8 -- >8 --
Subject: [PATCH] configure.ac: Improve description of NO_REGEX test

The commit 2f8952250a changed description of NO_REGEX build config
variable to be more neutral, and actually say that it is about
support for REG_STARTEND.  Change description in configure.ac to
be the same.

Change also the test message and variable name to match.  The test
just checks that REG_STARTEND is #defined.

Issue-found-by: Ramsay Jones 
Signed-off-by: Jakub Narębski 
---
 configure.ac | 13 +++--
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/configure.ac b/configure.ac
index aa9c91d..7f39fd0 100644
--- a/configure.ac
+++ b/configure.ac
@@ -835,9 +835,10 @@ AC_CHECK_TYPE([struct addrinfo],[
 ])
 GIT_CONF_SUBST([NO_IPV6])
 #
-# Define NO_REGEX if you have no or inferior regex support in your C library.
-AC_CACHE_CHECK([whether the platform regex can handle null bytes],
- [ac_cv_c_excellent_regex], [
+# Define NO_REGEX if your C library lacks regex support with REG_STARTEND
+# feature.
+AC_CACHE_CHECK([whether the platform regex supports REG_STARTEND],
+ [ac_cv_c_regex_with_reg_startend], [
 AC_EGREP_CPP(yippeeyeswehaveit,
AC_LANG_PROGRAM([AC_INCLUDES_DEFAULT
 #include 
@@ -846,10 +847,10 @@ AC_EGREP_CPP(yippeeyeswehaveit,
 yippeeyeswehaveit
 #endif
 ]),
-   [ac_cv_c_excellent_regex=yes],
-   [ac_cv_c_excellent_regex=no])
+   [ac_cv_c_regex_with_reg_startend=yes],
+   [ac_cv_c_regex_with_reg_startend=no])
 ])
-if test $ac_cv_c_excellent_regex = yes; then
+if test $ac_cv_c_regex_with_reg_startend = yes; then
NO_REGEX=
 else
NO_REGEX=YesPlease
-- 
2.10.0

Re: [PATCH/RFC] git.c: support "!!" aliases that do not move cwd

[Johannes Schindelin]

Hi Junio,

On Thu, 6 Oct 2016, Junio C Hamano wrote:

> Nguyễn Thái Ngọc Duy   writes:
> 
> > Throwing something at the mailing list to see if anybody is
> > interested.
> >
> > Current '!' aliases move cwd to $GIT_WORK_TREE first, which could make
> > handling path arguments hard because they are relative to the original
> > cwd. We set GIT_PREFIX to work around it, but I still think it's more
> > natural to keep cwd where it is.
> >
> > We have a way to do that now after 441981b (git: simplify environment
> > save/restore logic - 2016-01-26). It's just a matter of choosing the
> > right syntax. I'm going with '!!'. I'm not very happy with it. But I
> > do like this type of alias.
> 
> I do not know why you are not happy with the syntax, but I
> personally think it brilliant, both the idea and the preliminary
> clean-up that made this possible with a simple patch like this.

I guess he is not happy with it because "!!" is quite unintuitive a
construct. I know that *I* would have been puzzled by it, asking "What the
heck does this do?".

Ciao,
Dscho

[PATCH] clean up confusing suggestion for commit references

[Heiko Voigt]

The description for referencing commits looks as if it is contradicting
the example, since it is itself enclosed in double quotes. Lets use
single quotes around the description and include the double quotes in
the description so it matches the example.
---
Sorry for opening this up again but I just looked up the format and was
like: "Umm, which one is now the correct one..."

For this makes more sense. What do others think?

Cheers Heiko

 Documentation/SubmittingPatches | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/SubmittingPatches b/Documentation/SubmittingPatches
index 08352de..692f4ce 100644
--- a/Documentation/SubmittingPatches
+++ b/Documentation/SubmittingPatches
@@ -122,7 +122,7 @@ without external resources. Instead of giving a URL to a 
mailing list
 archive, summarize the relevant points of the discussion.
 
 If you want to reference a previous commit in the history of a stable
-branch, use the format "abbreviated sha1 (subject, date)",
+branch, use the format 'abbreviated sha1 ("subject", date)',
 with the subject enclosed in a pair of double-quotes, like this:
 
 Commit f86a374 ("pack-bitmap.c: fix a memleak", 2015-03-30)
-- 
2.10.0.645.g54f1e86

[PATCH v2 2/4] mergetool: move main program flow into a main() function

[David Aguilar]

Make it easier to follow the program's flow by isolating all
logic into functions.  Isolate the main execution code path into
a single unit instead of having prompt_after_failed_merge()
interrupt it partyway through.

The use of a main() function is borrowing a convention from C,
Python, Perl, and many other languages.  This helps readers more
familiar with other languages understand the purpose of each
function when diving into the codebase with fresh eyes.

Signed-off-by: David Aguilar 
---
As suggested by Hannes, v2 provides a better commit message.

This is a stylistic choice, but since I was about to start hacking on it,
I couldn't help but notice the organic sprawl that could use some tidying
before adding new features.

 git-mergetool.sh | 180 ---
 1 file changed, 93 insertions(+), 87 deletions(-)

diff --git a/git-mergetool.sh b/git-mergetool.sh
index 300ce7f..b2cd0a4 100755
--- a/git-mergetool.sh
+++ b/git-mergetool.sh
@@ -366,51 +366,6 @@ merge_file () {
return 0
 }
 
-prompt=$(git config --bool mergetool.prompt)
-guessed_merge_tool=false
-
-while test $# != 0
-do
-   case "$1" in
-   --tool-help=*)
-   TOOL_MODE=${1#--tool-help=}
-   show_tool_help
-   ;;
-   --tool-help)
-   show_tool_help
-   ;;
-   -t|--tool*)
-   case "$#,$1" in
-   *,*=*)
-   merge_tool=$(expr "z$1" : 'z-[^=]*=\(.*\)')
-   ;;
-   1,*)
-   usage ;;
-   *)
-   merge_tool="$2"
-   shift ;;
-   esac
-   ;;
-   -y|--no-prompt)
-   prompt=false
-   ;;
-   --prompt)
-   prompt=true
-   ;;
-   --)
-   shift
-   break
-   ;;
-   -*)
-   usage
-   ;;
-   *)
-   break
-   ;;
-   esac
-   shift
-done
-
 prompt_after_failed_merge () {
while true
do
@@ -427,57 +382,108 @@ prompt_after_failed_merge () {
done
 }
 
-git_dir_init
-require_work_tree
+main () {
+   prompt=$(git config --bool mergetool.prompt)
+   guessed_merge_tool=false
+
+   while test $# != 0
+   do
+   case "$1" in
+   --tool-help=*)
+   TOOL_MODE=${1#--tool-help=}
+   show_tool_help
+   ;;
+   --tool-help)
+   show_tool_help
+   ;;
+   -t|--tool*)
+   case "$#,$1" in
+   *,*=*)
+   merge_tool=$(expr "z$1" : 'z-[^=]*=\(.*\)')
+   ;;
+   1,*)
+   usage ;;
+   *)
+   merge_tool="$2"
+   shift ;;
+   esac
+   ;;
+   -y|--no-prompt)
+   prompt=false
+   ;;
+   --prompt)
+   prompt=true
+   ;;
+   --)
+   shift
+   break
+   ;;
+   -*)
+   usage
+   ;;
+   *)
+   break
+   ;;
+   esac
+   shift
+   done
+
+   git_dir_init
+   require_work_tree
 
-if test -z "$merge_tool"
-then
-   # Check if a merge tool has been configured
-   merge_tool=$(get_configured_merge_tool)
-   # Try to guess an appropriate merge tool if no tool has been set.
if test -z "$merge_tool"
then
-   merge_tool=$(guess_merge_tool) || exit
-   guessed_merge_tool=true
+   # Check if a merge tool has been configured
+   merge_tool=$(get_configured_merge_tool)
+   # Try to guess an appropriate merge tool if no tool has been 
set.
+   if test -z "$merge_tool"
+   then
+   merge_tool=$(guess_merge_tool) || exit
+   guessed_merge_tool=true
+   fi
fi
-fi
-merge_keep_backup="$(git config --bool mergetool.keepBackup || echo true)"
-merge_keep_temporaries="$(git config --bool mergetool.keepTemporaries || echo 
false)"
-
-files=
+   merge_keep_backup="$(git config --bool mergetool.keepBackup || echo 
true)"
+   merge_keep_temporaries="$(git config --bool mergetool.keepTemporaries 
|| echo false)"
 
-if test $# -eq 0
-then
-   cd_to_toplevel
+   files=
 
-   if test -e "$GIT_DIR/MERGE_RR"
+   if test $# -eq 0
then
-   files=$(git rerere remaining)
+   cd_to_toplevel
+
+