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

2016-10-06 Thread Johannes Sixt

Am 07.10.2016 um 01:47 schrieb David Aguilar:

Signed-off-by: David Aguilar 
---


An answer to "why?" is missing here. I guess it is just so that 
everything happens in functions, and that there is only the invocation 
of main at the top-level of the script. I am unsure whether this is 
sufficient justification; you are adding a level of indentation after all.


-- Hannes



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

2016-10-06 Thread Kevin Daudt
On Thu, Oct 06, 2016 at 03:24:17PM -0700, Junio C Hamano wrote:
> Here are the topics that have been cooking.  Commits prefixed with
> '-' are only in 'pu' (proposed updates) while commits prefixed with
> '+' are in 'next'.  The ones marked with '.' do not appear in any of
> the integration branches, but I am still holding onto them.
> 
> A handful of topics have been merged to 'master' and it's time to
> update the "What's cooking" report.  Linus's and Peff's auto scaling
> of default abbreviation length will cook a bit longer in 'next' to
> see if anybody screams; there may still be codepaths that assume
> that default-abbrev would never be negative and a reasonable
> fallback value that we need to locate and fix.
> 
> You can find the changes described here in the integration branches
> of the repositories listed at
> 
> http://git-blame.blogspot.com/p/git-public-repositories.html
> 
> --
> [Graduated to "master"]
> 

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".


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

2016-10-06 Thread David Aguilar
Teach mergetool to pass "-O" down to `git diff` when
specified on the command-line.

Signed-off-by: David Aguilar 
---
This is a replacement patch for 4/4 from the original series.

The changes are stylistic -- the "order_file" variable name and
"-O" in the usage were changed to "orderfile" and
"-O" for consistency with the documentation.

 Documentation/git-mergetool.txt | 10 ++
 git-mergetool.sh| 14 --
 t/t7610-mergetool.sh| 27 +++
 3 files changed, 45 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..1a0fe7a 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
@@ -459,8 +463,14 @@ main () {
fi
fi
 
+   if test -n "$orderfile"
+   then
+   set -- "$orderfile" -- "$@"
+   else
+   set -- -- "$@"
+   fi
files=$(git -c core.quotePath=false \
-   diff --name-only --diff-filter=U -- "$@")
+   diff --name-only --diff-filter=U "$@")
 
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 &&
+   test_cmp expect actual &&
+   git reset --hard >/dev/null 2>&1
 '
 
 test_done
-- 
2.10.1.386.g42aabb0



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

2016-10-06 Thread 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. :)

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 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.

> --- 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.

-Peff


[PATCH] documentation: clarify submodule..[url, path] variables

2016-10-06 Thread Stefan Beller
Signed-off-by: Stefan Beller 
---

This was raised in the discussion of 
https://public-inbox.org/git/20161006193725.31553-1-sbel...@google.com/raw

However this can go as a separate patch instead of adding it to the series.

Thanks,
Stefan

 Documentation/config.txt | 15 +++
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index e78293b..1f68d05 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -2812,11 +2812,18 @@ stash.showStat::
See description of 'show' command in linkgit:git-stash[1].
 
 submodule..path::
+   The path within this project for a submodule. This variable is
+   kept in the .gitmodules file. See linkgit:git-submodule[1] and
+   linkgit:gitmodules[5] for details.
+
 submodule..url::
-   The path within this project and URL for a submodule. These
-   variables are initially populated by 'git submodule init'. See
-   linkgit:git-submodule[1] and linkgit:gitmodules[5] for
-   details.
+   The URL for a submodule. This variable is copied from the .gitmodules
+   file to the git config via 'git submodule init'. The user can change
+   the configured URL before obtaining the submodule via 'git submodule
+   update'. After obtaining the submodule, this variable is kept in the
+   config as a boolean flag to indicate whether the submodule is of
+   interest to git commands.  See linkgit:git-submodule[1] and
+   linkgit:gitmodules[5] for details.
 
 submodule..update::
The default update procedure for a submodule. This variable
-- 
2.10.1.353.g1629400



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

2016-10-06 Thread 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 
---
 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..fb2aeef 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 | grep -A 2 Merging: >actual &&
+   test_cmp expect actual &&
+   git reset --hard >/dev/null
+'
+'
+
 test_done
-- 
2.10.1.355.g33afd83.dirty



[PATCH 4/4] mergetool: honor -O

2016-10-06 Thread David Aguilar
Teach mergetool to pass "-O" down to `git diff` when
specified on the command-line.

Signed-off-by: David Aguilar 
---
 Documentation/git-mergetool.txt | 10 ++
 git-mergetool.sh| 14 --
 t/t7610-mergetool.sh| 27 +++
 3 files changed, 45 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..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 "$@")
 
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 &&
+   test_cmp expect actual &&
+   git reset --hard >/dev/null 2>&1
 '
 
 test_done
-- 
2.10.1.355.g33afd83.dirty



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

2016-10-06 Thread David Aguilar
Signed-off-by: David Aguilar 
---
 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 |
+   sed -e 's/^[^   ]*  //' | sort -u)
+   fi
else
-   files=$(git ls-files -u | sed -e 's/^[^ ]*  //' | sort -u)
+   files=$(git ls-files -u -- "$@" |
+   sed -e 's/^[^   ]*  //' | sort -u)
fi
-else
-   files=$(git ls-files -u -- "$@" | sed -e 's/^[^ ]*  //' | sort -u)
-fi
-
-if test -z "$files"
-then
-   echo "No files need merging"
-   exit 0
-fi
-
-printf "Merging:\n"
-printf 

[PATCH 1/4] mergetool: add copyright

2016-10-06 Thread David Aguilar
Signed-off-by: David Aguilar 
---
 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.355.g33afd83.dirty



[PATCHv5] push: change submodule default to check when submodules exist

2016-10-06 Thread Stefan Beller
When working with submodules, it is easy to forget to push the submodules.
The setting 'check', which checks if any existing submodule is present on
at least one remote of the submodule remotes, is designed to prevent this
mistake.

Flipping the default to check for submodules is safer than the current
default of ignoring submodules while pushing.

However checking for submodules requires additional work[1], which annoys
users that do not use submodules, so we turn on the check for submodules
based on a cheap heuristic, the existence of
* any initialized submodules via checking submodule..url.
* the .git/modules directory. That directory doesn't exist when no
  submodules are used and is only created and populated when submodules
  are cloned/added.
* the existence of the .gitmodules file.

When the submodule directory doesn't exist, a user may have changed the
gitlinks via plumbing commands. Currently the default is to not check.
RECURSE_SUBMODULES_DEFAULT is effectively RECURSE_SUBMODULES_OFF currently,
though it may change in the future. When no submodules exist such a check
is pointless as it would fail anyway, so let's just turn it off.

[1] 
https://public-inbox.org/git/CA+55aFyos78qODyw57V=w13ux5-8svbqobjfaq22k+xkpwv...@mail.gmail.com/

Signed-off-by: Stefan Beller 
---

 * Reworded the commit message
 * added comment on why we only check submodule..url

 The first patch of the series is still good, so please use
 https://public-inbox.org/git/20161006193725.31553-2-sbel...@google.com/
 first and then build this on top.
 
 Thanks,
 Stefan 

 builtin/push.c | 16 +++-
 t/t5531-deep-submodule-push.sh |  6 +-
 2 files changed, 20 insertions(+), 2 deletions(-)

diff --git a/builtin/push.c b/builtin/push.c
index 3bb9d6b..9e0b8db 100644
--- a/builtin/push.c
+++ b/builtin/push.c
@@ -3,6 +3,7 @@
  */
 #include "cache.h"
 #include "refs.h"
+#include "dir.h"
 #include "run-command.h"
 #include "builtin.h"
 #include "remote.h"
@@ -22,6 +23,7 @@ static int deleterefs;
 static const char *receivepack;
 static int verbosity;
 static int progress = -1;
+static int has_submodules_configured;
 static int recurse_submodules = RECURSE_SUBMODULES_DEFAULT;
 static enum transport_family family;
 
@@ -31,6 +33,15 @@ static const char **refspec;
 static int refspec_nr;
 static int refspec_alloc;
 
+static void preset_submodule_default(void)
+{
+   if (has_submodules_configured || file_exists(git_path("modules")) ||
+   (!is_bare_repository() && file_exists(".gitmodules")))
+   recurse_submodules = RECURSE_SUBMODULES_CHECK;
+   else
+   recurse_submodules = RECURSE_SUBMODULES_OFF;
+}
+
 static void add_refspec(const char *ref)
 {
refspec_nr++;
@@ -495,7 +506,9 @@ static int git_push_config(const char *k, const char *v, 
void *cb)
const char *value;
if (!git_config_get_value("push.recursesubmodules", ))
recurse_submodules = 
parse_push_recurse_submodules_arg(k, value);
-   }
+   } else if (starts_with(k, "submodule.") && ends_with(k, ".url"))
+   /* The submodule..url is used as a bit to indicate 
existence */
+   has_submodules_configured = 1;
 
return git_default_config(k, v, NULL);
 }
@@ -552,6 +565,7 @@ int cmd_push(int argc, const char **argv, const char 
*prefix)
};
 
packet_trace_identity("push");
+   preset_submodule_default();
git_config(git_push_config, );
argc = parse_options(argc, argv, prefix, options, push_usage, 0);
set_push_cert_flags(, push_cert);
diff --git a/t/t5531-deep-submodule-push.sh b/t/t5531-deep-submodule-push.sh
index 198ce84..e690749 100755
--- a/t/t5531-deep-submodule-push.sh
+++ b/t/t5531-deep-submodule-push.sh
@@ -65,7 +65,11 @@ test_expect_success 'push fails if submodule commit not on 
remote' '
git add gar/bage &&
git commit -m "Third commit for gar/bage" &&
# the push should fail with --recurse-submodules=check
-   # on the command line...
+   # on the command line. "check" is the default for repos in
+   # which submodules are detected by existence of config,
+   # .gitmodules file or an internal .git/modules/
+   git submodule add -f ../submodule.git gar/bage &&
+   test_must_fail git push ../pub.git master &&
test_must_fail git push --recurse-submodules=check ../pub.git 
master &&
 
# ...or if specified in the configuration..
-- 
2.10.1.353.g1629400



Re: [PATCH v2 05/25] sequencer: allow the sequencer to take custody of malloc()ed data

2016-10-06 Thread Junio C Hamano
Jakub Narębski  writes:

>> We manage lifetime of a field in a structure in one of three ways in
>> our codebase [*1*].
>> 
>>  ...
>>  * A field can sometimes own and sometimes borrow the memory, and it
>>is accompanied by another field to tell which case it is, so that
>>cleaning-up can tell when it needs to be free(3)d.  This is a
>>minority case, and we generally avoid it especially in modern
>>code for small allocation, as it makes the lifetime rule more
>>complex than it is worth.
> ...
> On the other hand the _entrust() mechanism might be a good solution
> if the amount of memory was large, for example order of magnitude more
> than what would be needed to keep ownership info *and* borrowing would
> not be possible for some reason.

We have approach #3 exactly for that usage pattern.


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

2016-10-06 Thread Ramsay Jones


On 06/10/16 20:18, Ævar Arnfjörð Bjarmason wrote:
> On Tue, Oct 4, 2016 at 6:08 PM, Johannes Schindelin
>  wrote:
>> As to making NO_REGEX conditional on REG_STARTEND: you are talking about
>> apples and oranges here. NO_REGEX is a Makefile flag, while REG_STARTEND
>> is a C preprocessor macro.
>>
>> Unless you can convince the rest of the Git developers (you would not
>> convince me) to simulate autoconf by compiling an executable every time
>> `make` is run, to determine whether REG_STARTEND is defined, this is a
>> no-go.
> 
> 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.]

ATB,
Ramsay Jones




Re: [PATCH v2 05/25] sequencer: allow the sequencer to take custody of malloc()ed data

2016-10-06 Thread Jakub Narębski
W dniu 06.10.2016 o 21:23, Junio C Hamano pisze:
> Johannes Schindelin  writes:
> 
>> If you prefer to accept such sloppy work, I will change it of course,
>> feeling dirty that it has my name on it.
> 
> I do want neither leaks nor sloppyness, and I thought that was
> understood by everybody, hence I thought the last round made it
> clear that _entrust() must not exist.

Also, the *_entrust() mechanism in v1 was quite sequencer-specific,
rather than be a generic mechanism.

> 
> Let's try again.
> 
> We manage lifetime of a field in a structure in one of three ways in
> our codebase [*1*].
> 
>  * A field can always point at a borrowed piece of memory that the
>destruction of the containing structure or re-assignment to the
>field do not have to free the current value of the field.  This
>is a minority, simply because it is rare for a field to be always
>able to borrow from others.  The destruction of the containing
>structure or re-assignment to the field needs to do nothing
>special.
> 
>  * A field can own the piece of memory that it points at.  The
>destruction of the containing structure or re-assignment to the
>field needs to free the current value of the field [*2*].  A
>field that can be assigned a value from the configuration (which
>is typically xstrdup()'ed) is an example of such a field, and if
>a command line option also can assign to the field, we'd need to
>xstrdup() it, even though we know we could borrow from argv[],
>because cleaning-up needs to be able to free(3) it.
> 
>  * A field can sometimes own and sometimes borrow the memory, and it
>is accompanied by another field to tell which case it is, so that
>cleaning-up can tell when it needs to be free(3)d.  This is a
>minority case, and we generally avoid it especially in modern
>code for small allocation, as it makes the lifetime rule more
>complex than it is worth.

Great writeup!

> The _entrust() thing may have been an excellent fourth option if it
> were cost-free.  xstrdup() that the second approach necessitates for
> literal strings (like part of argv[]) would become unnecessary and
> we can treat as if all the fields in a structure were all borrowing
> the memory from elsewhere (in fact, _entrust() creates the missing
> owners of pieces of memory that need to be freed later); essentially
> it turns a "mixed ownership" case into the first approach.
> 
> But it is not cost-free.  The mechanism needs to allocate memory to
> become the missing owner and keep track of which pieces of memory
> need to be freed later, and the resulting code does not become any
> easier to follow.  The programmer still needs to know which ones to
> _entrust() just like the programmer in the model #2 needs to make
> sure xstrdup() is done for appropriate pieces of memory--the only
> difference is that an _entrust() programmer needs to mark the pieces
> of memory to be freed, while a #2 programmer needs to xstrdup() the
> pieces of memory that are being "borrowed".
> 
> So let's not add the fourth way to our codebase.  "mixed ownership"
> case should be turned into "field owns the memory", i.e. approach #2.

On the other hand the _entrust() mechanism might be a good solution
if the amount of memory was large, for example order of magnitude more
than what would be needed to keep ownership info *and* borrowing would
not be possible for some reason.

But the _entrust() mechanism reminds me on one hand side of memory
debuggers like dmalloc, Electric Fence (eFence), or valgrind; combined
a bit with garbage collection.  Namely, when we are in a library call,
record all allocations (perhaps by redirecting alloc functions), then
free those resources at the exit of library call.
 
> 
> [Footnotes]
> 
> *1* It is normal for different fields in the same structure follow
> different lifetime rules.
> 
> *2* Unless leaks are allowed, that is.  I think we have instances
> where "git cmd --opt=A --opt=B" allocates and stores a new piece
> of memory that is computed based on A and store it to a field,
> and then overwrites the field with another allocation of a value
> based on B without freeing old value, saying "the caller can
> avoid passing the same thing twice, and such a leak is miniscule
> anyway".  That is not nice, and we've been reducing them over
> time.
> 



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

2016-10-06 Thread Junio C Hamano
Here are the topics that have been cooking.  Commits prefixed with
'-' are only in 'pu' (proposed updates) while commits prefixed with
'+' are in 'next'.  The ones marked with '.' do not appear in any of
the integration branches, but I am still holding onto them.

A handful of topics have been merged to 'master' and it's time to
update the "What's cooking" report.  Linus's and Peff's auto scaling
of default abbreviation length will cook a bit longer in 'next' to
see if anybody screams; there may still be codepaths that assume
that default-abbrev would never be negative and a reasonable
fallback value that we need to locate and fix.

You can find the changes described here in the integration branches
of the repositories listed at

http://git-blame.blogspot.com/p/git-public-repositories.html

--
[Graduated to "master"]

* jc/blame-abbrev (2016-09-28) 1 commit
  (merged to 'next' on 2016-10-03 at 8ec86ff1e1)
 + blame: use DEFAULT_ABBREV macro

 Almost everybody uses DEFAULT_ABBREV to refer to the default
 setting for the abbreviation, but "git blame" peeked into
 underlying variable bypassing the macro for no good reason.


* jk/ambiguous-short-object-names (2016-09-27) 11 commits
  (merged to 'next' on 2016-09-28 at 1b85295323)
 + get_short_sha1: make default disambiguation configurable
 + get_short_sha1: list ambiguous objects on error
 + for_each_abbrev: drop duplicate objects
 + sha1_array: let callbacks interrupt iteration
 + get_short_sha1: mark ambiguity error for translation
 + get_short_sha1: NUL-terminate hex prefix
 + get_short_sha1: refactor init of disambiguation code
 + get_short_sha1: parse tags when looking for treeish
 + get_sha1: propagate flags to child functions
 + get_sha1: avoid repeating ourselves via ONLY_TO_DIE
 + get_sha1: detect buggy calls with multiple disambiguators
 (this branch is used by jk/abbrev-auto and lt/abbrev-auto.)

 When given an abbreviated object name that is not (or more
 realistically, "no longer") unique, we gave a fatal error
 "ambiguous argument".  This error is now accompanied by hints that
 lists the objects that begins with the given prefix.  During the
 course of development of this new feature, numerous minor bugs were
 uncovered and corrected, the most notable one of which is that we
 gave "short SHA1  is ambiguous." twice without good reason.


* jk/graph-padding-fix (2016-09-29) 1 commit
  (merged to 'next' on 2016-10-03 at 3f526e0f38)
 + graph: fix extra spaces in graph_padding_line

 The "graph" API used in "git log --graph" miscounted the number of
 output columns consumed so far when drawing a padding line, which
 has been fixed; this did not affect any existing code as nobody
 tried to write anything after the padding on such a line, though.


* ps/http-gssapi-cred-delegation (2016-09-29) 1 commit
  (merged to 'next' on 2016-10-03 at 310fbe8f24)
 + http: control GSSAPI credential delegation

 In recent versions of cURL, GSSAPI credential delegation is
 disabled by default due to CVE-2011-2192; introduce a configuration
 to selectively allow enabling this.


* rs/c-auto-resets-attributes (2016-09-29) 1 commit
  (merged to 'next' on 2016-10-03 at 6a0b946a79)
 + pretty: avoid adding reset for %C(auto) if output is empty

 When "%C(auto)" appears at the very beginning of the pretty format
 string, it did not need to issue the reset sequence, but it did.
 This is a small optimization to already graduated topic.


* rs/cocci (2016-10-03) 4 commits
  (merged to 'next' on 2016-10-03 at 758cc6de9c)
 + coccicheck: make transformation for strbuf_addf(sb, "...") more precise
  (merged to 'next' on 2016-09-28 at 26462645f9)
 + use strbuf_add_unique_abbrev() for adding short hashes, part 2
 + use strbuf_addstr() instead of strbuf_addf() with "%s", part 2
 + gitignore: ignore output files of coccicheck make target

 Code clean-up with help from coccinelle tool continues.


* sg/ref-filter-parse-optim (2016-10-03) 1 commit
  (merged to 'next' on 2016-10-03 at 9af6bb63e9)
 + ref-filter: strip format option after a field name only once while parsing

 The code that parses the format parameter of for-each-ref command
 has seen a micro-optimization.


* vn/revision-shorthand-for-side-branch-log (2016-09-27) 1 commit
  (merged to 'next' on 2016-09-28 at c1237b24f6)
 + revision: new rev^-n shorthand for rev^n..rev

 "git log rev^..rev" is an often-used revision range specification
 to show what was done on a side branch merged at rev.  This has
 gained a short-hand "rev^-1".  In general "rev^-$n" is the same as
 "^rev^$n rev", i.e. what has happened on other branches while the
 history leading to nth parent was looking the other way.

--
[New Topics]

* jc/ws-error-highlight (2016-10-04) 4 commits
 - diff: introduce diff.wsErrorHighlight option
 - diff.c: move ws-error-highlight parsing helpers up
 - diff.c: refactor parse_ws_error_highlight()
 - t4015: split out the 

Re: [PATCH v2] gpg-interface: use more status letters

2016-10-06 Thread Junio C Hamano
Junio C Hamano  writes:

> Michael J Gruber  writes:
>
>> Also, I'm open to using another letter for EXPKEYSIG but couldn't decide
>> between 'Y', 'Z', 'K'. 'K' could be confused with REVKEYSIG, I'm afraid.
>> 'Y' is next to 'X' and contained in 'KEY', it would be my first choice.
>
> Sounds good enough to me.  Thanks.

I really do not want to leave too many topics listed in the "What's
cooking" report to be in undecided / waiting state.  How about
squashing this in, with a fully updated log message to replace?

-- >8 --
From: Michael J Gruber 
Date: Wed, 28 Sep 2016 16:24:13 +0200
Subject: [PATCH] SQUASH: gpg-interface: use more status letters

According to gpg2's doc/DETAILS:

For each signature only one of the codes GOODSIG, BADSIG,
EXPSIG, EXPKEYSIG, REVKEYSIG or ERRSIG will be emitted.

gpg1 ("classic") behaves the same (although doc/DETAILS differs).

Currently, we parse gpg's status output for GOODSIG, BADSIG and
trust information and translate that into status codes G, B, U, N
for the %G?  format specifier.

git-verify-* returns success in the GOODSIG case only. This is
somewhat in disagreement with gpg, which considers the first 5 of
the 6 above as VALIDSIG, but we err on the very safe side.

Introduce additional status codes E, X, Y, R for ERRSIG, EXPSIG,
EXPKEYSIG, and REVKEYSIG so that a user of %G? gets more information
about the absence of a 'G' on first glance.

Requested-by: Alex 
Signed-off-by: Michael J Gruber 
Signed-off-by: Junio C Hamano 
---
 Documentation/pretty-formats.txt | 3 ++-
 gpg-interface.c  | 2 +-
 pretty.c | 1 +
 3 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/Documentation/pretty-formats.txt b/Documentation/pretty-formats.txt
index c28ff2b919..179c9389aa 100644
--- a/Documentation/pretty-formats.txt
+++ b/Documentation/pretty-formats.txt
@@ -146,7 +146,8 @@ endif::git-rev-list[]
 - '%G?': show "G" for a good (valid) signature,
   "B" for a bad signature,
   "U" for a good signature with unknown validity,
-  "X" for a good expired signature, or good signature made by an expired key,
+  "X" for a good signature that has expired,
+  "Y" for a good signature made by an expired key,
   "R" for a good signature made by a revoked key,
   "E" if the signature cannot be checked (e.g. missing key)
   and "N" for no signature
diff --git a/gpg-interface.c b/gpg-interface.c
index 6999e7b469..e44cc27da1 100644
--- a/gpg-interface.c
+++ b/gpg-interface.c
@@ -35,7 +35,7 @@ static struct {
{ 'U', "\n[GNUPG:] TRUST_UNDEFINED" },
{ 'E', "\n[GNUPG:] ERRSIG "},
{ 'X', "\n[GNUPG:] EXPSIG "},
-   { 'X', "\n[GNUPG:] EXPKEYSIG "},
+   { 'Y', "\n[GNUPG:] EXPKEYSIG "},
{ 'R', "\n[GNUPG:] REVKEYSIG "},
 };
 
diff --git a/pretty.c b/pretty.c
index 39a36cd825..f98b271069 100644
--- a/pretty.c
+++ b/pretty.c
@@ -1236,6 +1236,7 @@ static size_t format_commit_one(struct strbuf *sb, /* in 
UTF-8 */
case 'U':
case 'N':
case 'X':
+   case 'Y':
case 'R':
strbuf_addch(sb, c->signature_check.result);
}
-- 
2.10.1-520-ge127bfd383



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

2016-10-06 Thread Stefan Beller
On Thu, Oct 6, 2016 at 1:25 PM, Junio C Hamano  wrote:
> Stefan Beller  writes:
>
>> When working with submodules, it is easy to forget to push the submodules.
>> The setting 'check', which checks if any existing submodule is present on
>> at least one remote of the submodule remotes, is designed to prevent this
>> mistake.
>>
>> Flipping the default to check for submodules is safer than the current
>> default of ignoring submodules while pushing.
>>
>> However checking for submodules requires additional work[1], which annoys
>> users that do not use submodules, so we turn on the check for submodules
>> based on a cheap heuristic, the existence of the .git/modules directory.
>
> ... and other things like .gitmodules, submodule stuff in
> .git/config, etc.?

Oh right, I need to update this commit message to mention this, too

>
>> + } else if (starts_with(k, "submodule.") && ends_with(k, ".url"))
>> + has_submodules_configured = 1;
>
> An in-code comment to explain why ".url" is so special would be
> helpful.  Documentation/config.txt says .path and .url are both

   submodule..path, submodule..url
   The path within this project and URL for a submodule. These
   variables are initially populated by git submodule init. See git-
   submodule(1) and gitmodules(5) for details.

The correct version is:

submodule..path::
This is a config variable only found in .gitmodules files that
indicate at which
path relative to the repository root the submodule is found.
It is used to resolve the mapping (submodule name)<->path throughout all
time, i.e. also later when a submodule is initialized and checked out, any
subsequent operation that needs a submodule name/repsoitory uses it for
lookup.

submodule..url::
This is a config variable found both in .gitmodules files as well
as .git/config.
In the .gitmodules files it serves as a hint where to obtain a
repository from
to populate the gitlinks within the repository.
On submodule-init the value will be copied over to the .git/config
file and there
it serves as a holder of the value only until the first
submodule-update is run,
as the initial submodule-update is using that url to clone the submodule.
From then on it is only used as a boolean flag in the .git/config
to indicate
whether the submodule is considered interesting for further submodule
commands. (See the similar competing thing with submodule..ignore)

Given that it is obvious to my current me, that .url is the only
sensible thing as .path
is not found in the .git/config which we are searching in here.
So for the purpose of this patch I'd just add a line like this to
remind my future self:

/* See if any submodule is considered interesting: */

I would like to avoid references to .path as that is not helping here.
(Why does that
comment point to a variable name that is not supposed to exist in the file?)
So I think we really need to think how to reduce confusion in the
future by readers that
have more or less overview of the submodules concept.

A future version of the man page may read:

submodule..path::
, we may want to consider if we copy it over to
.git/config, so the
repository may trash the .gitmodules file and the submodules keep working.

submodule..url::
indication in .gitmodules where the submodule is cloned from. It
will be copied over
to .git/config on submodule-init. The user may adapt the
configured urls there before
proceeding to submodule-update, which will delete the url setting
again, as the
cloned submodule repository will hold the authoritative setting
where the remote
of the submodule is.
So it only exists in .git/config for submodules that are
initialized but have never
been cloned. Even when the submodule is deinit'd and reinited we
don't even need
to copy the url, as we still have the old submodule repository in
.git/module/

submodule..exists::
This setting is per working tree (unlike the previous 2) that
indicates if submodule
related commands pay attention to the given submodule. It is still
unclear how this
works together with the .ignore setting.


Note for this future to come true, we'd have to have only 2 big series
One that Duy started to introduce per working tree configs, see
https://public-inbox.org/git/20160720172419.25473-1-pclo...@gmail.com/
The other one is a series that hasn't seen the mailing list yet, that
I started to
separate the 2 modes of the url both as a value holder (to point at a URL) as
well as just a boolean flag for commands to acknowledge the existence of the
submodule.


> initially populated by 'git submodule init', which might be outdated
> information that confuses readers of the above code.

A lot of text but I guess we can use some ideas from here to update the
.path and .url documentation. (The wording in this email is not of man
page quality).


Re: [PATCH v8 11/11] convert: add filter..process option

2016-10-06 Thread Lars Schneider

> On 04 Oct 2016, at 23:00, Jakub Narębski  wrote:
> 
> [Some of answers and comments may got invalidated by v9]
> 
> W dniu 30.09.2016 o 21:38, Lars Schneider pisze:
>>> On 27 Sep 2016, at 17:37, Jakub Narębski  wrote:
>>> 
>>> Part second of the review of 11/11.
> [...]
 +
 +  if (start_command(process)) {
 +  error("cannot fork to run external filter '%s'", cmd);
 +  kill_multi_file_filter(hashmap, entry);
 +  return NULL;
 +  }
>>> 
>>> I guess there is a reason why we init hashmap entry, try to start
>>> external process, then kill entry of unable to start, instead of
>>> trying to start external process, and adding hashmap entry when
>>> we succeed?
>> 
>> Yes. This way I can reuse the kill_multi_file_filter() function.
> 
> I don't quite understand.  If you didn't fill the entry before
> using start_command(process), you would not need kill_multi_file_filter(),
> which in that case IIUC just removes the just created entry from hashmap.
> Couldn't you add entry to hashmap in the 'else' part?  Or would it
> be racy?

You are right. I'll fix that.


>> 
 +  if (pair[0] && pair[0]->len && pair[1]) {
 +  if (!strcmp(pair[0]->buf, "status=")) {
 +  strbuf_reset(status);
 +  strbuf_addbuf(status, pair[1]);
 +  }
>>> 
>>> So it is last status= line wins behavior?
>> 
>> Correct.
> 
> Perhaps this should be described in code comment.

OK


 
 +  fflush(NULL);
>>> 
>>> Why this fflush(NULL) is needed here?
>> 
>> This flushes all open output streams. The single filter does the same.
> 
> I know what it does, but I don't know why.  But "single filter does it"
> is good enough for me.  Still would want to know why, though ;-)

TBH I am not 100% sure why, too. I think this ensures that we don't have 
any outdated/unrelated/previous data in the stream buffers.


 
 +  if (fd >= 0 && !src) {
 +  if (fstat(fd, _stat) == -1)
 +  return 0;
 +  len = xsize_t(file_stat.st_size);
 +  }
>>> 
>>> Errr... is it necessary?  The protocol no longer provides size=
>>> hint, and neither uses such hint if provided.
>> 
>> We require the size in write_packetized_from_buf() later.
> 
> Don't we use write_packetized_from_fd() in the case of fd >= 0?

Of course! Ah too many refactorings :-)
I'll remove that.

Thank you,
Lars



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

2016-10-06 Thread Junio C Hamano
Stefan Beller  writes:

> When working with submodules, it is easy to forget to push the submodules.
> The setting 'check', which checks if any existing submodule is present on
> at least one remote of the submodule remotes, is designed to prevent this
> mistake.
>
> Flipping the default to check for submodules is safer than the current
> default of ignoring submodules while pushing.
>
> However checking for submodules requires additional work[1], which annoys
> users that do not use submodules, so we turn on the check for submodules
> based on a cheap heuristic, the existence of the .git/modules directory.

... and other things like .gitmodules, submodule stuff in
.git/config, etc.?

> + } else if (starts_with(k, "submodule.") && ends_with(k, ".url"))
> + has_submodules_configured = 1;

An in-code comment to explain why ".url" is so special would be
helpful.  Documentation/config.txt says .path and .url are both
initially populated by 'git submodule init', which might be outdated
information that confuses readers of the above code.

> @@ -552,6 +564,7 @@ int cmd_push(int argc, const char **argv, const char 
> *prefix)
>   };
>  
>   packet_trace_identity("push");
> + preset_submodule_default();
>   git_config(git_push_config, );
>   argc = parse_options(argc, argv, prefix, options, push_usage, 0);
>   set_push_cert_flags(, push_cert);
> diff --git a/t/t5531-deep-submodule-push.sh b/t/t5531-deep-submodule-push.sh
> index 198ce84..e690749 100755
> --- a/t/t5531-deep-submodule-push.sh
> +++ b/t/t5531-deep-submodule-push.sh
> @@ -65,7 +65,11 @@ test_expect_success 'push fails if submodule commit not on 
> remote' '
>   git add gar/bage &&
>   git commit -m "Third commit for gar/bage" &&
>   # the push should fail with --recurse-submodules=check
> - # on the command line...
> + # on the command line. "check" is the default for repos in
> + # which submodules are detected by existence of config,
> + # .gitmodules file or an internal .git/modules/
> + git submodule add -f ../submodule.git gar/bage &&
> + test_must_fail git push ../pub.git master &&
>   test_must_fail git push --recurse-submodules=check ../pub.git 
> master &&
>  
>   # ...or if specified in the configuration..


[BUG?] git log with --follow and --skip

2016-10-06 Thread Christian Couder
Hi,

At GitLab, we are annoyed by the fact that `git log` doesn't seem to
work well when using it with both --follow and --skip.

For example:

> git log --oneline -6 README.md
a299e3a README.md: format CLI commands with code syntax
c9a014e README.md: don't take 'commandname' literally
a217f07 README.md: move down historical explanation about the name
28513c4 README.md: don't call git stupid in the title
d9b297d README.md: move the link to git-scm.com up
6164972 README.md: add hyperlinks on filenames

> git log --skip 2 --oneline -6 README.md
a217f07 README.md: move down historical explanation about the name
28513c4 README.md: don't call git stupid in the title
d9b297d README.md: move the link to git-scm.com up
6164972 README.md: add hyperlinks on filenames
4ad21f5 README: use markdown syntax

The above 2 commands seem to work well, but:

> git log --follow --skip 2 --oneline -6 README.md
a299e3a README.md: format CLI commands with code syntax
c9a014e README.md: don't take 'commandname' literally
a217f07 README.md: move down historical explanation about the name
28513c4 README.md: don't call git stupid in the title
d9b297d README.md: move the link to git-scm.com up
6164972 README.md: add hyperlinks on filenames

So with --follow it looks like the top 2 commits are not skipped any more.

Thanks,
Christian.


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

2016-10-06 Thread Junio C Hamano
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.
>
> 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.



Re: [PATCH v2 3/3] gitweb: Link to "git describe"'d commits in log messages

2016-10-06 Thread Junio C Hamano
Ævar Arnfjörð Bjarmason   writes:

> Change the log formatting function to know about "git describe" output
> such as "v2.8.0-4-g867ad08", in addition to just plain "867ad08".
>
> There are still many valid refnames that we don't link to
> e.g. v2.10.0-rc1~2^2~1 is also a valid way to refer to
> v2.8.0-4-g867ad08, but I'm not supporting that with this commit,
> similarly it's trivially possible to create some refnames like
> "æ/var-gf6727b0" or which won't be picked up by this regex.

Not a serious counter-proposal or suggestion, and certainly not an
objection to the patch I am responding to, but I wonder if it is so
bad if we made the 867ad08 into link when showing v2.8.0-4-g867ad08.

IOW, in addition to \b followed by [0-9a-f]+ followed by \b, if we
allowed an optional 'g' in front of the hex, e.g.

-   $line =~ s{\b([0-9a-fA-F]{7,40})\b}{
+   $line =~ s{\bg?([0-9a-fA-F]{7,40})\b}{

wouldn't that be much simpler, covers more cases and sufficient?

> There's surely room for improvement here, but I just wanted to address
> the very common case of sticking "git describe" output into commit
> messages without trying to link to all possible refnames, that's going
> to be a rather futile exercise given that this is free text, and it
> would be prohibitively expensive to look up whether the references in
> question exist in our repository.
>
> There was on-list discussion about how we could do better than this
> patch. Junio suggested to update parse_commits() to call a new
> "gitweb--helper" command which would pass each of the revision
> candidates through "rev-parse --verify --quiet". That would cut down
> on our false positives (e.g. we'll link to "deadbeef"), and also allow
> us to be more aggressive in selecting candidate revisions.
>
> That may be too expensive to work in practice, or it may
> not. Investigating that would be a good follow-up to this patch.
>
> Signed-off-by: Ævar Arnfjörð Bjarmason 
> ---
>  gitweb/gitweb.perl | 18 --
>  1 file changed, 16 insertions(+), 2 deletions(-)
>
> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
> index 92b5e91..7cf68f0 100755
> --- a/gitweb/gitweb.perl
> +++ b/gitweb/gitweb.perl
> @@ -2036,10 +2036,24 @@ sub format_log_line_html {
>   my $line = shift;
>  
>   $line = esc_html($line, -nbsp=>1);
> - $line =~ s{\b([0-9a-fA-F]{7,40})\b}{
> + $line =~ s{
> +\b
> +(
> +# The output of "git describe", e.g. v2.10.0-297-gf6727b0
> +# or hadoop-20160921-113441-20-g094fb7d
> +(? +[A-Za-z0-9.-]+
> +(?!\.) # refs can't end with ".", see check_refname_format()
> +-g[0-9a-fA-F]{7,40}
> +|
> +# Just a normal looking Git SHA1
> +[0-9a-fA-F]{7,40}
> +)
> +\b
> +}{
>   $cgi->a({-href => href(action=>"object", hash=>$1),
>   -class => "text"}, $1);
> - }eg;
> + }egx;
>  
>   return $line;
>  }


Re: [PATCH v2 2/3] gitweb: Link to 7-char+ SHA1s, not only 8-char+

2016-10-06 Thread Junio C Hamano
Ævar Arnfjörð Bjarmason   writes:

> Change the minimum length of an abbreviated object identifier in the
> commit message gitweb tries to turn into link from 8 hexchars to 7.
>
> This arbitrary minimum length of 8 was introduced in bfe2191 ("gitweb:
> SHA-1 in commit log message links to "object" view", 2006-12-10), but
> the default abbreviation length is 7, and has been for a long time.
>
> It's still possible to reference SHA1s down to 4 characters in length,
> see v1.7.4-1-gdce9648's MINIMUM_ABBREV, but I can't see how to make
> git actually produce that, so I doubt anyone is putting that into log
> messages in practice, but people definitely do put 7 character SHA1s
> into log messages.
>
> I think it's fairly dubious to link to things matching [0-9a-fA-F]
> here as opposed to just [0-9a-f], that dates back to the initial
> version of gitweb from 161332a ("first working version",
> 2005-08-07). Git will accept all-caps SHA1s, but didn't ever produce
> them as far as I can tell.
>
> Signed-off-by: Ævar Arnfjörð Bjarmason 
> ---

s/SHA1/SHA-1/g.  I agree that A-F are dubious.

>  gitweb/gitweb.perl | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
> index cba7405..92b5e91 100755
> --- a/gitweb/gitweb.perl
> +++ b/gitweb/gitweb.perl
> @@ -2036,7 +2036,7 @@ sub format_log_line_html {
>   my $line = shift;
>  
>   $line = esc_html($line, -nbsp=>1);
> - $line =~ s{\b([0-9a-fA-F]{8,40})\b}{
> + $line =~ s{\b([0-9a-fA-F]{7,40})\b}{
>   $cgi->a({-href => href(action=>"object", hash=>$1),
>   -class => "text"}, $1);
>   }eg;


[PATCH v2 1/2] files_read_raw_ref: avoid infinite loop on broken symlinks

2016-10-06 Thread Jeff King
On Thu, Oct 06, 2016 at 09:31:22PM +0200, Johannes Sixt wrote:

> Am 06.10.2016 um 18:48 schrieb Jeff King:
> > +test_expect_success SYMLINKS 'ref resolution not confused by broken 
> > symlinks' '
> > +   ln -s does-not-exist .git/broken &&
> > +   test_must_fail git rev-parse --verify broken
> 
> Hm, lower-case named refs directly in .git are frowned upon, no? If we ever
> decide to forbid them outright, this ref-parse might still fail, but for the
> wrong reason. Should you not better pick an example below ref/?

I suppose so. The test case was adapted from a real-world example, but
it fails equally well with .git/refs/heads/broken. The only restriction
is that the symlink _destination_ cannot look like "refs/heads/...".

Here's a replacement 1/2. The second patch remains unchanged.

-- >8 --
Subject: files_read_raw_ref: avoid infinite loop on broken symlinks

Our ref resolution first runs lstat() on any path we try to
look up, because we want to treat symlinks specially (by
resolving them manually and considering them symrefs). But
if the results of `readlink` do _not_ look like a ref, we
fall through to treating it like a normal file, and just
read the contents of the linked path.

Since fcb7c76 (resolve_ref_unsafe(): close race condition
reading loose refs, 2013-06-19), that "normal file" code
path will stat() the file and if we see ENOENT, will jump
back to the lstat(), thinking we've seen inconsistent
results between the two calls. But for a symbolic ref, this
isn't a race: the lstat() found the symlink, and the stat()
is looking at the path it points to. We end up in an
infinite loop calling lstat() and stat().

We can fix this by avoiding the retry-on-inconsistent jump
when we know that we found a symlink. While we're at it,
let's add a comment explaining why the symlink case gets to
this code in the first place; without that, it is not
obvious that the correct solution isn't to avoid the stat()
code path entirely.

Signed-off-by: Jeff King 
---
 refs/files-backend.c| 7 ++-
 t/t1503-rev-parse-verify.sh | 5 +
 2 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/refs/files-backend.c b/refs/files-backend.c
index 0709f60..d826557 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -1403,6 +1403,11 @@ static int files_read_raw_ref(struct ref_store 
*ref_store,
ret = 0;
goto out;
}
+   /*
+* It doesn't look like a refname; fall through to just
+* treating it like a non-symlink, and reading whatever it
+* points to.
+*/
}
 
/* Is it a directory? */
@@ -1426,7 +1431,7 @@ static int files_read_raw_ref(struct ref_store *ref_store,
 */
fd = open(path, O_RDONLY);
if (fd < 0) {
-   if (errno == ENOENT)
+   if (errno == ENOENT && !S_ISLNK(st.st_mode))
/* inconsistent with lstat; retry */
goto stat_ref;
else
diff --git a/t/t1503-rev-parse-verify.sh b/t/t1503-rev-parse-verify.sh
index ab27d0d..492edff 100755
--- a/t/t1503-rev-parse-verify.sh
+++ b/t/t1503-rev-parse-verify.sh
@@ -139,4 +139,9 @@ test_expect_success 'master@{n} for various n' '
test_must_fail git rev-parse --verify master@{$Np1}
 '
 
+test_expect_success SYMLINKS 'ref resolution not confused by broken symlinks' '
+   ln -s does-not-exist .git/refs/heads/broken &&
+   test_must_fail git rev-parse --verify broken
+'
+
 test_done
-- 
2.10.1.506.g904834d



[PATCHv4 2/2] push: change submodule default to check when submodules exist

2016-10-06 Thread Stefan Beller
When working with submodules, it is easy to forget to push the submodules.
The setting 'check', which checks if any existing submodule is present on
at least one remote of the submodule remotes, is designed to prevent this
mistake.

Flipping the default to check for submodules is safer than the current
default of ignoring submodules while pushing.

However checking for submodules requires additional work[1], which annoys
users that do not use submodules, so we turn on the check for submodules
based on a cheap heuristic, the existence of the .git/modules directory.
That directory doesn't exist when no submodules are used and is only
created and populated when submodules are cloned/added.

When the submodule directory doesn't exist, a user may have changed the
gitlinks via plumbing commands. Currently the default is to not check.
RECURSE_SUBMODULES_DEFAULT is effectively RECURSE_SUBMODULES_OFF currently,
though it may change in the future. When no submodules exist such a check
is pointless as it would fail anyway, so let's just turn it off.

[1] 
https://public-inbox.org/git/CA+55aFyos78qODyw57V=w13ux5-8svbqobjfaq22k+xkpwv...@mail.gmail.com/

Signed-off-by: Stefan Beller 
---
 builtin/push.c | 15 ++-
 t/t5531-deep-submodule-push.sh |  6 +-
 2 files changed, 19 insertions(+), 2 deletions(-)

diff --git a/builtin/push.c b/builtin/push.c
index 3bb9d6b..683f270 100644
--- a/builtin/push.c
+++ b/builtin/push.c
@@ -3,6 +3,7 @@
  */
 #include "cache.h"
 #include "refs.h"
+#include "dir.h"
 #include "run-command.h"
 #include "builtin.h"
 #include "remote.h"
@@ -22,6 +23,7 @@ static int deleterefs;
 static const char *receivepack;
 static int verbosity;
 static int progress = -1;
+static int has_submodules_configured;
 static int recurse_submodules = RECURSE_SUBMODULES_DEFAULT;
 static enum transport_family family;
 
@@ -31,6 +33,15 @@ static const char **refspec;
 static int refspec_nr;
 static int refspec_alloc;
 
+static void preset_submodule_default(void)
+{
+   if (has_submodules_configured || file_exists(git_path("modules")) ||
+   (!is_bare_repository() && file_exists(".gitmodules")))
+   recurse_submodules = RECURSE_SUBMODULES_CHECK;
+   else
+   recurse_submodules = RECURSE_SUBMODULES_OFF;
+}
+
 static void add_refspec(const char *ref)
 {
refspec_nr++;
@@ -495,7 +506,8 @@ static int git_push_config(const char *k, const char *v, 
void *cb)
const char *value;
if (!git_config_get_value("push.recursesubmodules", ))
recurse_submodules = 
parse_push_recurse_submodules_arg(k, value);
-   }
+   } else if (starts_with(k, "submodule.") && ends_with(k, ".url"))
+   has_submodules_configured = 1;
 
return git_default_config(k, v, NULL);
 }
@@ -552,6 +564,7 @@ int cmd_push(int argc, const char **argv, const char 
*prefix)
};
 
packet_trace_identity("push");
+   preset_submodule_default();
git_config(git_push_config, );
argc = parse_options(argc, argv, prefix, options, push_usage, 0);
set_push_cert_flags(, push_cert);
diff --git a/t/t5531-deep-submodule-push.sh b/t/t5531-deep-submodule-push.sh
index 198ce84..e690749 100755
--- a/t/t5531-deep-submodule-push.sh
+++ b/t/t5531-deep-submodule-push.sh
@@ -65,7 +65,11 @@ test_expect_success 'push fails if submodule commit not on 
remote' '
git add gar/bage &&
git commit -m "Third commit for gar/bage" &&
# the push should fail with --recurse-submodules=check
-   # on the command line...
+   # on the command line. "check" is the default for repos in
+   # which submodules are detected by existence of config,
+   # .gitmodules file or an internal .git/modules/
+   git submodule add -f ../submodule.git gar/bage &&
+   test_must_fail git push ../pub.git master &&
test_must_fail git push --recurse-submodules=check ../pub.git 
master &&
 
# ...or if specified in the configuration..
-- 
2.10.1.353.g1629400



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

2016-10-06 Thread Stefan Beller
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.

Signed-off-by: Stefan Beller 
---
 git-submodule.sh   | 10 --
 t/t7400-submodule-basic.sh | 14 ++
 2 files changed, 22 insertions(+), 2 deletions(-)

diff --git a/git-submodule.sh b/git-submodule.sh
index a024a13..3762616 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -207,8 +207,14 @@ cmd_add()
tstart
s|/*$||
')
-   git ls-files --error-unmatch "$sm_path" > /dev/null 2>&1 &&
-   die "$(eval_gettext "'\$sm_path' already exists in the index")"
+   if test -z "$force"
+   then
+   git ls-files --error-unmatch "$sm_path" > /dev/null 2>&1 &&
+   die "$(eval_gettext "'\$sm_path' already exists in the index")"
+   else
+   git ls-files -s "$sm_path" | sane_grep -v "^16" > /dev/null 
2>&1 &&
+   die "$(eval_gettext "'\$sm_path' already exists in the index 
and is not a submodule")"
+   fi
 
if test -z "$force" && ! git add --dry-run --ignore-missing "$sm_path" 
> /dev/null 2>&1
then
diff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh
index b77cce8..c09ce0d 100755
--- a/t/t7400-submodule-basic.sh
+++ b/t/t7400-submodule-basic.sh
@@ -152,6 +152,20 @@ test_expect_success 'submodule add to .gitignored path 
with --force' '
)
 '
 
+test_expect_success 'submodule add to reconfigure existing submodule with 
--force' '
+   (
+   cd addtest-ignore &&
+   git submodule add --force bogus-url submod &&
+   git submodule add -b initial "$submodurl" submod-branch &&
+   test "bogus-url" = "$(git config -f .gitmodules 
submodule.submod.url)" &&
+   test "bogus-url" = "$(git config submodule.submod.url)" &&
+   # Restore the url
+   git submodule add --force "$submodurl" submod
+   test "$submodurl" = "$(git config -f .gitmodules 
submodule.submod.url)" &&
+   test "$submodurl" = "$(git config submodule.submod.url)"
+   )
+'
+
 test_expect_success 'submodule add --branch' '
echo "refs/heads/initial" >expect-head &&
cat <<-\EOF >expect-heads &&
-- 
2.10.1.353.g1629400



[PATCH 0/2] submodule pushes be extra careful

2016-10-06 Thread Stefan Beller
This is a reroll of
http://public-inbox.org/git/20161004210359.15266-1-sbel...@google.com/
([PATCHv3 1/2] push: change submodule default to check when submodules exist)
but with a test.

As we only have a heuristic, the test failed initially as these tests don't
have any configuration at all nor do they have the submodule repos in the
superprojects git dir. So I was looking for a cheap way to add a config.

I could have written the config myself via git config, I think it is more
idiomatic to use a submodule command for that. However just getting the
configuration added is not possible with a submodule command, so I made one
in the first patch.

Thanks,
Stefan

Stefan Beller (2):
  submodule add: extend force flag to add existing repos
  push: change submodule default to check when submodules exist

 builtin/push.c | 15 ++-
 git-submodule.sh   | 10 --
 t/t5531-deep-submodule-push.sh |  6 +-
 t/t7400-submodule-basic.sh | 14 ++
 4 files changed, 41 insertions(+), 4 deletions(-)

-- 
2.10.1.353.g1629400



Re: [PATCH 1/2] files_read_raw_ref: avoid infinite loop on broken symlinks

2016-10-06 Thread Johannes Sixt

Am 06.10.2016 um 18:48 schrieb Jeff King:

+test_expect_success SYMLINKS 'ref resolution not confused by broken symlinks' '
+   ln -s does-not-exist .git/broken &&
+   test_must_fail git rev-parse --verify broken


Hm, lower-case named refs directly in .git are frowned upon, no? If we 
ever decide to forbid them outright, this ref-parse might still fail, 
but for the wrong reason. Should you not better pick an example below ref/?


-- Hannes



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

2016-10-06 Thread Jeff King
On Thu, Oct 06, 2016 at 03:25:00PM -0400, Rich Felker wrote:

> > No, I think that is the exact purpose of configure.ac and autoconf.
> > 
> > It would be neat if we could auto-fallback during the build. Rich
> > suggested always compiling compat/regex.c, and just having it be a noop
> > at the preprocessor level. I'm not sure if that would work, though,
> > because we'd have to include the system "regex.h" to know if we have
> > REG_STARTEND, at which point it is potentially too late to compile our
> > own regex routines (we're potentially going to conflict with the system
> > declarations).
> 
> If you have autoconf testing for REG_STARTEND at configure time then
> compat/regex.c can #include "config.h" and test for HAVE_REG_STARTEND
> rather than for REG_STARTEND, or something like that.

Right, that part is easy; we do not even have to touch compat/regex.c,
because we already have such a knob in the Makefile (NO_REGEX), and
autoconf just needs to tweak that knob.

My question was whether we could do it without running a separate
compile (via autoconf or via the Makefile), and I think the answer is
"no".

-Peff


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

2016-10-06 Thread Rich Felker
On Thu, Oct 06, 2016 at 03:23:40PM -0400, Jeff King wrote:
> On Thu, Oct 06, 2016 at 09:18:29PM +0200, Ævar Arnfjörð Bjarmason wrote:
> 
> > On Tue, Oct 4, 2016 at 6:08 PM, Johannes Schindelin
> >  wrote:
> > > As to making NO_REGEX conditional on REG_STARTEND: you are talking about
> > > apples and oranges here. NO_REGEX is a Makefile flag, while REG_STARTEND
> > > is a C preprocessor macro.
> > >
> > > Unless you can convince the rest of the Git developers (you would not
> > > convince me) to simulate autoconf by compiling an executable every time
> > > `make` is run, to determine whether REG_STARTEND is defined, this is a
> > > no-go.
> > 
> > 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.
> 
> No, I think that is the exact purpose of configure.ac and autoconf.
> 
> It would be neat if we could auto-fallback during the build. Rich
> suggested always compiling compat/regex.c, and just having it be a noop
> at the preprocessor level. I'm not sure if that would work, though,
> because we'd have to include the system "regex.h" to know if we have
> REG_STARTEND, at which point it is potentially too late to compile our
> own regex routines (we're potentially going to conflict with the system
> declarations).

If you have autoconf testing for REG_STARTEND at configure time then
compat/regex.c can #include "config.h" and test for HAVE_REG_STARTEND
rather than for REG_STARTEND, or something like that.

Rich


Re: [RFC/PATCH 0/2] place cherry pick line below commit title

2016-10-06 Thread Junio C Hamano
Jonathan Tan  writes:

> How important is this feature? It doesn't seem too difficult to add,
> although it does break compatibility (in particular, "--signoff" must
> now be documented as "after the last trailer" instead of "at the end
> of the commit message").

The sign-off has always been meant to be appended at the end of the
trailer block, so there is no "compatibility" issue.  It's just how
you phrase the explanation of the behaviour, I would think.


Re: [PATCH v2 05/25] sequencer: allow the sequencer to take custody of malloc()ed data

2016-10-06 Thread Junio C Hamano
Johannes Schindelin  writes:

> If you prefer to accept such sloppy work, I will change it of course,
> feeling dirty that it has my name on it.

I do want neither leaks nor sloppyness, and I thought that was
understood by everybody, hence I thought the last round made it
clear that _entrust() must not exist.

Let's try again.

We manage lifetime of a field in a structure in one of three ways in
our codebase [*1*].

 * A field can always point at a borrowed piece of memory that the
   destruction of the containing structure or re-assignment to the
   field do not have to free the current value of the field.  This
   is a minority, simply because it is rare for a field to be always
   able to borrow from others.  The destruction of the containing
   structure or re-assignment to the field needs to do nothing
   special.

 * A field can own the piece of memory that it points at.  The
   destruction of the containing structure or re-assignment to the
   field needs to free the current value of the field [*2*].  A
   field that can be assigned a value from the configuration (which
   is typically xstrdup()'ed) is an example of such a field, and if
   a command line option also can assign to the field, we'd need to
   xstrdup() it, even though we know we could borrow from argv[],
   because cleaning-up needs to be able to free(3) it.

 * A field can sometimes own and sometimes borrow the memory, and it
   is accompanied by another field to tell which case it is, so that
   cleaning-up can tell when it needs to be free(3)d.  This is a
   minority case, and we generally avoid it especially in modern
   code for small allocation, as it makes the lifetime rule more
   complex than it is worth.

The _entrust() thing may have been an excellent fourth option if it
were cost-free.  xstrdup() that the second approach necessitates for
literal strings (like part of argv[]) would become unnecessary and
we can treat as if all the fields in a structure were all borrowing
the memory from elsewhere (in fact, _entrust() creates the missing
owners of pieces of memory that need to be freed later); essentially
it turns a "mixed ownership" case into the first approach.

But it is not cost-free.  The mechanism needs to allocate memory to
become the missing owner and keep track of which pieces of memory
need to be freed later, and the resulting code does not become any
easier to follow.  The programmer still needs to know which ones to
_entrust() just like the programmer in the model #2 needs to make
sure xstrdup() is done for appropriate pieces of memory--the only
difference is that an _entrust() programmer needs to mark the pieces
of memory to be freed, while a #2 programmer needs to xstrdup() the
pieces of memory that are being "borrowed".

So let's not add the fourth way to our codebase.  "mixed ownership"
case should be turned into "field owns the memory", i.e. approach #2.


[Footnotes]

*1* It is normal for different fields in the same structure follow
different lifetime rules.

*2* Unless leaks are allowed, that is.  I think we have instances
where "git cmd --opt=A --opt=B" allocates and stores a new piece
of memory that is computed based on A and store it to a field,
and then overwrites the field with another allocation of a value
based on B without freeing old value, saying "the caller can
avoid passing the same thing twice, and such a leak is miniscule
anyway".  That is not nice, and we've been reducing them over
time.


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

2016-10-06 Thread Jeff King
On Thu, Oct 06, 2016 at 09:18:29PM +0200, Ævar Arnfjörð Bjarmason wrote:

> On Tue, Oct 4, 2016 at 6:08 PM, Johannes Schindelin
>  wrote:
> > As to making NO_REGEX conditional on REG_STARTEND: you are talking about
> > apples and oranges here. NO_REGEX is a Makefile flag, while REG_STARTEND
> > is a C preprocessor macro.
> >
> > Unless you can convince the rest of the Git developers (you would not
> > convince me) to simulate autoconf by compiling an executable every time
> > `make` is run, to determine whether REG_STARTEND is defined, this is a
> > no-go.
> 
> 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.

No, I think that is the exact purpose of configure.ac and autoconf.

It would be neat if we could auto-fallback during the build. Rich
suggested always compiling compat/regex.c, and just having it be a noop
at the preprocessor level. I'm not sure if that would work, though,
because we'd have to include the system "regex.h" to know if we have
REG_STARTEND, at which point it is potentially too late to compile our
own regex routines (we're potentially going to conflict with the system
declarations).

-Peff


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

2016-10-06 Thread Ævar Arnfjörð Bjarmason
On Tue, Oct 4, 2016 at 6:08 PM, Johannes Schindelin
 wrote:
> As to making NO_REGEX conditional on REG_STARTEND: you are talking about
> apples and oranges here. NO_REGEX is a Makefile flag, while REG_STARTEND
> is a C preprocessor macro.
>
> Unless you can convince the rest of the Git developers (you would not
> convince me) to simulate autoconf by compiling an executable every time
> `make` is run, to determine whether REG_STARTEND is defined, this is a
> no-go.

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.


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

2016-10-06 Thread Junio C Hamano
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.

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 do not believe current "diff" behavior wrt. i-t-a entries is right
> either. There's no point in pursuing this series then. Feel free to
> revert 3f6d56d (commit: ignore intent-to-add entries instead of
> refusing - 2012-02-07) and bring back the old i-t-a behavior. All the
> problems would go away.

It is way too late to revert 3f6d56d.  Even though I think it was
overall a mistake to treat i-t-a as "not exist" while committing,
people are now used to the behaviour with that change, and we need
to make our progress within the constraint of the real world.


[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
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".

"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 

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

2016-10-06 Thread Jeff King
On Thu, Oct 06, 2016 at 10:15:49AM +0100, Richard Lloyd wrote:

> Unfortunately, on systems with an older regex shipped as
> standard (e.g. HP-UX 11), the include path picks up
> /usr/include/regex.h first, which doesn't define REG_STARTEND
> and the git-compat-util.h check fails.
> 
> The fix I applied on HP-UX 11 was to add -Icompat/regex
> to the CFLAGS ahead of other -I directives. Another possible
> change needed might be to line 69 of compat/regex/regex.c:

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.

-Peff


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

2016-10-06 Thread Jeff King
On Thu, Oct 06, 2016 at 03:00:14PM -0400, Jeff King wrote:

> PS I think your "!!" syntax conflicts with something like:
> 
> git -c alias.has-changes='!! git diff --quiet' has-changes
> 
>I don't know if that is worth worrying about or not. I also notice
>that using "!!git diff" with no space seems broken, as it seems to
>skip using the shell. I wonder if that is a bug in run-command.

Nevermind this last bit. It is the shell that is complaining
(rightfully) about "!git"; the error message is just confusing because
it mentions $0, which contains the whole script:

  $ git -c alias.has-changes='!!git diff --quiet' has-changes
  !git diff --quiet: 1: !git diff --quiet: !git: not found

The "!! git diff" syntax sill has the conflict I mentioned (though note
I screwed up the quotes in what I wrote above).

-Peff


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

2016-10-06 Thread Jeff King
On Thu, Oct 06, 2016 at 06:41:24PM +0700, Nguyễn Thái Ngọc Duy wrote:

> 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.

Hmm. I wonder if any commands will be fooled by not being moved to
GIT_WORK_TREE. I know that there is a bug already in this case:

  export HOME=$PWD

  for i in one two; do
git init $i &&
(cd $i &&
 echo "* diff=$i" >.gitattributes &&
 git add . &&
 git commit -m $i) &&
git config --global diff.$i.textconv "sed s/^/$i:/"
  done

  cd two &&
  git --git-dir=$(pwd)/../one/.git --work-tree=$(pwd)/../one show

This shows the contents of repo one using the .gitattributes from repo
two. I am not sure if the bug is that when GIT_WORK_TREE is set,
git-show doesn't position itself at the toplevel of that tree, or if the
attributes machinery should be more careful about looking up paths in
the work-tree versus the current directory.

This bug is tangential to your patch (it has nothing to do with aliases,
and should be fixed regardless). But I wonder if your "!!" aliases will
expose this bug more frequently.

-Peff

PS I think your "!!" syntax conflicts with something like:

git -c alias.has-changes="!! git diff --quiet' has-changes

   I don't know if that is worth worrying about or not. I also notice
   that using "!!git diff" with no space seems broken, as it seems to
   skip using the shell. I wonder if that is a bug in run-command.


Re: Format for %d includes unwanted preceding space

2016-10-06 Thread Junio C Hamano
"Ravi (Tom) Hale"  writes:

> The log --format="%d" includes preceding space:
>
> $ git log -n1 --format="XX%dXX" HEAD
> XX (HEAD -> master)XX
>
> I know of no other % that is output in this way.

That is primarily because %d predates %[-+ ] mechanism,
I think.  It is too late to redefine what "%d" means and force all
users to update their script to use "% d" instead, unfortunately.




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

2016-10-06 Thread Junio C Hamano
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.


> Signed-off-by: Nguyễn Thái Ngọc Duy 
> ---
>  git.c | 4 
>  1 file changed, 4 insertions(+)
>
> diff --git a/git.c b/git.c
> index 296857a..4c1dcf4 100644
> --- a/git.c
> +++ b/git.c
> @@ -252,6 +252,10 @@ static int handle_alias(int *argcp, const char ***argv)
>  
>   alias_string++;
>   commit_pager_choice();
> + if (*alias_string == '!') {
> + keep_cwd = 0;
> + alias_string++;
> + }
>   restore_env(keep_cwd);
>  
>   child.use_shell = 1;


Re: [PATCH v2 20/25] sequencer: left-trim lines read from the script

2016-10-06 Thread Junio C Hamano
Johannes Sixt  writes:

> Let me take the opportunity to say
>
>Thank You Very Much!
>
> for the new implementation of rebase -i. I'm using your branch
> rebase-i-extra, and it is such a pleasure to work with on Windows
> compared to the old fully scripted version.

Thanks for testing.  

Having more guinea pigs ^W ^W testers before the series is reviewed
here would be a nice boost and would hopefully encourage more
reviewers to help the series into a good shape to be upstreamed ;-)

>  8< 
> [PATCH] sequencer: strip CR from the end of exec insns
>
> It is not unheard of that editors on Windows write CRLF even if the file
> originally had only LF. This is particularly awkward for exec lines of a
> rebase -i todo sheet. Take for example the insn "exec echo": The shell
> script parser (either of the sequencer or of the shell that is invoked,
> I do not know) splits at the LF and leaves the CR attached to "echo",
> which leads to the unknown command "echo\r".

Interesting find.  So it's not just ltrim is being lenient only to
end-user typo, but not doing rtrim can actively hurt ;-)

> Work it around by stripping CR before the command specified with exec is
> passed to the shell.

Makes sense.



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

2016-10-06 Thread Junio C Hamano
Richard Lloyd  writes:

> git ships with a compat/regex tree containing a recent regex
> release, presumably with the intention of allowing systems with
> either no regex or an old regex installed to use the newer compat
> version.

Quick question.  Does NO_REGEX help?  cf. Makefile

# Define NO_REGEX if your C library lacks regex support with REG_STARTEND
# feature.


Re: [PATCH] push: Re-include "push.default=tracking" in the documentation

2016-10-06 Thread Junio C Hamano
Matthieu Moy  writes:

> Ævar Arnfjörð Bjarmason  writes:
>
>> Junio, looks like from the 2013 discussion that you preferred just
>> having that mention in parenthesis instead of its own item, how about
>> just re-applying your fa23348 (with conflicts resolved)?
>
> (fa23348 did this:
> --- a/Documentation/config.txt
> +++ b/Documentation/config.txt
> @@ -1749,7 +1749,8 @@ push.default::
>+
>This is currently the default, but Git 2.0 will change the default
>to `simple`.
> -* `upstream` - push the current branch to its upstream branch.
> +* `upstream` - push the current branch to its upstream branch
> +  (`tracking` is a deprecated synonym for this).
> )
>
> I agree that doing the same thing is the best option.

Sorry, I wasn't paying attention to this thread.  

It seems that 87a70e4ce8 ("config doc: rewrite push.default
section", 2013-06-19) removed that mention by accident?  The log
message of the commit does not say it actively wanted to remove
mention of `tracking` and/or why it wanted to do so, so I agree that
resurrecting that parenthetical mention is the easiest course of
action at this point.

However.

With today's description of push.default choices, each of which is a
full fledged paragraph, I no longer have the objection I had in

https://public-inbox.org/git/7vip6dgmx2@alter.siamese.dyndns.org/

against having `tracking` as a separate bullet item.  If we add

* `tracking` - a deprecated synonym for `upstream`; do not use this.

to today's list, it would stand out as something different from
others and it will not cause the confusion I feared in the
discussion we had in early 2013.  As Jonathan Nieder argued in the
thread back then, having it as one of the bullet point would help
people locate it without using "search" \C-s or / feature.





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

2016-10-06 Thread Junio C Hamano
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.

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.

> diff --git a/Documentation/git-merge.txt b/Documentation/git-merge.txt
> index b758d55..479400f 100644
> --- a/Documentation/git-merge.txt
> +++ b/Documentation/git-merge.txt
> @@ -135,15 +135,17 @@ will exit early with the message "Already up-to-date."
>  FAST-FORWARD MERGE
>  --
>  
> -Often the current branch head is an ancestor of the named commit.
> -This is the most common case especially when invoked from 'git
> -pull': you are tracking an upstream repository, you have committed
> -no local changes, and now you want to update to a newer upstream
> -revision.  In this case, a new commit is not needed to store the
> +When current branch head is an ancestor of the named commit,
> +a new commit is not needed to store the
>  combined history; instead, the `HEAD` (along with the index) is
>  updated to point at the named commit, without creating an extra
>  merge commit.
>  
> +This is very common case when 'git merge' is invoked from 'git
> +pull': you are tracking an upstream repository, you have committed
> +no local changes, and now you want to update to a newer upstream
> +revision.  
> +
>  This behavior can be suppressed with the `--no-ff` option.
>  
>  TRUE MERGE
>
> -- Sergey


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

2016-10-06 Thread Junio C Hamano
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").

It may be debatable if we want to give the description of what is
exactly recorded at that point of the document, but I personally
think that the users deserve a chance to learn how a merge is
recorded in "git merge" documentation.




Re: [PATCH v9 09/14] pkt-line: add packet_write_gently()

2016-10-06 Thread Junio C Hamano
Lars Schneider  writes:

> You are right. Would the solution below be acceptable?
> I would like to keep the `packet_size` variable as it eases the rest
> of the function.
>
>  
>   const size_t packet_size = size + 4;
>  
> - if (packet_size > sizeof(packet_write_buffer))
> + if (size > sizeof(packet_write_buffer) - 4)
>   return error("packet write failed - data exceeds max packet 
> size");

Sounds fine; packet_size may have invalid value if size is large
enough but in such a case the function would return without using
it, so no harm is expected, I would say.  I'd prefer to see the
definition of packet_size separate from the assignment of size + 4
to it in a case like this, though.



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

2016-10-06 Thread Stefan Beller
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.

Thanks,
Stefan


Re: [PATCH v8 00/11] Git filter protocol

2016-10-06 Thread Junio C Hamano
Jeff King  writes:

> I am not sure if I have followed all of this discussion, but I am of the
> opinion that Git should not be doing any timeouts at all.
> ...
> If this is debugging output of the form "now I am calling wait() on all
> of the filters", without respect to any timeout, that sounds reasonable.
> Though I would argue that run-command should simply trace_printf()
> any processes it is waiting for, which covers _any_ process, not just
> the filters. That seems like a good match for the rest of the GIT_TRACE
> output, which describes how and when we spawn the sub-processes.

Yup, I agree that having trace_printf() report the wait for any
process is the cleanest way to go.  As you guessed the reason why
Lars is bringing up "now we are waiting for this filter" is because
I suggested it as a way to encourage users to file bugs when they
see a hung Git.  Originally Lars wanted to have a timeout on wait
and after the timeout wanted to kill the process, and because I
really did not want such a random "you are too slow to die, so I'll
send a signal to you and exit myself without making sure you died"
there, I suggested that if we were to have a timeout, that would be
to timeout the wait only to have a chance to tell the user "we are
stuck waiting on this thing" (and then go back to wait), as it would
either be a buggy filter (i.e. the users need to debug their own
filter code) or a buggy use of wait on Git side (i.e. we would want
to hear about such bugs from them).

Without such a "wait with timeout so that we can tell the user", we
can still respond to "my 'git checkout' hangs forever" with "try
running with GIT_TRACE" as you outlined above, so I do not think we
need the timeout.

Thanks for straightening us out.




Re: [PATCH 2/2] Feature Request: user defined suffix for temp files created by git-mergetool

2016-10-06 Thread Junio C Hamano
Josef Ridky  writes:

> + --local=*)
> + temp_name=${1#--local=}
> + if [ "$temp_name" != "" ] && [ "$temp_name" != "$REMOTE_NAME" ] 
> && [ "$temp_name" != "$BASE_NAME" ] && [ "$temp_name" != "$BACKUP_NAME" ]
> + then
> + LOCAL_NAME=$temp_name
> + fi

It is not a good idea to ignore an explicit user request without
giving any indication and without giving any explanation.

You may have noticed that we do not say "[ cond ]" in this shell
script (we say "test" instead; see Documentation/CodingGuidelines).

Instead of having such a test all over the place, I'd suggest doing
it outside the loop:

while test $# != 0
do
case "$1" in
...
--local=*)
LOCAL_NAME=${1#--local=} ;;
--remote=*)
REMOTE_NAME=${1#--local=} ;;
...
esac
done

# sanity check _after_ parsing the command line
case ",$LOCAL_NAME,$REMOTE_NAME,$BASE_NAME,$BACKUP_NAME," in
*,,*) die "You cannot set --local/remote/... to empty" ;;
esac
... similarly, duplicate check comes here ...



★★★《出国参展邀请函》2017第52届世界JEC复合材料展览及会议,请转——董事长/总经理(收) |GA3/L100-M| gfytyt...@163.com

2016-10-06 Thread ndsvsqq
web1_re...@theadagio.com.tw
(凭此邮件优惠1000元/公司,需回信专用接收邮箱“12809...@qq.com”报名参展)
  
  
JEC world Composites Show & Conferences 2017
2017第52届JEC世界复合材料展览及会议
  
—— 参加JEC展会是一个复合材料及新材料企业走向国际化的标志和途径
  
【中文名称】 2017第52届世界JEC复合材料展览及会议(法国巴黎)
【英文名称】 The 52th JEC Composites Show & Conferences 2017 in Paris France (JEC 
world 2017)
  
【展会时间】 2017年03月14-16日
【展会地点】 法国巴黎北维勒班展览中心 (Paris Nord Villepinte Exhibition Center)
  
【展会周期】 一年一届 (本年度为第52届)
【报名截止】 2016年10月28日
  
【主办单位】 法国JEC复合材料发展促进会、法国JEC集团
【组办单位】 中国国际复合材料协会、 映德国际会展集团中国代表处、 馨德会展产业管理有限公司
  
【在线客服】 邮箱/ 12809395#qq.com; QQ/ 82775507; 微信/ yondexpo; 微博/ 
http://weibo.com/jecshow 
【咨询电话】 4000-580-850(转8144、5220); 139-1031-8144; 010—8699-7155;
  
  
【市场机遇】
  新材料为21世纪三大共性关键技术之一,已成为全球经济迅速增长的源动力和提升核心竞争力的战略焦点。
  材料作为制造业的基础,特别是新材料研究和产业发展的水平与规模,已经成为衡量一个国家科技进步和综合实力的重要标志。
  
在新材料发展与应用中,复合材料占有相当重要的地位,特别广泛的应用在汽车、交通、风能、航空、航天、兵器、船舶、国防、机械、电子、化工、建筑、农业、渔业、纺织、运动器材等领域,一直是世界各国优先发展和竞争激烈的重要行业。
  
  
【本展简介】
  “JEC世界复合材料展览及会议”(JEC world Composites Show & 
Conferences)创办于1963年,每年举办一届,至2016年总共举办了51届,主办单位是法国JEC复合材料发展促进会/JEC集团,中国总展团展商组织单位为映德会展(中国)有限公司,在北京、上海等地设有分支机构,负责该展会在中国的推广和招商工作(JEC中国总展团报名热线:4000-680-860转8144、5220)。JEC复合材料展已成为世界上历史最悠久、规模最大的复合材料行业专业展览会,展示和反映了当前复合材料行业的最新技术和应用成果。
  
JEC复合材料展览及会议以欧洲为基础致力于打造一个复合材料交流、贸易的平台。在最近的10年中,专业观众数量增长了86%,其中观众来源最多的国家依次为德国、英国、意大利、比利时、美国、荷兰、西班牙、瑞士、俄罗斯和中国。关注复合材料最多的行业为汽车制造、航空制造和建筑材料。参展展品应用领域多为汽车、船舶游艇、航天航空、建筑材料、轨道交通、风力发电、休闲产品、管道和电力,辐射行业之广是其他同类展会无法匹敌的。JEC复合材料展览及会议是唯一一个将全球复合材料工业联合起来的展览会,也是一个复合材料应用商与供应商、相关科研人员及专家广泛交流的平台,还是一个企业走向国际化的标志和途径。
  
2015年3月10-12日举办的第50届欧洲JEC复合材料展参展商超过2000家,专业观众超过37500人,展出面积超过56500平方米,展示了行业中的最新技术、新材料、新工艺、新方法,同期举办有丰富的学术交流活动,全面呈现当今国际最新产业动态及发展趋势。
  为了增进国内外复合材料行业的交流与合作,同时展示我国复合材料产业的发展与成就,帮助境内企业开拓国内外市场,中国国际复材协会、映德国际会展集团(YOND 
EXPO)中国代表处已近十年组织中国企业参与该展会,为中国复合材料集团、中材科技、中钢集团、中国建材集团、中国商飞、北京玻钢院、上海杰事杰新材料集团、重庆国际复合材料、中南控股集团、秦皇岛耀华玻璃钢、烟台氨纶、天马集团、华东理工大学、哈尔滨工业大学、巨石集团、中冶集团、金光集团、江苏恒神纤维材料、重庆大学、上海玻璃钢研究院、中南大学、哈尔滨玻璃钢研究院等众多行业巨头和知名机构提供了优质高效的境外展贸服务。
  “JEC world 2017 
第52届世界复合材料展览及会议”将于“3月14-16日”在“法国巴黎北维勒班展览中心”再度举行,我们诚邀全国各地复合材料及新材料相关单位与业界人士加入咱们的中国总展团前往参展参观。JEC中国总展团报名热线:4000-680-860转8144、5220。
  
  
【历届回顾】
  2008年参展商超过900家,专业观众25500人,展出面积36500平方米。
  2009年参展商超过1000家,专业观众27336人,展出面积39000平方米。 
  2010年参展商有1053家,专业观众3人,展出面积达43500平方米。
  2011年参展商有1120家,专业观众32000人,展出面积达48500平方米。
  2014年参展商超过2000家,专业观众超过37500人,展出面积超过56500平方米。
  
  
【展品范围】
◆ 成品应用类: 应用于车辆、船艇、交通、风能、建筑、航空、航天、国防、机械、电子、防腐、农林、渔业、纺织、日常生活等领域的复合材料/玻璃钢新产品、新技术;
◆ 原材料类: 
纤维类,玻璃纤维,玻璃钢,填料,碳纤维,硼纤维,碳化硅纤维,陶瓷纤维,聚乙烯纤维,石英棉,聚酯玻纤布,聚乙交酯,泡沫纤维,膨化纤维,天然纤维,矾土,碳酸钙,微珠与宏珠,微硅粉,硅灰石粉,树脂类,环氧树脂,聚醯胺,聚碳酸酯,填加剂类,抗浮剂,脱模剂,染料,颜料,增塑剂,挤塑剂,织物,毡片等;
  
◆ 加工机械类: 
喷胶机器,机器边框和部件,注塑机,冲压模塑机,触压成型机,纤维缠绕机,树脂传递成型机,聚亚安酯机械,玻璃纤维喷漆机,SMC和TRE加工机器,成型油压机,调整冲模用压力机,拉挤成型机械,催化剂配剂器,形件分离机器,模具,模具维护和清洁,抛光膏,超声波探伤,水射流切割,激光设备,机械加工设备,成型模具设计加工技术等;
◆ 生产线类: 
热固性塑胶加工,热塑性加工,高低压模具生产,模压成型,树脂传递成型,注塑焊接,真空成型,液压成型,糊成型,喷射成型,纤维缠绕成型,拉挤成型,热压罐成型,隔膜成型,迁移成型,反应注射成型,软膜膨胀成型,冲压成型,扩散焊接,粉末冶金,热轧,热拔,产品质量检验技术及装备,生产自动化控制与软件,质量监控技术,无损检测技术及仪器等。
  
  
【参展形式】
◆ 参 展: 租赁【展位】,展示企业形象及产品,主要费用有【展位费】【运输费】【搭建费】【人员费】等。
◆ 参 观: 派遣【人员】,莅临展场参观及考察,主要费用有【报名费】【注册费】【交通费】【食宿费】等。
  
  
【参展联系】
有关参展参观“JEC世界复合材料展”事宜,请联络【中国总展团】组办方:
办公地址: 北京市朝阳区北三环东路六号中国国际展览中心四层
参展顾问: 王先生、 张先生、 孙小姐、 吴小姐
参展热线: 139-1031-8144、 131-2662-5206;  010— 8699- 7155.
在线客服: 邮箱/ 12809395#qq.com; QQ/ 82775507; 微信/ yondexpo; 微博/ 
http://weibo.com/yondexpo
全国统一客服热线: 4000- 580- 850.(分机: /8144 /5220 /5206)
全国统一报名专线: 4000- 680- 860.(兼传真)
  
  
  
【公众平台】
  
微信: 参展消息 (ID:CanZhanXiaoXi)—— 品牌扩张的平台 市场开拓的桥梁
微信: 展商之家 (ID:ZhanShangZhiJia)—— 为展商提供最佳营地 为阁下营造参展价值
  
  
  
  
  
百万群发系统|为您发送|如不希望再收到此行业资讯|请回复“退订+JEC”至邮箱1055800...@qq.com


Re: Feature Request: user defined suffix for temp files created by git-mergetool

2016-10-06 Thread Junio C Hamano
Josef Ridky  writes:

> I agree, that this patch is written as general as possible and can
> possibly bring more confusion than benefits.

I am not sure about that.  Other people would have similar but
different workflow needs where they compare local new one with local
old one that would be helped by renaming local to old and remote to
new (i.e. the other way around from your need).  If you just add a
toggle between local-remote vs new-old, that would be just an
additional code baggage that does not help people other than you.

I think J6t's "EDIT THIS" hits the center of the issue.  If users
are trained to know LOCAL is the one to be edited, would the current
UI work well enough for them thru your custom workflow tools?  If we
rename LOCAL to "EDIT THIS" and do nothing else, would such UI work
well for even untrained users thru your custom workflow tools?



[PATCH 2/2] files_read_raw_ref: prevent infinite retry loops in general

2016-10-06 Thread Jeff King
Limit the number of retries to 3. That should be adequate to
prevent any races, while preventing the possibility of
infinite loops if the logic fails to handle any other
possible error modes correctly.

After the fix in the previous commit, there's no known way
to trigger an infinite loop, but I did manually verify that
this fixes the test in that commit even when the code change
is not applied.

Signed-off-by: Jeff King 
---
 refs/files-backend.c | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/refs/files-backend.c b/refs/files-backend.c
index d826557..76e5902 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -1353,6 +1353,7 @@ static int files_read_raw_ref(struct ref_store *ref_store,
int fd;
int ret = -1;
int save_errno;
+   int remaining_retries = 3;
 
*type = 0;
strbuf_reset(_path);
@@ -1373,8 +1374,14 @@ static int files_read_raw_ref(struct ref_store 
*ref_store,
 * <-> symlink) between the lstat() and reading, then
 * we don't want to report that as an error but rather
 * try again starting with the lstat().
+*
+* We'll keep a count of the retries, though, just to avoid
+* any confusing situation sending us into an infinite loop.
 */
 
+   if (remaining_retries-- <= 0)
+   goto out;
+
if (lstat(path, ) < 0) {
if (errno != ENOENT)
goto out;
-- 
2.10.1.506.g904834d


[PATCH 1/2] files_read_raw_ref: avoid infinite loop on broken symlinks

2016-10-06 Thread Jeff King
Our ref resolution first runs lstat() on any path we try to
look up, because we want to treat symlinks specially (by
resolving them manually and considering them symrefs). But
if the results of `readlink` do _not_ look like a ref, we
fall through to treating it like a normal file, and just
read the contents of the linked path.

Since fcb7c76 (resolve_ref_unsafe(): close race condition
reading loose refs, 2013-06-19), that "normal file" code
path will stat() the file and if we see ENOENT, will jump
back to the lstat(), thinking we've seen inconsistent
results between the two calls. But for a symbolic ref, this
isn't a race: the lstat() found the symlink, and the stat()
is looking at the path it points to. We end up in an
infinite loop calling lstat() and stat().

We can fix this by avoiding the retry-on-inconsistent jump
when we know that we found a symlink. While we're at it,
let's add a comment explaining why the symlink case gets to
this code in the first place; without that, it is not
obvious that the correct solution isn't to avoid the stat()
code path entirely.

Signed-off-by: Jeff King 
---
 refs/files-backend.c| 7 ++-
 t/t1503-rev-parse-verify.sh | 5 +
 2 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/refs/files-backend.c b/refs/files-backend.c
index 0709f60..d826557 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -1403,6 +1403,11 @@ static int files_read_raw_ref(struct ref_store 
*ref_store,
ret = 0;
goto out;
}
+   /*
+* It doesn't look like a refname; fall through to just
+* treating it like a non-symlink, and reading whatever it
+* points to.
+*/
}
 
/* Is it a directory? */
@@ -1426,7 +1431,7 @@ static int files_read_raw_ref(struct ref_store *ref_store,
 */
fd = open(path, O_RDONLY);
if (fd < 0) {
-   if (errno == ENOENT)
+   if (errno == ENOENT && !S_ISLNK(st.st_mode))
/* inconsistent with lstat; retry */
goto stat_ref;
else
diff --git a/t/t1503-rev-parse-verify.sh b/t/t1503-rev-parse-verify.sh
index ab27d0d..69d5135 100755
--- a/t/t1503-rev-parse-verify.sh
+++ b/t/t1503-rev-parse-verify.sh
@@ -139,4 +139,9 @@ test_expect_success 'master@{n} for various n' '
test_must_fail git rev-parse --verify master@{$Np1}
 '
 
+test_expect_success SYMLINKS 'ref resolution not confused by broken symlinks' '
+   ln -s does-not-exist .git/broken &&
+   test_must_fail git rev-parse --verify broken
+'
+
 test_done
-- 
2.10.1.506.g904834d



[PATCH 0/2] fix infinite loop in ref resolution

2016-10-06 Thread Jeff King
This fixes an infinite loop bug dating back to the v1.8.x era.
Triggering it requires creating a broken symbolic link in the .git
directory, so I don't think it's security-interesting. It should apply
cleanly on "maint".

  [1/2]: files_read_raw_ref: avoid infinite loop on broken symlinks
  [2/2]: files_read_raw_ref: prevent infinite retry loops in general

 refs/files-backend.c| 14 +-
 t/t1503-rev-parse-verify.sh |  5 +
 2 files changed, 18 insertions(+), 1 deletion(-)


Re: [PATCH] push: Re-include "push.default=tracking" in the documentation

2016-10-06 Thread Matthieu Moy
Ævar Arnfjörð Bjarmason  writes:

> Junio, looks like from the 2013 discussion that you preferred just
> having that mention in parenthesis instead of its own item, how about
> just re-applying your fa23348 (with conflicts resolved)?

(fa23348 did this:
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1749,7 +1749,8 @@ push.default::
   +
   This is currently the default, but Git 2.0 will change the default
   to `simple`.
-* `upstream` - push the current branch to its upstream branch.
+* `upstream` - push the current branch to its upstream branch
+  (`tracking` is a deprecated synonym for this).
)

I agree that doing the same thing is the best option.

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


Re: [PATCH v2 20/25] sequencer: left-trim lines read from the script

2016-10-06 Thread Johannes Sixt
Am 06.10.2016 um 15:08 schrieb Johannes Schindelin:
> Hi Junio,
> 
> On Mon, 12 Sep 2016, Junio C Hamano wrote:
> 
>> Johannes Schindelin  writes:
>>
 I do not offhand see why we want to be lenient here,
 especially only to the left.
>>>
>>> Postel's Law.
>>
>> How would that compare/relate to yagni, though?
> 
> I did need it, though. Blame it on being overworked or simply tired: I
> ended up inserting a spurious space before a "fixup" command. This command
> was handled as intended by the scripted rebase -i, and with the patch in
> question the rebase--helper agreed, too.
> 
> Note: we have no problem to the right because we do handle variable-length
> whitespace between command and SHA-1, and we do not care one iota about
> the exact form of the oneline (hence right-stripping is not necessary).

The last sentence is not entirely correct. See the patch below. You
may pick up the idea in one form or another, or just queue the patch
near the end of your series.

Let me take the opportunity to say

   Thank You Very Much!

for the new implementation of rebase -i. I'm using your branch
rebase-i-extra, and it is such a pleasure to work with on Windows
compared to the old fully scripted version.

 8< 
[PATCH] sequencer: strip CR from the end of exec insns

It is not unheard of that editors on Windows write CRLF even if the file
originally had only LF. This is particularly awkward for exec lines of a
rebase -i todo sheet. Take for example the insn "exec echo": The shell
script parser (either of the sequencer or of the shell that is invoked,
I do not know) splits at the LF and leaves the CR attached to "echo",
which leads to the unknown command "echo\r".

Work it around by stripping CR before the command specified with exec is
passed to the shell.

This happens to fix t9903.14 and .15 in my environment: the insn sheet
constructed here contains CRLF thanks to MSYS1's bash cleverness.

Signed-off-by: Johannes Sixt 
---
 sequencer.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/sequencer.c b/sequencer.c
index daf8f13..6961820 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -2005,8 +2005,11 @@ static int pick_commits(struct todo_list *todo_list, 
struct replay_opts *opts)
}
else if (item->command == TODO_EXEC) {
char *end_of_arg = (char *)(item->arg + item->arg_len);
-   int saved = *end_of_arg;
+   int saved;
 
+   while (end_of_arg != item->arg && end_of_arg[-1] == 
'\r')
+   end_of_arg--;
+   saved = *end_of_arg;
*end_of_arg = '\0';
res = do_exec(item->arg);
*end_of_arg = saved;
-- 
2.10.0.343.g37bc62b



Re: [PATCH] push: Re-include "push.default=tracking" in the documentation

2016-10-06 Thread Ævar Arnfjörð Bjarmason
On Thu, Oct 6, 2016 at 2:13 PM, Matthieu Moy
 wrote:
> Ęvar Arnfjörš Bjarmason  writes:
>
>> That's bad, either we shouldn't support it at all, or we should
>> document what it does. This patch does the latter.
>
> I vaguely remember a similar discussion and probably even a patch in the
> past (maybe by you actually). I think the proposal was to add a mention
> of tracking but avoid promoting it at the same level as the others.
>
> I have a slight preference for a patch adding stg like "`tracking` is a
> deprecated alias supported only for backward compatibility" to the item
> of `upstream`, but I'm OK with the current patch too.

You're right! I wasn't trying to be sneaky here, but apparently I just
keep running into the same things. We talked about this back in 2012,
kicked off by you proposing a patch to remove it completely from
config.txt and me submitting this:
http://www.spinics.net/lists/git/msg198264.html

After some back & forth Junio ended up applying fa23348 to address
that, but that was wiped away just a few months later in 87a70e4.

I'd just like it mentioned in the docs in some way, because I *did*
run into exactly the situation I described in my E-Mail back in 2012,
i.e. found a repo with push.default=tracking and couldn't find any
mention of what it did in the docs.

Junio, looks like from the 2013 discussion that you preferred just
having that mention in parenthesis instead of its own item, how about
just re-applying your fa23348 (with conflicts resolved)?

>> --- a/Documentation/config.txt
>> +++ b/Documentation/config.txt
>> @@ -2344,6 +2344,10 @@ push.default::
>>pushing to the same repository you would normally pull from
>>(i.e. central workflow).
>>
>> +* `tracking` - Deprecated synonym for `upstream`, which we still
>> +  support for backwards compatibility with existing configuration
>> +  files.
>
> Nit: I think the doc normally doesn't use "we" this way (we = the Git
> developers or the Git tool). Hence my s/which we still support/still
> supported/ above.
>
> --
> Matthieu Moy
> http://www-verimag.imag.fr/~moy/


Re: Bug Report: "git submodule deinit" fails right after a clone

2016-10-06 Thread Thomas Bétous
Hello,

Sorry again for the mailing list...

On Thu, Oct 6, 2016 at 11:20 AM, Heiko Voigt  wrote:
> So I guess the same applies to 'git status'?

No, it is the strange thing.
As told in my very first message here what happens after git diff and
git status:

$ git clone https://github.com/githubtraining/example-dependency.git
Cloning into 'example-dependency'...
remote: Counting objects: 35, done.
remote: Total 35 (delta 0), reused 0 (delta 0), pack-reused 35
Unpacking objects: 100% (35/35), done.
Checking connectivity... done.
$ cd example-dependency
$ git submodule deinit js
fatal: Please stage your changes to .gitmodules or stash them to proceed
Submodule work tree 'js' contains local modifications; use '-f' to discard them
$ git diff
[no output]
$ git submodule deinit js
fatal: Please stage your changes to .gitmodules or stash them to proceed
Submodule work tree 'js' contains local modifications; use '-f' to discard them
$ git status
On branch master
Your branch is up-to-date with 'origin/master'.
nothing to commit, working directory clean
$ git submodule deinit js
Cleared directory 'js'

So as you can see, the 'git status' makes the problem magically disappear.

Thomas


Re: [PATCH v8 00/11] Git filter protocol

2016-10-06 Thread Jeff King
On Thu, Oct 06, 2016 at 03:13:19PM +0200, Lars Schneider wrote:

> >>> Well, it would be good to tell users _why_ Git is hanging, see below.
> >> 
> >> Agreed. Do you think it is OK to write the message to stderr?
> > 
> > On the other hand, this is why GIT_TRACE (and GIT_TRACE_PERFORMANCE)
> > was invented for.  We do not signal troubles with single-shot filters,
> > so I guess doing it for multi-file filters is not needed.
> 
> I am on the fence with this one.
> 
> @Junio/Peff:
> Where would you prefer to see a "Waiting for filter 'XYZ'... " message?
> On stderr or via GIT_TRACE?

I am not sure if I have followed all of this discussion, but I am of the
opinion that Git should not be doing any timeouts at all. There are
simply too many places where the filter (or any other process that git
is depending on) could inexplicably hang, and I
do not want to pepper random timeouts for all parts of the procedure
where we say "woah, this is taking longer than expected" (nor do I want
to have a timeout for _one_ spot, and ignore all the others).

If this is debugging output of the form "now I am calling wait() on all
of the filters", without respect to any timeout, that sounds reasonable.
Though I would argue that run-command should simply trace_printf()
any processes it is waiting for, which covers _any_ process, not just
the filters. That seems like a good match for the rest of the GIT_TRACE
output, which describes how and when we spawn the sub-processes.

Something like:

diff --git a/run-command.c b/run-command.c
index 5a4dbb6..b884605 100644
--- a/run-command.c
+++ b/run-command.c
@@ -226,6 +226,9 @@ static int wait_or_whine(pid_t pid, const char *argv0, int 
in_signal)
pid_t waiting;
int failed_errno = 0;
 
+   if (!in_signal)
+   trace_printf("waiting for pid %d", (int)pid);
+
while ((waiting = waitpid(pid, , 0)) < 0 && errno == EINTR)
;   /* nothing */
if (in_signal)

but it does not have to be part of this series, I think.

-Peff


Re: Feature Request: user defined suffix for temp files created by git-mergetool

2016-10-06 Thread Junio C Hamano
Johannes Sixt  writes:

> Therefore, I think that your patch as written does not help to reduce
> the confusion. It may be a building block for further improvement, but
> if you stop here, it is pointless.

Yup, you're right.



Re: [PATCH v9 04/14] run-command: add wait_on_exit

2016-10-06 Thread Junio C Hamano
Johannes Schindelin  writes:

> Hi Junio & Lars,
>
> On Wed, 5 Oct 2016, Junio C Hamano wrote:
>
>> Lars Schneider  writes:
>> 
>> > OK. Something like the patch below would work nicely.
>> 
>> Yeah, something along that line; it would eliminate the need to
>> worry about a field named "stdin" ;-)
>
> Not only a need to worry

Thanks, but I (and more importantly Lars, too) knew that stdin is
problematic when I sent the message you are responding to, as raised
by Ramsay in a separate thread:





Re: Fork Errors

2016-10-06 Thread Konstantin Khomoutov
On Thu, 6 Oct 2016 14:02:09 +
"Vacha, Brian [USA]"  wrote:

> When starting Git Bash, I receive the following errors:
> 0 [main] bash 18088 fork: child 14072 - died waiting for dll loading,
> errno 11 bash: fork: retry: No child processes
> 1190419 [main] bash 18088 fork: child 8744 - died waiting for dll
> loading, errno 11 bash: fork: retry: No child processes
> 3343518 [main] bash 18088 fork: child 12324 - died waiting for dll
> loading, errno 11 bash: fork: retry: No child processes
> 7480858 [main] bash 18088 fork: child 17008 - died waiting for dll
> loading, errno 11 bash: fork: retry: No child processes
> 15635036 [main] bash 18088 fork: child 8108 - died waiting for dll
> loading, errno 11 bash: fork: Resource temporarily unavailable
> bash-4.3$
> 
> My connection is great at 72 Mbps download and 93 Mbps upload.  I
> don't receive other errors so it appears to be a Git Bash issue to me.

Have you tried searching through Git for Windows bugtracker [1] for
your problem.  I'm pretty sure it was recently discussed there.
The issue #776 [2] looks like the one you're experiencing.

1. https://github.com/git-for-windows/git/issues
2. https://github.com/git-for-windows/git/issues/776


Re: [PATCH v2 25/25] sequencer: remove bogus hint for translators

2016-10-06 Thread Johannes Schindelin
Hi Junio,

On Sun, 11 Sep 2016, Junio C Hamano wrote:

> Johannes Schindelin  writes:
> 
> > When translating error messages, we need to be careful *not* to translate
> > the todo commands such as "pick", "reword", etc because they are commands,
> > and Git would not understand translated versions of those commands.
> >
> > Therefore, translating the commands in the error messages would simply be
> > misleading.
> 
> This comes from b9c993e0 ("i18n: git-revert literal "me" messages",
> 2011-02-22) where it said
> 
> Author: Ævar Arnfjörð Bjarmason 
> 
> i18n: git-revert literal "me" messages
> 
> Translate messages that use the `me' variable. These are all error
> messages referencing the command name, so the name shouldn't be
> translated.
> 
> Reported-by: Jonathan Nieder 
> Signed-off-by: Ævar Arnfjörð Bjarmason 
> Signed-off-by: Junio C Hamano 
> 
> This looks like a positive attempt to remind translators that their
> translation must flow with these literal command names that will not
> get translated in place, not a bogus hint at all, at least to me.

Alright, I misread it to mean: "You need to translate these terms".

I fixed it by skipping this patch, and amending the "revamp the todo
parsing" patch: the first %s now actually refers to the much more helpful
todo command rather than the overall Git command.

Thanks,
Dscho

Fork Errors

2016-10-06 Thread Vacha, Brian [USA]
When starting Git Bash, I receive the following errors:
0 [main] bash 18088 fork: child 14072 - died waiting for dll loading, errno 11
bash: fork: retry: No child processes
1190419 [main] bash 18088 fork: child 8744 - died waiting for dll loading, 
errno 11
bash: fork: retry: No child processes
3343518 [main] bash 18088 fork: child 12324 - died waiting for dll loading, 
errno 11
bash: fork: retry: No child processes
7480858 [main] bash 18088 fork: child 17008 - died waiting for dll loading, 
errno 11
bash: fork: retry: No child processes
15635036 [main] bash 18088 fork: child 8108 - died waiting for dll loading, 
errno 11
bash: fork: Resource temporarily unavailable
bash-4.3$

My connection is great at 72 Mbps download and 93 Mbps upload.  I don't receive 
other errors so it appears to be a Git Bash issue to me.

Thanks,
Brian


Re: [PATCH v2 24/25] sequencer: quote filenames in error messages

2016-10-06 Thread Johannes Schindelin
Hi Junio,

On Sun, 11 Sep 2016, Junio C Hamano wrote:

> Johannes Schindelin  writes:
> 
> > This makes the code consistent by fixing quite a couple of error messages.
> 
> Looks OK.  While at it, we may want another one to downcase the
> first word, perhaps?
> 
> These may not be messages added by your series and can be left
> outside this series, but I have to point out that
> 
> if (commit_lock_file(_file) < 0)
> return error(_("Error wrapping up '%s'."), filename);
> 
> results in "error: Error wrapping up", which sounds quite funny.
> 
> "failed to finalize" or something would flow a bit better, I'd say.

Fair enough. I added a patch to make it so.

Thanks,
Dscho


[PATCH 1/2] Feature Request: user defined suffix for temp files created by git-mergetool

2016-10-06 Thread Josef Ridky
This is the first of two variant for request to add option to change
suffix of name of temporary files generated by git mergetool. This
change is requested for cases, when is git mergetool used for local
comparision between two version of same package during package rebase.

Signed-off-by: Josef Ridky 
---
 Documentation/git-mergetool.txt |  7 ++-
 git-mergetool.sh| 23 ++-
 2 files changed, 24 insertions(+), 6 deletions(-)

diff --git a/Documentation/git-mergetool.txt b/Documentation/git-mergetool.txt
index e846c2e..a212cef 100644
--- a/Documentation/git-mergetool.txt
+++ b/Documentation/git-mergetool.txt
@@ -8,7 +8,7 @@ git-mergetool - Run merge conflict resolution tools to resolve 
merge conflicts
 SYNOPSIS
 
 [verse]
-'git mergetool' [--tool=] [-y | --[no-]prompt] [...]
+'git mergetool' [--tool=] [-y | --[no-]prompt] [-l | --local] [...]
 
 DESCRIPTION
 ---
@@ -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.
 
+-l::
+--local::
+   Change suffix of name of temporary files generated by git
+   mergetool from `_REMOTE_` and `_LOCAL_` to `_OLD_` and `_NEW_`.
+
 TEMPORARY FILES
 ---
 `git mergetool` creates `*.orig` backup files while resolving merges.
diff --git a/git-mergetool.sh b/git-mergetool.sh
index bf86270..1e04368 100755
--- a/git-mergetool.sh
+++ b/git-mergetool.sh
@@ -8,10 +8,11 @@
 # 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] [-l|--local] 
[file to merge] ...'
 SUBDIRECTORY_OK=Yes
 NONGIT_OK=Yes
 OPTIONS_SPEC=
+LOCAL_MODE=false
 TOOL_MODE=merge
 . git-sh-setup
 . git-mergetool--lib
@@ -271,10 +272,19 @@ merge_file () {
BASE=${BASE##*/}
fi
 
-   BACKUP="$MERGETOOL_TMPDIR/${BASE}_BACKUP_$$$ext"
-   LOCAL="$MERGETOOL_TMPDIR/${BASE}_LOCAL_$$$ext"
-   REMOTE="$MERGETOOL_TMPDIR/${BASE}_REMOTE_$$$ext"
-   BASE="$MERGETOOL_TMPDIR/${BASE}_BASE_$$$ext"
+   if test "$LOCAL_MODE"
+   then
+
+   BACKUP="$MERGETOOL_TMPDIR/${BASE}_BACKUP_$$$ext"
+   LOCAL="$MERGETOOL_TMPDIR/${BASE}_NEW_$$$ext"
+   REMOTE="$MERGETOOL_TMPDIR/${BASE}_OLD_$$$ext"
+   BASE="$MERGETOOL_TMPDIR/${BASE}_BASE_$$$ext"
+   else
+   BACKUP="$MERGETOOL_TMPDIR/${BASE}_BACKUP_$$$ext"
+   LOCAL="$MERGETOOL_TMPDIR/${BASE}_LOCAL_$$$ext"
+   REMOTE="$MERGETOOL_TMPDIR/${BASE}_REMOTE_$$$ext"
+   BASE="$MERGETOOL_TMPDIR/${BASE}_BASE_$$$ext"
+   fi
 
base_mode=$(git ls-files -u -- "$MERGED" | awk '{if ($3==1) print $1;}')
local_mode=$(git ls-files -u -- "$MERGED" | awk '{if ($3==2) print 
$1;}')
@@ -396,6 +406,9 @@ do
--prompt)
prompt=true
;;
+   -l|--local)
+   LOCAL_MODE=true
+   ;;
--)
shift
break
-- 
2.7.4


Re: [PATCH v8 11/11] convert: add filter..process option

2016-10-06 Thread Lars Schneider

> On 04 Oct 2016, at 22:50, Jakub Narębski  wrote:
> 
> [Some of answers may get invalidated by v9]
> 
> W dniu 30.09.2016 o 20:56, Lars Schneider pisze:
>>> On 27 Sep 2016, at 00:41, Jakub Narębski  wrote:
>>> 
 +
 +After the filter has processed a blob it is expected to wait for
 +the next "key=value" list containing a command. Git will close
 +the command pipe on exit. The filter is expected to detect EOF
 +and exit gracefully on its own.
> 
> Is this still true?

Yes


>>> 
>>> Good to have it documented.  
>>> 
>>> Anyway, as it is Git command that spawns the filter driver process,
>>> assuming that the filter process doesn't daemonize itself, wouldn't
>>> the operating system reap it after its parent process, that is the
>>> git command it invoked, dies? So detecting EOF is good, but not
>>> strictly necessary for simple filter that do not need to free
>>> its resources, or can leave freeing resources to the operating
>>> system? But I may be wrong here.
>> 
>> The filter process runs independent of Git.
> 
> Ah.  So without some way to tell long-lived filter process that
> it can shut down, because no further data will be incoming, or
> killing it by Git, it would hang indefinitely?

Yes

- Lars

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

2016-10-06 Thread Sergey Organov
Jakub Narębski  writes:

> W dniu 05.10.2016 o 16:46, sorga...@gmail.com pisze:
>> From: Sergey Organov 
>> 
>> Old description had a few problems:
>> 
>> - sounded as if commits have changes
>> 
>> - stated that changes are taken since some "divergence point"
>>   that was not defined.
>> 
>> New description rather uses "common ancestor" and "merge base",
>> definitions of which are easily discoverable in the rest of GIT
>> documentation.
>
> This is a step in a good direction, but it has a few issues.
>
>> 
>> Signed-off-by: Sergey Organov 
>> ---
>>  Documentation/git-merge.txt | 25 +++--
>>  1 file changed, 15 insertions(+), 10 deletions(-)
>> 
>> diff --git a/Documentation/git-merge.txt b/Documentation/git-merge.txt
>> index cc0329d..351b8fc 100644
>> --- a/Documentation/git-merge.txt
>> +++ b/Documentation/git-merge.txt
>> @@ -16,11 +16,16 @@ SYNOPSIS
>>  
>>  DESCRIPTION
>>  ---
>> -Incorporates changes from the named commits (since the time their
>> -histories diverged from the current branch) into the current
>> -branch.  This command is used by 'git pull' to incorporate changes
>> -from another repository and can be used by hand to merge changes
>> -from one branch into another.
>> +
>> +Incorporates changes that lead to the named commits into the current
>> +branch, and joins corresponding histories. The best common ancestor of
>> +named commits and the current branch, called "merge base", is
>> +calculated, and then net changes taken from the merge base to
>> +the named commits are applied.
>
> The first sentence is all right; it reads better than the original
> without the introduced part in parentheses.  The only minor issue
> is with "joins corresponding histories" - it is a good description,
> but may imply that the branch we are merging vanishes: it doesn't.
> But all in all, it is a good change.

I've got "joins corresponding histories" from original NAME section, so
it's likely to be good enough.

>
> Second sentence has some problems.  First, while it is a good idea
> to use well defined term "merge base", I think writing "since the
> time their histories diverged" or "(which is the point where histories
> diverged)" would be a good plain language description; it was removed
> entirely in the proposal.

I was not sure about it myself, but it sounded as if it could be the
case that I might need to resolve the same conflicts again and again, as
changes are taken from some "divergence point" that is apparently fixed.

OTOH, "merge base" not only is well-defined term, but it also doesn't
sound as some fixed point in history.

>
> Second, while "common ancestor" and "least common ancestor" are well
> defined in mathematics of graphs, "best common ancestor" isn't...
> but this is what git-merge-base(1) documentation uses.

That's were I took it from indeed, git-merge-base manual page. I wanted
things we mention to be discoverable.

> Also, the "best common ancestor" doesn't need to be only one.  There
> might be many such ancestors... though Git would generate then a
> virtual best common ancestor thanks to recursive merge strategy.
> And usually there is only one "best common ancestor", that is a single
> merge base.  So this may need clarification, but it is not much of
> a problem.
>
> Third, and most important, is that "net changes taken from the merge
> base to the named commits are applied" is simply not true.  The
> `git merge` command does not reapply changes - that is what rebase
> and cherry-pick do.

"are applied" meant to be generic enough in this context to cover, say,
"-s ours" as well. Perhaps "are integrated" you suggest below, or "are
incorporated" that is already used in the beginning of DESCRIPTION, is
better indeed.

As for merge vs rebase (or cherry-pick), the only essential difference
for me is that merge applies net (cumulative, integrated) changes, while
rebase applies each change sequentially, one commit to another. Please
correct me if I'm wrong.

BTW, one can imagine "rebase" merge strategy: make a new chain of
commits on top of destination, then squash all that to get a new single
merge commit.

> The merge operation uses 3-way merge strategy (diff3) between
> merge-base, current branch, and merged commit. That is, it finds
> differences between differences, and "applies" that.

The problem is that at this point we need to cover other merge
strategies as well, so we can't writ the description as if 3-way merge
is the only strategy.

> See
> "A Formal Investigation of Diff3" paper by Sanjeev Khanna, Keshav
> Kunal, and Benjamin C. Pierce:
> http://www.cis.upenn.edu/~bcpierce/papers/diff3-short.pdf
>
> I'm not sure how to explain it succintly.  Perhaps
>
>   net changes between merge base to the current (merged into)
>   branch and named commits are integrated
>
> There is description of trivial 3-way merge somewhere in Git docs,
> though in very unobvious place; we can link it.


Re: [PATCH v8 00/11] Git filter protocol

2016-10-06 Thread Lars Schneider

> On 04 Oct 2016, at 21:04, Jakub Narębski  wrote:
> 
> W dniu 03.10.2016 o 19:13, Lars Schneider pisze: 
>>> On 01 Oct 2016, at 22:48, Jakub Narębski  wrote:
>>> W dniu 01.10.2016 o 20:59, Lars Schneider pisze: 
 On 29 Sep 2016, at 23:27, Junio C Hamano  wrote:
> Lars Schneider  writes:
> 
> If the filter process refuses to die forever when Git told it to
> shutdown (by closing the pipe to it, for example), that filter
> process is simply buggy.  I think we want users to become aware of
> that, instead of Git leaving it behind, which essentially is to
> sweep the problem under the rug.
>>> 
>>> Well, it would be good to tell users _why_ Git is hanging, see below.
>> 
>> Agreed. Do you think it is OK to write the message to stderr?
> 
> On the other hand, this is why GIT_TRACE (and GIT_TRACE_PERFORMANCE)
> was invented for.  We do not signal troubles with single-shot filters,
> so I guess doing it for multi-file filters is not needed.

I am on the fence with this one.

@Junio/Peff:
Where would you prefer to see a "Waiting for filter 'XYZ'... " message?
On stderr or via GIT_TRACE?


> 
> I agree with what Peff said elsewhere in the thread; if a filter
> process wants to take time to clean things up while letting Git
> proceed, it can do its own process management, but I think it is
> sensible for Git to wait the filter process it directly spawned.
 
 To realize the approach above I prototyped the run-command patch below:
 
 I added an "exit_timeout" variable to the "child_process" struct.
 On exit, Git will close the pipe to the process and wait "exit_timeout" 
 seconds until it kills the child process. If "exit_timeout" is negative
 then Git will wait until the process is done.
>>> 
>>> That might be good approach.  Probably the default would be to wait.
>> 
>> I think I would prefer a 2sec timeout or something as default. This way
>> we can ensure Git would not wait indefinitely for a buggy filter by default.
> 
> Actually this waiting for multi-file filter is only about waiting for
> the shutdown process of the filter.  The filter could still hang during
> processing a file, and git would hang too, if I understand it correctly.

Correct.


>> [...] this function is also used with the async struct... 
> 
> Hmmm... now I wonder if it is a good idea (similar treatment for
> single-file async-invoked filter, and multi-file pkt-line filters).
> 
> For single-file one-shot filter (correct me if I am wrong):
> 
> - git sends contents to filter, signals end with EOF
>   (after process is started)
> - in an async process:
>   - process is started
>   - git reads contents from filter, until EOF
>   - if process did not end, it is killed
> 
> 
> For multi-process pkt-line based filter (simplified):
> 
> - process is started
> - handshake
> - for each file
>   - file is send to filter process over pkt-line,
> end signalled with flush packet
>   - git reads from filter from pkt-line, until flush
> - ...
> 
> 
> See how single-shot filter is sent EOF, though in different part
> of code.  We need to signal multi-file filter that no more files
> will be coming.  Simplest solution is to send EOF (we could send
> "command=shutdown" for example...) to filter, and wait for EOF
> from filter (or for "status=finished" and EOF).

That's what we do. EOF does signal the multi-filter to shutdown.


> For full patch, you would need also to add to Documentation/config.txt

Why config.txt?


Thanks,
Lars

[PATCH 2/2] Feature Request: user defined suffix for temp files created by git-mergetool

2016-10-06 Thread Josef Ridky
This is the second of two variant for request to add option to change
suffix of name of temporary files generated by git mergetool. This
change is requested for cases, when is git mergetool used for local
comparison between two version of same package during package rebase.

Signed-off-by: Josef Ridky 
---
 Documentation/git-mergetool.txt | 26 -
 git-mergetool.sh| 43 -
 2 files changed, 63 insertions(+), 6 deletions(-)

diff --git a/Documentation/git-mergetool.txt b/Documentation/git-mergetool.txt
index e846c2e..6cf5935 100644
--- a/Documentation/git-mergetool.txt
+++ b/Documentation/git-mergetool.txt
@@ -8,7 +8,7 @@ git-mergetool - Run merge conflict resolution tools to resolve 
merge conflicts
 SYNOPSIS
 
 [verse]
-'git mergetool' [--tool=] [-y | --[no-]prompt] [...]
+'git mergetool' [--tool=] [-y | --[no-]prompt] [--local=] 
[--remote=] [--backup=] [--base=] [...]
 
 DESCRIPTION
 ---
@@ -79,6 +79,30 @@ 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.
 
+--local=::
+   Use string from  as part of suffix of name of temporary
+   file (local) for merging. If not used or is equal with any
+   other (remote|backup|base), default value is used.
+   Default suffix is LOCAL.
+
+--remote=::
+   Use string from  as part of suffix of name of temporary
+   file (remote) for merging. If not used or is equal with any
+   other (local|backup|base), default value is used.
+   Default suffix is REMOTE.
+
+--backup=::
+   Use string from  as part of suffix of name of temporary
+   file (backup) for merging. If not used or is equal with any
+   other (local|remote|base), default value is used.
+   Default suffix is BACKUP.
+
+--base=::
+   Use string from  as part of suffix of name of temporary
+   file (base) for merging. If not used or is equal with any
+   other (local|remote|backup), default value is used.
+   Default suffix is BASE.
+
 TEMPORARY FILES
 ---
 `git mergetool` creates `*.orig` backup files while resolving merges.
diff --git a/git-mergetool.sh b/git-mergetool.sh
index bf86270..096ee5e 100755
--- a/git-mergetool.sh
+++ b/git-mergetool.sh
@@ -8,7 +8,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] [--local=name] 
[--remote=name] [--backup=name] [--base=name] [file to merge] ...'
 SUBDIRECTORY_OK=Yes
 NONGIT_OK=Yes
 OPTIONS_SPEC=
@@ -16,6 +16,12 @@ TOOL_MODE=merge
 . git-sh-setup
 . git-mergetool--lib
 
+# Can be changed by user
+LOCAL_NAME='LOCAL'
+BASE_NAME='BASE'
+BACKUP_NAME='BACKUP'
+REMOTE_NAME='REMOTE'
+
 # Returns true if the mode reflects a symlink
 is_symlink () {
test "$1" = 12
@@ -271,10 +277,10 @@ merge_file () {
BASE=${BASE##*/}
fi
 
-   BACKUP="$MERGETOOL_TMPDIR/${BASE}_BACKUP_$$$ext"
-   LOCAL="$MERGETOOL_TMPDIR/${BASE}_LOCAL_$$$ext"
-   REMOTE="$MERGETOOL_TMPDIR/${BASE}_REMOTE_$$$ext"
-   BASE="$MERGETOOL_TMPDIR/${BASE}_BASE_$$$ext"
+   BACKUP="$MERGETOOL_TMPDIR/${BASE}_${BACKUP_NAME}_$$$ext"
+   LOCAL="$MERGETOOL_TMPDIR/${BASE}_${LOCAL_NAME}_$$$ext"
+   REMOTE="$MERGETOOL_TMPDIR/${BASE}_${REMOTE_NAME}_$$$ext"
+   BASE="$MERGETOOL_TMPDIR/${BASE}_${BASE_NAME}_$$$ext"
 
base_mode=$(git ls-files -u -- "$MERGED" | awk '{if ($3==1) print $1;}')
local_mode=$(git ls-files -u -- "$MERGED" | awk '{if ($3==2) print 
$1;}')
@@ -396,6 +402,33 @@ do
--prompt)
prompt=true
;;
+   --local=*)
+   temp_name=${1#--local=}
+   if [ "$temp_name" != "" ] && [ "$temp_name" != "$REMOTE_NAME" ] 
&& [ "$temp_name" != "$BASE_NAME" ] && [ "$temp_name" != "$BACKUP_NAME" ]
+   then
+   LOCAL_NAME=$temp_name
+   fi
+   ;;
+   --remote=*)
+   temp_name=${1#--remote=}
+   if [ "$temp_name" != "" ] && [ "$temp_name" != "$LOCAL_NAME" ] 
&& [ "$temp_name" != "$BASE_NAME" ] && [ "$temp_name" != "$BACKUP_NAME" ]
+   then
+   REMOTE_NAME=$temp_name
+   fi
+   ;;
+   --base=*)
+   temp_name=${1#--base=}
+   if [ "$temp_name" != "" ] && [ "$temp_name" != "$LOCAL_NAME" ] 
&& [ "$temp_name" != "$REMOTE_NAME" ] && [ "$temp_name" != "$BACKUP_NAME" ]
+   then
+   BASE_NAME=$temp_name
+   fi
+   ;;
+   --backup=*)
+   temp_name=${1#--backup=}
+   if [ "$temp_name" != "" ] && [ "$temp_name" != "$LOCAL_NAME" ] 
&& [ "$temp_name" != "$REMOTE_NAME" ] && [ "$temp_name" != 

Re: [PATCH v2 20/25] sequencer: left-trim lines read from the script

2016-10-06 Thread Johannes Schindelin
Hi Junio,

On Mon, 12 Sep 2016, Junio C Hamano wrote:

> Johannes Schindelin  writes:
> 
> >> I do not offhand see why we want to be lenient here,
> >> especially only to the left.
> >
> > Postel's Law.
> 
> How would that compare/relate to yagni, though?

I did need it, though. Blame it on being overworked or simply tired: I
ended up inserting a spurious space before a "fixup" command. This command
was handled as intended by the scripted rebase -i, and with the patch in
question the rebase--helper agreed, too.

Note: we have no problem to the right because we do handle variable-length
whitespace between command and SHA-1, and we do not care one iota about
the exact form of the oneline (hence right-stripping is not necessary).

Ciao,
Dscho


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

2016-10-06 Thread Sergey Organov
Junio C Hamano  writes:

> Sergey Organov  writes:
>
>> So I believe this change is inline with the rest of the patch. The
>> reference to git-pull (if it remains) should be a side-note, not part of
>> explanation of operation.
>
> Not really.  The thing is, "This is the most common" needs to be
> close to "Often...".  "git merge" directly invoked by the end user
> is much less likely to encounter a fast forward situation; getting
> invoked indirectly by "git pull" makes it common.

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:

diff --git a/Documentation/git-merge.txt b/Documentation/git-merge.txt
index b758d55..479400f 100644
--- a/Documentation/git-merge.txt
+++ b/Documentation/git-merge.txt
@@ -135,15 +135,17 @@ will exit early with the message "Already up-to-date."
 FAST-FORWARD MERGE
 --
 
-Often the current branch head is an ancestor of the named commit.
-This is the most common case especially when invoked from 'git
-pull': you are tracking an upstream repository, you have committed
-no local changes, and now you want to update to a newer upstream
-revision.  In this case, a new commit is not needed to store the
+When current branch head is an ancestor of the named commit,
+a new commit is not needed to store the
 combined history; instead, the `HEAD` (along with the index) is
 updated to point at the named commit, without creating an extra
 merge commit.
 
+This is very common case when 'git merge' is invoked from 'git
+pull': you are tracking an upstream repository, you have committed
+no local changes, and now you want to update to a newer upstream
+revision.  
+
 This behavior can be suppressed with the `--no-ff` option.
 
 TRUE MERGE

-- Sergey



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

2016-10-06 Thread Sergey Organov
Junio C Hamano  writes:

> Sergey Organov  writes:
>
>> OK, I see. So, what is the best way to handle this? Immediately follow
>> content change patch with another patch that only re-flows?
>
> Or no reflowing at all.
>
>>> the parents".  I do not know if the updated phrasing is better.  The
>>> "name" in the original was meant to be a short-hand for "object name",
>>> and I would support a change to spell it out to clarify; "reference"
>>> can be a vague word that can mean different things in Git, and when
>>> the word is given without context, most Git people would think that
>>> the word refers to "refs", but that is definitely not what the new
>>> commit records, so...
>>
>> I won't insist on the change, but "name" sounded wrong to me, and
>> "reference" was most general term I was able to come up with in this
>> context.
>> ...
>> 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 resulting merge commit is an origin for 2 chains, so it stores 2
references to its successors. No need to be aware of any [object] names
to understand all this.

-- Sergey


Re: [PATCH] push: Re-include "push.default=tracking" in the documentation

2016-10-06 Thread Matthieu Moy
Ævar Arnfjörð Bjarmason  writes:

> That's bad, either we shouldn't support it at all, or we should
> document what it does. This patch does the latter.

I vaguely remember a similar discussion and probably even a patch in the
past (maybe by you actually). I think the proposal was to add a mention
of tracking but avoid promoting it at the same level as the others.

I have a slight preference for a patch adding stg like "`tracking` is a
deprecated alias supported only for backward compatibility" to the item
of `upstream`, but I'm OK with the current patch too.

> --- a/Documentation/config.txt
> +++ b/Documentation/config.txt
> @@ -2344,6 +2344,10 @@ push.default::
>pushing to the same repository you would normally pull from
>(i.e. central workflow).
>  
> +* `tracking` - Deprecated synonym for `upstream`, which we still
> +  support for backwards compatibility with existing configuration
> +  files.

Nit: I think the doc normally doesn't use "we" this way (we = the Git
developers or the Git tool). Hence my s/which we still support/still
supported/ above.

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


Dearest Chosen One

2016-10-06 Thread Mrs. Amina
Dearest Chosen One

With Due Respect And Humanity,I was compelled to write to you under a 
humanitarian ground. My name is Mrs Amina Mohammed, My Husband was a director 
in a trading company here in Cote D Ivoire.We were married for 36 years without 
a child.  He died after Cardiac Arteries operation.

And Recently,My Doctor told me that I would not last for the next Eight months 
due to my cancer problem and stroke) .Before my husband died he lodged the sum 
of four million five hundred thousand dollars,(US$15,500.000.) in a bank here 
In Ivory Coast.

Presently,this money is still in a bank here. Having known my condition I 
decided to donate this fund to any good God fearing man or woman that will 
utilize this fund the way I am going to instruct herein.

I want somebody that will use this fund according to the desire of my late 
husband to help Less privilaged people,orphanages,widows and propagating the 
word of God.

I took this decision because I don't have any child that will inherit this 
fund,And I don't want in away where this money will be used in an unGodly way. 
This is why I am taking this decision to hand you over this Fund.

I am not afraid of death hence I know where I am going. I want you to always 
remember me in your daily prayers because of my up coming Cancer Surgery.

Write back as soon as possible any delay in your reply will give me room in 
sourcing another person for this same purpose.

God bless you as I hope to hear from you as soon.

Yours Sincerely Sister

Amina Mohammed


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

2016-10-06 Thread Nguyễn Thái Ngọc Duy
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.

Signed-off-by: Nguyễn Thái Ngọc Duy 
---
 git.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/git.c b/git.c
index 296857a..4c1dcf4 100644
--- a/git.c
+++ b/git.c
@@ -252,6 +252,10 @@ static int handle_alias(int *argcp, const char ***argv)
 
alias_string++;
commit_pager_choice();
+   if (*alias_string == '!') {
+   keep_cwd = 0;
+   alias_string++;
+   }
restore_env(keep_cwd);
 
child.use_shell = 1;
-- 
2.8.2.524.g6ff3d78



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

2016-10-06 Thread Johannes Schindelin
Hi James,

On Wed, 5 Oct 2016, James B wrote:

> On Wed, 5 Oct 2016 12:41:50 +0200 (CEST)
> Johannes Schindelin  wrote:
> 
> It's a very sad day for a tool that was developed originally to maintain
> Linux kernel, by the Linux kernel author, now is restricted to avoid
> use/optimise on Linux/POSIX features *because* it has to run on another
> Windows ...

Please note that this thread started because Git tried to shed the
restrictions of POSIX by using REG_STARTEND. In other words, we tried very
much *not* to be restricted.

And pragmatically, I must add that the REG_STARTEND feature is a very,
very useful one.

If you want to turn your sadness into something productive (you will allow
me that little prod after all you said in your mail), why don't you
dig back through the Git mailing list, fish out my original patch that
fell back to the "malloc();memcpy();/*add NUL*/" strategy, and contribute
that as a patch to the Git project, making a case that musl requires it?

Ciao,
Johannes


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

2016-10-06 Thread Johannes Schindelin
Hi Kuba,

On Wed, 5 Oct 2016, Jakub Narębski wrote:

> W dniu 04.10.2016 o 15:06, Johannes Schindelin pisze:
> 
> > The previous code still followed the old git-pull.sh code which did not
> > adhere to our new convention.
> 
> Good to know why it used its own convention.

Yeah, I figured that it is late, but still a good thing to explain this...

> > diff --git a/builtin/pull.c b/builtin/pull.c
> > index c639167..0bf9802 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);
> >  
> 
> Shouldn't those also be marked for translation with N_() or _()?

Of course!

Thanks,
Dscho

Re: [PATCH] run-command: fix build on cygwin (stdin is a macro)

2016-10-06 Thread Johannes Schindelin
Hi,

On Thu, 6 Oct 2016, Torsten Bögershausen wrote:

> >I am not suggesting that you apply this exact patch (stdin_ is not a good
> choice
> 
> How about fd_stdin ?

Better: stdin_fd. Why? Prior art:

$ git grep -c stdin_fd
builtin/remote-ext.c:3

$ git grep -c fd_stdin

Ciao,
Dscho

Re: [PATCH] push: Re-include "push.default=tracking" in the documentation

2016-10-06 Thread Johan Herland
On Thu, Oct 6, 2016 at 10:49 AM, Ævar Arnfjörð Bjarmason
 wrote:
> Change the documentation for push.tracking=* to re-include a mention
> of what "tracking" does.
>
> The "tracking" option was renamed to "upstream" back in
> 53c4031 ("push.default: Rename 'tracking' to 'upstream'", 2011-02-16),
> this section was then subsequently rewritten in 87a70e4 ("config doc:
> rewrite push.default section", 2013-06-19) to remove any mention of
> "tracking".
>
> Maybe we should just warn or die nowadays if this option is in the
> config, but I had some old config of mine use this option, I'd
> forgotten that it was a synonym, and nothing in git's documentation
> mentioned that.
>
> That's bad, either we shouldn't support it at all, or we should
> document what it does. This patch does the latter.
>
> Signed-off-by: Ævar Arnfjörð Bjarmason 

Acked-by: Johan Herland 


-- 
Johan Herland, 
www.herland.net


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

2016-10-06 Thread Richard Lloyd

git ships with a compat/regex tree containing a recent regex
release, presumably with the intention of allowing systems with
either no regex or an old regex installed to use the newer compat
version.

With the release of git-2.10.1, the use of a recent regex
is now specifically checked in git-compat-util.h via a
#ifndef REG_STARTEND check at line 979.

Unfortunately, on systems with an older regex shipped as
standard (e.g. HP-UX 11), the include path picks up
/usr/include/regex.h first, which doesn't define REG_STARTEND
and the git-compat-util.h check fails.

The fix I applied on HP-UX 11 was to add -Icompat/regex
to the CFLAGS ahead of other -I directives. Another possible
change needed might be to line 69 of compat/regex/regex.c:

#include "regex.h"

This is to ensure that /usr/include/regex.h isn't picked up
(since git ships its own regex.h in the compat/regex
dir, it probably shouldn't #include  anyway,
which risks picking up an incompatible regex.h).

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



--
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


Re: [PATCH v9 04/14] run-command: add wait_on_exit

2016-10-06 Thread Johannes Schindelin
Hi Junio & Lars,

On Wed, 5 Oct 2016, Junio C Hamano wrote:

> Lars Schneider  writes:
> 
> > OK. Something like the patch below would work nicely.
> 
> Yeah, something along that line; it would eliminate the need to
> worry about a field named "stdin" ;-)

Not only a need to worry. Git for Windows' SDK's headers define

#define stdin (&__iob_func()[0])

leading to the compile error

In file included from git-compat-util.h:159:0,
 from cache.h:4,
 from run-command.c:1:
run-command.c:25:6: error: expected identifier or '(' before '&' token
  int stdin;
  ^

I meant to investigate this build failure of `pu` earlier but only got
around to do it today.

Ciao,
Dscho


Re: [PATCH v9 04/14] run-command: add wait_on_exit

2016-10-06 Thread Lars Schneider

> On 06 Oct 2016, at 11:32, Johannes Schindelin  
> wrote:
> 
> Hi Junio & Lars,
> 
> On Wed, 5 Oct 2016, Junio C Hamano wrote:
> 
>> Lars Schneider  writes:
>> 
>>> OK. Something like the patch below would work nicely.
>> 
>> Yeah, something along that line; it would eliminate the need to
>> worry about a field named "stdin" ;-)
> 
> Not only a need to worry. Git for Windows' SDK's headers define
> 
>   #define stdin (&__iob_func()[0])
> 
> leading to the compile error
> 
>   In file included from git-compat-util.h:159:0,
> from cache.h:4,
> from run-command.c:1:
>   run-command.c:25:6: error: expected identifier or '(' before '&' token
> int stdin;
> ^
> 
> I meant to investigate this build failure of `pu` earlier but only got
> around to do it today.
> 
> Ciao,
> Dscho

Sorry for the trouble. The "stdin" will go away in the next round
as we agreed on a more generic solution:
http://public-inbox.org/git/1fd7fb64-0f40-47f0-a047-25b91b170...@gmail.com/

- Lars



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

2016-10-06 Thread Heiko Voigt
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?

Cheers Heiko


Format for %d includes unwanted preceding space

2016-10-06 Thread Ravi (Tom) Hale

The log --format="%d" includes preceding space:

$ git log -n1 --format="XX%dXX" HEAD
XX (HEAD -> master)XX

I know of no other % that is output in this way.

Is there a way of removing this preceding space through the format 
string, or will it need to be a code change?


--
Tom Hale


Re: Bug Report: "git submodule deinit" fails right after a clone

2016-10-06 Thread Heiko Voigt
Hi,

please also keep the mailinglist in the CC so everyone can read this.

On Thu, Oct 06, 2016 at 09:11:05AM +0200, Thomas Bétous wrote:
> On Wed, Oct 5, 2016 at 3:36 PM, Heiko Voigt  wrote:
> 
> >
> > My initial reaction is that this might be a problem with line endings. Did
> > you
> > check whether you get any diff when you do a 'git diff' after the clone?
> >
> > Maybe the variable 'core.autocrlf' is set to 'input' ? Have a look at 'git
> > help
> > config'
> 
> 
> When I do a 'git diff' right after the clone, nothing appears.
> Moreover my global setting for core.autocrlf is true. (This was configured
> on purpose as I work on Windows whereas the repositories are hosted on an
> UNIX server.)

So I guess the same applies to 'git status'?

> Nevertheless when I change core.autocrlf to 'input', the error disappears
> and I got the expected behavior for git submodule deinit...
> So I guess it is just a configuration problem but I do not understand why
> core.autocrlf should be set to 'input' to remove this error. Do you have
> any idea?

This is indeed strange. That's why I asked whether 'git diff' shows
anything. I was suspecting that the .gitmodules is somehow checked out
in UNIX format. And the fact that setting core.autocrlf to
'input' seems to fix it supports that.

I currently do not have access to a windows machine as the moment to
test this. Copying the windows mailing list maybe someone over there
can reproduce and help with the issue[1].

Cheers Heiko

[1] 
http://public-inbox.org/git/%3ccapoqyv+xsrlk7y1hjyhzfy8ofkxvrwpczbdqhdgrhthqdzy...@mail.gmail.com%3E/


[PATCH v2 2/3] gitweb: Link to 7-char+ SHA1s, not only 8-char+

2016-10-06 Thread Ævar Arnfjörð Bjarmason
Change the minimum length of an abbreviated object identifier in the
commit message gitweb tries to turn into link from 8 hexchars to 7.

This arbitrary minimum length of 8 was introduced in bfe2191 ("gitweb:
SHA-1 in commit log message links to "object" view", 2006-12-10), but
the default abbreviation length is 7, and has been for a long time.

It's still possible to reference SHA1s down to 4 characters in length,
see v1.7.4-1-gdce9648's MINIMUM_ABBREV, but I can't see how to make
git actually produce that, so I doubt anyone is putting that into log
messages in practice, but people definitely do put 7 character SHA1s
into log messages.

I think it's fairly dubious to link to things matching [0-9a-fA-F]
here as opposed to just [0-9a-f], that dates back to the initial
version of gitweb from 161332a ("first working version",
2005-08-07). Git will accept all-caps SHA1s, but didn't ever produce
them as far as I can tell.

Signed-off-by: Ævar Arnfjörð Bjarmason 
---
 gitweb/gitweb.perl | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index cba7405..92b5e91 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -2036,7 +2036,7 @@ sub format_log_line_html {
my $line = shift;
 
$line = esc_html($line, -nbsp=>1);
-   $line =~ s{\b([0-9a-fA-F]{8,40})\b}{
+   $line =~ s{\b([0-9a-fA-F]{7,40})\b}{
$cgi->a({-href => href(action=>"object", hash=>$1),
-class => "text"}, $1);
}eg;
-- 
2.9.3



[PATCH v2 0/3] gitweb: Be smarter about linking to SHA1s in log messages

2016-10-06 Thread Ævar Arnfjörð Bjarmason
This is v2 of patches I sent on September 21st starting at
<20160921114428.28664-1-ava...@gmail.com>. Jakub Narębski had a lot of
feedback for that series (thanks!). Which as far as I can tell I've
incorporated entirely in this re-roll.

Ævar Arnfjörð Bjarmason (3):
  gitweb: Fix a typo in a comment
  gitweb: Link to 7-char+ SHA1s, not only 8-char+
  gitweb: Link to "git describe"'d commits in log messages

 gitweb/gitweb.perl | 20 +---
 1 file changed, 17 insertions(+), 3 deletions(-)

-- 
2.9.3



[PATCH v2 3/3] gitweb: Link to "git describe"'d commits in log messages

2016-10-06 Thread Ævar Arnfjörð Bjarmason
Change the log formatting function to know about "git describe" output
such as "v2.8.0-4-g867ad08", in addition to just plain "867ad08".

There are still many valid refnames that we don't link to
e.g. v2.10.0-rc1~2^2~1 is also a valid way to refer to
v2.8.0-4-g867ad08, but I'm not supporting that with this commit,
similarly it's trivially possible to create some refnames like
"æ/var-gf6727b0" or which won't be picked up by this regex.

There's surely room for improvement here, but I just wanted to address
the very common case of sticking "git describe" output into commit
messages without trying to link to all possible refnames, that's going
to be a rather futile exercise given that this is free text, and it
would be prohibitively expensive to look up whether the references in
question exist in our repository.

There was on-list discussion about how we could do better than this
patch. Junio suggested to update parse_commits() to call a new
"gitweb--helper" command which would pass each of the revision
candidates through "rev-parse --verify --quiet". That would cut down
on our false positives (e.g. we'll link to "deadbeef"), and also allow
us to be more aggressive in selecting candidate revisions.

That may be too expensive to work in practice, or it may
not. Investigating that would be a good follow-up to this patch.

Signed-off-by: Ævar Arnfjörð Bjarmason 
---
 gitweb/gitweb.perl | 18 --
 1 file changed, 16 insertions(+), 2 deletions(-)

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 92b5e91..7cf68f0 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -2036,10 +2036,24 @@ sub format_log_line_html {
my $line = shift;
 
$line = esc_html($line, -nbsp=>1);
-   $line =~ s{\b([0-9a-fA-F]{7,40})\b}{
+   $line =~ s{
+\b
+(
+# The output of "git describe", e.g. v2.10.0-297-gf6727b0
+# or hadoop-20160921-113441-20-g094fb7d
+(?a({-href => href(action=>"object", hash=>$1),
-class => "text"}, $1);
-   }eg;
+   }egx;
 
return $line;
 }
-- 
2.9.3



[PATCH v2 1/3] gitweb: Fix a typo in a comment

2016-10-06 Thread Ævar Arnfjörð Bjarmason
Change a typo'd MIME type in a comment. The Content-Type is
application/xhtml+xml, not application/xhtm+xml.

Fixes up code originally added in 53c4031 ("gitweb: Strip
non-printable characters from syntax highlighter output", 2011-09-16).

Signed-off-by: Ævar Arnfjörð Bjarmason 
---
 gitweb/gitweb.perl | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 44094f4..cba7405 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -1616,7 +1616,7 @@ sub esc_path {
return $str;
 }
 
-# Sanitize for use in XHTML + application/xml+xhtm (valid XML 1.0)
+# Sanitize for use in XHTML + application/xml+xhtml (valid XML 1.0)
 sub sanitize {
my $str = shift;
 
-- 
2.9.3



[PATCH] git-gui: Update translation template

2016-10-06 Thread Dimitriy Ryazantcev
Signed-off-by: Dimitriy Ryazantcev 
---
 po/git-gui.pot | 2203 +++-
 1 file changed, 1205 insertions(+), 998 deletions(-)

diff --git a/po/git-gui.pot b/po/git-gui.pot
index 0c94f9c..634af68 100644
--- a/po/git-gui.pot
+++ b/po/git-gui.pot
@@ -8,41 +8,42 @@ msgid ""
 msgstr ""
 "Project-Id-Version: PACKAGE VERSION\n"
 "Report-Msgid-Bugs-To: \n"
-"POT-Creation-Date: 2010-01-26 15:47-0800\n"
+"POT-Creation-Date: 2016-10-06 12:00+0300\n"
 "PO-Revision-Date: YEAR-MO-DA HO:MI+ZONE\n"
 "Last-Translator: FULL NAME \n"
 "Language-Team: LANGUAGE \n"
+"Language: \n"
 "MIME-Version: 1.0\n"
 "Content-Type: text/plain; charset=CHARSET\n"
 "Content-Transfer-Encoding: 8bit\n"
 
-#: git-gui.sh:41 git-gui.sh:793 git-gui.sh:807 git-gui.sh:820 git-gui.sh:903
-#: git-gui.sh:922
-msgid "git-gui: fatal error"
-msgstr ""
-
-#: git-gui.sh:743
+#: git-gui.sh:865
 #, tcl-format
 msgid "Invalid font specified in %s:"
 msgstr ""
 
-#: git-gui.sh:779
+#: git-gui.sh:919
 msgid "Main Font"
 msgstr ""
 
-#: git-gui.sh:780
+#: git-gui.sh:920
 msgid "Diff/Console Font"
 msgstr ""
 
-#: git-gui.sh:794
+#: git-gui.sh:935 git-gui.sh:949 git-gui.sh:962 git-gui.sh:1052 git-gui.sh:1071
+#: git-gui.sh:3147
+msgid "git-gui: fatal error"
+msgstr ""
+
+#: git-gui.sh:936
 msgid "Cannot find git in PATH."
 msgstr ""
 
-#: git-gui.sh:821
+#: git-gui.sh:963
 msgid "Cannot parse Git version string:"
 msgstr ""
 
-#: git-gui.sh:839
+#: git-gui.sh:988
 #, tcl-format
 msgid ""
 "Git version cannot be determined.\n"
@@ -54,473 +55,501 @@ msgid ""
 "Assume '%s' is version 1.5.0?\n"
 msgstr ""
 
-#: git-gui.sh:1128
+#: git-gui.sh:1285
 msgid "Git directory not found:"
 msgstr ""
 
-#: git-gui.sh:1146
+#: git-gui.sh:1319
 msgid "Cannot move to top of working directory:"
 msgstr ""
 
-#: git-gui.sh:1154
+#: git-gui.sh:1327
 msgid "Cannot use bare repository:"
 msgstr ""
 
-#: git-gui.sh:1162
+#: git-gui.sh:1335
 msgid "No working directory"
 msgstr ""
 
-#: git-gui.sh:1334 lib/checkout_op.tcl:306
+#: git-gui.sh:1507 lib/checkout_op.tcl:306
 msgid "Refreshing file status..."
 msgstr ""
 
-#: git-gui.sh:1390
+#: git-gui.sh:1567
 msgid "Scanning for modified files ..."
 msgstr ""
 
-#: git-gui.sh:1454
+#: git-gui.sh:1645
 msgid "Calling prepare-commit-msg hook..."
 msgstr ""
 
-#: git-gui.sh:1471
+#: git-gui.sh:1662
 msgid "Commit declined by prepare-commit-msg hook."
 msgstr ""
 
-#: git-gui.sh:1629 lib/browser.tcl:246
+#: git-gui.sh:1820 lib/browser.tcl:252
 msgid "Ready."
 msgstr ""
 
-#: git-gui.sh:1787
+#: git-gui.sh:1984
 #, tcl-format
-msgid "Displaying only %s of %s files."
+msgid ""
+"Display limit (gui.maxfilesdisplayed = %s) reached, not showing all %s files."
 msgstr ""
 
-#: git-gui.sh:1913
+#: git-gui.sh:2107
 msgid "Unmodified"
 msgstr ""
 
-#: git-gui.sh:1915
+#: git-gui.sh:2109
 msgid "Modified, not staged"
 msgstr ""
 
-#: git-gui.sh:1916 git-gui.sh:1924
+#: git-gui.sh:2110 git-gui.sh:2122
 msgid "Staged for commit"
 msgstr ""
 
-#: git-gui.sh:1917 git-gui.sh:1925
+#: git-gui.sh:2111 git-gui.sh:2123
 msgid "Portions staged for commit"
 msgstr ""
 
-#: git-gui.sh:1918 git-gui.sh:1926
+#: git-gui.sh:2112 git-gui.sh:2124
 msgid "Staged for commit, missing"
 msgstr ""
 
-#: git-gui.sh:1920
+#: git-gui.sh:2114
 msgid "File type changed, not staged"
 msgstr ""
 
-#: git-gui.sh:1921
+#: git-gui.sh:2115 git-gui.sh:2116
+msgid "File type changed, old type staged for commit"
+msgstr ""
+
+#: git-gui.sh:2117
 msgid "File type changed, staged"
 msgstr ""
 
-#: git-gui.sh:1923
+#: git-gui.sh:2118
+msgid "File type change staged, modification not staged"
+msgstr ""
+
+#: git-gui.sh:2119
+msgid "File type change staged, file missing"
+msgstr ""
+
+#: git-gui.sh:2121
 msgid "Untracked, not staged"
 msgstr ""
 
-#: git-gui.sh:1928
+#: git-gui.sh:2126
 msgid "Missing"
 msgstr ""
 
-#: git-gui.sh:1929
+#: git-gui.sh:2127
 msgid "Staged for removal"
 msgstr ""
 
-#: git-gui.sh:1930
+#: git-gui.sh:2128
 msgid "Staged for removal, still present"
 msgstr ""
 
-#: git-gui.sh:1932 git-gui.sh:1933 git-gui.sh:1934 git-gui.sh:1935
-#: git-gui.sh:1936 git-gui.sh:1937
+#: git-gui.sh:2130 git-gui.sh:2131 git-gui.sh:2132 git-gui.sh:2133
+#: git-gui.sh:2134 git-gui.sh:2135
 msgid "Requires merge resolution"
 msgstr ""
 
-#: git-gui.sh:1972
+#: git-gui.sh:2170
 msgid "Starting gitk... please wait..."
 msgstr ""
 
-#: git-gui.sh:1984
+#: git-gui.sh:2182
 msgid "Couldn't find gitk in PATH"
 msgstr ""
 
-#: git-gui.sh:2043
+#: git-gui.sh:2241
 msgid "Couldn't find git gui in PATH"
 msgstr ""
 
-#: git-gui.sh:2455 lib/choose_repository.tcl:36
+#: git-gui.sh:2676 lib/choose_repository.tcl:41
 msgid "Repository"
 msgstr ""
 
-#: git-gui.sh:2456
+#: git-gui.sh:2677
 msgid "Edit"
 msgstr ""
 
-#: git-gui.sh:2458 lib/choose_rev.tcl:561
+#: git-gui.sh:2679 lib/choose_rev.tcl:567
 msgid "Branch"
 msgstr ""
 
-#: git-gui.sh:2461 lib/choose_rev.tcl:548
+#: git-gui.sh:2682 

[PATCH] push: Re-include "push.default=tracking" in the documentation

2016-10-06 Thread Ævar Arnfjörð Bjarmason
Change the documentation for push.tracking=* to re-include a mention
of what "tracking" does.

The "tracking" option was renamed to "upstream" back in
53c4031 ("push.default: Rename 'tracking' to 'upstream'", 2011-02-16),
this section was then subsequently rewritten in 87a70e4 ("config doc:
rewrite push.default section", 2013-06-19) to remove any mention of
"tracking".

Maybe we should just warn or die nowadays if this option is in the
config, but I had some old config of mine use this option, I'd
forgotten that it was a synonym, and nothing in git's documentation
mentioned that.

That's bad, either we shouldn't support it at all, or we should
document what it does. This patch does the latter.

Signed-off-by: Ævar Arnfjörð Bjarmason 
---
 Documentation/config.txt | 4 
 1 file changed, 4 insertions(+)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 32f065c..f30d52a 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -2344,6 +2344,10 @@ push.default::
   pushing to the same repository you would normally pull from
   (i.e. central workflow).
 
+* `tracking` - Deprecated synonym for `upstream`, which we still
+  support for backwards compatibility with existing configuration
+  files.
+
 * `simple` - in centralized workflow, work like `upstream` with an
   added safety to refuse to push if the upstream branch's name is
   different from the local one.
-- 
2.9.3



Re: Feature Request: user defined suffix for temp files created by git-mergetool

2016-10-06 Thread Josef Ridky
Hi Johannes,

thank you very much for this reply.

| Sent: Wednesday, October 5, 2016 11:04:17 PM
| 
| Am 05.10.2016 um 09:47 schrieb Josef Ridky:
| > Add support for user defined suffix part of name of temporary files
| > created by git mergetool
| 
| Do I understand correctly that your users have problems to identify
| which of the "_BASE_", "_LOCAL_", "_REMOTE_" and "_BACKUP_" files they
| must edit? I agree that there is some room for improvement.
| 
| The goal is that you want the user to see the label, e.g., "_EDIT_THIS_"
| instead of "_LOCAL_". Now you have to teach your users that they have to
| pass --local=_EDIT_THIS_. Why don't you just teach your users to edit
| the file labeled "_LOCAL_"?

The goal of this request is not to teach our users, how to change labels using 
git mergetool.
The point is, that git mergetool is used by several of our packages in specific 
use-case. 

If I understand well, git mergetool expecting comparing LOCAL changes against 
REMOTE status in git repository.
We use git mergetool to compare LOCAL OLD version of package with LOCAL NEW 
version of package.

In this use-case is really nonsense to label old version of package as LOCAL 
and new version of package as REMOTE.
This is point of confusion. This is the reason, why I am asking for the 
possibility to change the suffix of temporary files, which are shown to user 
for comparison.

And as well, users do not open these files themselves, these files are 
automatically opened by our packages, so users are not responsible for the name 
of temporary files. They just see the results.

| 
| Therefore, I think that your patch as written does not help to reduce
| the confusion. It may be a building block for further improvement, but
| if you stop here, it is pointless.
| 

I agree, that this patch is written as general as possible and can possibly 
bring more confusion than benefits.
I will make new version of this patch, where will be just simple switch between 
LOCAL/REMOTE and NEW/OLD.
 
| >  SYNOPSIS
| >  
| >  [verse]
| > -'git mergetool' [--tool=] [-y | --[no-]prompt] [...]
| > +'git mergetool' [--tool=] [-y | --[no-]prompt] [--local=]
| > [--remote=] [--backup=] [--base=] [...]
| >
| >  DESCRIPTION
| >  ---
| > @@ -79,6 +79,30 @@ 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.
| >
| > +--local=::
| > +   Use string from  as part of suffix of name of temporary
| > +   file (local) for merging. If not used or is equal with any
| > +   other (remote|backup|base), default value is used.
| > +   Default suffix is LOCAL.
| > +
| > +--remote=::
| > +   Use string from  as part of suffix of name of temporary
| > +   file (remote) for merging. If not used or is equal with any
| > +   other (local|backup|base), default value is used.
| > +   Default suffix is REMOTE.
| > +
| > +--backup=::
| > +   Use string from  as part of suffix of name of temporary
| > +   file (backup) for merging. If not used or is equal with any
| > +   other (local|remote|base), default value is used.
| > +   Default suffix is BACKUP.
| > +
| > +--base=::
| > +   Use string from  as part of suffix of name of temporary
| > +   file (base) for merging. If not used or is equal with any
| > +   other (local|remote|backup), default value is used.
| > +   Default suffix is BASE.
| 
| 


Re: Feature Request: user defined suffix for temp files created by git-mergetool

2016-10-06 Thread Josef Ridky
Hi Junio,

thank you very much for the tips. I'll make new patch with a simpler code.

Josef

| Sent: Wednesday, October 5, 2016 6:05:59 PM
| Josef Ridky  writes:
| 
| > Hi,
| >
| > I have just realize, that my attachment has been cut off from my previous
| > message.
| > Below you can find patch with requested change.
| >
| > Add support for user defined suffix part of name of temporary files
| > created by git mergetool
| > ---
| 
| The first two paragraphs above do not look like they are meant for
| the commit log for this change.
| 
| Please sign-off your patch.
| 
| > -USAGE='[--tool=tool] [--tool-help] [-y|--no-prompt|--prompt] [file to
| > merge] ...'
| > +USAGE='[--tool=tool] [--tool-help] [-y|--no-prompt|--prompt]
| > [--local=name] [--remote=name] [--backup=name] [--base=name] [file to
| > merge] ...'
| >  SUBDIRECTORY_OK=Yes
| >  NONGIT_OK=Yes
| >  OPTIONS_SPEC=
| >  TOOL_MODE=merge
| > +
| > +#optional name space convention
| > +local_name=""
| > +remote_name=""
| > +base_name=""
| > +backup_name=""
| 
| If you initialize these to LOCAL, REMOTE, etc. instead of empty,
| wouldn't it make the remainder of the code a lot simpler?  For
| example,
| 
| > +   if [ "$local_name" != "" ]  && [ "$local_name" != "$remote_name" ] && [
| > "$local_name" != "$backup_name" ] && [ "$local_name" != "$base_name" ]
| > +   then
| > +   LOCAL="$MERGETOOL_TMPDIR/${BASE}_${local_name}_$$$ext"
| > +   else
| > +   LOCAL="$MERGETOOL_TMPDIR/${BASE}_LOCAL_$$$ext"
| > +   fi
| 
| This can just be made an unconditional
| 
|   LOCAL="$MERGETOOL_TMPDIR/${BASE}_${local_name}_$$$ext"
| 
| without any "if" check in front.  The same for all others.
| 
| The conditional you added is doing two unrelated things.  It is
| trying to switch between an unset $local_name and default LOCAL,
| while it tries to make sure the user did not give the same string
| for two different things (which is a nonsense).  It is probably
| better to check for nonsense just once just before all these
| assuments of LOCAL, REMOTE, etc. begins, not at each point where
| they are set like this patch does.
|