[PATCH 1/1] rebase: fix GIT_REFLOG_ACTION regression
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
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
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
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
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
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
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
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"
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
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
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
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
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
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
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
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
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
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/
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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"
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
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
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
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
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
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/
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
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
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
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()
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
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
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
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
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
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()
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
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
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
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
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
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
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
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()
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()
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
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
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
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
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
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()
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()
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
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()
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
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
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
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
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)
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
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
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
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)
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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