[PATCH 1/1] rebase: fix GIT_REFLOG_ACTION regression

2018-11-29 Thread Johannes Schindelin via GitGitGadget
From: Johannes Schindelin 

The scripted version (partially) heeded the `GIT_REFLOG_ACTION` and when
we converted to a built-in, this regressed.

Fix that, and add a regression test, both with `GIT_REFLOG_ACTION` set
and unset.

Note: the reflog message for "rebase finished" did *not* heed
GIT_REFLOG_ACTION, and as we are very late in the v2.20.0-rcN phase, we
leave that bug for later (as it seems that that bug has been with us
from the very beginning).

Reported by Ian Jackson.

Signed-off-by: Johannes Schindelin 
---
 builtin/rebase.c  | 29 ++---
 t/t3406-rebase-message.sh | 26 ++
 2 files changed, 52 insertions(+), 3 deletions(-)

diff --git a/builtin/rebase.c b/builtin/rebase.c
index 5b3e5baec8..ba0c3c954b 100644
--- a/builtin/rebase.c
+++ b/builtin/rebase.c
@@ -776,6 +776,23 @@ static void NORETURN 
error_on_missing_default_upstream(void)
exit(1);
 }
 
+static void set_reflog_action(struct rebase_options *options)
+{
+   const char *env;
+   struct strbuf buf = STRBUF_INIT;
+
+   if (!is_interactive(options))
+   return;
+
+   env = getenv(GIT_REFLOG_ACTION_ENVIRONMENT);
+   if (env && strcmp("rebase", env))
+   return; /* only override it if it is "rebase" */
+
+   strbuf_addf(, "rebase -i (%s)", options->action);
+   setenv(GIT_REFLOG_ACTION_ENVIRONMENT, buf.buf, 1);
+   strbuf_release();
+}
+
 int cmd_rebase(int argc, const char **argv, const char *prefix)
 {
struct rebase_options options = {
@@ -978,6 +995,7 @@ int cmd_rebase(int argc, const char **argv, const char 
*prefix)
 
if (action != NO_ACTION && !in_progress)
die(_("No rebase in progress?"));
+   setenv(GIT_REFLOG_ACTION_ENVIRONMENT, "rebase", 0);
 
if (action == ACTION_EDIT_TODO && !is_interactive())
die(_("The --edit-todo action can only be used during "
@@ -990,6 +1008,7 @@ int cmd_rebase(int argc, const char **argv, const char 
*prefix)
int fd;
 
options.action = "continue";
+   set_reflog_action();
 
/* Sanity check */
if (get_oid("HEAD", ))
@@ -1018,6 +1037,7 @@ int cmd_rebase(int argc, const char **argv, const char 
*prefix)
struct string_list merge_rr = STRING_LIST_INIT_DUP;
 
options.action = "skip";
+   set_reflog_action();
 
rerere_clear(_rr);
string_list_clear(_rr, 1);
@@ -1033,6 +1053,7 @@ int cmd_rebase(int argc, const char **argv, const char 
*prefix)
case ACTION_ABORT: {
struct string_list merge_rr = STRING_LIST_INIT_DUP;
options.action = "abort";
+   set_reflog_action();
 
rerere_clear(_rr);
string_list_clear(_rr, 1);
@@ -1440,11 +1461,12 @@ int cmd_rebase(int argc, const char **argv, const char 
*prefix)
}
 
strbuf_reset();
-   strbuf_addf(, "rebase: checkout %s",
+   strbuf_addf(, "%s: checkout %s",
+   
getenv(GIT_REFLOG_ACTION_ENVIRONMENT),
options.switch_to);
if (reset_head(, "checkout",
   options.head_name, 0,
-  NULL, NULL) < 0) {
+  NULL, buf.buf) < 0) {
ret = !!error(_("could not switch to "
"%s"),
  options.switch_to);
@@ -1508,7 +1530,8 @@ int cmd_rebase(int argc, const char **argv, const char 
*prefix)
printf(_("First, rewinding head to replay your work on top of "
 "it...\n"));
 
-   strbuf_addf(, "rebase: checkout %s", options.onto_name);
+   strbuf_addf(, "%s: checkout %s",
+   getenv(GIT_REFLOG_ACTION_ENVIRONMENT), options.onto_name);
if (reset_head(>object.oid, "checkout", NULL,
   RESET_HEAD_DETACH, NULL, msg.buf))
die(_("Could not detach HEAD"));
diff --git a/t/t3406-rebase-message.sh b/t/t3406-rebase-message.sh
index 38bd876cab..db8505eb86 100755
--- a/t/t3406-rebase-message.sh
+++ b/t/t3406-rebase-message.sh
@@ -91,4 +91,30 @@ test_expect_success 'error out early upon -C or 
--whitespace=' '
test_i18ngrep "Invalid whitespace option" err
 '
 
+test_expect_success 'GIT_REFLOG_ACTION' '
+   git checkout start &&
+   test_commit reflog-onto &&
+   git checkout -b reflog-topic start &&
+   test_commit reflog-to-rebase &&
+
+   git rebase reflog-onto &&
+   git log -g --format=%gs -3 >actual &&
+   cat >expect <<-\EOF &&
+   rebase finished: 

[PATCH 0/1] Fix built-in rebase regression noticed by Debian's dgit

2018-11-29 Thread Johannes Schindelin via GitGitGadget
It has been reported on the Debian bug tracker
[https://bugs.debian.org/914695] that the built-in rebase regresses on the
scripted version, and later details emerged that this has something to do
with the reflog messages: they were different with the built-in rebase than
with the scripted one.

This here patch fixes that.

Johannes Schindelin (1):
  rebase: fix GIT_REFLOG_ACTION regression

 builtin/rebase.c  | 29 ++---
 t/t3406-rebase-message.sh | 26 ++
 2 files changed, 52 insertions(+), 3 deletions(-)


base-commit: 7068cbc4abac53d9c3675dfba81c1e97d25e8eeb
Published-As: 
https://github.com/gitgitgadget/git/releases/tags/pr-91%2Fdscho%2Ffix-reflog-action-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git 
pr-91/dscho/fix-reflog-action-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/91
-- 
gitgitgadget


[PATCH v2 0/1] Fix git rebase --stat -i

2018-11-29 Thread Johannes Schindelin via GitGitGadget
We're really entering obscure territory here, I would say.

To trigger the bug, two things have to come together: the user must have
asked for a diffstat afterwards, and the commits need to have been rebased
onto a completely unrelated commit history (i.e. there must exist no merge
base between the pre-rebase HEAD and the post-rebase HEAD).

Please note that this bug existed already in the scripted rebase, but it was
never detected because the scripted version would not even perform any error
checking.

It will make Junio very happy that I squashed the regression test in to the
patch that fixes it. The reason, however, was not to make Junio happy (I
hope to make him happy by fixing bugs), but simply that an earlier iteration
of test would only fail with the built-in rebase, but not with the scripted
version. The current version would fail with the scripted version, but I'll
save the time to split the patch again now.

Changes since v1:

 * The commit message now talks more about what we should do in case that
   there is no merge base, rather than stressing the differences between the
   way scripted vs built-in rebase handled it (both buggy, and fixed by this
   patch).
 * In case that there is no merge base, it no longer reports "Changes from
   (empty) to ..." but "Changes to ...", which should be a lot less
   controversial.

Johannes Schindelin (1):
  rebase --stat: fix when rebasing to an unrelated history

 builtin/rebase.c  | 18 --
 git-legacy-rebase.sh  | 10 --
 t/t3406-rebase-message.sh | 10 ++
 3 files changed, 30 insertions(+), 8 deletions(-)


base-commit: a1598010f775d82b5adf12c29d0f5bc9b41434c6
Published-As: 
https://github.com/gitgitgadget/git/releases/tags/pr-88%2Fdscho%2Frebase-stat-fix-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git 
pr-88/dscho/rebase-stat-fix-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/88

Range-diff vs v1:

 1:  680385e4bd ! 1:  190c7856ad rebase --stat: fix when rebasing to an 
unrelated history
 @@ -3,22 +3,21 @@
  rebase --stat: fix when rebasing to an unrelated history
  
  When rebasing to a commit history that has no common commits with the
 -current branch, there is no merge base. The scripted version of the 
`git
 -rebase` command was not prepared for that and spewed out
 +current branch, there is no merge base. In diffstat mode, this means
 +that we cannot compare to the merge base, but we have to compare to 
the
 +empty tree instead.
  
 -fatal: ambiguous argument '': unknown revision or path not in
 -the working tree.
 +Also, if running in verbose diffstat mode, we should not output
  
 -but then continued (due to lack of error checking).
 +Changes from  to 
  
 -The built-in version of the `git rebase` command blindly translated 
that
 -shell script code, assuming that there is no need to test whether 
there
 -*was* a merge base, and due to its better error checking, exited with 
a
 -fatal error (because it tried to read the object with hash ...
 -as a tree).
 +as that does not make sense without any merge base.
  
 -Fix both scripted and built-in versions to output something sensibly,
 -and add a regression test to keep this working in all eternity.
 +Note: neither scripted nor built-in versoin of `git rebase` were
 +prepared for this situation well. We use this opportunity not only to
 +fix the bug(s), but also to make both versions' output consistent in
 +this instance. And add a regression test to keep this working in all
 +eternity.
  
  Reported-by: Ævar Arnfjörð Bjarmason 
  Signed-off-by: Johannes Schindelin 
 @@ -27,15 +26,25 @@
  --- a/builtin/rebase.c
  +++ b/builtin/rebase.c
  @@
 +  if (options.flags & REBASE_DIFFSTAT) {
 +  struct diff_options opts;
   
 -  if (options.flags & REBASE_VERBOSE)
 -  printf(_("Changes from %s to %s:\n"),
 +- if (options.flags & REBASE_VERBOSE)
 +- printf(_("Changes from %s to %s:\n"),
  - oid_to_hex(_base),
 -+ is_null_oid(_base) ?
 -+ "(empty)" : oid_to_hex(_base),
 -  oid_to_hex(>object.oid));
 +- oid_to_hex(>object.oid));
 ++ if (options.flags & REBASE_VERBOSE) {
 ++ if (is_null_oid(_base))
 ++ printf(_("Changes to %s:\n"),
 ++oid_to_hex(>object.oid));
 ++ else
 ++ printf(_("Changes from %s to %s:\n"),
 ++oid_to_hex(_base),
 ++ 

[PATCH v2 1/1] rebase --stat: fix when rebasing to an unrelated history

2018-11-29 Thread Johannes Schindelin via GitGitGadget
From: Johannes Schindelin 

When rebasing to a commit history that has no common commits with the
current branch, there is no merge base. In diffstat mode, this means
that we cannot compare to the merge base, but we have to compare to the
empty tree instead.

Also, if running in verbose diffstat mode, we should not output

Changes from  to 

as that does not make sense without any merge base.

Note: neither scripted nor built-in versoin of `git rebase` were
prepared for this situation well. We use this opportunity not only to
fix the bug(s), but also to make both versions' output consistent in
this instance. And add a regression test to keep this working in all
eternity.

Reported-by: Ævar Arnfjörð Bjarmason 
Signed-off-by: Johannes Schindelin 
---
 builtin/rebase.c  | 18 --
 git-legacy-rebase.sh  | 10 --
 t/t3406-rebase-message.sh | 10 ++
 3 files changed, 30 insertions(+), 8 deletions(-)

diff --git a/builtin/rebase.c b/builtin/rebase.c
index 5b3e5baec8..1c6f817f4b 100644
--- a/builtin/rebase.c
+++ b/builtin/rebase.c
@@ -1481,10 +1481,15 @@ int cmd_rebase(int argc, const char **argv, const char 
*prefix)
if (options.flags & REBASE_DIFFSTAT) {
struct diff_options opts;
 
-   if (options.flags & REBASE_VERBOSE)
-   printf(_("Changes from %s to %s:\n"),
-   oid_to_hex(_base),
-   oid_to_hex(>object.oid));
+   if (options.flags & REBASE_VERBOSE) {
+   if (is_null_oid(_base))
+   printf(_("Changes to %s:\n"),
+  oid_to_hex(>object.oid));
+   else
+   printf(_("Changes from %s to %s:\n"),
+  oid_to_hex(_base),
+  oid_to_hex(>object.oid));
+   }
 
/* We want color (if set), but no pager */
diff_setup();
@@ -1494,8 +1499,9 @@ int cmd_rebase(int argc, const char **argv, const char 
*prefix)
DIFF_FORMAT_SUMMARY | DIFF_FORMAT_DIFFSTAT;
opts.detect_rename = DIFF_DETECT_RENAME;
diff_setup_done();
-   diff_tree_oid(_base, >object.oid,
- "", );
+   diff_tree_oid(is_null_oid(_base) ?
+ the_hash_algo->empty_tree : _base,
+ >object.oid, "", );
diffcore_std();
diff_flush();
}
diff --git a/git-legacy-rebase.sh b/git-legacy-rebase.sh
index b97ffdc9dd..b4c7dbfa57 100755
--- a/git-legacy-rebase.sh
+++ b/git-legacy-rebase.sh
@@ -718,10 +718,16 @@ if test -n "$diffstat"
 then
if test -n "$verbose"
then
-   echo "$(eval_gettext "Changes from \$mb to \$onto:")"
+   if test -z "$mb"
+   then
+   echo "$(eval_gettext "Changes to \$onto:")"
+   else
+   echo "$(eval_gettext "Changes from \$mb to \$onto:")"
+   fi
fi
+   mb_tree="${mb:-$(git hash-object -t tree /dev/null)}"
# We want color (if set), but no pager
-   GIT_PAGER='' git diff --stat --summary "$mb" "$onto"
+   GIT_PAGER='' git diff --stat --summary "$mb_tree" "$onto"
 fi
 
 test -n "$interactive_rebase" && run_specific_rebase
diff --git a/t/t3406-rebase-message.sh b/t/t3406-rebase-message.sh
index 38bd876cab..c2c9950568 100755
--- a/t/t3406-rebase-message.sh
+++ b/t/t3406-rebase-message.sh
@@ -91,4 +91,14 @@ test_expect_success 'error out early upon -C or 
--whitespace=' '
test_i18ngrep "Invalid whitespace option" err
 '
 
+test_expect_success 'rebase -i onto unrelated history' '
+   git init unrelated &&
+   test_commit -C unrelated 1 &&
+   git -C unrelated remote add -f origin "$PWD" &&
+   git -C unrelated branch --set-upstream-to=origin/master &&
+   git -C unrelated -c core.editor=true rebase -i -v --stat >actual &&
+   test_i18ngrep "Changes to " actual &&
+   test_i18ngrep "5 files changed" actual
+'
+
 test_done
-- 
gitgitgadget


[PATCH 1/1] rebase --stat: fix when rebasing to an unrelated history

2018-11-27 Thread Johannes Schindelin via GitGitGadget
From: Johannes Schindelin 

When rebasing to a commit history that has no common commits with the
current branch, there is no merge base. The scripted version of the `git
rebase` command was not prepared for that and spewed out

fatal: ambiguous argument '': unknown revision or path not in
the working tree.

but then continued (due to lack of error checking).

The built-in version of the `git rebase` command blindly translated that
shell script code, assuming that there is no need to test whether there
*was* a merge base, and due to its better error checking, exited with a
fatal error (because it tried to read the object with hash ...
as a tree).

Fix both scripted and built-in versions to output something sensibly,
and add a regression test to keep this working in all eternity.

Reported-by: Ævar Arnfjörð Bjarmason 
Signed-off-by: Johannes Schindelin 
---
 builtin/rebase.c  |  8 +---
 git-legacy-rebase.sh  |  6 --
 t/t3406-rebase-message.sh | 10 ++
 3 files changed, 19 insertions(+), 5 deletions(-)

diff --git a/builtin/rebase.c b/builtin/rebase.c
index 5b3e5baec8..9e4b0b564f 100644
--- a/builtin/rebase.c
+++ b/builtin/rebase.c
@@ -1483,7 +1483,8 @@ int cmd_rebase(int argc, const char **argv, const char 
*prefix)
 
if (options.flags & REBASE_VERBOSE)
printf(_("Changes from %s to %s:\n"),
-   oid_to_hex(_base),
+   is_null_oid(_base) ?
+   "(empty)" : oid_to_hex(_base),
oid_to_hex(>object.oid));
 
/* We want color (if set), but no pager */
@@ -1494,8 +1495,9 @@ int cmd_rebase(int argc, const char **argv, const char 
*prefix)
DIFF_FORMAT_SUMMARY | DIFF_FORMAT_DIFFSTAT;
opts.detect_rename = DIFF_DETECT_RENAME;
diff_setup_done();
-   diff_tree_oid(_base, >object.oid,
- "", );
+   diff_tree_oid(is_null_oid(_base) ?
+ the_hash_algo->empty_tree : _base,
+ >object.oid, "", );
diffcore_std();
diff_flush();
}
diff --git a/git-legacy-rebase.sh b/git-legacy-rebase.sh
index b97ffdc9dd..be3b241676 100755
--- a/git-legacy-rebase.sh
+++ b/git-legacy-rebase.sh
@@ -718,10 +718,12 @@ if test -n "$diffstat"
 then
if test -n "$verbose"
then
-   echo "$(eval_gettext "Changes from \$mb to \$onto:")"
+   mb_display="${mb:-(empty)}"
+   echo "$(eval_gettext "Changes from \$mb_display to \$onto:")"
fi
+   mb_tree="${mb:-$(git hash-object -t tree /dev/null)}"
# We want color (if set), but no pager
-   GIT_PAGER='' git diff --stat --summary "$mb" "$onto"
+   GIT_PAGER='' git diff --stat --summary "$mb_tree" "$onto"
 fi
 
 test -n "$interactive_rebase" && run_specific_rebase
diff --git a/t/t3406-rebase-message.sh b/t/t3406-rebase-message.sh
index 38bd876cab..a1ee912118 100755
--- a/t/t3406-rebase-message.sh
+++ b/t/t3406-rebase-message.sh
@@ -91,4 +91,14 @@ test_expect_success 'error out early upon -C or 
--whitespace=' '
test_i18ngrep "Invalid whitespace option" err
 '
 
+test_expect_success 'rebase -i onto unrelated history' '
+   git init unrelated &&
+   test_commit -C unrelated 1 &&
+   git -C unrelated remote add -f origin "$PWD" &&
+   git -C unrelated branch --set-upstream-to=origin/master &&
+   git -C unrelated -c core.editor=true rebase -i -v --stat >actual &&
+   test_i18ngrep "Changes from (empty)" actual &&
+   test_i18ngrep "5 files changed" actual
+'
+
 test_done
-- 
gitgitgadget


[PATCH 0/1] Fix git rebase --stat -i

2018-11-27 Thread Johannes Schindelin via GitGitGadget
We're really entering obscure territory here, I would say.

To trigger the bug, two things have to come together: the user must have
asked for a diffstat afterwards, and the commits need to have been rebased
onto a completely unrelated commit history (i.e. there must exist no merge
base between the pre-rebase HEAD and the post-rebase HEAD).

Please note that this bug existed already in the scripted rebase, but it was
never detected because the scripted version would not even perform any error
checking.

It will make Junio very happy that I squashed the regression test in to the
patch that fixes it. The reason, however, was not to make Junio happy (I
hope to make him happy by fixing bugs), but simply that an earlier iteration
of test would only fail with the built-in rebase, but not with the scripted
version. The current version would fail with the scripted version, but I'll
save the time to split the patch again now.

Johannes Schindelin (1):
  rebase --stat: fix when rebasing to an unrelated history

 builtin/rebase.c  |  8 +---
 git-legacy-rebase.sh  |  6 --
 t/t3406-rebase-message.sh | 10 ++
 3 files changed, 19 insertions(+), 5 deletions(-)


base-commit: a1598010f775d82b5adf12c29d0f5bc9b41434c6
Published-As: 
https://github.com/gitgitgadget/git/releases/tags/pr-88%2Fdscho%2Frebase-stat-fix-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git 
pr-88/dscho/rebase-stat-fix-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/88
-- 
gitgitgadget


[PATCH 0/1] Fix Windows build of next

2018-11-22 Thread Johannes Schindelin via GitGitGadget
The topic ot/ref-filter-object-info broke the Windows build since it entered 
pu, and as a consequence we have no test coverage of the other topics in pu.

Sadly, this topic now advanced to next. Junio, I would like to ask you to
merge this fix down to next and to advance to master together with 
ot/ref-filter-object-info.

Johannes Schindelin (1):
  ref-filter: replace unportable `%lld` format

 ref-filter.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)


base-commit: d364d6b33e15dbc6e57d83f9f1457a8e8fe77046
Published-As: 
https://github.com/gitgitgadget/git/releases/tags/pr-87%2Fdscho%2Fno-percent-lld-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git 
pr-87/dscho/no-percent-lld-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/87
-- 
gitgitgadget


[PATCH 1/1] ref-filter: replace unportable `%lld` format

2018-11-22 Thread Johannes Schindelin via GitGitGadget
From: Johannes Schindelin 

The `%lld` format is supported on Linux and macOS, but not on Windows.
This issue has been reported ten days ago (Message-ID:
nycvar.qro.7.76.6.1811121300520...@tvgsbejvaqbjf.bet), but the
corresponding topic still advanced to `next` in the meantime, breaking
the Windows build.

Let's use `PRIdMAX` and a cast to `intmax_t` instead, which unbreaks the
build, and imitates how we do things e.g. in `json-writer.c` already.

Signed-off-by: Johannes Schindelin 
---
 ref-filter.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ref-filter.c b/ref-filter.c
index 3cfe01a039..69cdf2dbb5 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -897,7 +897,7 @@ static void grab_common_values(struct atom_value *val, int 
deref, struct expand_
v->s = xstrdup(type_name(oi->type));
else if (!strcmp(name, "objectsize:disk")) {
v->value = oi->disk_size;
-   v->s = xstrfmt("%lld", (long long)oi->disk_size);
+   v->s = xstrfmt("%"PRIdMAX, (intmax_t)oi->disk_size);
} else if (!strcmp(name, "objectsize")) {
v->value = oi->size;
v->s = xstrfmt("%lu", oi->size);
-- 
gitgitgadget


[PATCH 0/1] legacy-rebase: fix "regression"

2018-11-20 Thread Johannes Schindelin via GitGitGadget
This is a backport, really, to accommodate a new regression test that was
introduced when the built-in rebase learned to validate the -C and 
--whitespace= arguments early.

Johannes Schindelin (1):
  legacy-rebase: backport -C and --whitespace= checks

 git-legacy-rebase.sh | 8 
 1 file changed, 8 insertions(+)


base-commit: bb75be6cb916297f271c846f2f9caa3daaaec718
Published-As: 
https://github.com/gitgitgadget/git/releases/tags/pr-86%2Fdscho%2Fscripted-rebase-Cn-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git 
pr-86/dscho/scripted-rebase-Cn-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/86
-- 
gitgitgadget


[PATCH 1/1] legacy-rebase: backport -C and --whitespace= checks

2018-11-20 Thread Johannes Schindelin via GitGitGadget
From: Johannes Schindelin 

Since 04519d720114 (rebase: validate -C and --whitespace=
parameters early, 2018-11-14), the built-in rebase validates the -C and
--whitespace arguments early. As this commit also introduced a
regression test for this, and as a later commit introduced the
GIT_TEST_REBASE_USE_BUILTIN mode to run tests, we now have a
"regression" in the scripted version of `git rebase` on our hands.

Backport the validation to fix this.

Reported-by: Ævar Arnfjörð Bjarmason 
Signed-off-by: Johannes Schindelin 
---
 git-legacy-rebase.sh | 8 
 1 file changed, 8 insertions(+)

diff --git a/git-legacy-rebase.sh b/git-legacy-rebase.sh
index 75a08b2683..ced0635326 100755
--- a/git-legacy-rebase.sh
+++ b/git-legacy-rebase.sh
@@ -337,6 +337,11 @@ do
fix|strip)
force_rebase=t
;;
+   warn|nowarn|error|error-all)
+   ;; # okay, known whitespace option
+   *)
+   die "Invalid whitespace option: '${1%*=}'"
+   ;;
esac
;;
--ignore-whitespace)
@@ -352,6 +357,9 @@ do
git_am_opt="$git_am_opt $1"
force_rebase=t
;;
+   -C*[!0-9]*)
+   die "switch \`C' expects a numerical value"
+   ;;
-C*)
git_am_opt="$git_am_opt $1"
;;
-- 
gitgitgadget


[PATCH 1/1] rebase: warn about the correct tree's OID

2018-11-20 Thread Johannes Schindelin via GitGitGadget
From: Johannes Schindelin 

This was a simple copy/paste error, and an obvious one at that: if we
cannot fill the tree descriptor, we should show an error message about
*that* tree, not another one.

Signed-off-by: Johannes Schindelin 
---
 builtin/rebase.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/builtin/rebase.c b/builtin/rebase.c
index 1a2758756a..5b3e5baec8 100644
--- a/builtin/rebase.c
+++ b/builtin/rebase.c
@@ -582,7 +582,8 @@ static int reset_head(struct object_id *oid, const char 
*action,
}
 
if (!reset_hard && !fill_tree_descriptor([nr++], _oid)) {
-   ret = error(_("failed to find tree of %s"), oid_to_hex(oid));
+   ret = error(_("failed to find tree of %s"),
+   oid_to_hex(_oid));
goto leave_reset_head;
}
 
-- 
gitgitgadget


[PATCH 0/1] rebase: warn about the correct tree's OID

2018-11-20 Thread Johannes Schindelin via GitGitGadget
A quick fix for a recent topic. Not overly critical, but I would deem this
v2.20.0-rc1 material.

Johannes Schindelin (1):
  rebase: warn about the correct tree's OID

 builtin/rebase.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)


base-commit: bb75be6cb916297f271c846f2f9caa3daaaec718
Published-As: 
https://github.com/gitgitgadget/git/releases/tags/pr-85%2Fdscho%2Freset_head-typo-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git 
pr-85/dscho/reset_head-typo-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/85
-- 
gitgitgadget


[PATCH 0/1] mingw: update a link to the official documentation

2018-11-15 Thread Johannes Schindelin via GitGitGadget
It is a pretty neat thing that the bulky MSDN documentation has been
replaced by something a lot more open, something that can be updated via
Pull Requests on GitHub. Let's link to this new documentation instead of the
obsolete one.

I know, it is really close to the code freeze leading up to Git v2.20.0. But
this is just an update to a code comment... :-)

Johannes Schindelin (1):
  mingw: replace an obsolete link with the superseding one

 compat/mingw.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)


base-commit: d166e6afe5f257217836ef24a73764eba390c58d
Published-As: 
https://github.com/gitgitgadget/git/releases/tags/pr-81%2Fdscho%2Fmingw-update-msdn-link-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git 
pr-81/dscho/mingw-update-msdn-link-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/81
-- 
gitgitgadget


[PATCH 1/1] mingw: replace an obsolete link with the superseding one

2018-11-15 Thread Johannes Schindelin via GitGitGadget
From: Johannes Schindelin 

The MSDN documentation has been superseded by Microsoft Docs (which is
backed by a repository on GitHub containing many, many files in Markdown
format).

Signed-off-by: Johannes Schindelin 
---
 compat/mingw.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/compat/mingw.c b/compat/mingw.c
index d2f4fabb44..9e42b0ee26 100644
--- a/compat/mingw.c
+++ b/compat/mingw.c
@@ -1028,8 +1028,8 @@ char *mingw_getcwd(char *pointer, int len)
 }
 
 /*
- * See http://msdn2.microsoft.com/en-us/library/17w5ykft(vs.71).aspx
- * (Parsing C++ Command-Line Arguments)
+ * See "Parsing C++ Command-Line Arguments" at Microsoft's Docs:
+ * https://docs.microsoft.com/en-us/cpp/cpp/parsing-cpp-command-line-arguments
  */
 static const char *quote_arg(const char *arg)
 {
-- 
gitgitgadget


[PATCH v2 2/5] tests: respect GIT_TEST_INSTALLED when initializing repositories

2018-11-14 Thread Johannes Schindelin via GitGitGadget
From: Johannes Schindelin 

It really makes very, very little sense to use a different git
executable than the one the caller indicated via setting the environment
variable GIT_TEST_INSTALLED.

Signed-off-by: Johannes Schindelin 
---
 t/test-lib-functions.sh | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
index d158c8d0bf..3472716651 100644
--- a/t/test-lib-functions.sh
+++ b/t/test-lib-functions.sh
@@ -923,7 +923,8 @@ test_create_repo () {
mkdir -p "$repo"
(
cd "$repo" || error "Cannot setup test environment"
-   "$GIT_EXEC_PATH/git-init" 
"--template=$GIT_BUILD_DIR/templates/blt/" >&3 2>&4 ||
+   "${GIT_TEST_INSTALLED:-$GIT_EXEC_PATH}/git" init \
+   "--template=$GIT_BUILD_DIR/templates/blt/" >&3 2>&4 ||
error "cannot run git init -- have you built things yet?"
mv .git/hooks .git/hooks-disabled
) || exit
-- 
gitgitgadget



[PATCH v2 4/5] tests: do not require Git to be built when testing an installed Git

2018-11-14 Thread Johannes Schindelin via GitGitGadget
From: Johannes Schindelin 

We really only need the test helpers to be built in the worktree in that
case, but that is not what we test for.

On the other hand it is a perfect opportunity to verify that
`GIT_TEST_INSTALLED` points to a working Git.

So let's test the appropriate Git executable. While at it, also adjust
the error message in the `GIT_TEST_INSTALLED` case.

This patch is best viewed with `-w --patience`.

Helped-by: Jeff King 
Signed-off-by: Johannes Schindelin 
---
 t/test-lib.sh | 9 +++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/t/test-lib.sh b/t/test-lib.sh
index 93883580a8..3d3a65ed0e 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -51,10 +51,15 @@ export LSAN_OPTIONS
 
 
 # It appears that people try to run tests without building...
-"$GIT_BUILD_DIR/git" >/dev/null
+"${GIT_TEST_INSTALLED:-$GIT_BUILD_DIR}/git" >/dev/null
 if test $? != 1
 then
-   echo >&2 'error: you do not seem to have built git yet.'
+   if test -n "$GIT_TEST_INSTALLED"
+   then
+   echo >&2 "error: there is no working Git at 
'$GIT_TEST_INSTALLED'"
+   else
+   echo >&2 'error: you do not seem to have built git yet.'
+   fi
exit 1
 fi
 
-- 
gitgitgadget



[PATCH v2 3/5] t/lib-gettext: test installed git-sh-i18n if GIT_TEST_INSTALLED is set

2018-11-14 Thread Johannes Schindelin via GitGitGadget
From: Johannes Schindelin 

It makes very, very little sense to test the built git-sh-i18n when the
user asked specifically to test another one.

Signed-off-by: Johannes Schindelin 
---
 t/lib-gettext.sh | 7 ++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/t/lib-gettext.sh b/t/lib-gettext.sh
index eec757f104..9eb160c997 100644
--- a/t/lib-gettext.sh
+++ b/t/lib-gettext.sh
@@ -10,7 +10,12 @@ GIT_TEXTDOMAINDIR="$GIT_BUILD_DIR/po/build/locale"
 GIT_PO_PATH="$GIT_BUILD_DIR/po"
 export GIT_TEXTDOMAINDIR GIT_PO_PATH
 
-. "$GIT_BUILD_DIR"/git-sh-i18n
+if test -n "$GIT_TEST_INSTALLED"
+then
+   . "$(git --exec-path)"/git-sh-i18n
+else
+   . "$GIT_BUILD_DIR"/git-sh-i18n
+fi
 
 if test_have_prereq GETTEXT && ! test_have_prereq GETTEXT_POISON
 then
-- 
gitgitgadget



[PATCH v2 5/5] tests: explicitly use `git.exe` on Windows

2018-11-14 Thread Johannes Schindelin via GitGitGadget
From: Johannes Schindelin 

On Windows, when we refer to `/an/absolute/path/to/git`, it magically
resolves `git.exe` at that location. Except if something of the name
`git` exists next to that `git.exe`. So if we call `$BUILD_DIR/git`, it
will find `$BUILD_DIR/git.exe` *only* if there is not, say, a directory
called `$BUILD_DIR/git`.

Such a directory, however, exists in Git for Windows when building with
Visual Studio (our Visual Studio project generator defaults to putting
the build files into a directory whose name is the base name of the
corresponding `.exe`).

In the bin-wrappers/* scripts, we already take pains to use `git.exe`
rather than `git`, as this could pick up the wrong thing on Windows
(i.e. if there exists a `git` file or directory in the build directory).

Now we do the same in the tests' start-up code.

This also helps when testing an installed Git, as there might be even
more likely some stray file or directory in the way.

Note: the only way we can record whether the `.exe` suffix is by writing
it to the `GIT-BUILD-OPTIONS` file and sourcing it at the beginning of
`t/test-lib.sh`. This is not a requirement introduced by this patch, but
we move the call to be able to use the `$X` variable that holds the file
extension, if any.

Note also: the many, many calls to `git this` and `git that` are
unaffected, as the regular PATH search will find the `.exe` files on
Windows (and not be confused by a directory of the name `git` that is
in one of the directories listed in the `PATH` variable), while
`/path/to/git` would not, per se, know that it is looking for an
executable and happily prefer such a directory.

Signed-off-by: Johannes Schindelin 
---
 Makefile|  1 +
 t/test-lib-functions.sh |  2 +-
 t/test-lib.sh   | 13 +
 3 files changed, 11 insertions(+), 5 deletions(-)

diff --git a/Makefile b/Makefile
index 016fdcdb81..21b3978744 100644
--- a/Makefile
+++ b/Makefile
@@ -2591,6 +2591,7 @@ GIT-BUILD-OPTIONS: FORCE
@echo NO_UNIX_SOCKETS=\''$(subst ','\'',$(subst 
','\'',$(NO_UNIX_SOCKETS)))'\' >>$@+
@echo PAGER_ENV=\''$(subst ','\'',$(subst ','\'',$(PAGER_ENV)))'\' >>$@+
@echo DC_SHA1=\''$(subst ','\'',$(subst ','\'',$(DC_SHA1)))'\' >>$@+
+   @echo X=\'$(X)\' >>$@+
 ifdef TEST_OUTPUT_DIRECTORY
@echo TEST_OUTPUT_DIRECTORY=\''$(subst ','\'',$(subst 
','\'',$(TEST_OUTPUT_DIRECTORY)))'\' >>$@+
 endif
diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
index 3472716651..274cbc2d6e 100644
--- a/t/test-lib-functions.sh
+++ b/t/test-lib-functions.sh
@@ -923,7 +923,7 @@ test_create_repo () {
mkdir -p "$repo"
(
cd "$repo" || error "Cannot setup test environment"
-   "${GIT_TEST_INSTALLED:-$GIT_EXEC_PATH}/git" init \
+   "${GIT_TEST_INSTALLED:-$GIT_EXEC_PATH}/git$X" init \
"--template=$GIT_BUILD_DIR/templates/blt/" >&3 2>&4 ||
error "cannot run git init -- have you built things yet?"
mv .git/hooks .git/hooks-disabled
diff --git a/t/test-lib.sh b/t/test-lib.sh
index 3d3a65ed0e..e12addc324 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -49,9 +49,17 @@ export ASAN_OPTIONS
 : ${LSAN_OPTIONS=abort_on_error=1}
 export LSAN_OPTIONS
 
+if test ! -f "$GIT_BUILD_DIR"/GIT-BUILD-OPTIONS
+then
+   echo >&2 'error: GIT-BUILD-OPTIONS missing (has Git been built?).'
+   exit 1
+fi
+. "$GIT_BUILD_DIR"/GIT-BUILD-OPTIONS
+export PERL_PATH SHELL_PATH
+
 
 # It appears that people try to run tests without building...
-"${GIT_TEST_INSTALLED:-$GIT_BUILD_DIR}/git" >/dev/null
+"${GIT_TEST_INSTALLED:-$GIT_BUILD_DIR}/git$X" >/dev/null
 if test $? != 1
 then
if test -n "$GIT_TEST_INSTALLED"
@@ -63,9 +71,6 @@ then
exit 1
 fi
 
-. "$GIT_BUILD_DIR"/GIT-BUILD-OPTIONS
-export PERL_PATH SHELL_PATH
-
 # if --tee was passed, write the output not only to the terminal, but
 # additionally to the file test-results/$BASENAME.out, too.
 case "$GIT_TEST_TEE_STARTED, $* " in
-- 
gitgitgadget


[PATCH v2 1/5] tests: fix GIT_TEST_INSTALLED's PATH to include t/helper/

2018-11-14 Thread Johannes Schindelin via GitGitGadget
From: Johannes Schindelin 

We really need to be able to find the test helpers... Really. This
change was forgotten when we moved the test helpers into t/helper/

Signed-off-by: Johannes Schindelin 
---
 t/test-lib.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/t/test-lib.sh b/t/test-lib.sh
index aba66cafa2..93883580a8 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -957,7 +957,7 @@ elif test -n "$GIT_TEST_INSTALLED"
 then
GIT_EXEC_PATH=$($GIT_TEST_INSTALLED/git --exec-path)  ||
error "Cannot run git from $GIT_TEST_INSTALLED."
-   PATH=$GIT_TEST_INSTALLED:$GIT_BUILD_DIR:$PATH
+   PATH=$GIT_TEST_INSTALLED:$GIT_BUILD_DIR/t/helper:$PATH
GIT_EXEC_PATH=${GIT_TEST_EXEC_PATH:-$GIT_EXEC_PATH}
 else # normal case, use ../bin-wrappers only unless $with_dashes:
git_bin_dir="$GIT_BUILD_DIR/bin-wrappers"
-- 
gitgitgadget



[PATCH v2 0/5] tests: various improvements to the GIT_TEST_INSTALLED feature

2018-11-14 Thread Johannes Schindelin via GitGitGadget
By setting the GIT_TEST_INSTALLED variable to the path of an installed Git
executable, it is possible to run the test suite also on a specific
installed version (as opposed to a version built from scratch).

The only thing this needs that is unlikely to be installed is the test
helper(s).

However, there have been a few rough edges around that, identified in my
(still ongoing) work to support building Git in Visual Studio (where we do
not want to run GNU Make, and where we have no canonical way to create, say,
hard-linked copies of the built-in commands, and other work to let Git for
Windows play better with BusyBox.

Triggered by a comment of AEvar
[https://public-inbox.org/git/20181102223743.4331-1-ava...@gmail.com/], I
hereby contribute these assorted fixes for the GIT_TEST_INSTALLED feature.

Changes since v1:

 * Now we verify in test-lib.sh also in the GIT_TEST_INSTALLED case whether
   the Git executable is working (thanks, Peff!).
 * The commit message of 5/5 was touched up.

Johannes Schindelin (5):
  tests: fix GIT_TEST_INSTALLED's PATH to include t/helper/
  tests: respect GIT_TEST_INSTALLED when initializing repositories
  t/lib-gettext: test installed git-sh-i18n if GIT_TEST_INSTALLED is set
  tests: do not require Git to be built when testing an installed Git
  tests: explicitly use `git.exe` on Windows

 Makefile|  1 +
 t/lib-gettext.sh|  7 ++-
 t/test-lib-functions.sh |  3 ++-
 t/test-lib.sh   | 22 --
 4 files changed, 25 insertions(+), 8 deletions(-)


base-commit: d166e6afe5f257217836ef24a73764eba390c58d
Published-As: 
https://github.com/gitgitgadget/git/releases/tags/pr-73%2Fdscho%2Ftest-git-installed-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git 
pr-73/dscho/test-git-installed-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/73

Range-diff vs v1:

 1:  2b04f9f086 = 1:  3b68e0fe8a tests: fix GIT_TEST_INSTALLED's PATH to 
include t/helper/
 2:  948b3dc146 = 2:  80d50d5932 tests: respect GIT_TEST_INSTALLED when 
initializing repositories
 3:  eddea552e4 = 3:  49e408677a t/lib-gettext: test installed git-sh-i18n if 
GIT_TEST_INSTALLED is set
 4:  316e215e54 < -:  -- tests: do not require Git to be built when 
testing an installed Git
 -:  -- > 4:  b801dc8027 tests: do not require Git to be built when 
testing an installed Git
 5:  cd314e1384 ! 5:  fbdb659de6 tests: explicitly use `git.exe` on Windows
 @@ -2,6 +2,17 @@
  
  tests: explicitly use `git.exe` on Windows
  
 +On Windows, when we refer to `/an/absolute/path/to/git`, it magically
 +resolves `git.exe` at that location. Except if something of the name
 +`git` exists next to that `git.exe`. So if we call `$BUILD_DIR/git`, 
it
 +will find `$BUILD_DIR/git.exe` *only* if there is not, say, a 
directory
 +called `$BUILD_DIR/git`.
 +
 +Such a directory, however, exists in Git for Windows when building 
with
 +Visual Studio (our Visual Studio project generator defaults to putting
 +the build files into a directory whose name is the base name of the
 +corresponding `.exe`).
 +
  In the bin-wrappers/* scripts, we already take pains to use `git.exe`
  rather than `git`, as this could pick up the wrong thing on Windows
  (i.e. if there exists a `git` file or directory in the build 
directory).
 @@ -68,11 +79,12 @@
  +
   
   # It appears that people try to run tests without building...
 --test -n "$GIT_TEST_INSTALLED" || "$GIT_BUILD_DIR/git" >/dev/null ||
 -+test -n "$GIT_TEST_INSTALLED" || "$GIT_BUILD_DIR/git$X" >/dev/null ||
 +-"${GIT_TEST_INSTALLED:-$GIT_BUILD_DIR}/git" >/dev/null
 ++"${GIT_TEST_INSTALLED:-$GIT_BUILD_DIR}/git$X" >/dev/null
   if test $? != 1
   then
 -  echo >&2 'error: you do not seem to have built git yet.'
 +  if test -n "$GIT_TEST_INSTALLED"
 +@@
exit 1
   fi
   

-- 
gitgitgadget


[PATCH v2 2/2] rebase: validate -C and --whitespace= parameters early

2018-11-14 Thread Johannes Schindelin via GitGitGadget
From: Johannes Schindelin 

It is a good idea to error out early upon seeing, say, `-Cbad`, rather
than starting the rebase only to have the `--am` backend complain later.

Let's do this.

The only options accepting parameters which we pass through to `git am`
(which may, or may not, forward them to `git apply`) are `-C` and
`--whitespace`. The other options we pass through do not accept
parameters, so we do not have to validate them here.

Signed-off-by: Johannes Schindelin 
---
 builtin/rebase.c  | 12 +++-
 t/t3406-rebase-message.sh |  7 +++
 2 files changed, 18 insertions(+), 1 deletion(-)

diff --git a/builtin/rebase.c b/builtin/rebase.c
index 96ffa80b71..571cf899d5 100644
--- a/builtin/rebase.c
+++ b/builtin/rebase.c
@@ -1064,12 +1064,22 @@ int cmd_rebase(int argc, const char **argv, const char 
*prefix)
}
 
for (i = 0; i < options.git_am_opts.argc; i++) {
-   const char *option = options.git_am_opts.argv[i];
+   const char *option = options.git_am_opts.argv[i], *p;
if (!strcmp(option, "--committer-date-is-author-date") ||
!strcmp(option, "--ignore-date") ||
!strcmp(option, "--whitespace=fix") ||
!strcmp(option, "--whitespace=strip"))
options.flags |= REBASE_FORCE;
+   else if (skip_prefix(option, "-C", )) {
+   while (*p)
+   if (!isdigit(*(p++)))
+   die(_("switch `C' expects a "
+ "numerical value"));
+   } else if (skip_prefix(option, "--whitespace=", )) {
+   if (*p && strcmp(p, "warn") && strcmp(p, "nowarn") &&
+   strcmp(p, "error") && strcmp(p, "error-all"))
+   die("Invalid whitespace option: '%s'", p);
+   }
}
 
if (!(options.flags & REBASE_NO_QUIET))
diff --git a/t/t3406-rebase-message.sh b/t/t3406-rebase-message.sh
index 0392e36d23..2c79eed4fe 100755
--- a/t/t3406-rebase-message.sh
+++ b/t/t3406-rebase-message.sh
@@ -84,4 +84,11 @@ test_expect_success 'rebase --onto outputs the invalid ref' '
test_i18ngrep "invalid-ref" err
 '
 
+test_expect_success 'error out early upon -C or --whitespace=' '
+   test_must_fail git rebase -Cnot-a-number HEAD 2>err &&
+   test_i18ngrep "numerical value" err &&
+   test_must_fail git rebase --whitespace=bad HEAD 2>err &&
+   test_i18ngrep "Invalid whitespace option" err
+'
+
 test_done
-- 
gitgitgadget


[PATCH v2 1/2] rebase: really just passthru the `git am` options

2018-11-14 Thread Johannes Schindelin via GitGitGadget
From: Johannes Schindelin 

Currently, we parse the options intended for `git am` as if we wanted to
handle them in `git rebase`, and then reconstruct them painstakingly to
define the `git_am_opt` variable.

However, there is a much better way (that I was unaware of, at the time
when I mentored Pratik to implement these options): OPT_PASSTHRU_ARGV.
It is intended for exactly this use case, where command-line options
want to be parsed into a separate `argv_array`.

Let's use this feature.

Incidentally, this also allows us to address a bug discovered by Phillip
Wood, where the built-in rebase failed to understand that the `-C`
option takes an optional argument.

Signed-off-by: Johannes Schindelin 
---
 builtin/rebase.c | 98 +---
 1 file changed, 35 insertions(+), 63 deletions(-)

diff --git a/builtin/rebase.c b/builtin/rebase.c
index 0ee06aa363..96ffa80b71 100644
--- a/builtin/rebase.c
+++ b/builtin/rebase.c
@@ -87,7 +87,7 @@ struct rebase_options {
REBASE_FORCE = 1<<3,
REBASE_INTERACTIVE_EXPLICIT = 1<<4,
} flags;
-   struct strbuf git_am_opt;
+   struct argv_array git_am_opts;
const char *action;
int signoff;
int allow_rerere_autoupdate;
@@ -339,7 +339,7 @@ N_("Resolve all conflicts manually, mark them as resolved 
with\n"
 static int run_specific_rebase(struct rebase_options *opts)
 {
const char *argv[] = { NULL, NULL };
-   struct strbuf script_snippet = STRBUF_INIT;
+   struct strbuf script_snippet = STRBUF_INIT, buf = STRBUF_INIT;
int status;
const char *backend, *backend_func;
 
@@ -433,7 +433,9 @@ static int run_specific_rebase(struct rebase_options *opts)
oid_to_hex(>restrict_revision->object.oid) : NULL);
add_var(_snippet, "GIT_QUIET",
opts->flags & REBASE_NO_QUIET ? "" : "t");
-   add_var(_snippet, "git_am_opt", opts->git_am_opt.buf);
+   sq_quote_argv_pretty(, opts->git_am_opts.argv);
+   add_var(_snippet, "git_am_opt", buf.buf);
+   strbuf_release();
add_var(_snippet, "verbose",
opts->flags & REBASE_VERBOSE ? "t" : "");
add_var(_snippet, "diffstat",
@@ -756,7 +758,7 @@ int cmd_rebase(int argc, const char **argv, const char 
*prefix)
struct rebase_options options = {
.type = REBASE_UNSPECIFIED,
.flags = REBASE_NO_QUIET,
-   .git_am_opt = STRBUF_INIT,
+   .git_am_opts = ARGV_ARRAY_INIT,
.allow_rerere_autoupdate  = -1,
.allow_empty_message = 1,
.git_format_patch_opt = STRBUF_INIT,
@@ -777,12 +779,7 @@ int cmd_rebase(int argc, const char **argv, const char 
*prefix)
ACTION_EDIT_TODO,
ACTION_SHOW_CURRENT_PATCH,
} action = NO_ACTION;
-   int committer_date_is_author_date = 0;
-   int ignore_date = 0;
-   int ignore_whitespace = 0;
const char *gpg_sign = NULL;
-   int opt_c = -1;
-   struct string_list whitespace = STRING_LIST_INIT_NODUP;
struct string_list exec = STRING_LIST_INIT_NODUP;
const char *rebase_merges = NULL;
int fork_point = -1;
@@ -804,15 +801,20 @@ int cmd_rebase(int argc, const char **argv, const char 
*prefix)
{OPTION_NEGBIT, 'n', "no-stat", , NULL,
N_("do not show diffstat of what changed upstream"),
PARSE_OPT_NOARG, NULL, REBASE_DIFFSTAT },
-   OPT_BOOL(0, "ignore-whitespace", _whitespace,
-N_("passed to 'git apply'")),
OPT_BOOL(0, "signoff", ,
 N_("add a Signed-off-by: line to each commit")),
-   OPT_BOOL(0, "committer-date-is-author-date",
-_date_is_author_date,
-N_("passed to 'git am'")),
-   OPT_BOOL(0, "ignore-date", _date,
-N_("passed to 'git am'")),
+   OPT_PASSTHRU_ARGV(0, "ignore-whitespace", _am_opts,
+ NULL, N_("passed to 'git am'"),
+ PARSE_OPT_NOARG),
+   OPT_PASSTHRU_ARGV(0, "committer-date-is-author-date",
+ _am_opts, NULL,
+ N_("passed to 'git am'"), PARSE_OPT_NOARG),
+   OPT_PASSTHRU_ARGV(0, "ignore-date", _am_opts, NULL,
+ N_("passed to 'git am'"), PARSE_OPT_NOARG),
+   OPT_PASSTHRU_ARGV('C', NULL, _am_opts, N_("n"),
+ N_("passed to 'git apply'"), 0),
+   OPT_PASSTHRU_ARGV(0, "whitespace", _am_opts,
+ N_("action"), N_("passed to 'git apply'"), 0),
OPT_BIT('f', "force-rebase", ,
N_("cherry-pick all commits, even if unchanged"),
REBASE_FORCE),
@@ -856,10 +858,6 

[PATCH v2 0/2] rebase: understand -C again, refactor

2018-11-14 Thread Johannes Schindelin via GitGitGadget
Phillip Wood reported a problem where the built-in rebase did not understand
options like -C1, i.e. it did not expect the option argument.

While investigating how to address this best, I stumbled upon 
OPT_PASSTHRU_ARGV (which I was so far happily unaware of).

Instead of just fixing the -C bug, I decided to simply convert all of the
options intended for git am (or, eventually, for git apply). This happens to
fix that bug, and does so much more: it simplifies the entire logic (and
removes more lines than it adds).

Change since v1:

 * Introduce early parameter validation for the options passed through to 
   git am.

Johannes Schindelin (2):
  rebase: really just passthru the `git am` options
  rebase: validate -C and --whitespace= parameters early

 builtin/rebase.c  | 108 --
 t/t3406-rebase-message.sh |   7 +++
 2 files changed, 52 insertions(+), 63 deletions(-)


base-commit: 8858448bb49332d353febc078ce4a3abcc962efe
Published-As: 
https://github.com/gitgitgadget/git/releases/tags/pr-76%2Fdscho%2Frebase-Cn-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git 
pr-76/dscho/rebase-Cn-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/76

Range-diff vs v1:

 1:  dc36a45068 = 1:  dc36a45068 rebase: really just passthru the `git am` 
options
 -:  -- > 2:  4c2ba52766 rebase: validate -C and --whitespace= 
parameters early

-- 
gitgitgadget


[PATCH v2 0/1] bundle: fix issue when bundles would be empty

2018-11-14 Thread Johannes Schindelin via GitGitGadget
And yet another patch coming through Git for Windows...

Change since v1:

 * Using a better oneline now.

Gaël Lhez (1):
  bundle: cleanup lock files on error

 bundle.c| 7 ---
 t/t5607-clone-bundle.sh | 4 
 2 files changed, 8 insertions(+), 3 deletions(-)


base-commit: 8858448bb49332d353febc078ce4a3abcc962efe
Published-As: 
https://github.com/gitgitgadget/git/releases/tags/pr-79%2Fdscho%2Fcreate-empty-bundle-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git 
pr-79/dscho/create-empty-bundle-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/79

Range-diff vs v1:

 1:  6276372ad3 ! 1:  c7f05a bundle: refuse to create empty bundle
 @@ -1,6 +1,6 @@
  Author: Gaël Lhez 
  
 -bundle: refuse to create empty bundle
 +bundle: cleanup lock files on error
  
  When an user tries to create an empty bundle via `git bundle create
   ` where `` resolves to an empty list (for

-- 
gitgitgadget


[PATCH v2 0/1] Some left-over add-on for bw/config-h

2018-11-14 Thread Johannes Schindelin via GitGitGadget
Back when bw/config-h was developed (and backported to Git for Windows), I
came up with a patch to use git_dir if commondir is NULL, and contributed
that as v1 of this patch. However, it was deemed a bug if that happens, so
let's instead detect that condition and report it.

Change since v1:

 * Be loud about this bug instead of papering over it.

Johannes Schindelin (1):
  config: report a bug if git_dir exists without commondir

 config.c | 2 ++
 1 file changed, 2 insertions(+)


base-commit: 8858448bb49332d353febc078ce4a3abcc962efe
Published-As: 
https://github.com/gitgitgadget/git/releases/tags/pr-78%2Fdscho%2Fbw%2Fconfig-h-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git 
pr-78/dscho/bw/config-h-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/78

Range-diff vs v1:

 1:  a3854e4ed8 ! 1:  0767f98378 do_git_config_sequence(): fall back to git_dir 
if commondir is NULL
 @@ -1,8 +1,9 @@
  Author: Johannes Schindelin 
  
 -do_git_config_sequence(): fall back to git_dir if commondir is NULL
 +config: report a bug if git_dir exists without commondir
  
 -Just some defensive programming.
 +This did happen at some stage, and was fixed relatively quickly. Make
 +sure that we detect very quickly, too, should that happen again.
  
  Signed-off-by: Johannes Schindelin 
  
 @@ -14,7 +15,7 @@
if (opts->commondir)
repo_config = mkpathdup("%s/config", opts->commondir);
  + else if (opts->git_dir)
 -+ repo_config = mkpathdup("%s/config", opts->git_dir);
 ++ BUG("git_dir without commondir");
else
repo_config = NULL;
   

-- 
gitgitgadget


[PATCH v2 1/1] config: report a bug if git_dir exists without commondir

2018-11-14 Thread Johannes Schindelin via GitGitGadget
From: Johannes Schindelin 

This did happen at some stage, and was fixed relatively quickly. Make
sure that we detect very quickly, too, should that happen again.

Signed-off-by: Johannes Schindelin 
---
 config.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/config.c b/config.c
index 4051e38823..db6b0167c6 100644
--- a/config.c
+++ b/config.c
@@ -1676,6 +1676,8 @@ static int do_git_config_sequence(const struct 
config_options *opts,
 
if (opts->commondir)
repo_config = mkpathdup("%s/config", opts->commondir);
+   else if (opts->git_dir)
+   BUG("git_dir without commondir");
else
repo_config = NULL;
 
-- 
gitgitgadget


[PATCH 0/1] win32: modernize pthread_cond_*() shims

2018-11-13 Thread Johannes Schindelin via GitGitGadget
And yet another patch from Git for Windows' cache of treasures.

It was challenging to emulate the functions related to pthread_cond_t as
long as we tried to maintain support for Windows XP, which has nothing close
to that feature. Now that we require Windows Vista or later, we can make use
of the very nice functions associated with the CONDITION_VARIABLE data type.

Look at how much code this deletes, and how little it introduces. This is a
maintainer's dream.

Loo Rong Jie (1):
  win32: replace pthread_cond_*() with much simpler code

 compat/win32/pthread.c | 138 -
 compat/win32/pthread.h |  28 +++--
 2 files changed, 7 insertions(+), 159 deletions(-)


base-commit: 8858448bb49332d353febc078ce4a3abcc962efe
Published-As: 
https://github.com/gitgitgadget/git/releases/tags/pr-80%2Fdscho%2Fmingw-modernize-pthread_cond_t-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git 
pr-80/dscho/mingw-modernize-pthread_cond_t-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/80
-- 
gitgitgadget


[PATCH 0/1] bundle: fix issue when bundles would be empty

2018-11-13 Thread Johannes Schindelin via GitGitGadget
And yet another patch coming through Git for Windows...

Gaël Lhez (1):
  bundle: refuse to create empty bundle

 bundle.c| 7 ---
 t/t5607-clone-bundle.sh | 4 
 2 files changed, 8 insertions(+), 3 deletions(-)


base-commit: 8858448bb49332d353febc078ce4a3abcc962efe
Published-As: 
https://github.com/gitgitgadget/git/releases/tags/pr-79%2Fdscho%2Fcreate-empty-bundle-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git 
pr-79/dscho/create-empty-bundle-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/79
-- 
gitgitgadget


[PATCH 0/1] Some left-over add-on for bw/config-h

2018-11-13 Thread Johannes Schindelin via GitGitGadget
Back when bw/config-h was developed (and backported to Git for Windows), I
came up with this patch. It seems to not be strictly necessary, but I like
the safety of falling back to the Git directory when no common directory is
configured (for whatever reason).

Johannes Schindelin (1):
  do_git_config_sequence(): fall back to git_dir if commondir is NULL

 config.c | 2 ++
 1 file changed, 2 insertions(+)


base-commit: 8858448bb49332d353febc078ce4a3abcc962efe
Published-As: 
https://github.com/gitgitgadget/git/releases/tags/pr-78%2Fdscho%2Fbw%2Fconfig-h-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git 
pr-78/dscho/bw/config-h-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/78
-- 
gitgitgadget


[PATCH 1/1] do_git_config_sequence(): fall back to git_dir if commondir is NULL

2018-11-13 Thread Johannes Schindelin via GitGitGadget
From: Johannes Schindelin 

Just some defensive programming.

Signed-off-by: Johannes Schindelin 
---
 config.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/config.c b/config.c
index 4051e38823..279d0d7077 100644
--- a/config.c
+++ b/config.c
@@ -1676,6 +1676,8 @@ static int do_git_config_sequence(const struct 
config_options *opts,
 
if (opts->commondir)
repo_config = mkpathdup("%s/config", opts->commondir);
+   else if (opts->git_dir)
+   repo_config = mkpathdup("%s/config", opts->git_dir);
else
repo_config = NULL;
 
-- 
gitgitgadget


[PATCH 0/1] mingw: use CreateHardLink() directly

2018-11-13 Thread Johannes Schindelin via GitGitGadget
This is another tidbit from the stash of Git for Windows' patches: it avoids
loading the function address of CreateHardLink() at runtime. This was done
in case we were running on a Windows version that does not support that
function, but we no longer support any of these Windows versions.

Johannes Schindelin (1):
  mingw: use `CreateHardLink()` directly

 compat/mingw.c | 14 +-
 1 file changed, 1 insertion(+), 13 deletions(-)


base-commit: 8858448bb49332d353febc078ce4a3abcc962efe
Published-As: 
https://github.com/gitgitgadget/git/releases/tags/pr-77%2Fdscho%2Fmingw-CreateHardLink-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git 
pr-77/dscho/mingw-CreateHardLink-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/77
-- 
gitgitgadget


[PATCH 1/1] mingw: use `CreateHardLink()` directly

2018-11-13 Thread Johannes Schindelin via GitGitGadget
From: Johannes Schindelin 

The function `CreateHardLink()` is available in all supported Windows
versions (even since Windows XP), so there is no more need to resolve it
at runtime.

Helped-by: Max Kirillov 
Signed-off-by: Johannes Schindelin 
---
 compat/mingw.c | 14 +-
 1 file changed, 1 insertion(+), 13 deletions(-)

diff --git a/compat/mingw.c b/compat/mingw.c
index 3b44dd99d7..fdcf946275 100644
--- a/compat/mingw.c
+++ b/compat/mingw.c
@@ -2084,24 +2084,12 @@ int mingw_raise(int sig)
 
 int link(const char *oldpath, const char *newpath)
 {
-   typedef BOOL (WINAPI *T)(LPCWSTR, LPCWSTR, LPSECURITY_ATTRIBUTES);
-   static T create_hard_link = NULL;
wchar_t woldpath[MAX_PATH], wnewpath[MAX_PATH];
if (xutftowcs_path(woldpath, oldpath) < 0 ||
xutftowcs_path(wnewpath, newpath) < 0)
return -1;
 
-   if (!create_hard_link) {
-   create_hard_link = (T) GetProcAddress(
-   GetModuleHandle("kernel32.dll"), "CreateHardLinkW");
-   if (!create_hard_link)
-   create_hard_link = (T)-1;
-   }
-   if (create_hard_link == (T)-1) {
-   errno = ENOSYS;
-   return -1;
-   }
-   if (!create_hard_link(wnewpath, woldpath, NULL)) {
+   if (!CreateHardLinkW(wnewpath, woldpath, NULL)) {
errno = err_win_to_posix(GetLastError());
return -1;
}
-- 
gitgitgadget


[PATCH 0/1] rebase: understand -C again, refactor

2018-11-13 Thread Johannes Schindelin via GitGitGadget
Phillip Wood reported a problem where the built-in rebase did not understand
options like -C1, i.e. it did not expect the option argument.

While investigating how to address this best, I stumbled upon 
OPT_PASSTHRU_ARGV (which I was so far happily unaware of).

Instead of just fixing the -C bug, I decided to simply convert all of the
options intended for git am (or, eventually, for git apply). This happens to
fix that bug, and does so much more: it simplifies the entire logic (and
removes more lines than it adds).

Johannes Schindelin (1):
  rebase: really just passthru the `git am` options

 builtin/rebase.c | 98 +---
 1 file changed, 35 insertions(+), 63 deletions(-)


base-commit: 8858448bb49332d353febc078ce4a3abcc962efe
Published-As: 
https://github.com/gitgitgadget/git/releases/tags/pr-76%2Fdscho%2Frebase-Cn-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git 
pr-76/dscho/rebase-Cn-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/76
-- 
gitgitgadget


[PATCH 1/1] rebase: really just passthru the `git am` options

2018-11-13 Thread Johannes Schindelin via GitGitGadget
From: Johannes Schindelin 

Currently, we parse the options intended for `git am` as if we wanted to
handle them in `git rebase`, and then reconstruct them painstakingly to
define the `git_am_opt` variable.

However, there is a much better way (that I was unaware of, at the time
when I mentored Pratik to implement these options): OPT_PASSTHRU_ARGV.
It is intended for exactly this use case, where command-line options
want to be parsed into a separate `argv_array`.

Let's use this feature.

Incidentally, this also allows us to address a bug discovered by Phillip
Wood, where the built-in rebase failed to understand that the `-C`
option takes an optional argument.

Signed-off-by: Johannes Schindelin 
---
 builtin/rebase.c | 98 +---
 1 file changed, 35 insertions(+), 63 deletions(-)

diff --git a/builtin/rebase.c b/builtin/rebase.c
index 0ee06aa363..96ffa80b71 100644
--- a/builtin/rebase.c
+++ b/builtin/rebase.c
@@ -87,7 +87,7 @@ struct rebase_options {
REBASE_FORCE = 1<<3,
REBASE_INTERACTIVE_EXPLICIT = 1<<4,
} flags;
-   struct strbuf git_am_opt;
+   struct argv_array git_am_opts;
const char *action;
int signoff;
int allow_rerere_autoupdate;
@@ -339,7 +339,7 @@ N_("Resolve all conflicts manually, mark them as resolved 
with\n"
 static int run_specific_rebase(struct rebase_options *opts)
 {
const char *argv[] = { NULL, NULL };
-   struct strbuf script_snippet = STRBUF_INIT;
+   struct strbuf script_snippet = STRBUF_INIT, buf = STRBUF_INIT;
int status;
const char *backend, *backend_func;
 
@@ -433,7 +433,9 @@ static int run_specific_rebase(struct rebase_options *opts)
oid_to_hex(>restrict_revision->object.oid) : NULL);
add_var(_snippet, "GIT_QUIET",
opts->flags & REBASE_NO_QUIET ? "" : "t");
-   add_var(_snippet, "git_am_opt", opts->git_am_opt.buf);
+   sq_quote_argv_pretty(, opts->git_am_opts.argv);
+   add_var(_snippet, "git_am_opt", buf.buf);
+   strbuf_release();
add_var(_snippet, "verbose",
opts->flags & REBASE_VERBOSE ? "t" : "");
add_var(_snippet, "diffstat",
@@ -756,7 +758,7 @@ int cmd_rebase(int argc, const char **argv, const char 
*prefix)
struct rebase_options options = {
.type = REBASE_UNSPECIFIED,
.flags = REBASE_NO_QUIET,
-   .git_am_opt = STRBUF_INIT,
+   .git_am_opts = ARGV_ARRAY_INIT,
.allow_rerere_autoupdate  = -1,
.allow_empty_message = 1,
.git_format_patch_opt = STRBUF_INIT,
@@ -777,12 +779,7 @@ int cmd_rebase(int argc, const char **argv, const char 
*prefix)
ACTION_EDIT_TODO,
ACTION_SHOW_CURRENT_PATCH,
} action = NO_ACTION;
-   int committer_date_is_author_date = 0;
-   int ignore_date = 0;
-   int ignore_whitespace = 0;
const char *gpg_sign = NULL;
-   int opt_c = -1;
-   struct string_list whitespace = STRING_LIST_INIT_NODUP;
struct string_list exec = STRING_LIST_INIT_NODUP;
const char *rebase_merges = NULL;
int fork_point = -1;
@@ -804,15 +801,20 @@ int cmd_rebase(int argc, const char **argv, const char 
*prefix)
{OPTION_NEGBIT, 'n', "no-stat", , NULL,
N_("do not show diffstat of what changed upstream"),
PARSE_OPT_NOARG, NULL, REBASE_DIFFSTAT },
-   OPT_BOOL(0, "ignore-whitespace", _whitespace,
-N_("passed to 'git apply'")),
OPT_BOOL(0, "signoff", ,
 N_("add a Signed-off-by: line to each commit")),
-   OPT_BOOL(0, "committer-date-is-author-date",
-_date_is_author_date,
-N_("passed to 'git am'")),
-   OPT_BOOL(0, "ignore-date", _date,
-N_("passed to 'git am'")),
+   OPT_PASSTHRU_ARGV(0, "ignore-whitespace", _am_opts,
+ NULL, N_("passed to 'git am'"),
+ PARSE_OPT_NOARG),
+   OPT_PASSTHRU_ARGV(0, "committer-date-is-author-date",
+ _am_opts, NULL,
+ N_("passed to 'git am'"), PARSE_OPT_NOARG),
+   OPT_PASSTHRU_ARGV(0, "ignore-date", _am_opts, NULL,
+ N_("passed to 'git am'"), PARSE_OPT_NOARG),
+   OPT_PASSTHRU_ARGV('C', NULL, _am_opts, N_("n"),
+ N_("passed to 'git apply'"), 0),
+   OPT_PASSTHRU_ARGV(0, "whitespace", _am_opts,
+ N_("action"), N_("passed to 'git apply'"), 0),
OPT_BIT('f', "force-rebase", ,
N_("cherry-pick all commits, even if unchanged"),
REBASE_FORCE),
@@ -856,10 +858,6 

[PATCH 2/5] rebase -r: do not write MERGE_HEAD unless needed

2018-11-12 Thread Johannes Schindelin via GitGitGadget
From: Johannes Schindelin 

When we detect that a `merge` can be skipped because the merged commit
is already an ancestor of HEAD, we do not need to commit, therefore
writing the MERGE_HEAD file is useless.

It is actually worse than useless: a subsequent `git commit` will pick
it up and think that we want to merge that commit, still.

To avoid that, move the code that writes the MERGE_HEAD file to a
location where we already know that the `merge` cannot be skipped.

Signed-off-by: Johannes Schindelin 
---
 sequencer.c  | 8 
 t/t3430-rebase-merges.sh | 2 +-
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/sequencer.c b/sequencer.c
index 9e1ab3a2a7..7a9cd81afb 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -3191,10 +3191,6 @@ static int do_merge(struct commit *commit, const char 
*arg, int arg_len,
}
 
merge_commit = to_merge->item;
-   write_message(oid_to_hex(_commit->object.oid), GIT_SHA1_HEXSZ,
- git_path_merge_head(the_repository), 0);
-   write_message("no-ff", 5, git_path_merge_mode(the_repository), 0);
-
bases = get_merge_bases(head_commit, merge_commit);
if (bases && oideq(_commit->object.oid,
   >item->object.oid)) {
@@ -3203,6 +3199,10 @@ static int do_merge(struct commit *commit, const char 
*arg, int arg_len,
goto leave_merge;
}
 
+   write_message(oid_to_hex(_commit->object.oid), GIT_SHA1_HEXSZ,
+ git_path_merge_head(the_repository), 0);
+   write_message("no-ff", 5, git_path_merge_mode(the_repository), 0);
+
for (j = bases; j; j = j->next)
commit_list_insert(j->item, );
free_commit_list(bases);
diff --git a/t/t3430-rebase-merges.sh b/t/t3430-rebase-merges.sh
index 1f08a33687..cc5646836f 100755
--- a/t/t3430-rebase-merges.sh
+++ b/t/t3430-rebase-merges.sh
@@ -396,7 +396,7 @@ test_expect_success 'with --autosquash and --exec' '
grep "G: +G" actual
 '
 
-test_expect_failure '--continue after resolving conflicts after a merge' '
+test_expect_success '--continue after resolving conflicts after a merge' '
git checkout -b already-has-g E &&
git cherry-pick E..G &&
test_commit H2 &&
-- 
gitgitgadget



[PATCH 3/5] rebase -i: include MERGE_HEAD into files to clean up

2018-11-12 Thread Johannes Schindelin via GitGitGadget
From: Johannes Schindelin 

Every once in a while, the interactive rebase makes sure that no stale
files are lying around. These days, we need to include MERGE_HEAD into
that set of files, as the `merge` command will generate them.

Signed-off-by: Johannes Schindelin 
---
 sequencer.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/sequencer.c b/sequencer.c
index 7a9cd81afb..2f526390ac 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -3459,6 +3459,7 @@ static int pick_commits(struct todo_list *todo_list, 
struct replay_opts *opts)
unlink(rebase_path_author_script());
unlink(rebase_path_stopped_sha());
unlink(rebase_path_amend());
+   unlink(git_path_merge_head(the_repository));
delete_ref(NULL, "REBASE_HEAD", NULL, REF_NO_DEREF);
 
if (item->command == TODO_BREAK)
@@ -3829,6 +3830,7 @@ static int commit_staged_changes(struct replay_opts *opts,
   opts, flags))
return error(_("could not commit staged changes."));
unlink(rebase_path_amend());
+   unlink(git_path_merge_head(the_repository));
if (final_fixup) {
unlink(rebase_path_fixup_msg());
unlink(rebase_path_squash_msg());
-- 
gitgitgadget



[PATCH 5/5] status: rebase and merge can be in progress at the same time

2018-11-12 Thread Johannes Schindelin via GitGitGadget
From: Johannes Schindelin 

Since `git rebase -r` was introduced, that is possible. But our
machinery did not think that possible, and failed to say anything about
the rebase in progress when in the middle of a merge.

Let's work around that in the minimal fashion.

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

diff --git a/wt-status.c b/wt-status.c
index 187568a112..a24711374c 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -1559,6 +1559,7 @@ void wt_status_get_state(struct wt_status_state *state,
struct object_id oid;
 
if (!stat(git_path_merge_head(the_repository), )) {
+   wt_status_check_rebase(NULL, state);
state->merge_in_progress = 1;
} else if (wt_status_check_rebase(NULL, state)) {
;   /* all set */
@@ -1583,9 +1584,13 @@ static void wt_longstatus_print_state(struct wt_status 
*s)
const char *state_color = color(WT_STATUS_HEADER, s);
struct wt_status_state *state = >state;
 
-   if (state->merge_in_progress)
+   if (state->merge_in_progress) {
+   if (state->rebase_interactive_in_progress) {
+   show_rebase_information(s, state_color);
+   fputs("\n", s->fp);
+   }
show_merge_in_progress(s, state_color);
-   else if (state->am_in_progress)
+   } else if (state->am_in_progress)
show_am_in_progress(s, state_color);
else if (state->rebase_in_progress || 
state->rebase_interactive_in_progress)
show_rebase_in_progress(s, state_color);
-- 
gitgitgadget


[PATCH 1/5] rebase -r: demonstrate bug with conflicting merges

2018-11-12 Thread Johannes Schindelin via GitGitGadget
From: Johannes Schindelin 

When calling `merge` on a branch that has already been merged, that
`merge` is skipped quietly, but currently a MERGE_HEAD file is being
left behind and will then be grabbed by the next `pick` (that did
not want to create a *merge* commit).

Demonstrate this.

Signed-off-by: Johannes Schindelin 
---
 t/t3430-rebase-merges.sh | 16 
 1 file changed, 16 insertions(+)

diff --git a/t/t3430-rebase-merges.sh b/t/t3430-rebase-merges.sh
index aa7bfc88ec..1f08a33687 100755
--- a/t/t3430-rebase-merges.sh
+++ b/t/t3430-rebase-merges.sh
@@ -396,4 +396,20 @@ test_expect_success 'with --autosquash and --exec' '
grep "G: +G" actual
 '
 
+test_expect_failure '--continue after resolving conflicts after a merge' '
+   git checkout -b already-has-g E &&
+   git cherry-pick E..G &&
+   test_commit H2 &&
+
+   git checkout -b conflicts-in-merge H &&
+   test_commit H2 H2.t conflicts H2-conflict &&
+   test_must_fail git rebase -r already-has-g &&
+   grep conflicts H2.t &&
+   echo resolved >H2.t &&
+   git add -u &&
+   git rebase --continue &&
+   test_must_fail git rev-parse --verify HEAD^2 &&
+   test_path_is_missing .git/MERGE_HEAD
+'
+
 test_done
-- 
gitgitgadget



[PATCH 4/5] built-in rebase --skip/--abort: clean up stale .git/ files

2018-11-12 Thread Johannes Schindelin via GitGitGadget
From: Johannes Schindelin 

The scripted version of the rebase used to execute `git reset --hard`
when skipping or aborting. When we ported this to C, we did update the
worktree and some reflogs, but we failed to imitate `git reset --hard`'s
behavior regarding files in .git/ such as MERGE_HEAD.

Let's address this oversight.

Signed-off-by: Johannes Schindelin 
---
 builtin/rebase.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/builtin/rebase.c b/builtin/rebase.c
index 0ee06aa363..017dce1b50 100644
--- a/builtin/rebase.c
+++ b/builtin/rebase.c
@@ -23,6 +23,7 @@
 #include "revision.h"
 #include "commit-reach.h"
 #include "rerere.h"
+#include "branch.h"
 
 static char const * const builtin_rebase_usage[] = {
N_("git rebase [-i] [options] [--exec ] [--onto ] "
@@ -1002,6 +1003,7 @@ int cmd_rebase(int argc, const char **argv, const char 
*prefix)
 
if (reset_head(NULL, "reset", NULL, 0, NULL, NULL) < 0)
die(_("could not discard worktree changes"));
+   remove_branch_state();
if (read_basic_state())
exit(1);
goto run_rebase;
@@ -1019,6 +1021,7 @@ int cmd_rebase(int argc, const char **argv, const char 
*prefix)
   options.head_name, 0, NULL, NULL) < 0)
die(_("could not move back to %s"),
oid_to_hex(_head));
+   remove_branch_state();
ret = finish_rebase();
goto cleanup;
}
-- 
gitgitgadget



[PATCH 0/5] Assorted fixes revolving around rebase and merges

2018-11-12 Thread Johannes Schindelin via GitGitGadget
I noticed a couple of weeks ago that I had bogus merge commits after my
rebases, where the original commits had been regular commits.

This set me out on the adventure that is reflected in this patch series.

Of course, the thing I wanted to fix is demonstrated by 1/5 and fixed in
2/5. But while at it, I ran into other issues and fixed them since I was at
it anyway.

Johannes Schindelin (5):
  rebase -r: demonstrate bug with conflicting merges
  rebase -r: do not write MERGE_HEAD unless needed
  rebase -i: include MERGE_HEAD into files to clean up
  built-in rebase --skip/--abort: clean up stale .git/ files
  status: rebase and merge can be in progress at the same time

 builtin/rebase.c |  3 +++
 sequencer.c  | 10 ++
 t/t3430-rebase-merges.sh | 16 
 wt-status.c  |  9 +++--
 4 files changed, 32 insertions(+), 6 deletions(-)


base-commit: 8858448bb49332d353febc078ce4a3abcc962efe
Published-As: 
https://github.com/gitgitgadget/git/releases/tags/pr-75%2Fdscho%2Frebase-r-and-merge-head-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git 
pr-75/dscho/rebase-r-and-merge-head-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/75
-- 
gitgitgadget


[PATCH 1/1] apply --recount: allow "no-op hunks"

2018-11-12 Thread Johannes Schindelin via GitGitGadget
From: Johannes Schindelin 

When editing patches e.g. in `git add -e`, it is quite common that a
hunk ends up having no -/+ lines, i.e. it is now supposed to do nothing.

This use case was broken by ad6e8ed37bc1 (apply: reject a hunk that does
not do anything, 2015-06-01) with the good intention of catching a very
real, different issue in hand-edited patches.

So let's use the `--recount` option as the tell-tale whether the user
would actually be okay with no-op hunks.

Add a test case to make sure that this use case does not regress again.

Signed-off-by: Johannes Schindelin 
---
 apply.c|  2 +-
 t/t4136-apply-check.sh | 12 
 2 files changed, 13 insertions(+), 1 deletion(-)

diff --git a/apply.c b/apply.c
index 073d5f0451..76955afb00 100644
--- a/apply.c
+++ b/apply.c
@@ -1748,7 +1748,7 @@ static int parse_fragment(struct apply_state *state,
}
if (oldlines || newlines)
return -1;
-   if (!deleted && !added)
+   if (!patch->recount && !deleted && !added)
return -1;
 
fragment->leading = leading;
diff --git a/t/t4136-apply-check.sh b/t/t4136-apply-check.sh
index 6d92872318..4c3f264a63 100755
--- a/t/t4136-apply-check.sh
+++ b/t/t4136-apply-check.sh
@@ -29,6 +29,18 @@ test_expect_success 'apply exits non-zero with no-op patch' '
test_must_fail git apply --check input
 '
 
+test_expect_success '`apply --recount` allows no-op patch' '
+   echo 1 >1 &&
+   git apply --recount --check <<-\EOF
+   diff --get a/1 b/1
+   index 6696ea4..606eddd 100644
+   --- a/1
+   +++ b/1
+   @@ -1,1 +1,1 @@
+1
+   EOF
+'
+
 test_expect_success 'invalid combination: create and copy' '
test_must_fail git apply --check - <<-\EOF
diff --git a/1 b/2
-- 
gitgitgadget


[PATCH 0/1] Allow "no-op hunks" when editing the diff in git add -e

2018-11-12 Thread Johannes Schindelin via GitGitGadget
I use git add -e frequently. Often there are multiple hunks and I end up
deleting the + lines and converting the - lines to context lines, as I like
to stage massive changes in an incremental fashion (and commit those staged
changes incrementally, too).

Some time after I invented git add -e, this feature was broken, and I had to
start identifying and deleting those hunks with no changes.

I finally got around to find the regression, and to fix it. Here is the
outcome of this effort.

Johannes Schindelin (1):
  apply --recount: allow "no-op hunks"

 apply.c|  2 +-
 t/t4136-apply-check.sh | 12 
 2 files changed, 13 insertions(+), 1 deletion(-)


base-commit: 8858448bb49332d353febc078ce4a3abcc962efe
Published-As: 
https://github.com/gitgitgadget/git/releases/tags/pr-74%2Fdscho%2Fapply-recount-empty-hunk-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git 
pr-74/dscho/apply-recount-empty-hunk-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/74
-- 
gitgitgadget


[PATCH 4/5] tests: do not require Git to be built when testing an installed Git

2018-11-12 Thread Johannes Schindelin via GitGitGadget
From: Johannes Schindelin 

We really only need the test helpers in that case, but that is not what
we test for. So let's skip the test for now when we know that we want to
test an installed Git.

Signed-off-by: Johannes Schindelin 
---
 t/test-lib.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/t/test-lib.sh b/t/test-lib.sh
index 832ede5099..1ea20dc2dc 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -51,7 +51,7 @@ export LSAN_OPTIONS
 
 
 # It appears that people try to run tests without building...
-"$GIT_BUILD_DIR/git" >/dev/null
+test -n "$GIT_TEST_INSTALLED" || "$GIT_BUILD_DIR/git" >/dev/null ||
 if test $? != 1
 then
echo >&2 'error: you do not seem to have built git yet.'
-- 
gitgitgadget



[PATCH 5/5] tests: explicitly use `git.exe` on Windows

2018-11-12 Thread Johannes Schindelin via GitGitGadget
From: Johannes Schindelin 

In the bin-wrappers/* scripts, we already take pains to use `git.exe`
rather than `git`, as this could pick up the wrong thing on Windows
(i.e. if there exists a `git` file or directory in the build directory).

Now we do the same in the tests' start-up code.

This also helps when testing an installed Git, as there might be even
more likely some stray file or directory in the way.

Note: the only way we can record whether the `.exe` suffix is by writing
it to the `GIT-BUILD-OPTIONS` file and sourcing it at the beginning of
`t/test-lib.sh`. This is not a requirement introduced by this patch, but
we move the call to be able to use the `$X` variable that holds the file
extension, if any.

Note also: the many, many calls to `git this` and `git that` are
unaffected, as the regular PATH search will find the `.exe` files on
Windows (and not be confused by a directory of the name `git` that is
in one of the directories listed in the `PATH` variable), while
`/path/to/git` would not, per se, know that it is looking for an
executable and happily prefer such a directory.

Signed-off-by: Johannes Schindelin 
---
 Makefile|  1 +
 t/test-lib-functions.sh |  2 +-
 t/test-lib.sh   | 13 +
 3 files changed, 11 insertions(+), 5 deletions(-)

diff --git a/Makefile b/Makefile
index bbfbb4292d..5df0118ce9 100644
--- a/Makefile
+++ b/Makefile
@@ -2590,6 +2590,7 @@ GIT-BUILD-OPTIONS: FORCE
@echo NO_UNIX_SOCKETS=\''$(subst ','\'',$(subst 
','\'',$(NO_UNIX_SOCKETS)))'\' >>$@+
@echo PAGER_ENV=\''$(subst ','\'',$(subst ','\'',$(PAGER_ENV)))'\' >>$@+
@echo DC_SHA1=\''$(subst ','\'',$(subst ','\'',$(DC_SHA1)))'\' >>$@+
+   @echo X=\'$(X)\' >>$@+
 ifdef TEST_OUTPUT_DIRECTORY
@echo TEST_OUTPUT_DIRECTORY=\''$(subst ','\'',$(subst 
','\'',$(TEST_OUTPUT_DIRECTORY)))'\' >>$@+
 endif
diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
index 801cc9b2ef..c167b2e1af 100644
--- a/t/test-lib-functions.sh
+++ b/t/test-lib-functions.sh
@@ -900,7 +900,7 @@ test_create_repo () {
mkdir -p "$repo"
(
cd "$repo" || error "Cannot setup test environment"
-   "${GIT_TEST_INSTALLED:-$GIT_EXEC_PATH}/git" init \
+   "${GIT_TEST_INSTALLED:-$GIT_EXEC_PATH}/git$X" init \
"--template=$GIT_BUILD_DIR/templates/blt/" >&3 2>&4 ||
error "cannot run git init -- have you built things yet?"
mv .git/hooks .git/hooks-disabled
diff --git a/t/test-lib.sh b/t/test-lib.sh
index 1ea20dc2dc..3e2a9ce76d 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -49,18 +49,23 @@ export ASAN_OPTIONS
 : ${LSAN_OPTIONS=abort_on_error=1}
 export LSAN_OPTIONS
 
+if test ! -f "$GIT_BUILD_DIR"/GIT-BUILD-OPTIONS
+then
+   echo >&2 'error: GIT-BUILD-OPTIONS missing (has Git been built?).'
+   exit 1
+fi
+. "$GIT_BUILD_DIR"/GIT-BUILD-OPTIONS
+export PERL_PATH SHELL_PATH
+
 
 # It appears that people try to run tests without building...
-test -n "$GIT_TEST_INSTALLED" || "$GIT_BUILD_DIR/git" >/dev/null ||
+test -n "$GIT_TEST_INSTALLED" || "$GIT_BUILD_DIR/git$X" >/dev/null ||
 if test $? != 1
 then
echo >&2 'error: you do not seem to have built git yet.'
exit 1
 fi
 
-. "$GIT_BUILD_DIR"/GIT-BUILD-OPTIONS
-export PERL_PATH SHELL_PATH
-
 # if --tee was passed, write the output not only to the terminal, but
 # additionally to the file test-results/$BASENAME.out, too.
 case "$GIT_TEST_TEE_STARTED, $* " in
-- 
gitgitgadget


[PATCH 3/5] t/lib-gettext: test installed git-sh-i18n if GIT_TEST_INSTALLED is set

2018-11-12 Thread Johannes Schindelin via GitGitGadget
From: Johannes Schindelin 

It makes very, very little sense to test the built git-sh-i18n when the
user asked specifically to test another one.

Signed-off-by: Johannes Schindelin 
---
 t/lib-gettext.sh | 7 ++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/t/lib-gettext.sh b/t/lib-gettext.sh
index eec757f104..9eb160c997 100644
--- a/t/lib-gettext.sh
+++ b/t/lib-gettext.sh
@@ -10,7 +10,12 @@ GIT_TEXTDOMAINDIR="$GIT_BUILD_DIR/po/build/locale"
 GIT_PO_PATH="$GIT_BUILD_DIR/po"
 export GIT_TEXTDOMAINDIR GIT_PO_PATH
 
-. "$GIT_BUILD_DIR"/git-sh-i18n
+if test -n "$GIT_TEST_INSTALLED"
+then
+   . "$(git --exec-path)"/git-sh-i18n
+else
+   . "$GIT_BUILD_DIR"/git-sh-i18n
+fi
 
 if test_have_prereq GETTEXT && ! test_have_prereq GETTEXT_POISON
 then
-- 
gitgitgadget



[PATCH 2/5] tests: respect GIT_TEST_INSTALLED when initializing repositories

2018-11-12 Thread Johannes Schindelin via GitGitGadget
From: Johannes Schindelin 

It really makes very, very little sense to use a different git
executable than the one the caller indicated via setting the environment
variable GIT_TEST_INSTALLED.

Signed-off-by: Johannes Schindelin 
---
 t/test-lib-functions.sh | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
index 78d8c3783b..801cc9b2ef 100644
--- a/t/test-lib-functions.sh
+++ b/t/test-lib-functions.sh
@@ -900,7 +900,8 @@ test_create_repo () {
mkdir -p "$repo"
(
cd "$repo" || error "Cannot setup test environment"
-   "$GIT_EXEC_PATH/git-init" 
"--template=$GIT_BUILD_DIR/templates/blt/" >&3 2>&4 ||
+   "${GIT_TEST_INSTALLED:-$GIT_EXEC_PATH}/git" init \
+   "--template=$GIT_BUILD_DIR/templates/blt/" >&3 2>&4 ||
error "cannot run git init -- have you built things yet?"
mv .git/hooks .git/hooks-disabled
) || exit
-- 
gitgitgadget



[PATCH 1/5] tests: fix GIT_TEST_INSTALLED's PATH to include t/helper/

2018-11-12 Thread Johannes Schindelin via GitGitGadget
From: Johannes Schindelin 

We really need to be able to find the test helpers... Really. This
change was forgotten when we moved the test helpers into t/helper/

Signed-off-by: Johannes Schindelin 
---
 t/test-lib.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/t/test-lib.sh b/t/test-lib.sh
index 47a99aa0ed..832ede5099 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -957,7 +957,7 @@ elif test -n "$GIT_TEST_INSTALLED"
 then
GIT_EXEC_PATH=$($GIT_TEST_INSTALLED/git --exec-path)  ||
error "Cannot run git from $GIT_TEST_INSTALLED."
-   PATH=$GIT_TEST_INSTALLED:$GIT_BUILD_DIR:$PATH
+   PATH=$GIT_TEST_INSTALLED:$GIT_BUILD_DIR/t/helper:$PATH
GIT_EXEC_PATH=${GIT_TEST_EXEC_PATH:-$GIT_EXEC_PATH}
 else # normal case, use ../bin-wrappers only unless $with_dashes:
git_bin_dir="$GIT_BUILD_DIR/bin-wrappers"
-- 
gitgitgadget



[PATCH 0/5] tests: various improvements to the GIT_TEST_INSTALLED feature

2018-11-12 Thread Johannes Schindelin via GitGitGadget
By setting the GIT_TEST_INSTALLED variable to the path of an installed Git
executable, it is possible to run the test suite also on a specific
installed version (as opposed to a version built from scratch).

The only thing this needs that is unlikely to be installed is the test
helper(s).

However, there have been a few rough edges around that, identified in my
(still ongoing) work to support building Git in Visual Studio (where we do
not want to run GNU Make, and where we have no canonical way to create, say,
hard-linked copies of the built-in commands, and other work to let Git for
Windows play better with BusyBox.

Triggered by a comment of AEvar
[https://public-inbox.org/git/20181102223743.4331-1-ava...@gmail.com/], I
hereby contribute these assorted fixes for the GIT_TEST_INSTALLED feature.

Johannes Schindelin (5):
  tests: fix GIT_TEST_INSTALLED's PATH to include t/helper/
  tests: respect GIT_TEST_INSTALLED when initializing repositories
  t/lib-gettext: test installed git-sh-i18n if GIT_TEST_INSTALLED is set
  tests: do not require Git to be built when testing an installed Git
  tests: explicitly use `git.exe` on Windows

 Makefile|  1 +
 t/lib-gettext.sh|  7 ++-
 t/test-lib-functions.sh |  3 ++-
 t/test-lib.sh   | 15 ++-
 4 files changed, 19 insertions(+), 7 deletions(-)


base-commit: 8858448bb49332d353febc078ce4a3abcc962efe
Published-As: 
https://github.com/gitgitgadget/git/releases/tags/pr-73%2Fdscho%2Ftest-git-installed-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git 
pr-73/dscho/test-git-installed-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/73
-- 
gitgitgadget


[PATCH v2 2/3] rebase: prepare reset_head() for more flags

2018-11-12 Thread Johannes Schindelin via GitGitGadget
From: Johannes Schindelin 

Currently, we only accept the flag indicating whether the HEAD should be
detached not. In the next commit, we want to introduce another flag: to
toggle between emulating `reset --hard` vs `checkout -q`.

Signed-off-by: Johannes Schindelin 
---
 builtin/rebase.c | 9 ++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/builtin/rebase.c b/builtin/rebase.c
index e173654d56..074594cf10 100644
--- a/builtin/rebase.c
+++ b/builtin/rebase.c
@@ -522,10 +522,13 @@ finished_rebase:
 
 #define GIT_REFLOG_ACTION_ENVIRONMENT "GIT_REFLOG_ACTION"
 
+#define RESET_HEAD_DETACH (1<<0)
+
 static int reset_head(struct object_id *oid, const char *action,
- const char *switch_to_branch, int detach_head,
+ const char *switch_to_branch, unsigned flags,
  const char *reflog_orig_head, const char *reflog_head)
 {
+   unsigned detach_head = flags & RESET_HEAD_DETACH;
struct object_id head_oid;
struct tree_desc desc;
struct lock_file lock = LOCK_INIT;
@@ -1500,8 +1503,8 @@ int cmd_rebase(int argc, const char **argv, const char 
*prefix)
 "it...\n"));
 
strbuf_addf(, "rebase: checkout %s", options.onto_name);
-   if (reset_head(>object.oid, "checkout", NULL, 1,
-   NULL, msg.buf))
+   if (reset_head(>object.oid, "checkout", NULL,
+  RESET_HEAD_DETACH, NULL, msg.buf))
die(_("Could not detach HEAD"));
strbuf_release();
 
-- 
gitgitgadget



[PATCH v2 0/3] Fix built-in rebase perf regression

2018-11-12 Thread Johannes Schindelin via GitGitGadget
In our tests with large repositories, we noticed a serious regression of the
performance of git rebase when using the built-in vs the shell script
version. It boils down to an incorrect conversion of a git checkout -q:
instead of using a twoway_merge as git checkout does, we used a oneway_merge 
as git reset does. The latter, however, calls lstat() on all files listed in
the index, while the former essentially looks only at the files that are
different between the given two revisions.

Let's reinstate the original behavior by introducing a flag to the 
reset_head() function to indicate whether we want to emulate reset --hard 
(in which case we use the oneway_merge, otherwise we use twoway_merge).

Johannes Schindelin (3):
  rebase: consolidate clean-up code before leaving reset_head()
  rebase: prepare reset_head() for more flags
  built-in rebase: reinstate `checkout -q` behavior where appropriate

 builtin/rebase.c | 79 
 1 file changed, 46 insertions(+), 33 deletions(-)


base-commit: 8858448bb49332d353febc078ce4a3abcc962efe
Published-As: 
https://github.com/gitgitgadget/git/releases/tags/pr-72%2Fdscho%2Fbuiltin-rebase-perf-regression-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git 
pr-72/dscho/builtin-rebase-perf-regression-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/72

Range-diff vs v1:

 1:  64597fe827 ! 1:  28e24d98ab rebase: consolidate clean-up code before 
leaving reset_head()
 @@ -11,6 +11,33 @@
  --- a/builtin/rebase.c
  +++ b/builtin/rebase.c
  @@
 +  if (switch_to_branch && !starts_with(switch_to_branch, "refs/"))
 +  BUG("Not a fully qualified branch: '%s'", switch_to_branch);
 + 
 +- if (hold_locked_index(, LOCK_REPORT_ON_ERROR) < 0)
 +- return -1;
 ++ if (hold_locked_index(, LOCK_REPORT_ON_ERROR) < 0) {
 ++ ret = -1;
 ++ goto leave_reset_head;
 ++ }
 + 
 +  if (!oid) {
 +  if (get_oid("HEAD", _oid)) {
 +- rollback_lock_file();
 +- return error(_("could not determine HEAD revision"));
 ++ ret = error(_("could not determine HEAD revision"));
 ++ goto leave_reset_head;
 +  }
 +  oid = _oid;
 +  }
 +@@
 +  unpack_tree_opts.reset = 1;
 + 
 +  if (read_index_unmerged(the_repository->index) < 0) {
 +- rollback_lock_file();
 +- return error(_("could not read index"));
 ++ ret = error(_("could not read index"));
 ++ goto leave_reset_head;
}
   
if (!fill_tree_descriptor(, oid)) {
 @@ -31,15 +58,17 @@
}
   
tree = parse_tree_indirect(oid);
 -@@
 +  prime_cache_tree(the_repository->index, tree);
   
 -  if (write_locked_index(the_repository->index, , COMMIT_LOCK) < 0)
 +- if (write_locked_index(the_repository->index, , COMMIT_LOCK) < 0)
 ++ if (write_locked_index(the_repository->index, , COMMIT_LOCK) < 0) {
ret = error(_("could not write index"));
  - free((void *)desc.buffer);
 - 
 -  if (ret)
 +-
 +- if (ret)
  - return ret;
  + goto leave_reset_head;
 ++ }
   
reflog_action = getenv(GIT_REFLOG_ACTION_ENVIRONMENT);
strbuf_addf(, "%s: ", reflog_action ? reflog_action : "rebase");
 -:  -- > 2:  db963b2094 rebase: prepare reset_head() for more flags
 2:  070092b430 ! 3:  a7360b856f built-in rebase: reinstate `checkout -q` 
behavior where appropriate
 @@ -20,15 +20,18 @@
  @@
   #define GIT_REFLOG_ACTION_ENVIRONMENT "GIT_REFLOG_ACTION"
   
 + #define RESET_HEAD_DETACH (1<<0)
 ++#define RESET_HEAD_HARD (1<<1)
 + 
   static int reset_head(struct object_id *oid, const char *action,
 --   const char *switch_to_branch, int detach_head,
 -+   const char *switch_to_branch,
 -+   int detach_head, int reset_hard,
 +const char *switch_to_branch, unsigned flags,
  const char *reflog_orig_head, const char *reflog_head)
   {
 +  unsigned detach_head = flags & RESET_HEAD_DETACH;
 ++ unsigned reset_hard = flags & RESET_HEAD_HARD;
struct object_id head_oid;
  - struct tree_desc desc;
 -+ struct tree_desc desc[2];
 ++ struct tree_desc desc[2] = { { NULL }, { NULL } };
struct lock_file lock = LOCK_INIT;
struct unpack_trees_options unpack_tree_opts;
struct tree *tree;
 @@ -42,18 +45,18 @@
if (switch_to_branch && !starts_with(switch_to_branch, "refs/"))
BUG("Not a fully qualified branch: '%s'", switch_to_branch);
  @@
 -  if (hold_locked_index(, LOCK_REPORT_ON_ERROR) < 0)
 -  return -1;
 +  goto leave_reset_head;
 +  }
   
  - if (!oid) {
  - if 

[PATCH v2 1/3] rebase: consolidate clean-up code before leaving reset_head()

2018-11-12 Thread Johannes Schindelin via GitGitGadget
From: Johannes Schindelin 

The same clean-up code is repeated quite a few times; Let's DRY up the
code some.

Signed-off-by: Johannes Schindelin 
---
 builtin/rebase.c | 36 ++--
 1 file changed, 18 insertions(+), 18 deletions(-)

diff --git a/builtin/rebase.c b/builtin/rebase.c
index 0ee06aa363..e173654d56 100644
--- a/builtin/rebase.c
+++ b/builtin/rebase.c
@@ -541,13 +541,15 @@ static int reset_head(struct object_id *oid, const char 
*action,
if (switch_to_branch && !starts_with(switch_to_branch, "refs/"))
BUG("Not a fully qualified branch: '%s'", switch_to_branch);
 
-   if (hold_locked_index(, LOCK_REPORT_ON_ERROR) < 0)
-   return -1;
+   if (hold_locked_index(, LOCK_REPORT_ON_ERROR) < 0) {
+   ret = -1;
+   goto leave_reset_head;
+   }
 
if (!oid) {
if (get_oid("HEAD", _oid)) {
-   rollback_lock_file();
-   return error(_("could not determine HEAD revision"));
+   ret = error(_("could not determine HEAD revision"));
+   goto leave_reset_head;
}
oid = _oid;
}
@@ -564,32 +566,27 @@ static int reset_head(struct object_id *oid, const char 
*action,
unpack_tree_opts.reset = 1;
 
if (read_index_unmerged(the_repository->index) < 0) {
-   rollback_lock_file();
-   return error(_("could not read index"));
+   ret = error(_("could not read index"));
+   goto leave_reset_head;
}
 
if (!fill_tree_descriptor(, oid)) {
-   error(_("failed to find tree of %s"), oid_to_hex(oid));
-   rollback_lock_file();
-   free((void *)desc.buffer);
-   return -1;
+   ret = error(_("failed to find tree of %s"), oid_to_hex(oid));
+   goto leave_reset_head;
}
 
if (unpack_trees(1, , _tree_opts)) {
-   rollback_lock_file();
-   free((void *)desc.buffer);
-   return -1;
+   ret = -1;
+   goto leave_reset_head;
}
 
tree = parse_tree_indirect(oid);
prime_cache_tree(the_repository->index, tree);
 
-   if (write_locked_index(the_repository->index, , COMMIT_LOCK) < 0)
+   if (write_locked_index(the_repository->index, , COMMIT_LOCK) < 0) {
ret = error(_("could not write index"));
-   free((void *)desc.buffer);
-
-   if (ret)
-   return ret;
+   goto leave_reset_head;
+   }
 
reflog_action = getenv(GIT_REFLOG_ACTION_ENVIRONMENT);
strbuf_addf(, "%s: ", reflog_action ? reflog_action : "rebase");
@@ -622,7 +619,10 @@ static int reset_head(struct object_id *oid, const char 
*action,
 UPDATE_REFS_MSG_ON_ERR);
}
 
+leave_reset_head:
strbuf_release();
+   rollback_lock_file();
+   free((void *)desc.buffer);
return ret;
 }
 
-- 
gitgitgadget



[PATCH v2 3/3] built-in rebase: reinstate `checkout -q` behavior where appropriate

2018-11-12 Thread Johannes Schindelin via GitGitGadget
From: Johannes Schindelin 

When we converted a `git checkout -q $onto^0` call to use
`reset_head()`, we inadvertently incurred a change from a twoway_merge
to a oneway_merge, as if we wanted a `git reset --hard` instead.

This has performance ramifications under certain, though, as the
oneway_merge needs to lstat() every single index entry whereas
twoway_merge does not.

So let's go back to the old behavior.

Signed-off-by: Johannes Schindelin 
---
 builtin/rebase.c | 40 +---
 1 file changed, 25 insertions(+), 15 deletions(-)

diff --git a/builtin/rebase.c b/builtin/rebase.c
index 074594cf10..dc78c1497d 100644
--- a/builtin/rebase.c
+++ b/builtin/rebase.c
@@ -523,14 +523,16 @@ finished_rebase:
 #define GIT_REFLOG_ACTION_ENVIRONMENT "GIT_REFLOG_ACTION"
 
 #define RESET_HEAD_DETACH (1<<0)
+#define RESET_HEAD_HARD (1<<1)
 
 static int reset_head(struct object_id *oid, const char *action,
  const char *switch_to_branch, unsigned flags,
  const char *reflog_orig_head, const char *reflog_head)
 {
unsigned detach_head = flags & RESET_HEAD_DETACH;
+   unsigned reset_hard = flags & RESET_HEAD_HARD;
struct object_id head_oid;
-   struct tree_desc desc;
+   struct tree_desc desc[2] = { { NULL }, { NULL } };
struct lock_file lock = LOCK_INIT;
struct unpack_trees_options unpack_tree_opts;
struct tree *tree;
@@ -539,7 +541,7 @@ static int reset_head(struct object_id *oid, const char 
*action,
size_t prefix_len;
struct object_id *orig = NULL, oid_orig,
*old_orig = NULL, oid_old_orig;
-   int ret = 0;
+   int ret = 0, nr = 0;
 
if (switch_to_branch && !starts_with(switch_to_branch, "refs/"))
BUG("Not a fully qualified branch: '%s'", switch_to_branch);
@@ -549,20 +551,20 @@ static int reset_head(struct object_id *oid, const char 
*action,
goto leave_reset_head;
}
 
-   if (!oid) {
-   if (get_oid("HEAD", _oid)) {
-   ret = error(_("could not determine HEAD revision"));
-   goto leave_reset_head;
-   }
-   oid = _oid;
+   if ((!oid || !reset_hard) && get_oid("HEAD", _oid)) {
+   ret = error(_("could not determine HEAD revision"));
+   goto leave_reset_head;
}
 
+   if (!oid)
+   oid = _oid;
+
memset(_tree_opts, 0, sizeof(unpack_tree_opts));
setup_unpack_trees_porcelain(_tree_opts, action);
unpack_tree_opts.head_idx = 1;
unpack_tree_opts.src_index = the_repository->index;
unpack_tree_opts.dst_index = the_repository->index;
-   unpack_tree_opts.fn = oneway_merge;
+   unpack_tree_opts.fn = reset_hard ? oneway_merge : twoway_merge;
unpack_tree_opts.update = 1;
unpack_tree_opts.merge = 1;
if (!detach_head)
@@ -573,12 +575,17 @@ static int reset_head(struct object_id *oid, const char 
*action,
goto leave_reset_head;
}
 
-   if (!fill_tree_descriptor(, oid)) {
+   if (!reset_hard && !fill_tree_descriptor([nr++], _oid)) {
+   ret = error(_("failed to find tree of %s"), oid_to_hex(oid));
+   goto leave_reset_head;
+   }
+
+   if (!fill_tree_descriptor([nr++], oid)) {
ret = error(_("failed to find tree of %s"), oid_to_hex(oid));
goto leave_reset_head;
}
 
-   if (unpack_trees(1, , _tree_opts)) {
+   if (unpack_trees(nr, desc, _tree_opts)) {
ret = -1;
goto leave_reset_head;
}
@@ -625,7 +632,8 @@ static int reset_head(struct object_id *oid, const char 
*action,
 leave_reset_head:
strbuf_release();
rollback_lock_file();
-   free((void *)desc.buffer);
+   while (nr)
+   free((void *)desc[--nr].buffer);
return ret;
 }
 
@@ -1003,7 +1011,8 @@ int cmd_rebase(int argc, const char **argv, const char 
*prefix)
rerere_clear(_rr);
string_list_clear(_rr, 1);
 
-   if (reset_head(NULL, "reset", NULL, 0, NULL, NULL) < 0)
+   if (reset_head(NULL, "reset", NULL, RESET_HEAD_HARD,
+  NULL, NULL) < 0)
die(_("could not discard worktree changes"));
if (read_basic_state())
exit(1);
@@ -1019,7 +1028,8 @@ int cmd_rebase(int argc, const char **argv, const char 
*prefix)
if (read_basic_state())
exit(1);
if (reset_head(_head, "reset",
-  options.head_name, 0, NULL, NULL) < 0)
+  options.head_name, RESET_HEAD_HARD,
+  NULL, NULL) < 0)
die(_("could not move back to %s"),
oid_to_hex(_head));
ret = 

[PATCH v2 0/1] Update .mailmap

2018-11-09 Thread Johannes Schindelin via GitGitGadget
I noticed that there were a couple of duplicate entries in git shortlog -nse
v2.19.1.., and then continued a bit to remove the duplicate entries even for
v2.10.0..

Changes relative to v1:

 * Fixed the commit message (it talked about the opposite commit range than
   intended).
 * Added a formerly missing space between the email addresses of Masaya.

Johannes Schindelin (1):
  Update .mailmap

 .mailmap | 13 +
 1 file changed, 13 insertions(+)


base-commit: 8858448bb49332d353febc078ce4a3abcc962efe
Published-As: 
https://github.com/gitgitgadget/git/releases/tags/pr-71%2Fdscho%2Fmailmap-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git 
pr-71/dscho/mailmap-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/71

Range-diff vs v1:

 1:  b590a9bebf ! 1:  c121ebc474 Update .mailmap
 @@ -2,7 +2,7 @@
  
  Update .mailmap
  
 -This patch makes the output of `git shortlog -nse v2.10.0`
 +This patch makes the output of `git shortlog -nse v2.10.0..master`
  duplicate-free.
  
  Signed-off-by: Johannes Schindelin 
 @@ -47,7 +47,7 @@
   Mark Rada 
   Martin Langhoff  
   Martin von Zweigbergk  

 -+Masaya Suzuki 
 ++Masaya Suzuki  
   Matt Draisey  
   Matt Kraai  
   Matt McCutchen  

-- 
gitgitgadget


[PATCH v2 1/1] Update .mailmap

2018-11-09 Thread Johannes Schindelin via GitGitGadget
From: Johannes Schindelin 

This patch makes the output of `git shortlog -nse v2.10.0..master`
duplicate-free.

Signed-off-by: Johannes Schindelin 
---
 .mailmap | 13 +
 1 file changed, 13 insertions(+)

diff --git a/.mailmap b/.mailmap
index bef3352b0d..eb7b5fc7b9 100644
--- a/.mailmap
+++ b/.mailmap
@@ -21,6 +21,8 @@ Anders Kaseorg  
 Aneesh Kumar K.V 
 Amos Waterland  
 Amos Waterland  
+Ben Peart  
+Ben Peart  
 Ben Walton  
 Benoit Sigoure  
 Bernt Hansen  
@@ -32,6 +34,7 @@ Bryan Larsen  
 Cheng Renquan 
 Chris Shoemaker 
 Chris Wright  
+Christian Ludwig  
 Cord Seele  
 Christian Couder  
 Christian Stimming  
@@ -51,6 +54,7 @@ David Reiss  

 David S. Miller 
 David Turner  
 David Turner  
+Derrick Stolee  
 Deskin Miller 
 Dirk Süsserott 
 Eric Blake  
@@ -98,6 +102,7 @@ Jens Axboe  
 Jens Lindström  Jens Lindstrom 
 Jim Meyering  
 Joachim Berdal Haga 
+Joachim Jablon  
 Johannes Schindelin  
 Johannes Sixt  
 Johannes Sixt  
@@ -150,6 +155,7 @@ Mark Levedahl  
 Mark Rada 
 Martin Langhoff  
 Martin von Zweigbergk  
+Masaya Suzuki  
 Matt Draisey  
 Matt Kraai  
 Matt McCutchen  
@@ -157,6 +163,7 @@ Matthias Kestenholz  

 Matthias Rüster  Matthias Ruester
 Matthias Urlichs  
 Matthias Urlichs  
+Matthieu Moy  
 Michael Coleman 
 Michael J Gruber  
 Michael J Gruber  
@@ -180,7 +187,11 @@ Nick Stokoe  Nick Woolley 

 Nick Stokoe  Nick Woolley 
 Nicolas Morey-Chaisemartin  

 Nicolas Morey-Chaisemartin  

+Nicolas Morey-Chaisemartin  

+Nicolas Morey-Chaisemartin  

+Nicolas Morey-Chaisemartin  

 Nicolas Sebrecht  
+Orgad Shaneh  
 Paolo Bonzini  
 Pascal Obry  
 Pascal Obry  
@@ -200,6 +211,7 @@ Philipp A. Hartmann  
 Philippe Bruhat 
 Ralf Thielow  
 Ramsay Jones  
+Randall S. Becker  
 René Scharfe  
 René Scharfe  Rene Scharfe
 Richard Hansen  
@@ -238,6 +250,7 @@ Steven Walter  

 Sven Verdoolaege  
 Sven Verdoolaege  
 SZEDER Gábor  
+Tao Qingyun  <845767...@qq.com>
 Tay Ray Chuan 
 Ted Percival  
 Theodore Ts'o 
-- 
gitgitgadget


[PATCH 0/2] Fix built-in rebase perf regression

2018-11-09 Thread Johannes Schindelin via GitGitGadget
In our tests with large repositories, we noticed a serious regression of the
performance of git rebase when using the built-in vs the shell script
version. It boils down to an incorrect conversion of a git checkout -q:
instead of using a twoway_merge as git checkout does, we used a oneway_merge 
as git reset does. The latter, however, calls lstat() on all files listed in
the index, while the former essentially looks only at the files that are
different between the given two revisions.

Let's reinstate the original behavior by introducing a flag to the 
reset_head() function to indicate whether we want to emulate reset --hard 
(in which case we use the oneway_merge, otherwise we use twoway_merge).

Johannes Schindelin (2):
  rebase: consolidate clean-up code before leaving reset_head()
  built-in rebase: reinstate `checkout -q` behavior where appropriate

 builtin/rebase.c | 60 ++--
 1 file changed, 33 insertions(+), 27 deletions(-)


base-commit: 8858448bb49332d353febc078ce4a3abcc962efe
Published-As: 
https://github.com/gitgitgadget/git/releases/tags/pr-72%2Fdscho%2Fbuiltin-rebase-perf-regression-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git 
pr-72/dscho/builtin-rebase-perf-regression-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/72
-- 
gitgitgadget


[PATCH 2/2] built-in rebase: reinstate `checkout -q` behavior where appropriate

2018-11-09 Thread Johannes Schindelin via GitGitGadget
From: Johannes Schindelin 

When we converted a `git checkout -q $onto^0` call to use
`reset_head()`, we inadvertently incurred a change from a twoway_merge
to a oneway_merge, as if we wanted a `git reset --hard` instead.

This has performance ramifications under certain, though, as the
oneway_merge needs to lstat() every single index entry whereas
twoway_merge does not.

So let's go back to the old behavior.

Signed-off-by: Johannes Schindelin 
---
 builtin/rebase.c | 45 ++---
 1 file changed, 26 insertions(+), 19 deletions(-)

diff --git a/builtin/rebase.c b/builtin/rebase.c
index 6f6d7de156..c1cc50f3f8 100644
--- a/builtin/rebase.c
+++ b/builtin/rebase.c
@@ -523,11 +523,12 @@ finished_rebase:
 #define GIT_REFLOG_ACTION_ENVIRONMENT "GIT_REFLOG_ACTION"
 
 static int reset_head(struct object_id *oid, const char *action,
- const char *switch_to_branch, int detach_head,
+ const char *switch_to_branch,
+ int detach_head, int reset_hard,
  const char *reflog_orig_head, const char *reflog_head)
 {
struct object_id head_oid;
-   struct tree_desc desc;
+   struct tree_desc desc[2];
struct lock_file lock = LOCK_INIT;
struct unpack_trees_options unpack_tree_opts;
struct tree *tree;
@@ -536,7 +537,7 @@ static int reset_head(struct object_id *oid, const char 
*action,
size_t prefix_len;
struct object_id *orig = NULL, oid_orig,
*old_orig = NULL, oid_old_orig;
-   int ret = 0;
+   int ret = 0, nr = 0;
 
if (switch_to_branch && !starts_with(switch_to_branch, "refs/"))
BUG("Not a fully qualified branch: '%s'", switch_to_branch);
@@ -544,20 +545,20 @@ static int reset_head(struct object_id *oid, const char 
*action,
if (hold_locked_index(, LOCK_REPORT_ON_ERROR) < 0)
return -1;
 
-   if (!oid) {
-   if (get_oid("HEAD", _oid)) {
-   rollback_lock_file();
-   return error(_("could not determine HEAD revision"));
-   }
-   oid = _oid;
+   if (get_oid("HEAD", _oid)) {
+   rollback_lock_file();
+   return error(_("could not determine HEAD revision"));
}
 
+   if (!oid)
+   oid = _oid;
+
memset(_tree_opts, 0, sizeof(unpack_tree_opts));
setup_unpack_trees_porcelain(_tree_opts, action);
unpack_tree_opts.head_idx = 1;
unpack_tree_opts.src_index = the_repository->index;
unpack_tree_opts.dst_index = the_repository->index;
-   unpack_tree_opts.fn = oneway_merge;
+   unpack_tree_opts.fn = reset_hard ? oneway_merge : twoway_merge;
unpack_tree_opts.update = 1;
unpack_tree_opts.merge = 1;
if (!detach_head)
@@ -568,12 +569,17 @@ static int reset_head(struct object_id *oid, const char 
*action,
return error(_("could not read index"));
}
 
-   if (!fill_tree_descriptor(, oid)) {
+   if (!reset_hard && !fill_tree_descriptor([nr++], _oid)) {
+   ret = error(_("failed to find tree of %s"), oid_to_hex(oid));
+   goto leave_reset_head;
+   }
+
+   if (!fill_tree_descriptor([nr++], oid)) {
ret = error(_("failed to find tree of %s"), oid_to_hex(oid));
goto leave_reset_head;
}
 
-   if (unpack_trees(1, , _tree_opts)) {
+   if (unpack_trees(nr, desc, _tree_opts)) {
ret = -1;
goto leave_reset_head;
}
@@ -621,7 +627,8 @@ static int reset_head(struct object_id *oid, const char 
*action,
 leave_reset_head:
strbuf_release();
rollback_lock_file();
-   free((void *)desc.buffer);
+   while (nr)
+   free((void *)desc[--nr].buffer);
return ret;
 }
 
@@ -999,7 +1006,7 @@ int cmd_rebase(int argc, const char **argv, const char 
*prefix)
rerere_clear(_rr);
string_list_clear(_rr, 1);
 
-   if (reset_head(NULL, "reset", NULL, 0, NULL, NULL) < 0)
+   if (reset_head(NULL, "reset", NULL, 0, 1, NULL, NULL) < 0)
die(_("could not discard worktree changes"));
if (read_basic_state())
exit(1);
@@ -1015,7 +1022,7 @@ int cmd_rebase(int argc, const char **argv, const char 
*prefix)
if (read_basic_state())
exit(1);
if (reset_head(_head, "reset",
-  options.head_name, 0, NULL, NULL) < 0)
+  options.head_name, 0, 1, NULL, NULL) < 0)
die(_("could not move back to %s"),
oid_to_hex(_head));
ret = finish_rebase();
@@ -1379,7 +1386,7 @@ int cmd_rebase(int argc, const char **argv, const char 
*prefix)
write_file(autostash, "%s", 

[PATCH 1/2] rebase: consolidate clean-up code before leaving reset_head()

2018-11-09 Thread Johannes Schindelin via GitGitGadget
From: Johannes Schindelin 

The same clean-up code is repeated quite a few times; Let's DRY up the
code some.

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

diff --git a/builtin/rebase.c b/builtin/rebase.c
index 0ee06aa363..6f6d7de156 100644
--- a/builtin/rebase.c
+++ b/builtin/rebase.c
@@ -569,16 +569,13 @@ static int reset_head(struct object_id *oid, const char 
*action,
}
 
if (!fill_tree_descriptor(, oid)) {
-   error(_("failed to find tree of %s"), oid_to_hex(oid));
-   rollback_lock_file();
-   free((void *)desc.buffer);
-   return -1;
+   ret = error(_("failed to find tree of %s"), oid_to_hex(oid));
+   goto leave_reset_head;
}
 
if (unpack_trees(1, , _tree_opts)) {
-   rollback_lock_file();
-   free((void *)desc.buffer);
-   return -1;
+   ret = -1;
+   goto leave_reset_head;
}
 
tree = parse_tree_indirect(oid);
@@ -586,10 +583,9 @@ static int reset_head(struct object_id *oid, const char 
*action,
 
if (write_locked_index(the_repository->index, , COMMIT_LOCK) < 0)
ret = error(_("could not write index"));
-   free((void *)desc.buffer);
 
if (ret)
-   return ret;
+   goto leave_reset_head;
 
reflog_action = getenv(GIT_REFLOG_ACTION_ENVIRONMENT);
strbuf_addf(, "%s: ", reflog_action ? reflog_action : "rebase");
@@ -622,7 +618,10 @@ static int reset_head(struct object_id *oid, const char 
*action,
 UPDATE_REFS_MSG_ON_ERR);
}
 
+leave_reset_head:
strbuf_release();
+   rollback_lock_file();
+   free((void *)desc.buffer);
return ret;
 }
 
-- 
gitgitgadget



[PATCH 1/1] Update .mailmap

2018-11-08 Thread Johannes Schindelin via GitGitGadget
From: Johannes Schindelin 

This patch makes the output of `git shortlog -nse v2.10.0`
duplicate-free.

Signed-off-by: Johannes Schindelin 
---
 .mailmap | 13 +
 1 file changed, 13 insertions(+)

diff --git a/.mailmap b/.mailmap
index bef3352b0d..1d8310073a 100644
--- a/.mailmap
+++ b/.mailmap
@@ -21,6 +21,8 @@ Anders Kaseorg  
 Aneesh Kumar K.V 
 Amos Waterland  
 Amos Waterland  
+Ben Peart  
+Ben Peart  
 Ben Walton  
 Benoit Sigoure  
 Bernt Hansen  
@@ -32,6 +34,7 @@ Bryan Larsen  
 Cheng Renquan 
 Chris Shoemaker 
 Chris Wright  
+Christian Ludwig  
 Cord Seele  
 Christian Couder  
 Christian Stimming  
@@ -51,6 +54,7 @@ David Reiss  

 David S. Miller 
 David Turner  
 David Turner  
+Derrick Stolee  
 Deskin Miller 
 Dirk Süsserott 
 Eric Blake  
@@ -98,6 +102,7 @@ Jens Axboe  
 Jens Lindström  Jens Lindstrom 
 Jim Meyering  
 Joachim Berdal Haga 
+Joachim Jablon  
 Johannes Schindelin  
 Johannes Sixt  
 Johannes Sixt  
@@ -150,6 +155,7 @@ Mark Levedahl  
 Mark Rada 
 Martin Langhoff  
 Martin von Zweigbergk  
+Masaya Suzuki 
 Matt Draisey  
 Matt Kraai  
 Matt McCutchen  
@@ -157,6 +163,7 @@ Matthias Kestenholz  

 Matthias Rüster  Matthias Ruester
 Matthias Urlichs  
 Matthias Urlichs  
+Matthieu Moy  
 Michael Coleman 
 Michael J Gruber  
 Michael J Gruber  
@@ -180,7 +187,11 @@ Nick Stokoe  Nick Woolley 

 Nick Stokoe  Nick Woolley 
 Nicolas Morey-Chaisemartin  

 Nicolas Morey-Chaisemartin  

+Nicolas Morey-Chaisemartin  

+Nicolas Morey-Chaisemartin  

+Nicolas Morey-Chaisemartin  

 Nicolas Sebrecht  
+Orgad Shaneh  
 Paolo Bonzini  
 Pascal Obry  
 Pascal Obry  
@@ -200,6 +211,7 @@ Philipp A. Hartmann  
 Philippe Bruhat 
 Ralf Thielow  
 Ramsay Jones  
+Randall S. Becker  
 René Scharfe  
 René Scharfe  Rene Scharfe
 Richard Hansen  
@@ -238,6 +250,7 @@ Steven Walter  

 Sven Verdoolaege  
 Sven Verdoolaege  
 SZEDER Gábor  
+Tao Qingyun  <845767...@qq.com>
 Tay Ray Chuan 
 Ted Percival  
 Theodore Ts'o 
-- 
gitgitgadget


[PATCH 0/1] Update .mailmap

2018-11-08 Thread Johannes Schindelin via GitGitGadget
I noticed that there were a couple of duplicate entries in git shortlog -nse
v2.19.1.., and then continued a bit to remove the duplicate entries even for
v2.10.0..

Johannes Schindelin (1):
  Update .mailmap

 .mailmap | 13 +
 1 file changed, 13 insertions(+)


base-commit: 8858448bb49332d353febc078ce4a3abcc962efe
Published-As: 
https://github.com/gitgitgadget/git/releases/tags/pr-71%2Fdscho%2Fmailmap-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git 
pr-71/dscho/mailmap-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/71
-- 
gitgitgadget


[PATCH 0/2] built-in rebase --autostash: fix regression

2018-11-07 Thread Johannes Schindelin via GitGitGadget
It was reported that the new, shiny built-in git rebase has a bug where it
would detach the HEAD when it was not even necessary to detach it.

Keeping with my fine tradition to first demonstrate what is the actual
problem (and making it easy to verify my claim), this patch series first
introduces the regression test, and then the (quite simple) fix.

AEvar, sorry for the ASCII-fication of your name, I still did not find the
time to look at the GitGitGadget bug closely where it does the wrong thing
when Cc:ing with non-ASCII names.

Johannes Schindelin (2):
  built-in rebase: demonstrate regression with --autostash
  built-in rebase --autostash: leave the current branch alone if
possible

 builtin/rebase.c| 3 ++-
 t/t3420-rebase-autostash.sh | 8 
 2 files changed, 10 insertions(+), 1 deletion(-)


base-commit: 8858448bb49332d353febc078ce4a3abcc962efe
Published-As: 
https://github.com/gitgitgadget/git/releases/tags/pr-70%2Fdscho%2Ffix-built-in-rebase-autostash-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git 
pr-70/dscho/fix-built-in-rebase-autostash-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/70
-- 
gitgitgadget


[PATCH 1/2] built-in rebase: demonstrate regression with --autostash

2018-11-07 Thread Johannes Schindelin via GitGitGadget
From: Johannes Schindelin 

An unnamed colleague of Ævar Arnfjörð Bjarmason reported a breakage
where a `pull --rebase` (which did not really need to do anything but
stash, see that nothing was changed, and apply the stash again) also
detached the HEAD.

This patch adds a minimal reproducer for this regression.

Signed-off-by: Johannes Schindelin 
---
 t/t3420-rebase-autostash.sh | 8 
 1 file changed, 8 insertions(+)

diff --git a/t/t3420-rebase-autostash.sh b/t/t3420-rebase-autostash.sh
index f355c6825a..d4e2520bcb 100755
--- a/t/t3420-rebase-autostash.sh
+++ b/t/t3420-rebase-autostash.sh
@@ -361,4 +361,12 @@ test_expect_success 'autostash with dirty submodules' '
git rebase -i --autostash HEAD
 '
 
+test_expect_failure 'branch is left alone when possible' '
+   git checkout -b unchanged-branch &&
+   echo changed >file0 &&
+   git rebase --autostash unchanged-branch &&
+   test changed = "$(cat file0)" &&
+   test unchanged-branch = "$(git rev-parse --abbrev-ref HEAD)"
+'
+
 test_done
-- 
gitgitgadget



[PATCH 2/2] built-in rebase --autostash: leave the current branch alone if possible

2018-11-07 Thread Johannes Schindelin via GitGitGadget
From: Johannes Schindelin 

When we converted a `git reset --hard` call in the original Unix shell
script to built-in code, we asked to reset the worktree and the index
and explicitly *not* to detach the HEAD. By mistake, though, we still
did. Let's fix this.

Signed-off-by: Johannes Schindelin 
---
 builtin/rebase.c| 3 ++-
 t/t3420-rebase-autostash.sh | 2 +-
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/builtin/rebase.c b/builtin/rebase.c
index 0ee06aa363..4a608d0a78 100644
--- a/builtin/rebase.c
+++ b/builtin/rebase.c
@@ -613,7 +613,8 @@ static int reset_head(struct object_id *oid, const char 
*action,
reflog_head = msg.buf;
}
if (!switch_to_branch)
-   ret = update_ref(reflog_head, "HEAD", oid, orig, REF_NO_DEREF,
+   ret = update_ref(reflog_head, "HEAD", oid, orig,
+detach_head ? REF_NO_DEREF : 0,
 UPDATE_REFS_MSG_ON_ERR);
else {
ret = create_symref("HEAD", switch_to_branch, msg.buf);
diff --git a/t/t3420-rebase-autostash.sh b/t/t3420-rebase-autostash.sh
index d4e2520bcb..4c7494cc8f 100755
--- a/t/t3420-rebase-autostash.sh
+++ b/t/t3420-rebase-autostash.sh
@@ -361,7 +361,7 @@ test_expect_success 'autostash with dirty submodules' '
git rebase -i --autostash HEAD
 '
 
-test_expect_failure 'branch is left alone when possible' '
+test_expect_success 'branch is left alone when possible' '
git checkout -b unchanged-branch &&
echo changed >file0 &&
git rebase --autostash unchanged-branch &&
-- 
gitgitgadget


[PATCH 0/1] Windows: force-recompile git.res for differing architectures

2018-11-06 Thread Johannes Schindelin via GitGitGadget
This is a patch designed to help maintaining Git for Windows better: when
the same source code is "cross-compiled" for i686 as well as x86_64, we want
to rebuild the whole thing, including the resource file git.res.

Note: regular C files are re-compiled appropriately, as the default prefix
in Git for Windows is /mingw32 or /mingw64 depending on the architecture,
and this difference is manifested in the CFLAGS (which, upon change, trigger
a complete rebuild).

As non-Windows platforms do not even compile these .res files, this patch
should have exactly no effect on non-Windows builds.

Johannes Schindelin (1):
  Windows: force-recompile git.res for differing architectures

 Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)


base-commit: cd69ec8cde54af1817630331fc441f493866f0d4
Published-As: 
https://github.com/gitgitgadget/git/releases/tags/pr-67%2Fdscho%2Fmingw-git.res-bitness-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git 
pr-67/dscho/mingw-git.res-bitness-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/67
-- 
gitgitgadget


[PATCH 1/1] Windows: force-recompile git.res for differing architectures

2018-11-06 Thread Johannes Schindelin via GitGitGadget
From: Johannes Schindelin 

When git.rc is compiled into git.res, the result is actually dependent
on the architecture. That is, you cannot simply link a 32-bit git.res
into a 64-bit git.exe.

Therefore, to allow 32-bit and 64-bit builds in the same directory, we
let git.res depend on GIT-PREFIX so that it gets recompiled when
compiling for a different architecture (this works because the exec path
changes based on the architecture: /mingw32/libexec/git-core for 32-bit
and /mingw64/libexec/git-core for 64-bit).

Signed-off-by: Johannes Schindelin 
---
 Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Makefile b/Makefile
index bbfbb4292d..8375736c32 100644
--- a/Makefile
+++ b/Makefile
@@ -2110,7 +2110,7 @@ $(SCRIPT_LIB) : % : %.sh GIT-SCRIPT-DEFINES
$(QUIET_GEN)$(cmd_munge_script) && \
mv $@+ $@
 
-git.res: git.rc GIT-VERSION-FILE
+git.res: git.rc GIT-VERSION-FILE GIT-PREFIX
$(QUIET_RC)$(RC) \
  $(join -DMAJOR= -DMINOR= -DMICRO= -DPATCHLEVEL=, $(wordlist 1, 4, \
$(shell echo $(GIT_VERSION) 0 0 0 0 | tr '.a-zA-Z-' ' '))) \
-- 
gitgitgadget


[PATCH 1/1] mingw: handle absolute paths in expand_user_path()

2018-11-06 Thread Johannes Schindelin via GitGitGadget
From: Johannes Schindelin 

On Windows, an absolute POSIX path needs to be turned into a Windows
one.

Signed-off-by: Johannes Schindelin 
---
 path.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/path.c b/path.c
index 34f0f98349..a72abf0e1f 100644
--- a/path.c
+++ b/path.c
@@ -11,6 +11,7 @@
 #include "path.h"
 #include "packfile.h"
 #include "object-store.h"
+#include "exec-cmd.h"
 
 static int get_st_mode_bits(const char *path, int *mode)
 {
@@ -709,6 +710,10 @@ char *expand_user_path(const char *path, int real_home)
 
if (path == NULL)
goto return_null;
+#ifdef __MINGW32__
+   if (path[0] == '/')
+   return system_path(path + 1);
+#endif
if (path[0] == '~') {
const char *first_slash = strchrnul(path, '/');
const char *username = path + 1;
-- 
gitgitgadget


[PATCH 0/1] mingw: handle absolute paths in expand_user_path()

2018-11-06 Thread Johannes Schindelin via GitGitGadget
While this patch has been "in production" in Git for Windows for a good
while, this patch series is half meant as a request for comments.

The reason is this: something like this (make paths specified e.g. via 
http.sslCAInfo relative to the runtime prefix) is potentially useful also in
the non-Windows context, as long as Git was built with the runtime prefix
feature.

The main problem with non-Windows platforms is that paths starting with a
single slash are unambiguously absolute, whereas we can say with certainty
that they are not absolute Windows paths.

So maybe someone on this list has a clever idea how we could specify paths
(unambiguously, even on non-Windows platforms) that Git should interpret as
relative to the runtime prefix?

Johannes Schindelin (1):
  mingw: handle absolute paths in expand_user_path()

 path.c | 5 +
 1 file changed, 5 insertions(+)


base-commit: cd69ec8cde54af1817630331fc441f493866f0d4
Published-As: 
https://github.com/gitgitgadget/git/releases/tags/pr-66%2Fdscho%2Fmingw-expand-absolute-user-path-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git 
pr-66/dscho/mingw-expand-absolute-user-path-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/66
-- 
gitgitgadget


[PATCH 0/1] Make compat/poll safer on Windows

2018-10-31 Thread Johannes Schindelin via GitGitGadget
This is yet another piece from the Git for Windows cake. It avoids a
wrap-around in the poll emulation on Windows that occurs every 49 days.

Steve Hoelzer (1):
  poll: use GetTickCount64() to avoid wrap-around issues

 compat/poll/poll.c | 10 +++---
 1 file changed, 7 insertions(+), 3 deletions(-)


base-commit: 4ede3d42dfb57f9a41ac96a1f216c62eb7566cc2
Published-As: 
https://github.com/gitgitgadget/git/releases/tags/pr-64%2Fdscho%2Fmingw-safer-compat-poll-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git 
pr-64/dscho/mingw-safer-compat-poll-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/64
-- 
gitgitgadget


[PATCH 3/3] tests: optionally skip `git rebase -p` tests

2018-10-31 Thread Johannes Schindelin via GitGitGadget
From: Johannes Schindelin 

The `--preserve-merges` mode of the `rebase` command is slated to be
deprecated soon, as the more powerful `--rebase-merges` mode is
available now, and the latter was designed with the express intent to
address the shortcomings of `--preserve-merges`' design (e.g. the
inability to reorder commits in an interactive rebase).

As such, we will eventually even remove the `--preserve-merges` support,
and along with it, its tests.

In preparation for this, and also to allow the Windows phase of our
automated tests to save some well-needed time when running the test
suite, this commit introduces a new prerequisite REBASE_P, which can be
forced to being unmet by setting the environment variable
`GIT_TEST_SKIP_REBASE_P` to any non-empty string.

Signed-off-by: Johannes Schindelin 
---
 t/t3404-rebase-interactive.sh |  8 ++---
 t/t3408-rebase-multi-line.sh  |  2 +-
 t/t3409-rebase-preserve-merges.sh |  5 
 t/t3410-rebase-preserve-dropped-merges.sh |  5 
 t/t3411-rebase-preserve-around-merges.sh  |  5 
 t/t3412-rebase-root.sh| 12 
 t/t3414-rebase-preserve-onto.sh   |  5 
 t/t3418-rebase-continue.sh|  4 +--
 t/t3421-rebase-topology-linear.sh | 36 +++
 t/t3425-rebase-topology-merges.sh |  5 
 t/t5520-pull.sh   |  6 ++--
 t/t7505-prepare-commit-msg-hook.sh|  2 +-
 t/t7517-per-repo-email.sh |  6 ++--
 t/test-lib.sh |  4 +++
 14 files changed, 69 insertions(+), 36 deletions(-)

diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
index 99d1fb79a8..68ca8dc9bb 100755
--- a/t/t3404-rebase-interactive.sh
+++ b/t/t3404-rebase-interactive.sh
@@ -312,7 +312,7 @@ test_expect_success 'retain authorship when squashing' '
git show HEAD | grep "^Author: Twerp Snog"
 '
 
-test_expect_success '-p handles "no changes" gracefully' '
+test_expect_success REBASE_P '-p handles "no changes" gracefully' '
HEAD=$(git rev-parse HEAD) &&
set_fake_editor &&
git rebase -i -p HEAD^ &&
@@ -322,7 +322,7 @@ test_expect_success '-p handles "no changes" gracefully' '
test $HEAD = $(git rev-parse HEAD)
 '
 
-test_expect_failure 'exchange two commits with -p' '
+test_expect_failure REBASE_P 'exchange two commits with -p' '
git checkout H &&
set_fake_editor &&
FAKE_LINES="2 1" git rebase -i -p HEAD~2 &&
@@ -330,7 +330,7 @@ test_expect_failure 'exchange two commits with -p' '
test G = $(git cat-file commit HEAD | sed -ne \$p)
 '
 
-test_expect_success 'preserve merges with -p' '
+test_expect_success REBASE_P 'preserve merges with -p' '
git checkout -b to-be-preserved master^ &&
: > unrelated-file &&
git add unrelated-file &&
@@ -373,7 +373,7 @@ test_expect_success 'preserve merges with -p' '
test $(git show HEAD:unrelated-file) = 1
 '
 
-test_expect_success 'edit ancestor with -p' '
+test_expect_success REBASE_P 'edit ancestor with -p' '
set_fake_editor &&
FAKE_LINES="1 2 edit 3 4" git rebase -i -p HEAD~3 &&
echo 2 > unrelated-file &&
diff --git a/t/t3408-rebase-multi-line.sh b/t/t3408-rebase-multi-line.sh
index e7292f5b9b..d2bd7c17b0 100755
--- a/t/t3408-rebase-multi-line.sh
+++ b/t/t3408-rebase-multi-line.sh
@@ -52,7 +52,7 @@ test_expect_success rebase '
test_cmp expect actual
 
 '
-test_expect_success rebasep '
+test_expect_success REBASE_P rebasep '
 
git checkout side-merge &&
git rebase -p side &&
diff --git a/t/t3409-rebase-preserve-merges.sh 
b/t/t3409-rebase-preserve-merges.sh
index 8c251c57a6..3b340f1ece 100755
--- a/t/t3409-rebase-preserve-merges.sh
+++ b/t/t3409-rebase-preserve-merges.sh
@@ -8,6 +8,11 @@ Run "git rebase -p" and check that merges are properly carried 
along
 '
 . ./test-lib.sh
 
+if ! test_have_prereq REBASE_P; then
+   skip_all='skipping git rebase -p tests, as asked for'
+   test_done
+fi
+
 GIT_AUTHOR_EMAIL=bogus_email_address
 export GIT_AUTHOR_EMAIL
 
diff --git a/t/t3410-rebase-preserve-dropped-merges.sh 
b/t/t3410-rebase-preserve-dropped-merges.sh
index 6f73b95558..2e29866993 100755
--- a/t/t3410-rebase-preserve-dropped-merges.sh
+++ b/t/t3410-rebase-preserve-dropped-merges.sh
@@ -11,6 +11,11 @@ rewritten.
 '
 . ./test-lib.sh
 
+if ! test_have_prereq REBASE_P; then
+   skip_all='skipping git rebase -p tests, as asked for'
+   test_done
+fi
+
 # set up two branches like this:
 #
 # A - B - C - D - E
diff --git a/t/t3411-rebase-preserve-around-merges.sh 
b/t/t3411-rebase-preserve-around-merges.sh
index dc81bf27eb..fb45e7bf7b 100755
--- a/t/t3411-rebase-preserve-around-merges.sh
+++ b/t/t3411-rebase-preserve-around-merges.sh
@@ -10,6 +10,11 @@ a merge to before the merge.
 '
 . ./test-lib.sh
 
+if ! test_have_prereq REBASE_P; then
+   skip_all='skipping git rebase -p tests, as asked for'
+   

[PATCH 2/3] t3418: decouple test cases from a previous `rebase -p` test case

2018-10-31 Thread Johannes Schindelin via GitGitGadget
From: Johannes Schindelin 

It is in general a good idea for regression test cases to be as
independent of each other as possible (with the one exception of an
initial `setup` test case, which is only a test case in Git's test suite
because it does not have a notion of a fixture or setup).

This patch addresses one particular instance of this principle being
violated: a few test cases in t3418-rebase-continue.sh depend on a side
effect of a test case that verifies a specific `rebase -p` behavior. The
later test cases should, however, still succeed even if the `rebase -p`
test case is skipped.

Signed-off-by: Johannes Schindelin 
---
 t/t3418-rebase-continue.sh | 1 +
 1 file changed, 1 insertion(+)

diff --git a/t/t3418-rebase-continue.sh b/t/t3418-rebase-continue.sh
index 25099d715c..031e3d8ddb 100755
--- a/t/t3418-rebase-continue.sh
+++ b/t/t3418-rebase-continue.sh
@@ -177,6 +177,7 @@ test_expect_success 'setup rerere database' '
git checkout master &&
test_commit "commit-new-file-F3" F3 3 &&
test_config rerere.enabled true &&
+   git update-ref refs/heads/topic commit-new-file-F3-on-topic-branch &&
test_must_fail git rebase -m master topic &&
echo "Resolved" >F2 &&
cp F2 expected-F2 &&
-- 
gitgitgadget



[PATCH 1/3] t3404: decouple some test cases from outcomes of previous test cases

2018-10-31 Thread Johannes Schindelin via GitGitGadget
From: Johannes Schindelin 

Originally, the `--preserve-merges` option of the `git rebase` command
piggy-backed on top of the `--interactive` feature. For that reason, the
early test cases were added to the very same test script that contains
the `git rebase -i` tests: `t3404-rebase-interactive.sh`.

However, since c42abfe7857 (rebase: introduce a dedicated backend for
--preserve-merges, 2018-05-28), the `--preserve-merges` feature got its
own backend, in preparation for converting the rest of the
`--interactive` code to built-in code, written in C rather than shell.

The reason why the `--preserve-merges` feature was not converted at the
same time is that we have something much better now: `--rebase-merges`.
That option intends to supersede `--preserve-merges`, and we will
probably deprecate the latter soon.

Once `--preserve-merges` has been deprecated for a good amount of time,
it will be time to remove it, and along with it, its tests.

In preparation for that, let's make the rest of the test cases in
`t3404-rebase-interactive.sh` independent of the test cases dedicated to
`--preserve-merges`.

Signed-off-by: Johannes Schindelin 
---
 t/t3404-rebase-interactive.sh | 15 ---
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
index ff89b6341a..99d1fb79a8 100755
--- a/t/t3404-rebase-interactive.sh
+++ b/t/t3404-rebase-interactive.sh
@@ -387,6 +387,7 @@ test_expect_success 'edit ancestor with -p' '
 '
 
 test_expect_success '--continue tries to commit' '
+   git reset --hard D &&
test_tick &&
set_fake_editor &&
test_must_fail git rebase -i --onto new-branch1 HEAD^ &&
@@ -426,7 +427,7 @@ test_expect_success C_LOCALE_OUTPUT 'multi-fixup does not 
fire up editor' '
git rebase -i $base &&
test $base = $(git rev-parse HEAD^) &&
test 0 = $(git show | grep NEVER | wc -l) &&
-   git checkout to-be-rebased &&
+   git checkout @{-1} &&
git branch -D multi-fixup
 '
 
@@ -441,7 +442,7 @@ test_expect_success 'commit message used after conflict' '
git rebase --continue &&
test $base = $(git rev-parse HEAD^) &&
test 1 = $(git show | grep ONCE | wc -l) &&
-   git checkout to-be-rebased &&
+   git checkout @{-1} &&
git branch -D conflict-fixup
 '
 
@@ -456,7 +457,7 @@ test_expect_success 'commit message retained after 
conflict' '
git rebase --continue &&
test $base = $(git rev-parse HEAD^) &&
test 2 = $(git show | grep TWICE | wc -l) &&
-   git checkout to-be-rebased &&
+   git checkout @{-1} &&
git branch -D conflict-squash
 '
 
@@ -481,7 +482,7 @@ test_expect_success C_LOCALE_OUTPUT 'squash and fixup 
generate correct log messa
grep "^# This is a combination of 3 commits\."  &&
git cat-file commit HEAD@{3} |
grep "^# This is a combination of 2 commits\."  &&
-   git checkout to-be-rebased &&
+   git checkout @{-1} &&
git branch -D squash-fixup
 '
 
@@ -494,7 +495,7 @@ test_expect_success C_LOCALE_OUTPUT 'squash ignores 
comments' '
git rebase -i $base &&
test $base = $(git rev-parse HEAD^) &&
test 1 = $(git show | grep ONCE | wc -l) &&
-   git checkout to-be-rebased &&
+   git checkout @{-1} &&
git branch -D skip-comments
 '
 
@@ -507,7 +508,7 @@ test_expect_success C_LOCALE_OUTPUT 'squash ignores blank 
lines' '
git rebase -i $base &&
test $base = $(git rev-parse HEAD^) &&
test 1 = $(git show | grep ONCE | wc -l) &&
-   git checkout to-be-rebased &&
+   git checkout @{-1} &&
git branch -D skip-blank-lines
 '
 
@@ -648,7 +649,7 @@ test_expect_success 'rebase with a file named HEAD in 
worktree' '
) &&
 
set_fake_editor &&
-   FAKE_LINES="1 squash 2" git rebase -i to-be-rebased &&
+   FAKE_LINES="1 squash 2" git rebase -i @{-1} &&
test "$(git show -s --pretty=format:%an)" = "Squashed Away"
 
 '
-- 
gitgitgadget



[PATCH 0/3] tests: allow to skip git rebase -p tests

2018-10-31 Thread Johannes Schindelin via GitGitGadget
The --preserve-merges mode of the git rebase command is on its way out,
being superseded by the --rebase-merges mode. My plan is to contribute
patches to deprecate the former in favor of the latter before long.

In the meantime, it seems a bit pointless to keep running the git rebase -p 
tests, in particular in the Windows phase of the automated testing. In
preparation for skipping those tests, this patch series starts out by
decoupling test cases so that no non-rebase -p ones depend on side effects
of rebase -p ones, and it concludes by a patch that allows skipping the 
rebase -p ones by setting the environment variable GIT_TEST_SKIP_REBASE_P.

In a quick 'n dirty test, skipping the rebase -p tests seems to shave off
about 8 minutes from the 1h20 running time of the test suite on Windows
(without git svn tests, we skip them for a long time already, as they are
really, really slow on Windows).

Johannes Schindelin (3):
  t3404: decouple some test cases from outcomes of previous test cases
  t3418: decouple test cases from a previous `rebase -p` test case
  tests: optionally skip `git rebase -p` tests

 t/t3404-rebase-interactive.sh | 23 ---
 t/t3408-rebase-multi-line.sh  |  2 +-
 t/t3409-rebase-preserve-merges.sh |  5 
 t/t3410-rebase-preserve-dropped-merges.sh |  5 
 t/t3411-rebase-preserve-around-merges.sh  |  5 
 t/t3412-rebase-root.sh| 12 
 t/t3414-rebase-preserve-onto.sh   |  5 
 t/t3418-rebase-continue.sh|  5 ++--
 t/t3421-rebase-topology-linear.sh | 36 +++
 t/t3425-rebase-topology-merges.sh |  5 
 t/t5520-pull.sh   |  6 ++--
 t/t7505-prepare-commit-msg-hook.sh|  2 +-
 t/t7517-per-repo-email.sh |  6 ++--
 t/test-lib.sh |  4 +++
 14 files changed, 78 insertions(+), 43 deletions(-)


base-commit: 4ede3d42dfb57f9a41ac96a1f216c62eb7566cc2
Published-As: 
https://github.com/gitgitgadget/git/releases/tags/pr-63%2Fdscho%2Fsplit-out-rebase-p-tests-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git 
pr-63/dscho/split-out-rebase-p-tests-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/63
-- 
gitgitgadget


[PATCH 0/1] mingw: fix isatty() after dup2()

2018-10-30 Thread Johannes Schindelin via GitGitGadget
This patch has been looong in the waiting (well over a year) for being sent
to the Git mailing list; it is a fixup for a change of isatty() on Windows
that had long made it into git.git's master branch.

Johannes Schindelin (1):
  mingw: fix isatty() after dup2()

 compat/mingw.h   |  3 +++
 compat/winansi.c | 12 
 2 files changed, 15 insertions(+)


base-commit: 4ede3d42dfb57f9a41ac96a1f216c62eb7566cc2
Published-As: 
https://github.com/gitgitgadget/git/releases/tags/pr-61%2Fdscho%2Fmingw-isatty-and-dup2-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git 
pr-61/dscho/mingw-isatty-and-dup2-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/61
-- 
gitgitgadget


[PATCH 1/1] mingw: fix isatty() after dup2()

2018-10-30 Thread Johannes Schindelin via GitGitGadget
From: Johannes Schindelin 

Since a9b8a09c3c30 (mingw: replace isatty() hack, 2016-12-22), we handle
isatty() by special-casing the stdin/stdout/stderr file descriptors,
caching the return value. However, we missed the case where dup2()
overrides the respective file descriptor.

That poses a problem e.g. where the `show` builtin asks for a pager very
early, the `setup_pager()` function sets the pager depending on the
return value of `isatty()` and then redirects stdout. Subsequently,
`cmd_log_init_finish()` calls `setup_pager()` *again*. What should
happen now is that `isatty()` reports that stdout is *not* a TTY and
consequently stdout should be left alone.

Let's override dup2() to handle this appropriately.

This fixes https://github.com/git-for-windows/git/issues/1077

Signed-off-by: Johannes Schindelin 
---
 compat/mingw.h   |  3 +++
 compat/winansi.c | 12 
 2 files changed, 15 insertions(+)

diff --git a/compat/mingw.h b/compat/mingw.h
index f31dcff2b..b04556ce0 100644
--- a/compat/mingw.h
+++ b/compat/mingw.h
@@ -390,6 +390,9 @@ int mingw_raise(int sig);
 int winansi_isatty(int fd);
 #define isatty winansi_isatty
 
+int winansi_dup2(int oldfd, int newfd);
+#define dup2 winansi_dup2
+
 void winansi_init(void);
 HANDLE winansi_get_osfhandle(int fd);
 
diff --git a/compat/winansi.c b/compat/winansi.c
index a11a0f16d..f4f08237f 100644
--- a/compat/winansi.c
+++ b/compat/winansi.c
@@ -474,6 +474,18 @@ static void die_lasterr(const char *fmt, ...)
va_end(params);
 }
 
+#undef dup2
+int winansi_dup2(int oldfd, int newfd)
+{
+   int ret = dup2(oldfd, newfd);
+
+   if (!ret && newfd >= 0 && newfd <= 2)
+   fd_is_interactive[newfd] = oldfd < 0 || oldfd > 2 ?
+   0 : fd_is_interactive[oldfd];
+
+   return ret;
+}
+
 static HANDLE duplicate_handle(HANDLE hnd)
 {
HANDLE hresult, hproc = GetCurrentProcess();
-- 
gitgitgadget


[PATCH 4/4] mingw: unset PERL5LIB by default

2018-10-30 Thread Johannes Schindelin via GitGitGadget
From: Johannes Schindelin 

Git for Windows ships with its own Perl interpreter, and insists on
using it, so it will most likely wreak havoc if PERL5LIB is set before
launching Git.

Let's just unset that environment variables when spawning processes.

To make this feature extensible (and overrideable), there is a new
config setting `core.unsetenvvars` that allows specifying a
comma-separated list of names to unset before spawning processes.

Reported by Gabriel Fuhrmann.

Signed-off-by: Johannes Schindelin 
---
 Documentation/config.txt |  6 ++
 compat/mingw.c   | 35 ++-
 t/t0029-core-unsetenvvars.sh | 30 ++
 3 files changed, 70 insertions(+), 1 deletion(-)
 create mode 100755 t/t0029-core-unsetenvvars.sh

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 09e95e9e9..f338f0b2c 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -921,6 +921,12 @@ relatively high IO latencies.  When enabled, Git will do 
the
 index comparison to the filesystem data in parallel, allowing
 overlapping IO's.  Defaults to true.
 
+core.unsetenvvars::
+   Windows-only: comma-separated list of environment variables'
+   names that need to be unset before spawning any other process.
+   Defaults to `PERL5LIB` to account for the fact that Git for
+   Windows insists on using its own Perl interpreter.
+
 core.createObject::
You can set this to 'link', in which case a hardlink followed by
a delete of the source are used to make sure that object creation
diff --git a/compat/mingw.c b/compat/mingw.c
index 272d5e11e..181e74c23 100644
--- a/compat/mingw.c
+++ b/compat/mingw.c
@@ -212,6 +212,7 @@ enum hide_dotfiles_type {
 };
 
 static enum hide_dotfiles_type hide_dotfiles = HIDE_DOTFILES_DOTGITONLY;
+static char *unset_environment_variables;
 
 int mingw_core_config(const char *var, const char *value, void *cb)
 {
@@ -223,6 +224,12 @@ int mingw_core_config(const char *var, const char *value, 
void *cb)
return 0;
}
 
+   if (!strcmp(var, "core.unsetenvvars")) {
+   free(unset_environment_variables);
+   unset_environment_variables = xstrdup(value);
+   return 0;
+   }
+
return 0;
 }
 
@@ -1180,6 +1187,27 @@ static wchar_t *make_environment_block(char **deltaenv)
return wenvblk;
 }
 
+static void do_unset_environment_variables(void)
+{
+   static int done;
+   char *p = unset_environment_variables;
+
+   if (done || !p)
+   return;
+   done = 1;
+
+   for (;;) {
+   char *comma = strchr(p, ',');
+
+   if (comma)
+   *comma = '\0';
+   unsetenv(p);
+   if (!comma)
+   break;
+   p = comma + 1;
+   }
+}
+
 struct pinfo_t {
struct pinfo_t *next;
pid_t pid;
@@ -1198,9 +1226,12 @@ static pid_t mingw_spawnve_fd(const char *cmd, const 
char **argv, char **deltaen
wchar_t wcmd[MAX_PATH], wdir[MAX_PATH], *wargs, *wenvblk = NULL;
unsigned flags = CREATE_UNICODE_ENVIRONMENT;
BOOL ret;
+   HANDLE cons;
+
+   do_unset_environment_variables();
 
/* Determine whether or not we are associated to a console */
-   HANDLE cons = CreateFile("CONOUT$", GENERIC_WRITE,
+   cons = CreateFile("CONOUT$", GENERIC_WRITE,
FILE_SHARE_WRITE, NULL, OPEN_EXISTING,
FILE_ATTRIBUTE_NORMAL, NULL);
if (cons == INVALID_HANDLE_VALUE) {
@@ -2437,6 +2468,8 @@ void mingw_startup(void)
/* fix Windows specific environment settings */
setup_windows_environment();
 
+   unset_environment_variables = xstrdup("PERL5LIB");
+
/* initialize critical section for waitpid pinfo_t list */
InitializeCriticalSection(_cs);
 
diff --git a/t/t0029-core-unsetenvvars.sh b/t/t0029-core-unsetenvvars.sh
new file mode 100755
index 0..24ce46a6e
--- /dev/null
+++ b/t/t0029-core-unsetenvvars.sh
@@ -0,0 +1,30 @@
+#!/bin/sh
+
+test_description='test the Windows-only core.unsetenvvars setting'
+
+. ./test-lib.sh
+
+if ! test_have_prereq MINGW
+then
+   skip_all='skipping Windows-specific tests'
+   test_done
+fi
+
+test_expect_success 'setup' '
+   mkdir -p "$TRASH_DIRECTORY/.git/hooks" &&
+   write_script "$TRASH_DIRECTORY/.git/hooks/pre-commit" <<-\EOF
+   echo $HOBBES >&2
+   EOF
+'
+
+test_expect_success 'core.unsetenvvars works' '
+   HOBBES=Calvin &&
+   export HOBBES &&
+   git commit --allow-empty -m with 2>err &&
+   grep Calvin err &&
+   git -c core.unsetenvvars=FINDUS,HOBBES,CALVIN \
+   commit --allow-empty -m without 2>err &&
+   ! grep Calvin err
+'
+
+test_done
-- 
gitgitgadget


[PATCH 1/4] config: rename `dummy` parameter to `cb` in git_default_config()

2018-10-30 Thread Johannes Schindelin via GitGitGadget
From: Johannes Schindelin 

This is the convention elsewhere (and prepares for the case where we may
need to pass callback data).

Signed-off-by: Johannes Schindelin 
---
 config.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/config.c b/config.c
index 4051e3882..3687c6783 100644
--- a/config.c
+++ b/config.c
@@ -1448,13 +1448,13 @@ static int git_default_mailmap_config(const char *var, 
const char *value)
return 0;
 }
 
-int git_default_config(const char *var, const char *value, void *dummy)
+int git_default_config(const char *var, const char *value, void *cb)
 {
if (starts_with(var, "core."))
return git_default_core_config(var, value);
 
if (starts_with(var, "user."))
-   return git_ident_config(var, value, dummy);
+   return git_ident_config(var, value, cb);
 
if (starts_with(var, "i18n."))
return git_default_i18n_config(var, value);
-- 
gitgitgadget



[PATCH 3/4] Move Windows-specific config settings into compat/mingw.c

2018-10-30 Thread Johannes Schindelin via GitGitGadget
From: Johannes Schindelin 

Signed-off-by: Johannes Schindelin 
---
 cache.h|  8 
 compat/mingw.c | 18 ++
 config.c   |  8 
 environment.c  |  1 -
 4 files changed, 18 insertions(+), 17 deletions(-)

diff --git a/cache.h b/cache.h
index f7fabdde8..0ce95c5a8 100644
--- a/cache.h
+++ b/cache.h
@@ -906,14 +906,6 @@ int use_optional_locks(void);
 extern char comment_line_char;
 extern int auto_comment_line_char;
 
-/* Windows only */
-enum hide_dotfiles_type {
-   HIDE_DOTFILES_FALSE = 0,
-   HIDE_DOTFILES_TRUE,
-   HIDE_DOTFILES_DOTGITONLY
-};
-extern enum hide_dotfiles_type hide_dotfiles;
-
 enum log_refs_config {
LOG_REFS_UNSET = -1,
LOG_REFS_NONE = 0,
diff --git a/compat/mingw.c b/compat/mingw.c
index 293f286af..272d5e11e 100644
--- a/compat/mingw.c
+++ b/compat/mingw.c
@@ -6,6 +6,7 @@
 #include "../run-command.h"
 #include "../cache.h"
 #include "win32/lazyload.h"
+#include "../config.h"
 
 #define HCAST(type, handle) ((type)(intptr_t)handle)
 
@@ -203,8 +204,25 @@ static int ask_yes_no_if_possible(const char *format, ...)
}
 }
 
+/* Windows only */
+enum hide_dotfiles_type {
+   HIDE_DOTFILES_FALSE = 0,
+   HIDE_DOTFILES_TRUE,
+   HIDE_DOTFILES_DOTGITONLY
+};
+
+static enum hide_dotfiles_type hide_dotfiles = HIDE_DOTFILES_DOTGITONLY;
+
 int mingw_core_config(const char *var, const char *value, void *cb)
 {
+   if (!strcmp(var, "core.hidedotfiles")) {
+   if (value && !strcasecmp(value, "dotgitonly"))
+   hide_dotfiles = HIDE_DOTFILES_DOTGITONLY;
+   else
+   hide_dotfiles = git_config_bool(var, value);
+   return 0;
+   }
+
return 0;
 }
 
diff --git a/config.c b/config.c
index 646b6cca9..5bf19a23c 100644
--- a/config.c
+++ b/config.c
@@ -1344,14 +1344,6 @@ static int git_default_core_config(const char *var, 
const char *value, void *cb)
return 0;
}
 
-   if (!strcmp(var, "core.hidedotfiles")) {
-   if (value && !strcasecmp(value, "dotgitonly"))
-   hide_dotfiles = HIDE_DOTFILES_DOTGITONLY;
-   else
-   hide_dotfiles = git_config_bool(var, value);
-   return 0;
-   }
-
if (!strcmp(var, "core.partialclonefilter")) {
return git_config_string(_partial_clone_filter_default,
 var, value);
diff --git a/environment.c b/environment.c
index 3f3c8746c..30ecd4e78 100644
--- a/environment.c
+++ b/environment.c
@@ -71,7 +71,6 @@ int core_apply_sparse_checkout;
 int merge_log_config = -1;
 int precomposed_unicode = -1; /* see probe_utf8_pathname_composition() */
 unsigned long pack_size_limit_cfg;
-enum hide_dotfiles_type hide_dotfiles = HIDE_DOTFILES_DOTGITONLY;
 enum log_refs_config log_all_ref_updates = LOG_REFS_UNSET;
 
 #ifndef PROTECT_HFS_DEFAULT
-- 
gitgitgadget



[PATCH 0/4] mingw: prevent external PERL5LIB from interfering

2018-10-30 Thread Johannes Schindelin via GitGitGadget
In Git for Windows, we do not support running the Perl scripts with any
random Perl interpreter. Instead, we insist on using the shipped one (except
for MinGit, where we do not even ship the Perl scripts, to save on space).

However, if Git is called from, say, a Perl script running in a different
Perl interpreter with appropriately configured PERL5LIB, it messes with
Git's ability to run its Perl scripts.

For that reason, we devised the presented method of defining a list of
environment variables (via core.unsetEnvVars) that would then be unset
before spawning any process, defaulting to PERL5LIB.

An alternative approach which was rejected at the time (because it
interfered with the then-ongoing work to compile Git for Windows using MS
Visual C++) would patch the make_environment_block() function to skip the
specified environment variables before handing down the environment block to
the spawned process. Currently it would interfere with the mingw-utf-8-env
patch series I sent earlier today
[https://public-inbox.org/git/pull.57.git.gitgitgad...@gmail.com].

While at it, this patch series also cleans up house and moves the
Windows-specific core.* variable handling to compat/mingw.c rather than
cluttering environment.c and config.c with things that e.g. developers on
Linux do not want to care about.

Johannes Schindelin (4):
  config: rename `dummy` parameter to `cb` in git_default_config()
  Allow for platform-specific core.* config settings
  Move Windows-specific config settings into compat/mingw.c
  mingw: unset PERL5LIB by default

 Documentation/config.txt |  6 
 cache.h  |  8 -
 compat/mingw.c   | 58 +++-
 compat/mingw.h   |  3 ++
 config.c | 18 ---
 environment.c|  1 -
 git-compat-util.h|  8 +
 t/t0029-core-unsetenvvars.sh | 30 +++
 8 files changed, 109 insertions(+), 23 deletions(-)
 create mode 100755 t/t0029-core-unsetenvvars.sh


base-commit: 4ede3d42dfb57f9a41ac96a1f216c62eb7566cc2
Published-As: 
https://github.com/gitgitgadget/git/releases/tags/pr-62%2Fdscho%2Fperl5lib-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git 
pr-62/dscho/perl5lib-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/62
-- 
gitgitgadget


[PATCH 2/4] Allow for platform-specific core.* config settings

2018-10-30 Thread Johannes Schindelin via GitGitGadget
From: Johannes Schindelin 

In the Git for Windows project, we have ample precendent for config
settings that apply to Windows, and to Windows only.

Let's formalize this concept by introducing a platform_core_config()
function that can be #define'd in a platform-specific manner.

This will allow us to contain platform-specific code better, as the
corresponding variables no longer need to be exported so that they can
be defined in environment.c and be set in config.c

Signed-off-by: Johannes Schindelin 
---
 compat/mingw.c| 5 +
 compat/mingw.h| 3 +++
 config.c  | 6 +++---
 git-compat-util.h | 8 
 4 files changed, 19 insertions(+), 3 deletions(-)

diff --git a/compat/mingw.c b/compat/mingw.c
index 81ef24286..293f286af 100644
--- a/compat/mingw.c
+++ b/compat/mingw.c
@@ -203,6 +203,11 @@ static int ask_yes_no_if_possible(const char *format, ...)
}
 }
 
+int mingw_core_config(const char *var, const char *value, void *cb)
+{
+   return 0;
+}
+
 /* Normalizes NT paths as returned by some low-level APIs. */
 static wchar_t *normalize_ntpath(wchar_t *wbuf)
 {
diff --git a/compat/mingw.h b/compat/mingw.h
index f31dcff2b..e9d2b9cdd 100644
--- a/compat/mingw.h
+++ b/compat/mingw.h
@@ -11,6 +11,9 @@ typedef _sigset_t sigset_t;
 #undef _POSIX_THREAD_SAFE_FUNCTIONS
 #endif
 
+extern int mingw_core_config(const char *var, const char *value, void *cb);
+#define platform_core_config mingw_core_config
+
 /*
  * things that are not available in header files
  */
diff --git a/config.c b/config.c
index 3687c6783..646b6cca9 100644
--- a/config.c
+++ b/config.c
@@ -1093,7 +1093,7 @@ int git_config_color(char *dest, const char *var, const 
char *value)
return 0;
 }
 
-static int git_default_core_config(const char *var, const char *value)
+static int git_default_core_config(const char *var, const char *value, void 
*cb)
 {
/* This needs a better name */
if (!strcmp(var, "core.filemode")) {
@@ -1363,7 +1363,7 @@ static int git_default_core_config(const char *var, const 
char *value)
}
 
/* Add other config variables here and to Documentation/config.txt. */
-   return 0;
+   return platform_core_config(var, value, cb);
 }
 
 static int git_default_i18n_config(const char *var, const char *value)
@@ -1451,7 +1451,7 @@ static int git_default_mailmap_config(const char *var, 
const char *value)
 int git_default_config(const char *var, const char *value, void *cb)
 {
if (starts_with(var, "core."))
-   return git_default_core_config(var, value);
+   return git_default_core_config(var, value, cb);
 
if (starts_with(var, "user."))
return git_ident_config(var, value, cb);
diff --git a/git-compat-util.h b/git-compat-util.h
index 96a3f86d8..3a08d9916 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -342,6 +342,14 @@ typedef uintmax_t timestamp_t;
 #define _PATH_DEFPATH "/usr/local/bin:/usr/bin:/bin"
 #endif
 
+#ifndef platform_core_config
+static inline int noop_core_config(const char *var, const char *value, void 
*cb)
+{
+   return 0;
+}
+#define platform_core_config noop_core_config
+#endif
+
 #ifndef has_dos_drive_prefix
 static inline int git_has_dos_drive_prefix(const char *path)
 {
-- 
gitgitgadget



[PATCH 1/2] t7800: fix quoting

2018-10-30 Thread Johannes Schindelin via GitGitGadget
From: Johannes Schindelin 

When passing a command-line to call an external diff command to the
difftool, we must be prepared for paths containing special characters,
e.g. backslashes in the temporary directory's path on Windows.

This patch is needed in preparation for the next commit, which will
make the MinGW version of Git *not* rewrite TMP to use forward slashes
instead of backslashes.

Signed-off-by: Johannes Schindelin 
---
 t/t7800-difftool.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/t/t7800-difftool.sh b/t/t7800-difftool.sh
index 562bd215a..22b9199d5 100755
--- a/t/t7800-difftool.sh
+++ b/t/t7800-difftool.sh
@@ -332,7 +332,7 @@ test_expect_success 'difftool --extcmd cat arg1' '
 test_expect_success 'difftool --extcmd cat arg2' '
echo branch >expect &&
git difftool --no-prompt \
-   --extcmd sh\ -c\ \"cat\ \$2\" branch >actual &&
+   --extcmd sh\ -c\ \"cat\ \\\"\$2\\\"\" branch >actual &&
test_cmp expect actual
 '
 
-- 
gitgitgadget



[PATCH 2/2] mingw: reencode environment variables on the fly (UTF-16 <-> UTF-8)

2018-10-30 Thread Johannes Schindelin via GitGitGadget
From: Johannes Schindelin 

On Windows, the authoritative environment is encoded in UTF-16. In Git
for Windows, we convert that to UTF-8 (because UTF-16 is *such* a
foreign idea to Git that its source code is unprepared for it).

Previously, out of performance concerns, we converted the entire
environment to UTF-8 in one fell swoop at the beginning, and upon
putenv() and run_command() converted it back.

Having a private copy of the environment comes with its own perils: when
a library used by Git's source code tries to modify the environment, it
does not really work (in Git for Windows' case, libcurl, see
https://github.com/git-for-windows/git/compare/bcad1e6d58^...bcad1e6d58^2
for a glimpse of the issues).

Hence, it makes our environment handling substantially more robust if we
switch to on-the-fly-conversion in `getenv()`/`putenv()` calls. Based
on an initial version in the MSVC context by Jeff Hostetler, this patch
makes it so.

Surprisingly, this has a *positive* effect on speed: at the time when
the current code was written, we tested the performance, and there were
*so many* `getenv()` calls that it seemed better to convert everything
in one go. In the meantime, though, Git has obviously been cleaned up a
bit with regards to `getenv()` calls so that the Git processes spawned
by the test suite use an average of only 40 `getenv()`/`putenv()` calls
over the process lifetime.

Speaking of the entire test suite: the total time spent in the
re-encoding in the current code takes about 32.4 seconds (out of 113
minutes runtime), whereas the code introduced in this patch takes only
about 8.2 seconds in total. Not much, but it proves that we need not be
concerned about the performance impact introduced by this patch.

Helped-by: Jeff Hostetler 
Signed-off-by: Johannes Schindelin 
---
 compat/mingw.c | 280 +
 compat/mingw.h |  32 +-
 2 files changed, 196 insertions(+), 116 deletions(-)

diff --git a/compat/mingw.c b/compat/mingw.c
index 44264fe3f..0cd432441 100644
--- a/compat/mingw.c
+++ b/compat/mingw.c
@@ -1071,44 +1071,121 @@ static char *path_lookup(const char *cmd, int exe_only)
return prog;
 }
 
-static int do_putenv(char **env, const char *name, int size, int free_old);
+static const wchar_t *wcschrnul(const wchar_t *s, wchar_t c)
+{
+   while (*s && *s != c)
+   s++;
+   return s;
+}
+
+/* Compare only keys */
+static int wenvcmp(const void *a, const void *b)
+{
+   wchar_t *p = *(wchar_t **)a, *q = *(wchar_t **)b;
+   size_t p_len, q_len;
+
+   /* Find the keys */
+   p_len = wcschrnul(p, L'=') - p;
+   q_len = wcschrnul(q, L'=') - q;
+
+   /* If the length differs, include the shorter key's NUL */
+   if (p_len < q_len)
+   p_len++;
+   else if (p_len > q_len)
+   p_len = q_len + 1;
+
+   return _wcsnicmp(p, q, p_len);
+}
 
-/* used number of elements of environ array, including terminating NULL */
-static int environ_size = 0;
-/* allocated size of environ array, in bytes */
-static int environ_alloc = 0;
+/* We need a stable sort to convert the environment between UTF-16 <-> UTF-8 */
+#ifndef INTERNAL_QSORT
+#include "qsort.c"
+#endif
 
 /*
- * Create environment block suitable for CreateProcess. Merges current
- * process environment and the supplied environment changes.
+ * Build an environment block combining the inherited environment
+ * merged with the given list of settings.
+ *
+ * Values of the form "KEY=VALUE" in deltaenv override inherited values.
+ * Values of the form "KEY" in deltaenv delete inherited values.
+ *
+ * Multiple entries in deltaenv for the same key are explicitly allowed.
+ *
+ * We return a contiguous block of UNICODE strings with a final trailing
+ * zero word.
  */
 static wchar_t *make_environment_block(char **deltaenv)
 {
-   wchar_t *wenvblk = NULL;
-   char **tmpenv;
-   int i = 0, size = environ_size, wenvsz = 0, wenvpos = 0;
+   wchar_t *wenv = GetEnvironmentStringsW(), *wdeltaenv, *result, *p;
+   size_t wlen, s, delta_size, size;
+
+   wchar_t **array = NULL;
+   size_t alloc = 0, nr = 0, i;
+
+   size = 1; /* for extra NUL at the end */
+
+   /* If there is no deltaenv to apply, simply return a copy. */
+   if (!deltaenv || !*deltaenv) {
+   for (p = wenv; p && *p; ) {
+   size_t s = wcslen(p) + 1;
+   size += s;
+   p += s;
+   }
+
+   ALLOC_ARRAY(result, size);
+   memcpy(result, wenv, size * sizeof(*wenv));
+   FreeEnvironmentStringsW(wenv);
+   return result;
+   }
+
+   /*
+* If there is a deltaenv, let's accumulate all keys into `array`,
+* sort them using the stable git_qsort() and then copy, skipping
+* duplicate keys
+*/
+   for (p = wenv; p && *p; ) {
+   ALLOC_GROW(array, nr + 1, 

[PATCH 0/2] mingw: rework the environment handling

2018-10-30 Thread Johannes Schindelin via GitGitGadget
Once upon a time, the Git for Windows project had to decide what to do about
Unicode support, including how to deal with the environment. Karsten Blees
spent a ton of work on this, culminating in the final version
[https://groups.google.com/d/msg/msysgit/wNZAyScbJG4/viWz2KXU0VYJ] which
made it into Git for Windows and at least partially into core Git, too.

The environment handling in particular is a bit tricky: Windows actually has 
two copies of the environment, one encoded in UTF-16, and the other one in
the local encoding. Since we want UTF-8 encoded values (which is not an
option for the local encoding), we had to convert from/to the UTF-16
environment.

At the time those patches were developed, there were so many getenv()/
putenv() calls in Git's code base that it seemed the best solution to
convert the entire environment into UTF-8 in one go, at startup.

There are good reasons for us to change that paradigm now (and this patch
series does that):

 * The method we use does not work with modern MSVC runtimes (__environ can
   no longer be overridden).
 * Our method of having a malloc()ed environment wreaks havoc if a library
   we use calls MSVC's version of setenv() (I am looking at you, libcurl).
 * In the meantime, core Git's usage of getenv()/putenv() was reduced
   dramatically (for unrelated reasons), so that it is actually advantageous
   nowadays to convert on the fly, i.e. with each getenv()/putenv() call,
   rather than doing one wholesale conversion at process startup. See also
   the commit message of the second patch.

Note: in contrast to other patches flowing from Git for Windows to Git these
days this patch has not been in Git for Windows for ages. Its approach has
been tested in some MS Visual C++ builds (thanks, Jeff Hostetler!), though,
so I am quite confident that it is correct, and the test suite agrees.

Johannes Schindelin (2):
  t7800: fix quoting
  mingw: reencode environment variables on the fly (UTF-16 <-> UTF-8)

 compat/mingw.c  | 280 ++--
 compat/mingw.h  |  32 -
 t/t7800-difftool.sh |   2 +-
 3 files changed, 197 insertions(+), 117 deletions(-)


base-commit: c670b1f876521c9f7cd40184bf7ed05aad843433
Published-As: 
https://github.com/gitgitgadget/git/releases/tags/pr-57%2Fdscho%2Fmingw-utf-8-env-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git 
pr-57/dscho/mingw-utf-8-env-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/57
-- 
gitgitgadget


[PATCH v2 3/3] http: when using Secure Channel, ignore sslCAInfo by default

2018-10-25 Thread Johannes Schindelin via GitGitGadget
From: Johannes Schindelin 

As of cURL v7.60.0, the Secure Channel backend can use the certificate
bundle provided via `http.sslCAInfo`, but that would override the
Windows Certificate Store. Since this is not desirable by default, let's
tell Git to not ask cURL to use that bundle by default when the `schannel`
backend was configured via `http.sslBackend`, unless
`http.schannelUseSSLCAInfo` overrides this behavior.

Signed-off-by: Johannes Schindelin 
---
 Documentation/config.txt |  8 
 http.c   | 19 ++-
 2 files changed, 26 insertions(+), 1 deletion(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index d569ebd49..1f6a6a4a6 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1997,6 +1997,14 @@ http.schannelCheckRevoke::
certificate. This option is ignored if cURL lacks support for
setting the relevant SSL option at runtime.
 
+http.schannelUseSSLCAInfo::
+   As of cURL v7.60.0, the Secure Channel backend can use the
+   certificate bundle provided via `http.sslCAInfo`, but that would
+   override the Windows Certificate Store. Since this is not desirable
+   by default, Git will tell cURL not to use that bundle by default
+   when the `schannel` backend was configured via `http.sslBackend`,
+   unless `http.schannelUseSSLCAInfo` overrides this behavior.
+
 http.pinnedpubkey::
Public key of the https service. It may either be the filename of
a PEM or DER encoded public key file or a string starting with
diff --git a/http.c b/http.c
index 65daa9bfa..28009ca73 100644
--- a/http.c
+++ b/http.c
@@ -158,6 +158,12 @@ static char *cached_accept_language;
 static char *http_ssl_backend;
 
 static int http_schannel_check_revoke = 1;
+/*
+ * With the backend being set to `schannel`, setting sslCAinfo would override
+ * the Certificate Store in cURL v7.60.0 and later, which is not what we want
+ * by default.
+ */
+static int http_schannel_use_ssl_cainfo;
 
 size_t fread_buffer(char *ptr, size_t eltsize, size_t nmemb, void *buffer_)
 {
@@ -317,6 +323,11 @@ static int http_options(const char *var, const char 
*value, void *cb)
return 0;
}
 
+   if (!strcmp("http.schannelusesslcainfo", var)) {
+   http_schannel_use_ssl_cainfo = git_config_bool(var, value);
+   return 0;
+   }
+
if (!strcmp("http.minsessions", var)) {
min_curl_sessions = git_config_int(var, value);
 #ifndef USE_CURL_MULTI
@@ -869,7 +880,13 @@ static CURL *get_curl_handle(void)
if (ssl_pinnedkey != NULL)
curl_easy_setopt(result, CURLOPT_PINNEDPUBLICKEY, 
ssl_pinnedkey);
 #endif
-   if (ssl_cainfo != NULL)
+   if (http_ssl_backend && !strcmp("schannel", http_ssl_backend) &&
+   !http_schannel_use_ssl_cainfo) {
+   curl_easy_setopt(result, CURLOPT_CAINFO, NULL);
+#if LIBCURL_VERSION_NUM >= 0x073400
+   curl_easy_setopt(result, CURLOPT_PROXY_CAINFO, NULL);
+#endif
+   } else if (ssl_cainfo != NULL)
curl_easy_setopt(result, CURLOPT_CAINFO, ssl_cainfo);
 
if (curl_low_speed_limit > 0 && curl_low_speed_time > 0) {
-- 
gitgitgadget


[PATCH v2 1/3] http: add support for selecting SSL backends at runtime

2018-10-25 Thread Johannes Schindelin via GitGitGadget
From: Johannes Schindelin 

As of version 7.56.0, curl supports being compiled with multiple SSL
backends.

This patch adds the Git side of that feature: by setting http.sslBackend
to "openssl" or "schannel", Git for Windows can now choose the SSL
backend at runtime.

This comes in handy on Windows because Secure Channel ("schannel") is
the native solution, accessing the Windows Credential Store, thereby
allowing for enterprise-wide management of certificates. For historical
reasons, Git for Windows needs to support OpenSSL still, as it has
previously been the only supported SSL backend in Git for Windows for
almost a decade.

The patch has been carried in Git for Windows for over a year, and is
considered mature.

Signed-off-by: Johannes Schindelin 
---
 Documentation/config.txt |  5 +
 http.c   | 35 +++
 2 files changed, 40 insertions(+)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 154683321..7d38f0bf1 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1984,6 +1984,11 @@ http.sslCAPath::
with when fetching or pushing over HTTPS. Can be overridden
by the `GIT_SSL_CAPATH` environment variable.
 
+http.sslBackend::
+   Name of the SSL backend to use (e.g. "openssl" or "schannel").
+   This option is ignored if cURL lacks support for choosing the SSL
+   backend at runtime.
+
 http.pinnedpubkey::
Public key of the https service. It may either be the filename of
a PEM or DER encoded public key file or a string starting with
diff --git a/http.c b/http.c
index 98ff12258..7fb37a061 100644
--- a/http.c
+++ b/http.c
@@ -155,6 +155,8 @@ static struct active_request_slot *active_queue_head;
 
 static char *cached_accept_language;
 
+static char *http_ssl_backend;
+
 size_t fread_buffer(char *ptr, size_t eltsize, size_t nmemb, void *buffer_)
 {
size_t size = eltsize * nmemb;
@@ -302,6 +304,12 @@ static int http_options(const char *var, const char 
*value, void *cb)
curl_ssl_try = git_config_bool(var, value);
return 0;
}
+   if (!strcmp("http.sslbackend", var)) {
+   free(http_ssl_backend);
+   http_ssl_backend = xstrdup_or_null(value);
+   return 0;
+   }
+
if (!strcmp("http.minsessions", var)) {
min_curl_sessions = git_config_int(var, value);
 #ifndef USE_CURL_MULTI
@@ -995,6 +1003,33 @@ void http_init(struct remote *remote, const char *url, 
int proactive_auth)
git_config(urlmatch_config_entry, );
free(normalized_url);
 
+#if LIBCURL_VERSION_NUM >= 0x073800
+   if (http_ssl_backend) {
+   const curl_ssl_backend **backends;
+   struct strbuf buf = STRBUF_INIT;
+   int i;
+
+   switch (curl_global_sslset(-1, http_ssl_backend, )) {
+   case CURLSSLSET_UNKNOWN_BACKEND:
+   strbuf_addf(, _("Unsupported SSL backend '%s'. "
+   "Supported SSL backends:"),
+   http_ssl_backend);
+   for (i = 0; backends[i]; i++)
+   strbuf_addf(, "\n\t%s", backends[i]->name);
+   die("%s", buf.buf);
+   case CURLSSLSET_NO_BACKENDS:
+   die(_("Could not set SSL backend to '%s': "
+ "cURL was built without SSL backends"),
+   http_ssl_backend);
+   case CURLSSLSET_TOO_LATE:
+   die(_("Could not set SSL backend to '%s': already set"),
+   http_ssl_backend);
+   case CURLSSLSET_OK:
+   break; /* Okay! */
+   }
+   }
+#endif
+
if (curl_global_init(CURL_GLOBAL_ALL) != CURLE_OK)
die("curl_global_init failed");
 
-- 
gitgitgadget



[PATCH v2 0/3] Allow choosing the SSL backend cURL uses (plus related patches)

2018-10-25 Thread Johannes Schindelin via GitGitGadget
This topic branch brings support for choosing cURL's SSL backend (a feature
developed in Git for Windows' context) at runtime via http.sslBackend, and
two more patches that are related (and only of interest for Windows users).

Changes since v1:

 * Reworded the commit message of v1's patch 2/3, to talk about the original
   design instead of "an earlier iteration" that was never contributed to
   the Git mailing list.
 * Changed the confusing >= 7.44.0 to < 7.44.0.

Note: I had prepared 
https://github.com/dscho/git/commit/81e8c9a4006c919747a0b6a287f28f25799fcaf4
, intended to be included in v2, but Junio came up with 
https://public-inbox.org/git/xmqqsh0uln5c.fsf...@gitster-ct.c.googlers.com/ 
in the meantime, which I like better.

Brendan Forster (1):
  http: add support for disabling SSL revocation checks in cURL

Johannes Schindelin (2):
  http: add support for selecting SSL backends at runtime
  http: when using Secure Channel, ignore sslCAInfo by default

 Documentation/config.txt | 21 
 http.c   | 71 +++-
 2 files changed, 91 insertions(+), 1 deletion(-)


base-commit: 5a0cc8aca797dbd7d2be3b67458ff880ed45cddf
Published-As: 
https://github.com/gitgitgadget/git/releases/tags/pr-46%2Fdscho%2Fhttp-ssl-backend-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git 
pr-46/dscho/http-ssl-backend-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/46

Range-diff vs v1:

 1:  8c5ecdb6c = 1:  85bd0fb27 http: add support for selecting SSL backends at 
runtime
 2:  764791d13 ! 2:  951383695 http: add support for disabling SSL revocation 
checks in cURL
 @@ -14,10 +14,10 @@
  
  This is only supported in cURL 7.44 or later.
  
 -Note: an earlier iteration tried to use the config setting
 -http.schannel.checkRevoke, but the http.* config settings can be 
limited
 -to specific URLs via http..* (which would mistake `schannel` for 
a
 -URL).
 +Note: originally, we wanted to call the config setting
 +`http.schannel.checkRevoke`. This, however, does not work: the 
`http.*`
 +config settings can be limited to specific URLs via `http..*`
 +(and this feature would mistake `schannel` for a URL).
  
  Helped by Agustín Martín Barbero.
  
 @@ -77,7 +77,7 @@
  + curl_easy_setopt(result, CURLOPT_SSL_OPTIONS, 
CURLSSLOPT_NO_REVOKE);
  +#else
  + warning("CURLSSLOPT_NO_REVOKE not applied to curl SSL options 
because\n"
 -+ "your curl version is too old (>= 7.44.0)");
 ++ "your curl version is too old (< 7.44.0)");
  +#endif
  + }
  +
 3:  9927e4ce6 = 3:  a5f937a36 http: when using Secure Channel, ignore 
sslCAInfo by default

-- 
gitgitgadget


[PATCH v4 3/3] repack -ad: prune the list of shallow commits

2018-10-24 Thread Johannes Schindelin via GitGitGadget
From: Johannes Schindelin 

`git repack` can drop unreachable commits without further warning,
making the corresponding entries in `.git/shallow` invalid, which causes
serious problems when deepening the branches.

One scenario where unreachable commits are dropped by `git repack` is
when a `git fetch --prune` (or even a `git fetch` when a ref was
force-pushed in the meantime) can make a commit unreachable that was
reachable before.

Therefore it is not safe to assume that a `git repack -adlf` will keep
unreachable commits alone (under the assumption that they had not been
packed in the first place, which is an assumption at least some of Git's
code seems to make).

This is particularly important to keep in mind when looking at the
`.git/shallow` file: if any commits listed in that file become
unreachable, it is not a problem, but if they go missing, it *is* a
problem. One symptom of this problem is that a deepening fetch may now
fail with

fatal: error in object: unshallow 

To avoid this problem, let's prune the shallow list in `git repack` when
the `-d` option is passed, unless `-A` is passed, too (which would force
the now-unreachable objects to be turned into loose objects instead of
being deleted). Additionally, we also need to take `--keep-reachable`
and `--unpack-unreachable=` into account.

Note: an alternative solution discussed during the review of this patch
was to teach `git fetch` to simply ignore entries in .git/shallow if the
corresponding commits do not exist locally. A quick test, however,
revealed that the .git/shallow file is written during a shallow *clone*,
in which case the commits do not exist, either, but the "shallow" line
*does* need to be sent. Therefore, this approach would be a lot more
finicky than the approach presented by the this patch.

Signed-off-by: Johannes Schindelin 
---
 builtin/repack.c | 6 ++
 t/t5537-fetch-shallow.sh | 2 +-
 2 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/builtin/repack.c b/builtin/repack.c
index c6a7943d5..acd43c75d 100644
--- a/builtin/repack.c
+++ b/builtin/repack.c
@@ -549,6 +549,12 @@ int cmd_repack(int argc, const char **argv, const char 
*prefix)
if (!po_args.quiet && isatty(2))
opts |= PRUNE_PACKED_VERBOSE;
prune_packed_objects(opts);
+
+   if (!keep_unreachable &&
+   (!(pack_everything & LOOSEN_UNREACHABLE) ||
+unpack_unreachable) &&
+   is_repository_shallow(the_repository))
+   prune_shallow(PRUNE_QUICK);
}
 
if (!no_update_server_info)
diff --git a/t/t5537-fetch-shallow.sh b/t/t5537-fetch-shallow.sh
index 686fdc928..6faf17e17 100755
--- a/t/t5537-fetch-shallow.sh
+++ b/t/t5537-fetch-shallow.sh
@@ -186,7 +186,7 @@ EOF
test_cmp expect actual
 '
 
-test_expect_failure '.git/shallow is edited by repack' '
+test_expect_success '.git/shallow is edited by repack' '
git init shallow-server &&
test_commit -C shallow-server A &&
test_commit -C shallow-server B &&
-- 
gitgitgadget


[PATCH v4 2/3] shallow: offer to prune only non-existing entries

2018-10-24 Thread Johannes Schindelin via GitGitGadget
From: Johannes Schindelin 

The `prune_shallow()` function wants a full reachability check to be
completed before it goes to work, to ensure that all unreachable entries
are removed from the shallow file.

However, in the upcoming patch we do not even want to go that far. We
really only need to remove entries corresponding to pruned commits, i.e.
to commits that no longer exist.

Let's support that use case.

Rather than extending the signature of `prune_shallow()` to accept
another Boolean, let's turn it into a bit field and declare constants,
for readability.

Signed-off-by: Johannes Schindelin 
---
 builtin/prune.c |  2 +-
 commit.h|  4 +++-
 shallow.c   | 23 +--
 3 files changed, 21 insertions(+), 8 deletions(-)

diff --git a/builtin/prune.c b/builtin/prune.c
index 41230f821..1ec9ddd75 100644
--- a/builtin/prune.c
+++ b/builtin/prune.c
@@ -161,7 +161,7 @@ int cmd_prune(int argc, const char **argv, const char 
*prefix)
free(s);
 
if (is_repository_shallow(the_repository))
-   prune_shallow(show_only);
+   prune_shallow(show_only ? PRUNE_SHOW_ONLY : 0);
 
return 0;
 }
diff --git a/commit.h b/commit.h
index 1d260d62f..b80d6fdb5 100644
--- a/commit.h
+++ b/commit.h
@@ -249,7 +249,9 @@ extern void assign_shallow_commits_to_refs(struct 
shallow_info *info,
   uint32_t **used,
   int *ref_status);
 extern int delayed_reachability_test(struct shallow_info *si, int c);
-extern void prune_shallow(int show_only);
+#define PRUNE_SHOW_ONLY 1
+#define PRUNE_QUICK 2
+extern void prune_shallow(unsigned options);
 extern struct trace_key trace_shallow;
 
 extern int interactive_add(int argc, const char **argv, const char *prefix, 
int patch);
diff --git a/shallow.c b/shallow.c
index 732e18d54..02fdbfc55 100644
--- a/shallow.c
+++ b/shallow.c
@@ -247,6 +247,7 @@ static void check_shallow_file_for_update(struct repository 
*r)
 
 #define SEEN_ONLY 1
 #define VERBOSE   2
+#define QUICK 4
 
 struct write_shallow_data {
struct strbuf *out;
@@ -261,7 +262,10 @@ static int write_one_shallow(const struct commit_graft 
*graft, void *cb_data)
const char *hex = oid_to_hex(>oid);
if (graft->nr_parent != -1)
return 0;
-   if (data->flags & SEEN_ONLY) {
+   if (data->flags & QUICK) {
+   if (!has_object_file(>oid))
+   return 0;
+   } else if (data->flags & SEEN_ONLY) {
struct commit *c = lookup_commit(the_repository, >oid);
if (!c || !(c->object.flags & SEEN)) {
if (data->flags & VERBOSE)
@@ -371,16 +375,23 @@ void advertise_shallow_grafts(int fd)
 
 /*
  * mark_reachable_objects() should have been run prior to this and all
- * reachable commits marked as "SEEN".
+ * reachable commits marked as "SEEN", except when quick_prune is non-zero,
+ * in which case lines are excised from the shallow file if they refer to
+ * commits that do not exist (any longer).
  */
-void prune_shallow(int show_only)
+void prune_shallow(unsigned options)
 {
struct lock_file shallow_lock = LOCK_INIT;
struct strbuf sb = STRBUF_INIT;
+   unsigned flags = SEEN_ONLY;
int fd;
 
-   if (show_only) {
-   write_shallow_commits_1(, 0, NULL, SEEN_ONLY | VERBOSE);
+   if (options & PRUNE_QUICK)
+   flags |= QUICK;
+
+   if (options & PRUNE_SHOW_ONLY) {
+   flags |= VERBOSE;
+   write_shallow_commits_1(, 0, NULL, flags);
strbuf_release();
return;
}
@@ -388,7 +399,7 @@ void prune_shallow(int show_only)
   git_path_shallow(the_repository),
   LOCK_DIE_ON_ERROR);
check_shallow_file_for_update(the_repository);
-   if (write_shallow_commits_1(, 0, NULL, SEEN_ONLY)) {
+   if (write_shallow_commits_1(, 0, NULL, flags)) {
if (write_in_full(fd, sb.buf, sb.len) < 0)
die_errno("failed to write to %s",
  get_lock_file_path(_lock));
-- 
gitgitgadget



[PATCH v4 0/3] repack -ad: fix after fetch --prune in a shallow repository

2018-10-24 Thread Johannes Schindelin via GitGitGadget
Under certain circumstances, commits that were reachable can be made
unreachable, e.g. via git fetch --prune. These commits might have been
packed already, in which case git repack -adlf will just drop them without
giving them the usual grace period before an eventual git prune (via git gc)
prunes them.

This is a problem when the shallow file still lists them, which is the
reason why git prune edits that file. And with the proposed changes, git
repack -ad will now do the same.

Reported by Alejandro Pauly.

Changes since v3:

 * Made the regression test less confusing with regards to tags that might
   need to be dereferenced.
 * Introduced new global constants PRUNE_SHOW_ONLY and PRUNE_QUICK instead
   of extending the signature of prune_shallow() with Boolean parameters.
 * While at it, renamed the file-local constant from QUICK_PRUNE to QUICK.
 * Replaced the lookup_commit() && parse_commit() cadence (that wants to
   test for the existence of a commit) to has_object_file(), for ease of
   readability, and also to make it more obvious how to add a call to 
   is_promisor_object() if we ever figure out that that would be necessary.

Changes since v2:

 * Fixed a typo in the last commit message.
 * Added an explanation to the last commit message why we do not simply skip
   non-existing shallow commits at fetch time.
 * Introduced a new, "quick prune" mode where prune_shallow() does not try
   to drop unreachable commits, but only non-existing ones.
 * Rebased to current master because there were too many merge conflicts for
   my liking otherwise.

Changes since v1:

 * Also trigger prune_shallow() when --unpack-unreachable= was
   passed to git repack.
 * No need to trigger prune_shallow() when git repack was called with -k.

Johannes Schindelin (3):
  repack: point out a bug handling stale shallow info
  shallow: offer to prune only non-existing entries
  repack -ad: prune the list of shallow commits

 builtin/prune.c  |  2 +-
 builtin/repack.c |  6 ++
 commit.h |  4 +++-
 shallow.c| 23 +--
 t/t5537-fetch-shallow.sh | 27 +++
 5 files changed, 54 insertions(+), 8 deletions(-)


base-commit: c4df23f7927d8d00e666a3c8d1b3375f1dc8a3c1
Published-As: 
https://github.com/gitgitgadget/git/releases/tags/pr-9%2Fdscho%2Fshallow-and-fetch-prune-v4
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git 
pr-9/dscho/shallow-and-fetch-prune-v4
Pull-Request: https://github.com/gitgitgadget/git/pull/9

Range-diff vs v3:

 1:  ed8559b91 ! 1:  d9720bd25 repack: point out a bug handling stale shallow 
info
 @@ -29,7 +29,7 @@
  + test_commit -C shallow-server C &&
  + test_commit -C shallow-server E &&
  + test_commit -C shallow-server D &&
 -+ d="$(git -C shallow-server rev-parse --verify D)" &&
 ++ d="$(git -C shallow-server rev-parse --verify D^0)" &&
  + git -C shallow-server checkout master &&
  +
  + git clone --depth=1 --no-tags --no-single-branch \
 @@ -43,7 +43,7 @@
  + test_must_fail git -C shallow-client rev-parse --verify $d^0 &&
  + ! grep $d shallow-client/.git/shallow &&
  +
 -+ git -C shallow-server branch branch-orig D^0 &&
 ++ git -C shallow-server branch branch-orig $d &&
  + git -C shallow-client fetch --prune --depth=2 \
  + origin "+refs/heads/*:refs/remotes/origin/*"
  +'
 2:  f085eb4f7 ! 2:  18308c13e shallow: offer to prune only non-existing entries
 @@ -12,6 +12,10 @@
  
  Let's support that use case.
  
 +Rather than extending the signature of `prune_shallow()` to accept
 +another Boolean, let's turn it into a bit field and declare constants,
 +for readability.
 +
  Signed-off-by: Johannes Schindelin 
  
  diff --git a/builtin/prune.c b/builtin/prune.c
 @@ -22,7 +26,7 @@
   
if (is_repository_shallow(the_repository))
  - prune_shallow(show_only);
 -+ prune_shallow(show_only, 0);
 ++ prune_shallow(show_only ? PRUNE_SHOW_ONLY : 0);
   
return 0;
   }
 @@ -35,7 +39,9 @@
   int *ref_status);
   extern int delayed_reachability_test(struct shallow_info *si, int c);
  -extern void prune_shallow(int show_only);
 -+extern void prune_shallow(int show_only, int quick_prune);
 ++#define PRUNE_SHOW_ONLY 1
 ++#define PRUNE_QUICK 2
 ++extern void prune_shallow(unsigned options);
   extern struct trace_key trace_shallow;
   
   extern int interactive_add(int argc, const char **argv, const char 
*prefix, int patch);
 @@ -47,7 +53,7 @@
   
   #define SEEN_ONLY 1
   #define VERBOSE   2
 -+#define QUICK_PRUNE 4
 ++#define QUICK 4
   
   struct write_shallow_data {
struct strbuf *out;
 @@ -56,9 +62,8 @@
if (graft->nr_parent != -1)
return 0;
  - if (data->flags 

[PATCH v4 1/3] repack: point out a bug handling stale shallow info

2018-10-24 Thread Johannes Schindelin via GitGitGadget
From: Johannes Schindelin 

A `git fetch --prune` can turn previously-reachable objects unreachable,
even commits that are in the `shallow` list. A subsequent `git repack
-ad` will then unceremoniously drop those unreachable commits, and the
`shallow` list will become stale. This means that when we try to fetch
with a larger `--depth` the next time, we may end up with:

fatal: error in object: unshallow 

Reported by Alejandro Pauly.

Signed-off-by: Johannes Schindelin 
---
 t/t5537-fetch-shallow.sh | 27 +++
 1 file changed, 27 insertions(+)

diff --git a/t/t5537-fetch-shallow.sh b/t/t5537-fetch-shallow.sh
index 7045685e2..686fdc928 100755
--- a/t/t5537-fetch-shallow.sh
+++ b/t/t5537-fetch-shallow.sh
@@ -186,6 +186,33 @@ EOF
test_cmp expect actual
 '
 
+test_expect_failure '.git/shallow is edited by repack' '
+   git init shallow-server &&
+   test_commit -C shallow-server A &&
+   test_commit -C shallow-server B &&
+   git -C shallow-server checkout -b branch &&
+   test_commit -C shallow-server C &&
+   test_commit -C shallow-server E &&
+   test_commit -C shallow-server D &&
+   d="$(git -C shallow-server rev-parse --verify D^0)" &&
+   git -C shallow-server checkout master &&
+
+   git clone --depth=1 --no-tags --no-single-branch \
+   "file://$PWD/shallow-server" shallow-client &&
+
+   : now remove the branch and fetch with prune &&
+   git -C shallow-server branch -D branch &&
+   git -C shallow-client fetch --prune --depth=1 \
+   origin "+refs/heads/*:refs/remotes/origin/*" &&
+   git -C shallow-client repack -adfl &&
+   test_must_fail git -C shallow-client rev-parse --verify $d^0 &&
+   ! grep $d shallow-client/.git/shallow &&
+
+   git -C shallow-server branch branch-orig $d &&
+   git -C shallow-client fetch --prune --depth=2 \
+   origin "+refs/heads/*:refs/remotes/origin/*"
+'
+
 . "$TEST_DIRECTORY"/lib-httpd.sh
 start_httpd
 
-- 
gitgitgadget



[PATCH v2 1/2] mingw: ensure `getcwd()` reports the correct case

2018-10-24 Thread Johannes Schindelin via GitGitGadget
From: Johannes Schindelin 

When switching the current working directory, say, in PowerShell, it is
quite possible to use a different capitalization than the one that is
recorded on disk. While doing the same in `cmd.exe` adjusts the
capitalization magically, that does not happen in PowerShell so that
`getcwd()` returns the current directory in a different way than is
recorded on disk.

Typically this creates no problems except when you call

git log .

in a subdirectory called, say, "GIT/" but you switched to "Git/" and
your `getcwd()` reports the latter, then Git won't understand that you
wanted to see the history as per the `GIT/` subdirectory but it thinks you
wanted to see the history of some directory that may have existed in the
past (but actually never did).

So let's be extra careful to adjust the capitalization of the current
directory before working with it.

Reported by a few PowerShell power users ;-)

Signed-off-by: Johannes Schindelin 
---
 compat/mingw.c | 11 +--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/compat/mingw.c b/compat/mingw.c
index 18caf2196..2c3e27ce9 100644
--- a/compat/mingw.c
+++ b/compat/mingw.c
@@ -917,8 +917,15 @@ struct tm *localtime_r(const time_t *timep, struct tm 
*result)
 
 char *mingw_getcwd(char *pointer, int len)
 {
-   wchar_t wpointer[MAX_PATH];
-   if (!_wgetcwd(wpointer, ARRAY_SIZE(wpointer)))
+   wchar_t cwd[MAX_PATH], wpointer[MAX_PATH];
+   DWORD ret = GetCurrentDirectoryW(ARRAY_SIZE(cwd), cwd);
+
+   if (!ret || ret >= ARRAY_SIZE(cwd)) {
+   errno = ret ? ENAMETOOLONG : err_win_to_posix(GetLastError());
+   return NULL;
+   }
+   ret = GetLongPathNameW(cwd, wpointer, ARRAY_SIZE(wpointer));
+   if (!ret || ret >= ARRAY_SIZE(wpointer))
return NULL;
if (xwcstoutf(pointer, wpointer, len) < 0)
return NULL;
-- 
gitgitgadget



[PATCH v2 0/2] Work around case-insensitivity issues with cwd on Windows

2018-10-24 Thread Johannes Schindelin via GitGitGadget
On Windows, file names are recorded case-sensitively, but looked up
case-insensitively. Therefore, it is possible to switch to a directory by
using incorrect case, e.g. cd documentation will still get you into the 
Documentation subdirectory.

In Powershell, doing so will however report the current directory with the
specified spelling rather than the one recorded on disk, and Git will get
confused.

To remedy that, we fixed this in Git for Windows more than three years ago,
and needed only a small fix a couple of months later to accommodate for the
diverse scenarios encountered by the many Git for Windows users.

Not only to keep the story closer to what happened historically, but also to
make it easier to follow, I refrained from squashing these two patches.

Side note: the second patch is technically not battle-tested for that long:
it uses an API function that requires Windows Vista or later, and we only
recently started to clean up Git for Windows' code to drop fallbacks for
Windows XP. Read: this code used to load the GetFinalPathNameByHandle() 
function dynamically, and that is the only difference to the code that has
been "battle-tested" for close to three years.

Changes since v1:

 * Fixed a grammar mistake in the second commit message.

Anton Serbulov (1):
  mingw: fix getcwd when the parent directory cannot be queried

Johannes Schindelin (1):
  mingw: ensure `getcwd()` reports the correct case

 compat/mingw.c | 50 --
 1 file changed, 48 insertions(+), 2 deletions(-)


base-commit: c4df23f7927d8d00e666a3c8d1b3375f1dc8a3c1
Published-As: 
https://github.com/gitgitgadget/git/releases/tags/pr-54%2Fdscho%2Fmingw-getcwd-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git 
pr-54/dscho/mingw-getcwd-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/54

Range-diff vs v1:

 1:  e13ae2338 = 1:  e13ae2338 mingw: ensure `getcwd()` reports the correct case
 2:  3e3b1c3b1 ! 2:  87ef9ac63 mingw: fix getcwd when the parent directory 
cannot be queried
 @@ -4,7 +4,7 @@
  
  `GetLongPathName()` function may fail when it is unable to query
  the parent directory of a path component to determine the long name
 -for that component. It happens, because of it uses `FindFirstFile()`
 +for that component. It happens, because it uses `FindFirstFile()`
  function for each next short part of path. The `FindFirstFile()`
  requires `List Directory` and `Synchronize` desired access for a 
calling
  process.

-- 
gitgitgadget


[PATCH 2/2] rebase --autostash: fix issue with dirty submodules

2018-10-23 Thread Johannes Schindelin via GitGitGadget
From: Johannes Schindelin 

Since we cannot stash dirty submodules, there is no use in requiring
them to be clean (or stash them when they are not).

This brings the built-in rebase in line with the previous, scripted
version, which also did not care about dirty submodules (but it was
admittedly not very easy to figure that out).

This fixes https://github.com/git-for-windows/git/issues/1820

Signed-off-by: Johannes Schindelin 
---
 builtin/rebase.c| 2 +-
 t/t3420-rebase-autostash.sh | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/builtin/rebase.c b/builtin/rebase.c
index 09f55bfb9..1dd24301f 100644
--- a/builtin/rebase.c
+++ b/builtin/rebase.c
@@ -1349,7 +1349,7 @@ int cmd_rebase(int argc, const char **argv, const char 
*prefix)
update_index_if_able(_index, _file);
rollback_lock_file(_file);
 
-   if (has_unstaged_changes(0) || has_uncommitted_changes(0)) {
+   if (has_unstaged_changes(1) || has_uncommitted_changes(1)) {
const char *autostash =
state_dir_path("autostash", );
struct child_process stash = CHILD_PROCESS_INIT;
diff --git a/t/t3420-rebase-autostash.sh b/t/t3420-rebase-autostash.sh
index 7eb9f9041..f355c6825 100755
--- a/t/t3420-rebase-autostash.sh
+++ b/t/t3420-rebase-autostash.sh
@@ -351,7 +351,7 @@ test_expect_success 'autostash is saved on editor failure 
with conflict' '
test_cmp expected file0
 '
 
-test_expect_failure 'autostash with dirty submodules' '
+test_expect_success 'autostash with dirty submodules' '
test_when_finished "git reset --hard && git checkout master" &&
git checkout -b with-submodule &&
git submodule add ./ sub &&
-- 
gitgitgadget


[PATCH 0/2] Demonstrate and fix a rebase --autostash bug with dirty submodules

2018-10-23 Thread Johannes Schindelin via GitGitGadget
This bug report came in via Git for Windows (already with version 2.19.0,
but I misread the reporter's enthusiasm to take matters into his own hands).

The culprit is, in a nutshell, that the built-in rebase tries to run git
stash only when the worktree is dirty, but it includes submodules in that.
However, git stash cannot do anything about submodules, and if the only
changes are in submodules, then it won't even give us back an OID, and the
built-in rebase acts surprised.

The solution is easy: simply exclude the submodules from the question
whether the worktree is dirty.

What is surprisingly not simple is to get the regression test right. For
that reason, and because I firmly believe that it is easier to verify a fix
for a regression when the regression test is introduced separately (i.e.
making it simple to verify that there is a regression), I really want to
keep the first patch separate from the second one.

Since this bug concerns the built-in rebase, I based the patches on top of 
next.

Johannes Schindelin (2):
  rebase --autostash: demonstrate a problem with dirty submodules
  rebase --autostash: fix issue with dirty submodules

 builtin/rebase.c|  2 +-
 t/t3420-rebase-autostash.sh | 10 ++
 2 files changed, 11 insertions(+), 1 deletion(-)


base-commit: 209f214ca4ae4e301fc32e59ab26f937082f3ea3
Published-As: 
https://github.com/gitgitgadget/git/releases/tags/pr-56%2Fdscho%2Ffix-built-in-rebase-autostash-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git 
pr-56/dscho/fix-built-in-rebase-autostash-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/56
-- 
gitgitgadget


[PATCH 1/2] rebase --autostash: demonstrate a problem with dirty submodules

2018-10-23 Thread Johannes Schindelin via GitGitGadget
From: Johannes Schindelin 

It has been reported that dirty submodules cause problems with the
built-in rebase when it is asked to autostash. The symptom is:

fatal: Unexpected stash response: ''

This patch adds a regression test that demonstrates that bug.

Original report: https://github.com/git-for-windows/git/issues/1820

Signed-off-by: Johannes Schindelin 
---
 t/t3420-rebase-autostash.sh | 10 ++
 1 file changed, 10 insertions(+)

diff --git a/t/t3420-rebase-autostash.sh b/t/t3420-rebase-autostash.sh
index 0c4eefec7..7eb9f9041 100755
--- a/t/t3420-rebase-autostash.sh
+++ b/t/t3420-rebase-autostash.sh
@@ -351,4 +351,14 @@ test_expect_success 'autostash is saved on editor failure 
with conflict' '
test_cmp expected file0
 '
 
+test_expect_failure 'autostash with dirty submodules' '
+   test_when_finished "git reset --hard && git checkout master" &&
+   git checkout -b with-submodule &&
+   git submodule add ./ sub &&
+   test_tick &&
+   git commit -m add-submodule &&
+   echo changed >sub/file0 &&
+   git rebase -i --autostash HEAD
+'
+
 test_done
-- 
gitgitgadget



[PATCH 0/2] Work around case-insensitivity issues with cwd on Windows

2018-10-23 Thread Johannes Schindelin via GitGitGadget
On Windows, file names are recorded case-sensitively, but looked up
case-insensitively. Therefore, it is possible to switch to a directory by
using incorrect case, e.g. cd documentation will still get you into the 
Documentation subdirectory.

In Powershell, doing so will however report the current directory with the
specified spelling rather than the one recorded on disk, and Git will get
confused.

To remedy that, we fixed this in Git for Windows more than three years ago,
and needed only a small fix a couple of months later to accommodate for the
diverse scenarios encountered by the many Git for Windows users.

Not only to keep the story closer to what happened historically, but also to
make it easier to follow, I refrained from squashing these two patches.

Side note: the second patch is technically not battle-tested for that long:
it uses an API function that requires Windows Vista or later, and we only
recently started to clean up Git for Windows' code to drop fallbacks for
Windows XP. Read: this code used to load the GetFinalPathNameByHandle() 
function dynamically, and that is the only difference to the code that has
been "battle-tested" for close to three years.

Anton Serbulov (1):
  mingw: fix getcwd when the parent directory cannot be queried

Johannes Schindelin (1):
  mingw: ensure `getcwd()` reports the correct case

 compat/mingw.c | 50 --
 1 file changed, 48 insertions(+), 2 deletions(-)


base-commit: c4df23f7927d8d00e666a3c8d1b3375f1dc8a3c1
Published-As: 
https://github.com/gitgitgadget/git/releases/tags/pr-54%2Fdscho%2Fmingw-getcwd-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git 
pr-54/dscho/mingw-getcwd-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/54
-- 
gitgitgadget


[PATCH 1/2] mingw: ensure `getcwd()` reports the correct case

2018-10-23 Thread Johannes Schindelin via GitGitGadget
From: Johannes Schindelin 

When switching the current working directory, say, in PowerShell, it is
quite possible to use a different capitalization than the one that is
recorded on disk. While doing the same in `cmd.exe` adjusts the
capitalization magically, that does not happen in PowerShell so that
`getcwd()` returns the current directory in a different way than is
recorded on disk.

Typically this creates no problems except when you call

git log .

in a subdirectory called, say, "GIT/" but you switched to "Git/" and
your `getcwd()` reports the latter, then Git won't understand that you
wanted to see the history as per the `GIT/` subdirectory but it thinks you
wanted to see the history of some directory that may have existed in the
past (but actually never did).

So let's be extra careful to adjust the capitalization of the current
directory before working with it.

Reported by a few PowerShell power users ;-)

Signed-off-by: Johannes Schindelin 
---
 compat/mingw.c | 11 +--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/compat/mingw.c b/compat/mingw.c
index 18caf2196..2c3e27ce9 100644
--- a/compat/mingw.c
+++ b/compat/mingw.c
@@ -917,8 +917,15 @@ struct tm *localtime_r(const time_t *timep, struct tm 
*result)
 
 char *mingw_getcwd(char *pointer, int len)
 {
-   wchar_t wpointer[MAX_PATH];
-   if (!_wgetcwd(wpointer, ARRAY_SIZE(wpointer)))
+   wchar_t cwd[MAX_PATH], wpointer[MAX_PATH];
+   DWORD ret = GetCurrentDirectoryW(ARRAY_SIZE(cwd), cwd);
+
+   if (!ret || ret >= ARRAY_SIZE(cwd)) {
+   errno = ret ? ENAMETOOLONG : err_win_to_posix(GetLastError());
+   return NULL;
+   }
+   ret = GetLongPathNameW(cwd, wpointer, ARRAY_SIZE(wpointer));
+   if (!ret || ret >= ARRAY_SIZE(wpointer))
return NULL;
if (xwcstoutf(pointer, wpointer, len) < 0)
return NULL;
-- 
gitgitgadget



[PATCH 1/1] mingw: load system libraries the recommended way

2018-10-23 Thread Johannes Schindelin via GitGitGadget
From: Johannes Schindelin 

When we access IPv6-related functions, we load the corresponding system
library using the `LoadLibrary()` function, which is not the recommended
way to load system libraries.

In practice, it does not make a difference: the `ws2_32.dll` library
containing the IPv6 functions is already loaded into memory, so
LoadLibrary() simply reuses the already-loaded library.

Still, recommended way is recommended way, so let's use that instead.

While at it, also adjust the code in contrib/ that loads system libraries.

Signed-off-by: Johannes Schindelin 
---
 compat/mingw.c  | 3 ++-
 contrib/credential/wincred/git-credential-wincred.c | 3 ++-
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/compat/mingw.c b/compat/mingw.c
index 18caf2196..9fd7db571 100644
--- a/compat/mingw.c
+++ b/compat/mingw.c
@@ -1577,7 +1577,8 @@ static void ensure_socket_initialization(void)
WSAGetLastError());
 
for (name = libraries; *name; name++) {
-   ipv6_dll = LoadLibrary(*name);
+   ipv6_dll = LoadLibraryExA(*name, NULL,
+ LOAD_LIBRARY_SEARCH_SYSTEM32);
if (!ipv6_dll)
continue;
 
diff --git a/contrib/credential/wincred/git-credential-wincred.c 
b/contrib/credential/wincred/git-credential-wincred.c
index 86518cd93..5bdad41de 100644
--- a/contrib/credential/wincred/git-credential-wincred.c
+++ b/contrib/credential/wincred/git-credential-wincred.c
@@ -75,7 +75,8 @@ static CredDeleteWT CredDeleteW;
 static void load_cred_funcs(void)
 {
/* load DLLs */
-   advapi = LoadLibrary("advapi32.dll");
+   advapi = LoadLibraryExA("advapi32.dll", NULL,
+   LOAD_LIBRARY_SEARCH_SYSTEM32);
if (!advapi)
die("failed to load advapi32.dll");
 
-- 
gitgitgadget


[PATCH 0/1] Load system libraries the recommended way on Windows

2018-10-23 Thread Johannes Schindelin via GitGitGadget
The search order for DLLs on Windows is a bit funny, and for system
libraries, it is recommended to use a strict search path.

In practice, this should not make a difference, as the library has been
loaded into memory already, and therefore the LoadLibrary() call would just
return the handle instead of loading the library again.

Johannes Schindelin (1):
  mingw: load system libraries the recommended way

 compat/mingw.c  | 3 ++-
 contrib/credential/wincred/git-credential-wincred.c | 3 ++-
 2 files changed, 4 insertions(+), 2 deletions(-)


base-commit: c4df23f7927d8d00e666a3c8d1b3375f1dc8a3c1
Published-As: 
https://github.com/gitgitgadget/git/releases/tags/pr-55%2Fdscho%2Fmingw-load-sys-dll-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git 
pr-55/dscho/mingw-load-sys-dll-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/55
-- 
gitgitgadget


[PATCH 1/3] mingw: factor out code to set stat() data

2018-10-23 Thread Johannes Schindelin via GitGitGadget
From: Johannes Schindelin 

In our fstat() emulation, we convert the file metadata from Win32 data
structures to an emulated POSIX structure.

To structure the code better, let's factor that part out into its own
function.

Note: it would be tempting to try to unify this code with the part of
do_lstat() that does the same thing, but they operate on different data
structures: BY_HANDLE_FILE_INFORMATION vs WIN32_FILE_ATTRIBUTE_DATA. So
unfortunately, they cannot be unified.

Signed-off-by: Johannes Schindelin 
---
 compat/mingw.c | 39 +--
 1 file changed, 25 insertions(+), 14 deletions(-)

diff --git a/compat/mingw.c b/compat/mingw.c
index 18caf2196..d2e7d86db 100644
--- a/compat/mingw.c
+++ b/compat/mingw.c
@@ -736,6 +736,29 @@ static int do_stat_internal(int follow, const char 
*file_name, struct stat *buf)
return do_lstat(follow, alt_name, buf);
 }
 
+static int get_file_info_by_handle(HANDLE hnd, struct stat *buf)
+{
+   BY_HANDLE_FILE_INFORMATION fdata;
+
+   if (!GetFileInformationByHandle(hnd, )) {
+   errno = err_win_to_posix(GetLastError());
+   return -1;
+   }
+
+   buf->st_ino = 0;
+   buf->st_gid = 0;
+   buf->st_uid = 0;
+   buf->st_nlink = 1;
+   buf->st_mode = file_attr_to_st_mode(fdata.dwFileAttributes);
+   buf->st_size = fdata.nFileSizeLow |
+   (((off_t)fdata.nFileSizeHigh)<<32);
+   buf->st_dev = buf->st_rdev = 0; /* not used by Git */
+   buf->st_atime = filetime_to_time_t(&(fdata.ftLastAccessTime));
+   buf->st_mtime = filetime_to_time_t(&(fdata.ftLastWriteTime));
+   buf->st_ctime = filetime_to_time_t(&(fdata.ftCreationTime));
+   return 0;
+}
+
 int mingw_lstat(const char *file_name, struct stat *buf)
 {
return do_stat_internal(0, file_name, buf);
@@ -748,7 +771,6 @@ int mingw_stat(const char *file_name, struct stat *buf)
 int mingw_fstat(int fd, struct stat *buf)
 {
HANDLE fh = (HANDLE)_get_osfhandle(fd);
-   BY_HANDLE_FILE_INFORMATION fdata;
 
if (fh == INVALID_HANDLE_VALUE) {
errno = EBADF;
@@ -758,20 +780,9 @@ int mingw_fstat(int fd, struct stat *buf)
if (GetFileType(fh) != FILE_TYPE_DISK)
return _fstati64(fd, buf);
 
-   if (GetFileInformationByHandle(fh, )) {
-   buf->st_ino = 0;
-   buf->st_gid = 0;
-   buf->st_uid = 0;
-   buf->st_nlink = 1;
-   buf->st_mode = file_attr_to_st_mode(fdata.dwFileAttributes);
-   buf->st_size = fdata.nFileSizeLow |
-   (((off_t)fdata.nFileSizeHigh)<<32);
-   buf->st_dev = buf->st_rdev = 0; /* not used by Git */
-   buf->st_atime = filetime_to_time_t(&(fdata.ftLastAccessTime));
-   buf->st_mtime = filetime_to_time_t(&(fdata.ftLastWriteTime));
-   buf->st_ctime = filetime_to_time_t(&(fdata.ftCreationTime));
+   if (!get_file_info_by_handle(fh, buf))
return 0;
-   }
+
errno = EBADF;
return -1;
 }
-- 
gitgitgadget



[PATCH 0/3] Use nanosecond-precision file times on Windows

2018-10-23 Thread Johannes Schindelin via GitGitGadget
This is yet another patch series in the slow wave of patches coming over
from Git for Windows.

With this change, we now use preciser timestamps to determine e.g. whether
the Git index is out of date. This change made it into Git for Windows
already in version 2.6.0, i.e. for a little over three years.

Please note that this change originally caused a lot of trouble, as e.g.
libgit2 was unaware of our plans and used second-precision file times. So if
you used Git for Windows as well as a libgit2-based program to, say, update
the Git index, there would be a back-and-forth between index updates with
and without the fractional second parts, causing quite a bit of bad
performance.

These issues have been ironed out long ago, though, so it is high time to
contribute these patches to core Git.

Johannes Schindelin (1):
  mingw: factor out code to set stat() data

Karsten Blees (2):
  mingw: replace MSVCRT's fstat() with a Win32-based implementation
  mingw: implement nanosecond-precision file times

 compat/mingw.c   | 76 +++-
 compat/mingw.h   | 36 ---
 config.mak.uname |  2 --
 3 files changed, 76 insertions(+), 38 deletions(-)


base-commit: c4df23f7927d8d00e666a3c8d1b3375f1dc8a3c1
Published-As: 
https://github.com/gitgitgadget/git/releases/tags/pr-53%2Fdscho%2Fnanosecond-file-times-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git 
pr-53/dscho/nanosecond-file-times-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/53
-- 
gitgitgadget


[PATCH v2 2/3] rebase (autostash): store the full OID in /autostash

2018-10-22 Thread Johannes Schindelin via GitGitGadget
From: Johannes Schindelin 

It was reported by Gábor Szeder and analyzed by Alban Gruin that the
built-in rebase stores only abbreviated stash hashes in the `autostash`
file.

This is problematic e.g. in t5520-pull.sh, where the abbreviated hash is
so short that it sometimes consists only of digits, which are
subsequently mistaken ("DWIMmed") for numbers by `git stash apply`.

Let's align the behavior of the built-in rebase with the scripted rebase
and store the full stash hash instead. That makes it a lot less likely
that it consists only of digits.

Signed-off-by: Johannes Schindelin 
---
 builtin/rebase.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/builtin/rebase.c b/builtin/rebase.c
index d21504d9f..418624837 100644
--- a/builtin/rebase.c
+++ b/builtin/rebase.c
@@ -1375,7 +1375,7 @@ int cmd_rebase(int argc, const char **argv, const char 
*prefix)
if (safe_create_leading_directories_const(autostash))
die(_("Could not create directory for '%s'"),
options.state_dir);
-   write_file(autostash, "%s", buf.buf);
+   write_file(autostash, "%s", oid_to_hex());
printf(_("Created autostash: %s\n"), buf.buf);
if (reset_head(>object.oid, "reset --hard",
   NULL, 0, NULL, NULL) < 0)
-- 
gitgitgadget



  1   2   3   4   >