From e9d309e2fcd4faaeee9d20ef2a46bf86a4847bd0 Mon Sep 17 00:00:00 2001 From: Denton Liu Date: Wed, 24 Apr 2019 15:46:57 -0700 Subject: [PATCH 01/66] t7610: unsuppress output The output for commands used to be suppressed by redirecting both stdout and stderr to /dev/null. However, this should not happen since the output is useful for debugging and, without the "-v" flag, test scripts don't output anyway. Unsuppress the output by removing the redirections to /dev/null. Signed-off-by: Denton Liu Signed-off-by: Junio C Hamano --- t/t7610-mergetool.sh | 122 +++++++++++++++++++++---------------------- 1 file changed, 61 insertions(+), 61 deletions(-) diff --git a/t/t7610-mergetool.sh b/t/t7610-mergetool.sh index a9fb971615..69711487dd 100755 --- a/t/t7610-mergetool.sh +++ b/t/t7610-mergetool.sh @@ -130,14 +130,14 @@ test_expect_success 'custom mergetool' ' test_when_finished "git reset --hard" && git checkout -b test$test_count branch1 && git submodule update -N && - test_must_fail git merge master >/dev/null 2>&1 && - ( yes "" | git mergetool both >/dev/null 2>&1 ) && + test_must_fail git merge master && + ( yes "" | git mergetool both ) && ( yes "" | git mergetool file1 file1 ) && - ( yes "" | git mergetool file2 "spaced name" >/dev/null 2>&1 ) && - ( yes "" | git mergetool subdir/file3 >/dev/null 2>&1 ) && - ( yes "d" | git mergetool file11 >/dev/null 2>&1 ) && - ( yes "d" | git mergetool file12 >/dev/null 2>&1 ) && - ( yes "l" | git mergetool submod >/dev/null 2>&1 ) && + ( yes "" | git mergetool file2 "spaced name" ) && + ( yes "" | git mergetool subdir/file3 ) && + ( yes "d" | git mergetool file11 ) && + ( yes "d" | git mergetool file12 ) && + ( yes "l" | git mergetool submod ) && test "$(cat file1)" = "master updated" && test "$(cat file2)" = "master new" && test "$(cat subdir/file3)" = "master new sub" && @@ -153,15 +153,15 @@ test_expect_success 'mergetool crlf' ' # test_when_finished is LIFO.) test_config core.autocrlf true && git checkout -b test$test_count branch1 && - test_must_fail git merge master >/dev/null 2>&1 && - ( yes "" | git mergetool file1 >/dev/null 2>&1 ) && - ( yes "" | git mergetool file2 >/dev/null 2>&1 ) && - ( yes "" | git mergetool "spaced name" >/dev/null 2>&1 ) && - ( yes "" | git mergetool both >/dev/null 2>&1 ) && - ( yes "" | git mergetool subdir/file3 >/dev/null 2>&1 ) && - ( yes "d" | git mergetool file11 >/dev/null 2>&1 ) && - ( yes "d" | git mergetool file12 >/dev/null 2>&1 ) && - ( yes "r" | git mergetool submod >/dev/null 2>&1 ) && + test_must_fail git merge master && + ( yes "" | git mergetool file1 ) && + ( yes "" | git mergetool file2 ) && + ( yes "" | git mergetool "spaced name" ) && + ( yes "" | git mergetool both ) && + ( yes "" | git mergetool subdir/file3 ) && + ( yes "d" | git mergetool file11 ) && + ( yes "d" | git mergetool file12 ) && + ( yes "r" | git mergetool submod ) && test "$(printf x | cat file1 -)" = "$(printf "master updated\r\nx")" && test "$(printf x | cat file2 -)" = "$(printf "master new\r\nx")" && test "$(printf x | cat subdir/file3 -)" = "$(printf "master new sub\r\nx")" && @@ -176,8 +176,8 @@ test_expect_success 'mergetool in subdir' ' git submodule update -N && ( cd subdir && - test_must_fail git merge master >/dev/null 2>&1 && - ( yes "" | git mergetool file3 >/dev/null 2>&1 ) && + test_must_fail git merge master && + ( yes "" | git mergetool file3 ) && test "$(cat file3)" = "master new sub" ) ' @@ -188,14 +188,14 @@ test_expect_success 'mergetool on file in parent dir' ' git submodule update -N && ( cd subdir && - test_must_fail git merge master >/dev/null 2>&1 && - ( yes "" | git mergetool file3 >/dev/null 2>&1 ) && - ( yes "" | git mergetool ../file1 >/dev/null 2>&1 ) && - ( yes "" | git mergetool ../file2 ../spaced\ name >/dev/null 2>&1 ) && - ( yes "" | git mergetool ../both >/dev/null 2>&1 ) && - ( yes "d" | git mergetool ../file11 >/dev/null 2>&1 ) && - ( yes "d" | git mergetool ../file12 >/dev/null 2>&1 ) && - ( yes "l" | git mergetool ../submod >/dev/null 2>&1 ) && + test_must_fail git merge master && + ( yes "" | git mergetool file3 ) && + ( yes "" | git mergetool ../file1 ) && + ( yes "" | git mergetool ../file2 ../spaced\ name ) && + ( yes "" | git mergetool ../both ) && + ( yes "d" | git mergetool ../file11 ) && + ( yes "d" | git mergetool ../file12 ) && + ( yes "l" | git mergetool ../submod ) && test "$(cat ../file1)" = "master updated" && test "$(cat ../file2)" = "master new" && test "$(cat ../submod/bar)" = "branch1 submodule" && @@ -209,9 +209,9 @@ test_expect_success 'mergetool skips autoresolved' ' git submodule update -N && test_must_fail git merge master && test -n "$(git ls-files -u)" && - ( yes "d" | git mergetool file11 >/dev/null 2>&1 ) && - ( yes "d" | git mergetool file12 >/dev/null 2>&1 ) && - ( yes "l" | git mergetool submod >/dev/null 2>&1 ) && + ( yes "d" | git mergetool file11 ) && + ( yes "d" | git mergetool file12 ) && + ( yes "l" | git mergetool submod ) && output="$(git mergetool --no-prompt)" && test "$output" = "No files need merging" ' @@ -259,9 +259,9 @@ test_expect_success 'mergetool skips resolved paths when rerere is active' ' rm -rf .git/rr-cache && git checkout -b test$test_count branch1 && git submodule update -N && - test_must_fail git merge master >/dev/null 2>&1 && - ( yes "l" | git mergetool --no-prompt submod >/dev/null 2>&1 ) && - ( yes "d" "d" | git mergetool --no-prompt >/dev/null 2>&1 ) && + test_must_fail git merge master && + ( yes "l" | git mergetool --no-prompt submod ) && + ( yes "d" "d" | git mergetool --no-prompt ) && git submodule update -N && output="$(yes "n" | git mergetool --no-prompt)" && test "$output" = "No files need merging" @@ -369,9 +369,9 @@ test_expect_success 'deleted vs modified submodule' ' git checkout -b test$test_count.a test$test_count && test_must_fail git merge master && test -n "$(git ls-files -u)" && - ( yes "" | git mergetool file1 file2 spaced\ name subdir/file3 >/dev/null 2>&1 ) && - ( yes "" | git mergetool both >/dev/null 2>&1 ) && - ( yes "d" | git mergetool file11 file12 >/dev/null 2>&1 ) && + ( yes "" | git mergetool file1 file2 spaced\ name subdir/file3 ) && + ( yes "" | git mergetool both ) && + ( yes "d" | git mergetool file11 file12 ) && ( yes "r" | git mergetool submod ) && rmdir submod && mv submod-movedaside submod && test "$(cat submod/bar)" = "branch1 submodule" && @@ -386,9 +386,9 @@ test_expect_success 'deleted vs modified submodule' ' git submodule update -N && test_must_fail git merge master && test -n "$(git ls-files -u)" && - ( yes "" | git mergetool file1 file2 spaced\ name subdir/file3 >/dev/null 2>&1 ) && - ( yes "" | git mergetool both >/dev/null 2>&1 ) && - ( yes "d" | git mergetool file11 file12 >/dev/null 2>&1 ) && + ( yes "" | git mergetool file1 file2 spaced\ name subdir/file3 ) && + ( yes "" | git mergetool both ) && + ( yes "d" | git mergetool file11 file12 ) && ( yes "l" | git mergetool submod ) && test ! -e submod && output="$(git mergetool --no-prompt)" && @@ -400,9 +400,9 @@ test_expect_success 'deleted vs modified submodule' ' git submodule update -N && test_must_fail git merge test$test_count && test -n "$(git ls-files -u)" && - ( yes "" | git mergetool file1 file2 spaced\ name subdir/file3 >/dev/null 2>&1 ) && - ( yes "" | git mergetool both >/dev/null 2>&1 ) && - ( yes "d" | git mergetool file11 file12 >/dev/null 2>&1 ) && + ( yes "" | git mergetool file1 file2 spaced\ name subdir/file3 ) && + ( yes "" | git mergetool both ) && + ( yes "d" | git mergetool file11 file12 ) && ( yes "r" | git mergetool submod ) && test ! -e submod && test -d submod.orig && @@ -416,9 +416,9 @@ test_expect_success 'deleted vs modified submodule' ' git submodule update -N && test_must_fail git merge test$test_count && test -n "$(git ls-files -u)" && - ( yes "" | git mergetool file1 file2 spaced\ name subdir/file3 >/dev/null 2>&1 ) && - ( yes "" | git mergetool both >/dev/null 2>&1 ) && - ( yes "d" | git mergetool file11 file12 >/dev/null 2>&1 ) && + ( yes "" | git mergetool file1 file2 spaced\ name subdir/file3 ) && + ( yes "" | git mergetool both ) && + ( yes "d" | git mergetool file11 file12 ) && ( yes "l" | git mergetool submod ) && test "$(cat submod/bar)" = "master submodule" && git submodule update -N && @@ -440,9 +440,9 @@ test_expect_success 'file vs modified submodule' ' git checkout -b test$test_count.a branch1 && test_must_fail git merge master && test -n "$(git ls-files -u)" && - ( yes "" | git mergetool file1 file2 spaced\ name subdir/file3 >/dev/null 2>&1 ) && - ( yes "" | git mergetool both >/dev/null 2>&1 ) && - ( yes "d" | git mergetool file11 file12 >/dev/null 2>&1 ) && + ( yes "" | git mergetool file1 file2 spaced\ name subdir/file3 ) && + ( yes "" | git mergetool both ) && + ( yes "d" | git mergetool file11 file12 ) && ( yes "r" | git mergetool submod ) && rmdir submod && mv submod-movedaside submod && test "$(cat submod/bar)" = "branch1 submodule" && @@ -456,9 +456,9 @@ test_expect_success 'file vs modified submodule' ' git checkout -b test$test_count.b test$test_count && test_must_fail git merge master && test -n "$(git ls-files -u)" && - ( yes "" | git mergetool file1 file2 spaced\ name subdir/file3 >/dev/null 2>&1 ) && - ( yes "" | git mergetool both >/dev/null 2>&1 ) && - ( yes "d" | git mergetool file11 file12 >/dev/null 2>&1 ) && + ( yes "" | git mergetool file1 file2 spaced\ name subdir/file3 ) && + ( yes "" | git mergetool both ) && + ( yes "d" | git mergetool file11 file12 ) && ( yes "l" | git mergetool submod ) && git submodule update -N && test "$(cat submod)" = "not a submodule" && @@ -472,9 +472,9 @@ test_expect_success 'file vs modified submodule' ' git submodule update -N && test_must_fail git merge test$test_count && test -n "$(git ls-files -u)" && - ( yes "" | git mergetool file1 file2 spaced\ name subdir/file3 >/dev/null 2>&1 ) && - ( yes "" | git mergetool both >/dev/null 2>&1 ) && - ( yes "d" | git mergetool file11 file12 >/dev/null 2>&1 ) && + ( yes "" | git mergetool file1 file2 spaced\ name subdir/file3 ) && + ( yes "" | git mergetool both ) && + ( yes "d" | git mergetool file11 file12 ) && ( yes "r" | git mergetool submod ) && test -d submod.orig && git submodule update -N && @@ -488,9 +488,9 @@ test_expect_success 'file vs modified submodule' ' git submodule update -N && test_must_fail git merge test$test_count && test -n "$(git ls-files -u)" && - ( yes "" | git mergetool file1 file2 spaced\ name subdir/file3 >/dev/null 2>&1 ) && - ( yes "" | git mergetool both>/dev/null 2>&1 ) && - ( yes "d" | git mergetool file11 file12 >/dev/null 2>&1 ) && + ( yes "" | git mergetool file1 file2 spaced\ name subdir/file3 ) && + ( yes "" | git mergetool both ) && + ( yes "d" | git mergetool file11 file12 ) && ( yes "l" | git mergetool submod ) && test "$(cat submod/bar)" = "master submodule" && git submodule update -N && @@ -543,7 +543,7 @@ test_expect_success 'submodule in subdirectory' ' git add subdir/subdir_module && git commit -m "change submodule in subdirectory on test$test_count.b" && - test_must_fail git merge test$test_count.a >/dev/null 2>&1 && + test_must_fail git merge test$test_count.a && ( cd subdir && ( yes "l" | git mergetool subdir_module ) @@ -554,7 +554,7 @@ test_expect_success 'submodule in subdirectory' ' git reset --hard && git submodule update -N && - test_must_fail git merge test$test_count.a >/dev/null 2>&1 && + test_must_fail git merge test$test_count.a && ( yes "r" | git mergetool subdir/subdir_module ) && test "$(cat subdir/subdir_module/file15)" = "test$test_count.b" && git submodule update -N && @@ -641,7 +641,7 @@ test_expect_success 'filenames seen by tools start with ./' ' test_config mergetool.myecho.trustExitCode true && test_must_fail git merge master && git mergetool --no-prompt --tool myecho -- both >actual && - grep ^\./both_LOCAL_ actual >/dev/null + grep ^\./both_LOCAL_ actual ' test_lazy_prereq MKTEMP ' @@ -658,8 +658,8 @@ test_expect_success MKTEMP 'temporary filenames are used with mergetool.writeToT test_config mergetool.myecho.trustExitCode true && test_must_fail git merge master && git mergetool --no-prompt --tool myecho -- both >actual && - ! grep ^\./both_LOCAL_ actual >/dev/null && - grep /both_LOCAL_ actual >/dev/null + ! grep ^\./both_LOCAL_ actual && + grep /both_LOCAL_ actual ' test_expect_success 'diff.orderFile configuration is honored' ' From 57d93c1d2ce7deddf485e7c826278fef6815ec6b Mon Sep 17 00:00:00 2001 From: Denton Liu Date: Wed, 24 Apr 2019 15:46:59 -0700 Subject: [PATCH 02/66] t7610: add mergetool --gui tests In 063f2bdbf7 (mergetool: accept -g/--[no-]gui as arguments, 2018-10-24), mergetool was taught the --gui option but no tests were added to ensure that it was working properly. Add a test to ensure that it works. Signed-off-by: Denton Liu Signed-off-by: Junio C Hamano --- t/t7610-mergetool.sh | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/t/t7610-mergetool.sh b/t/t7610-mergetool.sh index 69711487dd..dad607e186 100755 --- a/t/t7610-mergetool.sh +++ b/t/t7610-mergetool.sh @@ -145,6 +145,28 @@ test_expect_success 'custom mergetool' ' git commit -m "branch1 resolved with mergetool" ' +test_expect_success 'gui mergetool' ' + test_config merge.guitool myguitool && + test_config mergetool.myguitool.cmd "(printf \"gui \" && cat \"\$REMOTE\") >\"\$MERGED\"" && + test_config mergetool.myguitool.trustExitCode true && + test_when_finished "git reset --hard" && + git checkout -b test$test_count branch1 && + git submodule update -N && + test_must_fail git merge master && + ( yes "" | git mergetool --gui both ) && + ( yes "" | git mergetool -g file1 file1 ) && + ( yes "" | git mergetool --gui file2 "spaced name" ) && + ( yes "" | git mergetool --gui subdir/file3 ) && + ( yes "d" | git mergetool --gui file11 ) && + ( yes "d" | git mergetool --gui file12 ) && + ( yes "l" | git mergetool --gui submod ) && + test "$(cat file1)" = "gui master updated" && + test "$(cat file2)" = "gui master new" && + test "$(cat subdir/file3)" = "gui master new sub" && + test "$(cat submod/bar)" = "branch1 submodule" && + git commit -m "branch1 resolved with mergetool" +' + test_expect_success 'mergetool crlf' ' test_when_finished "git reset --hard" && # This test_config line must go after the above reset line so that From 8e712ef6fc9742a9bf6f1826d81327f8da488041 Mon Sep 17 00:00:00 2001 From: Elijah Newren Date: Thu, 25 Apr 2019 07:58:54 -0700 Subject: [PATCH 03/66] Honor core.precomposeUnicode in more places MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit On Mac's HFS where git sets core.precomposeUnicode to true automatically by git init/clone, when a user creates a simple unicode refname (in NFC format) such as españa: $ git branch españa different commands would display the branch name differently. For example, git branch, git log --decorate, and git fast-export all used 65 73 70 61 c3 b1 61 (or "espa\xc3\xb1a") (NFC form) while show-ref would use 65 73 70 61 6e cc 83 61 (or "espan\xcc\x83a") (NFD form). A stress test for git filter-repo was tripped up by this inconsistency, though digging in I found that the problems could compound; for example, if the user ran $ git pack-refs --all and then tried to check out the branch, they would be met with: $ git checkout españa error: pathspec 'españa' did not match any file(s) known to git $ git checkout españa -- fatal: invalid reference: españa $ git branch españa * master Note that the user could run the `git branch` command first and copy and paste the `españa` portion of the output and still see the same two errors. Also, if the user added --no-prune to the pack-refs command, then they would see three branches: master, españa, and españa (those last two are NFC vs. NFD forms, even if they render the same). Further, if the user had the `españa` branch checked out before running `git pack-refs --all`, the user would be greeted with (note that I'm trimming trailing output with an ellipsis): $ git rev-parse HEAD fatal: ambiguous argument 'HEAD': unknown revision or path... $ git status On branch españa No commits yet... Or worse, if the user didn't check this stuff first, running `git commit` will create a new commit with all changes of all of history being squashed into it. In addition to pack-refs, one could also get into this state with upload-pack or anything that calls either pack-refs or upload-pack (e.g. gc or clone). Add code in a few places (pack-refs, show-ref, upload-pack) to check and honor the setting of core.precomposeUnicode to avoid these bugs. Signed-off-by: Elijah Newren Signed-off-by: Junio C Hamano --- builtin/pack-refs.c | 2 ++ builtin/show-ref.c | 3 +++ upload-pack.c | 2 ++ 3 files changed, 7 insertions(+) diff --git a/builtin/pack-refs.c b/builtin/pack-refs.c index f3353564f9..cfbd5c36c7 100644 --- a/builtin/pack-refs.c +++ b/builtin/pack-refs.c @@ -1,4 +1,5 @@ #include "builtin.h" +#include "config.h" #include "parse-options.h" #include "refs.h" #include "repository.h" @@ -16,6 +17,7 @@ int cmd_pack_refs(int argc, const char **argv, const char *prefix) OPT_BIT(0, "prune", &flags, N_("prune loose refs (default)"), PACK_REFS_PRUNE), OPT_END(), }; + git_config(git_default_config, NULL); if (parse_options(argc, argv, prefix, opts, pack_refs_usage, 0)) usage_with_options(pack_refs_usage, opts); return refs_pack_refs(get_main_ref_store(the_repository), flags); diff --git a/builtin/show-ref.c b/builtin/show-ref.c index ed888ffa48..07ff311b46 100644 --- a/builtin/show-ref.c +++ b/builtin/show-ref.c @@ -1,5 +1,6 @@ #include "builtin.h" #include "cache.h" +#include "config.h" #include "refs.h" #include "object-store.h" #include "object.h" @@ -182,6 +183,8 @@ static const struct option show_ref_options[] = { int cmd_show_ref(int argc, const char **argv, const char *prefix) { + git_config(git_default_config, NULL); + argc = parse_options(argc, argv, prefix, show_ref_options, show_ref_usage, 0); diff --git a/upload-pack.c b/upload-pack.c index 5e81f1ff24..c9b9176301 100644 --- a/upload-pack.c +++ b/upload-pack.c @@ -1041,6 +1041,8 @@ static int upload_pack_config(const char *var, const char *value, void *unused) allow_filter = git_config_bool(var, value); } else if (!strcmp("uploadpack.allowrefinwant", var)) { allow_ref_in_want = git_config_bool(var, value); + } else if (!strcmp("core.precomposeunicode", var)) { + precomposed_unicode = git_config_bool(var, value); } if (current_config_scope() != CONFIG_SCOPE_REPO) { From 960154b9c17afb276e12d0bec83513f3e46de565 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Mon, 6 May 2019 19:43:34 -0400 Subject: [PATCH 04/66] coccicheck: optionally batch spatch invocations In our "make coccicheck" rule, we currently feed each source file to its own individual invocation of spatch. This has a few downsides: - it repeats any overhead spatch has for starting up and reading the patch file - any included header files may get processed from multiple invocations. This is slow (we see the same header files multiple times) and may produce a resulting patch with repeated hunks (which cannot be applied without further cleanup) Ideally we'd just invoke a single instance of spatch per rule-file and feed it all source files. But spatch can be rather memory hungry when run in this way. I measured the peak RSS going from ~90MB for a single file to ~1900MB for all files. Multiplied by multiple rule files being processed at the same time (for "make -j"), this can make things slower or even cause them to fail (e.g., this is reported to happen on our Travis builds). Instead, let's provide a tunable knob. We'll leave the default at "1", but it can be cranked up to "999" for maximum CPU/memory tradeoff, or people can find points in between that serve their particular machines. Here are a few numbers running a single rule via: SIZES='1 4 16 999' RULE=contrib/coccinelle/object_id.cocci for i in $SIZES; do make clean /usr/bin/time -o $i.out --format='%e | %U | %S | %M' \ make $RULE.patch SPATCH_BATCH_SIZE=$i done for i in $SIZES; do printf '%4d | %s\n' $i "$(cat $i.out)" done which yields: 1 | 97.73 | 93.38 | 4.33 | 100128 4 | 52.80 | 51.14 | 1.69 | 135204 16 | 35.82 | 35.09 | 0.76 | 284124 999 | 23.30 | 23.13 | 0.20 | 1903852 The implementation is done with xargs, which should be widely available; it's in POSIX, we rely on it already in the test suite. And "coccicheck" is really a developer-only tool anyway, so it's not a big deal if obscure systems can't run it. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- Makefile | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/Makefile b/Makefile index 9f1b6e8926..daba958b8f 100644 --- a/Makefile +++ b/Makefile @@ -1174,8 +1174,10 @@ PTHREAD_CFLAGS = SPARSE_FLAGS ?= SP_EXTRA_FLAGS = -# For the 'coccicheck' target +# For the 'coccicheck' target; setting SPATCH_BATCH_SIZE higher will +# usually result in less CPU usage at the cost of higher peak memory. SPATCH_FLAGS = --all-includes --patch . +SPATCH_BATCH_SIZE = 1 include config.mak.uname -include config.mak.autogen @@ -2790,12 +2792,9 @@ endif %.cocci.patch: %.cocci $(COCCI_SOURCES) @echo ' ' SPATCH $<; \ - ret=0; \ - for f in $(COCCI_SOURCES); do \ - $(SPATCH) --sp-file $< $$f $(SPATCH_FLAGS) || \ - { ret=$$?; break; }; \ - done >$@+ 2>$@.log; \ - if test $$ret != 0; \ + if ! echo $(COCCI_SOURCES) | xargs -n $(SPATCH_BATCH_SIZE) \ + $(SPATCH) --sp-file $< $(SPATCH_FLAGS) \ + >$@+ 2>$@.log; \ then \ cat $@.log; \ exit 1; \ From 5c387428f10c27c24d3adb890cd466e2300518fa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nguy=E1=BB=85n=20Th=C3=A1i=20Ng=E1=BB=8Dc=20Duy?= Date: Mon, 29 Apr 2019 17:05:25 +0700 Subject: [PATCH 05/66] parse-options: don't emit "ambiguous option" for aliases MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Change the option parsing machinery so that e.g. "clone --recurs ..." doesn't error out because "clone" understands both "--recursive" and "--recurse-submodules" to mean the same thing. Initially "clone" just understood --recursive until the --recurses-submodules alias was added in ccdd3da652 ("clone: Add the --recurse-submodules option as alias for --recursive", 2010-11-04). Since bb62e0a99f ("clone: teach --recurse-submodules to optionally take a pathspec", 2017-03-17) the longer form has been promoted to the default. But due to the way the options parsing machinery works this resulted in the rather absurd situation of: $ git clone --recurs [...] error: ambiguous option: recurs (could be --recursive or --recurse-submodules) Add OPT_ALIAS() to express this link between two or more options and use it in git-clone. Multiple aliases of an option could be written as OPT_ALIAS(0, "alias1", "original-name"), OPT_ALIAS(0, "alias2", "original-name"), ... The current implementation is not exactly optimal in this case. But we can optimize it when it becomes a problem. So far we don't even have two aliases of any option. A big chunk of code is actually from Junio C Hamano. Signed-off-by: Nguyễn Thái Ngọc Duy Signed-off-by: Junio C Hamano --- builtin/clone.c | 5 +- parse-options.c | 143 ++++++++++++++++++++++++++++++++-- parse-options.h | 6 ++ t/helper/test-parse-options.c | 3 + t/t0040-parse-options.sh | 17 ++++ 5 files changed, 164 insertions(+), 10 deletions(-) diff --git a/builtin/clone.c b/builtin/clone.c index 50bde99618..703b7247ad 100644 --- a/builtin/clone.c +++ b/builtin/clone.c @@ -98,10 +98,7 @@ static struct option builtin_clone_options[] = { N_("don't use local hardlinks, always copy")), OPT_BOOL('s', "shared", &option_shared, N_("setup as shared repository")), - { OPTION_CALLBACK, 0, "recursive", &option_recurse_submodules, - N_("pathspec"), N_("initialize submodules in the clone"), - PARSE_OPT_OPTARG | PARSE_OPT_HIDDEN, recurse_submodules_cb, - (intptr_t)"." }, + OPT_ALIAS(0, "recursive", "recurse-submodules"), { OPTION_CALLBACK, 0, "recurse-submodules", &option_recurse_submodules, N_("pathspec"), N_("initialize submodules in the clone"), PARSE_OPT_OPTARG, recurse_submodules_cb, (intptr_t)"." }, diff --git a/parse-options.c b/parse-options.c index cb24f1aa8a..987e27cb91 100644 --- a/parse-options.c +++ b/parse-options.c @@ -261,6 +261,35 @@ static enum parse_opt_result parse_short_opt(struct parse_opt_ctx_t *p, return PARSE_OPT_UNKNOWN; } +static int has_string(const char *it, const char **array) +{ + while (*array) + if (!strcmp(it, *(array++))) + return 1; + return 0; +} + +static int is_alias(struct parse_opt_ctx_t *ctx, + const struct option *one_opt, + const struct option *another_opt) +{ + const char **group; + + if (!ctx->alias_groups) + return 0; + + if (!one_opt->long_name || !another_opt->long_name) + return 0; + + for (group = ctx->alias_groups; *group; group += 3) { + /* it and other are from the same family? */ + if (has_string(one_opt->long_name, group) && + has_string(another_opt->long_name, group)) + return 1; + } + return 0; +} + static enum parse_opt_result parse_long_opt( struct parse_opt_ctx_t *p, const char *arg, const struct option *options) @@ -298,7 +327,8 @@ again: if (!(p->flags & PARSE_OPT_KEEP_UNKNOWN) && !strncmp(long_name, arg, arg_end - arg)) { is_abbreviated: - if (abbrev_option) { + if (abbrev_option && + !is_alias(p, abbrev_option, options)) { /* * If this is abbreviated, it is * ambiguous. So when there is no @@ -447,6 +477,10 @@ static void parse_options_check(const struct option *opts) if (opts->callback) BUG("OPTION_LOWLEVEL_CALLBACK needs no high level callback"); break; + case OPTION_ALIAS: + BUG("OPT_ALIAS() should not remain at this point. " + "Are you using parse_options_step() directly?\n" + "That case is not supported yet."); default: ; /* ok. (usually accepts an argument) */ } @@ -458,11 +492,10 @@ static void parse_options_check(const struct option *opts) exit(128); } -void parse_options_start(struct parse_opt_ctx_t *ctx, - int argc, const char **argv, const char *prefix, - const struct option *options, int flags) +static void parse_options_start_1(struct parse_opt_ctx_t *ctx, + int argc, const char **argv, const char *prefix, + const struct option *options, int flags) { - memset(ctx, 0, sizeof(*ctx)); ctx->argc = argc; ctx->argv = argv; if (!(flags & PARSE_OPT_ONE_SHOT)) { @@ -484,6 +517,14 @@ void parse_options_start(struct parse_opt_ctx_t *ctx, parse_options_check(options); } +void parse_options_start(struct parse_opt_ctx_t *ctx, + int argc, const char **argv, const char *prefix, + const struct option *options, int flags) +{ + memset(ctx, 0, sizeof(*ctx)); + parse_options_start_1(ctx, argc, argv, prefix, options, flags); +} + static void show_negated_gitcomp(const struct option *opts, int nr_noopts) { int printed_dashdash = 0; @@ -575,6 +616,83 @@ static int show_gitcomp(const struct option *opts) return PARSE_OPT_COMPLETE; } +/* + * Scan and may produce a new option[] array, which should be used + * instead of the original 'options'. + * + * Right now this is only used to preprocess and substitue + * OPTION_ALIAS. + */ +static struct option *preprocess_options(struct parse_opt_ctx_t *ctx, + const struct option *options) +{ + struct option *newopt; + int i, nr, alias; + int nr_aliases = 0; + + for (nr = 0; options[nr].type != OPTION_END; nr++) { + if (options[nr].type == OPTION_ALIAS) + nr_aliases++; + } + + if (!nr_aliases) + return NULL; + + ALLOC_ARRAY(newopt, nr + 1); + COPY_ARRAY(newopt, options, nr + 1); + + /* each alias has two string pointers and NULL */ + CALLOC_ARRAY(ctx->alias_groups, 3 * (nr_aliases + 1)); + + for (alias = 0, i = 0; i < nr; i++) { + int short_name; + const char *long_name; + const char *source; + int j; + + if (newopt[i].type != OPTION_ALIAS) + continue; + + short_name = newopt[i].short_name; + long_name = newopt[i].long_name; + source = newopt[i].value; + + if (!long_name) + BUG("An alias must have long option name"); + + for (j = 0; j < nr; j++) { + const char *name = options[j].long_name; + + if (!name || strcmp(name, source)) + continue; + + if (options[j].type == OPTION_ALIAS) + BUG("No please. Nested aliases are not supported."); + + /* + * NEEDSWORK: this is a bit inconsistent because + * usage_with_options() on the original options[] will print + * help string as "alias of %s" but "git cmd -h" will + * print the original help string. + */ + memcpy(newopt + i, options + j, sizeof(*newopt)); + newopt[i].short_name = short_name; + newopt[i].long_name = long_name; + break; + } + + if (j == nr) + BUG("could not find source option '%s' of alias '%s'", + source, newopt[i].long_name); + ctx->alias_groups[alias * 3 + 0] = newopt[i].long_name; + ctx->alias_groups[alias * 3 + 1] = options[j].long_name; + ctx->alias_groups[alias * 3 + 2] = NULL; + alias++; + } + + return newopt; +} + static int usage_with_options_internal(struct parse_opt_ctx_t *, const char * const *, const struct option *, int, int); @@ -714,11 +832,16 @@ int parse_options(int argc, const char **argv, const char *prefix, int flags) { struct parse_opt_ctx_t ctx; + struct option *real_options; disallow_abbreviated_options = git_env_bool("GIT_TEST_DISALLOW_ABBREVIATED_OPTIONS", 0); - parse_options_start(&ctx, argc, argv, prefix, options, flags); + memset(&ctx, 0, sizeof(ctx)); + real_options = preprocess_options(&ctx, options); + if (real_options) + options = real_options; + parse_options_start_1(&ctx, argc, argv, prefix, options, flags); switch (parse_options_step(&ctx, options, usagestr)) { case PARSE_OPT_HELP: case PARSE_OPT_ERROR: @@ -741,6 +864,8 @@ int parse_options(int argc, const char **argv, const char *prefix, } precompose_argv(argc, argv); + free(real_options); + free(ctx.alias_groups); return parse_options_end(&ctx); } @@ -835,6 +960,12 @@ static int usage_with_options_internal(struct parse_opt_ctx_t *ctx, fputc('\n', outfile); pad = USAGE_OPTS_WIDTH; } + if (opts->type == OPTION_ALIAS) { + fprintf(outfile, "%*s", pad + USAGE_GAP, ""); + fprintf_ln(outfile, _("alias of --%s"), + (const char *)opts->value); + continue; + } fprintf(outfile, "%*s%s\n", pad + USAGE_GAP, "", _(opts->help)); } fputc('\n', outfile); diff --git a/parse-options.h b/parse-options.h index cc9230adac..bb49efc2da 100644 --- a/parse-options.h +++ b/parse-options.h @@ -7,6 +7,7 @@ enum parse_opt_type { OPTION_ARGUMENT, OPTION_GROUP, OPTION_NUMBER, + OPTION_ALIAS, /* options with no arguments */ OPTION_BIT, OPTION_NEGBIT, @@ -183,6 +184,9 @@ struct option { N_("no-op (backward compatibility)"), \ PARSE_OPT_HIDDEN | PARSE_OPT_NOARG, parse_opt_noop_cb } +#define OPT_ALIAS(s, l, source_long_name) \ + { OPTION_ALIAS, (s), (l), (source_long_name) } + /* * parse_options() will filter out the processed options and leave the * non-option arguments in argv[]. argv0 is assumed program name and @@ -258,6 +262,8 @@ struct parse_opt_ctx_t { const char *opt; int flags; const char *prefix; + const char **alias_groups; /* must be in groups of 3 elements! */ + struct option *updated_options; }; void parse_options_start(struct parse_opt_ctx_t *ctx, diff --git a/t/helper/test-parse-options.c b/t/helper/test-parse-options.c index 2232b2f79e..af82db06ac 100644 --- a/t/helper/test-parse-options.c +++ b/t/helper/test-parse-options.c @@ -149,6 +149,9 @@ int cmd__parse_options(int argc, const char **argv) OPT_CALLBACK(0, "expect", &expect, "string", "expected output in the variable dump", collect_expect), + OPT_GROUP("Alias"), + OPT_STRING('A', "alias-source", &string, "string", "get a string"), + OPT_ALIAS('Z', "alias-target", "alias-source"), OPT_END(), }; int i; diff --git a/t/t0040-parse-options.sh b/t/t0040-parse-options.sh index 800b3ea5f5..cebc77fab0 100755 --- a/t/t0040-parse-options.sh +++ b/t/t0040-parse-options.sh @@ -48,6 +48,12 @@ Standard options -q, --quiet be quiet --expect expected output in the variable dump +Alias + -A, --alias-source + get a string + -Z, --alias-target + get a string + EOF test_expect_success 'test help' ' @@ -224,6 +230,17 @@ test_expect_success 'non ambiguous option (after two options it abbreviates)' ' test-tool parse-options --expect="string: 123" --st 123 ' +test_expect_success 'Alias options do not contribute to abbreviation' ' + test-tool parse-options --alias-source 123 >output && + grep "^string: 123" output && + test-tool parse-options --alias-target 123 >output && + grep "^string: 123" output && + test_must_fail test-tool parse-options --alias && + GIT_TEST_DISALLOW_ABBREVIATED_OPTIONS=false \ + test-tool parse-options --alias 123 >output && + grep "^string: 123" output +' + cat >typo.err <<\EOF error: did you mean `--boolean` (with two dashes ?) EOF From 27434bf08c6c7c065e6323e8aa3f285d8f345e83 Mon Sep 17 00:00:00 2001 From: Denton Liu Date: Sat, 27 Apr 2019 05:02:19 -0700 Subject: [PATCH 06/66] t2018: cleanup in current test Before, in t2018, if do_checkout failed to create `branch2`, the next test-case would run `git branch -D branch2` but then fail because it was expecting `branch2` to exist, even though it doesn't. As a result, an early failure could cause a cascading failure of tests. Make test-case responsible for cleaning up their own branches so that future tests can start with a sane environment. Signed-off-by: Denton Liu Signed-off-by: Junio C Hamano --- t/t2018-checkout-branch.sh | 43 +++++++++++++++----------------------- 1 file changed, 17 insertions(+), 26 deletions(-) diff --git a/t/t2018-checkout-branch.sh b/t/t2018-checkout-branch.sh index c5014ad9a6..f1c7023e1a 100755 --- a/t/t2018-checkout-branch.sh +++ b/t/t2018-checkout-branch.sh @@ -60,38 +60,40 @@ test_expect_success 'setup' ' ' test_expect_success 'checkout -b to a new branch, set to HEAD' ' + test_when_finished " + git checkout branch1 && + test_might_fail git branch -D branch2" && do_checkout branch2 ' test_expect_success 'checkout -b to a new branch, set to an explicit ref' ' - git checkout branch1 && - git branch -D branch2 && - + test_when_finished " + git checkout branch1 && + test_might_fail git branch -D branch2" && do_checkout branch2 $HEAD1 ' test_expect_success 'checkout -b to a new branch with unmergeable changes fails' ' - git checkout branch1 && - - # clean up from previous test - git branch -D branch2 && - setup_dirty_unmergeable && test_must_fail do_checkout branch2 $HEAD1 && test_dirty_unmergeable ' test_expect_success 'checkout -f -b to a new branch with unmergeable changes discards changes' ' + test_when_finished " + git checkout branch1 && + test_might_fail git branch -D branch2" && + # still dirty and on branch1 do_checkout branch2 $HEAD1 "-f -b" && test_must_fail test_dirty_unmergeable ' test_expect_success 'checkout -b to a new branch preserves mergeable changes' ' - git checkout branch1 && - - # clean up from previous test - git branch -D branch2 && + test_when_finished " + git reset --hard && + git checkout branch1 && + test_might_fail git branch -D branch2" && setup_dirty_mergeable && do_checkout branch2 $HEAD1 && @@ -99,27 +101,18 @@ test_expect_success 'checkout -b to a new branch preserves mergeable changes' ' ' test_expect_success 'checkout -f -b to a new branch with mergeable changes discards changes' ' - # clean up from previous test - git reset --hard && - - git checkout branch1 && - - # clean up from previous test - git branch -D branch2 && - + test_when_finished git reset --hard HEAD && setup_dirty_mergeable && do_checkout branch2 $HEAD1 "-f -b" && test_must_fail test_dirty_mergeable ' test_expect_success 'checkout -b to an existing branch fails' ' - git reset --hard HEAD && - + test_when_finished git reset --hard HEAD && test_must_fail do_checkout branch2 $HEAD2 ' test_expect_success 'checkout -b to @{-1} fails with the right branch name' ' - git reset --hard HEAD && git checkout branch1 && git checkout branch2 && echo >expect "fatal: A branch named '\''branch1'\'' already exists." && @@ -160,6 +153,7 @@ test_expect_success 'checkout -f -B to an existing branch with unmergeable chang ' test_expect_success 'checkout -B to an existing branch preserves mergeable changes' ' + test_when_finished git reset --hard && git checkout branch1 && setup_dirty_mergeable && @@ -168,9 +162,6 @@ test_expect_success 'checkout -B to an existing branch preserves mergeable chang ' test_expect_success 'checkout -f -B to an existing branch with mergeable changes discards changes' ' - # clean up from previous test - git reset --hard && - git checkout branch1 && setup_dirty_mergeable && From e3d6539d58238f046ff955330f6e10c447150164 Mon Sep 17 00:00:00 2001 From: Denton Liu Date: Sat, 27 Apr 2019 05:02:22 -0700 Subject: [PATCH 07/66] branch: make create_branch accept a merge base rev When we ran something like $ git checkout -b test master... it would fail with the message fatal: Not a valid object name: 'master...'. This was caused by the call to `create_branch` where `start_name` is expected to be a valid rev. However, git-checkout allows the branch to be a valid _merge base_ rev (i.e. with a "...") so it was possible for an invalid rev to be passed in. Make `create_branch` accept a merge base rev so that this case does not error out. As a side-effect, teach git-branch how to handle merge base revs as well. Helped-by: Junio C Hamano Signed-off-by: Denton Liu Signed-off-by: Junio C Hamano --- Documentation/git-branch.txt | 6 +++++- Documentation/git-checkout.txt | 4 ++++ branch.c | 2 +- t/t2018-checkout-branch.sh | 13 +++++++++++++ t/t3200-branch.sh | 14 ++++++++++---- 5 files changed, 33 insertions(+), 6 deletions(-) diff --git a/Documentation/git-branch.txt b/Documentation/git-branch.txt index 3bd83a7cbd..8719e4cab9 100644 --- a/Documentation/git-branch.txt +++ b/Documentation/git-branch.txt @@ -45,7 +45,11 @@ argument is missing it defaults to `HEAD` (i.e. the tip of the current branch). The command's second form creates a new branch head named -which points to the current `HEAD`, or if given. +which points to the current `HEAD`, or if given. As a +special case, for , you may use `"A...B"` as a shortcut for +the merge base of `A` and `B` if there is exactly one merge base. You +can leave out at most one of `A` and `B`, in which case it defaults to +`HEAD`. Note that this will create the new branch, but it will not switch the working tree to it; use "git checkout " to switch to the diff --git a/Documentation/git-checkout.txt b/Documentation/git-checkout.txt index 8c3d4128c2..d32f2eb611 100644 --- a/Documentation/git-checkout.txt +++ b/Documentation/git-checkout.txt @@ -301,6 +301,10 @@ leave out at most one of `A` and `B`, in which case it defaults to `HEAD`. :: The name of a commit at which to start the new branch; see linkgit:git-branch[1] for details. Defaults to HEAD. ++ +As a special case, you may use `"A...B"` as a shortcut for the +merge base of `A` and `B` if there is exactly one merge base. You can +leave out at most one of `A` and `B`, in which case it defaults to `HEAD`. :: Tree to checkout from (when paths are given). If not specified, diff --git a/branch.c b/branch.c index 28b81a7e02..a84c8aaca2 100644 --- a/branch.c +++ b/branch.c @@ -268,7 +268,7 @@ void create_branch(struct repository *r, } real_ref = NULL; - if (get_oid(start_name, &oid)) { + if (get_oid_mb(start_name, &oid)) { if (explicit_tracking) { if (advice_set_upstream_failure) { error(_(upstream_missing), start_name); diff --git a/t/t2018-checkout-branch.sh b/t/t2018-checkout-branch.sh index f1c7023e1a..822381dd9d 100755 --- a/t/t2018-checkout-branch.sh +++ b/t/t2018-checkout-branch.sh @@ -66,6 +66,13 @@ test_expect_success 'checkout -b to a new branch, set to HEAD' ' do_checkout branch2 ' +test_expect_success 'checkout -b to a merge base' ' + test_when_finished " + git checkout branch1 && + test_might_fail git branch -D branch2" && + git checkout -b branch2 branch1... +' + test_expect_success 'checkout -b to a new branch, set to an explicit ref' ' test_when_finished " git checkout branch1 && @@ -126,6 +133,12 @@ test_expect_success 'checkout -B to an existing branch resets branch to HEAD' ' do_checkout branch2 "" -B ' +test_expect_success 'checkout -B to a merge base' ' + git checkout branch1 && + + git checkout -B branch2 branch1... +' + test_expect_success 'checkout -B to an existing branch from detached HEAD resets branch to HEAD' ' git checkout $(git rev-parse --verify HEAD) && diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh index 478b82cf9b..acb16b62dd 100755 --- a/t/t3200-branch.sh +++ b/t/t3200-branch.sh @@ -42,6 +42,10 @@ test_expect_success 'git branch a/b/c should create a branch' ' git branch a/b/c && test_path_is_file .git/refs/heads/a/b/c ' +test_expect_success 'git branch mb master... should create a branch' ' + git branch mb master... && test_path_is_file .git/refs/heads/mb +' + test_expect_success 'git branch HEAD should fail' ' test_must_fail git branch HEAD ' @@ -292,8 +296,8 @@ test_expect_success 'git branch --list -v with --abbrev' ' test_expect_success 'git branch --column' ' COLUMNS=81 git branch --column=column >actual && cat >expected <<\EOF && - a/b/c bam foo l * master n o/p r - abc bar j/k m/m master2 o/o q + a/b/c bam foo l * master mb o/o q + abc bar j/k m/m master2 n o/p r EOF test_cmp expected actual ' @@ -315,6 +319,7 @@ test_expect_success 'git branch --column with an extremely long branch name' ' m/m * master master2 + mb n o/o o/p @@ -332,8 +337,8 @@ test_expect_success 'git branch with column.*' ' git config --unset column.branch && git config --unset column.ui && cat >expected <<\EOF && - a/b/c bam foo l * master n o/p r - abc bar j/k m/m master2 o/o q + a/b/c bam foo l * master mb o/o q + abc bar j/k m/m master2 n o/p r EOF test_cmp expected actual ' @@ -357,6 +362,7 @@ test_expect_success 'git branch -v with column.ui ignored' ' m/m * master master2 + mb n o/o o/p From 64404a24cf60761baba0e04a337c419f0e86c0f9 Mon Sep 17 00:00:00 2001 From: Derrick Stolee Date: Mon, 29 Apr 2019 09:18:55 -0700 Subject: [PATCH 08/66] midx: pass a repository pointer Much of the multi-pack-index code focuses on the multi_pack_index struct, and so we only pass a pointer to the current one. However, we will insert a dependency on the packed_git linked list in a future change, so we will need a repository reference. Inserting these parameters is a significant enough change to split out. Signed-off-by: Derrick Stolee Signed-off-by: Junio C Hamano --- builtin/multi-pack-index.c | 2 +- builtin/pack-objects.c | 2 +- midx.c | 22 ++++++++++++++-------- midx.h | 7 ++++--- packfile.c | 4 ++-- 5 files changed, 22 insertions(+), 15 deletions(-) diff --git a/builtin/multi-pack-index.c b/builtin/multi-pack-index.c index ae6e476ad5..72dfd3dadc 100644 --- a/builtin/multi-pack-index.c +++ b/builtin/multi-pack-index.c @@ -46,7 +46,7 @@ int cmd_multi_pack_index(int argc, const char **argv, if (!strcmp(argv[0], "write")) return write_midx_file(opts.object_dir); if (!strcmp(argv[0], "verify")) - return verify_midx_file(opts.object_dir); + return verify_midx_file(the_repository, opts.object_dir); die(_("unrecognized verb: %s"), argv[0]); } diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c index 2d9a3bdc9d..e606b9ef03 100644 --- a/builtin/pack-objects.c +++ b/builtin/pack-objects.c @@ -1078,7 +1078,7 @@ static int want_object_in_pack(const struct object_id *oid, for (m = get_multi_pack_index(the_repository); m; m = m->next) { struct pack_entry e; - if (fill_midx_entry(oid, &e, m)) { + if (fill_midx_entry(the_repository, oid, &e, m)) { struct packed_git *p = e.p; off_t offset; diff --git a/midx.c b/midx.c index d5d2e9522f..8b8faec35a 100644 --- a/midx.c +++ b/midx.c @@ -201,7 +201,7 @@ void close_midx(struct multi_pack_index *m) FREE_AND_NULL(m->pack_names); } -int prepare_midx_pack(struct multi_pack_index *m, uint32_t pack_int_id) +int prepare_midx_pack(struct repository *r, struct multi_pack_index *m, uint32_t pack_int_id) { struct strbuf pack_name = STRBUF_INIT; @@ -261,7 +261,10 @@ static uint32_t nth_midxed_pack_int_id(struct multi_pack_index *m, uint32_t pos) return get_be32(m->chunk_object_offsets + pos * MIDX_CHUNK_OFFSET_WIDTH); } -static int nth_midxed_pack_entry(struct multi_pack_index *m, struct pack_entry *e, uint32_t pos) +static int nth_midxed_pack_entry(struct repository *r, + struct multi_pack_index *m, + struct pack_entry *e, + uint32_t pos) { uint32_t pack_int_id; struct packed_git *p; @@ -271,7 +274,7 @@ static int nth_midxed_pack_entry(struct multi_pack_index *m, struct pack_entry * pack_int_id = nth_midxed_pack_int_id(m, pos); - if (prepare_midx_pack(m, pack_int_id)) + if (prepare_midx_pack(r, m, pack_int_id)) die(_("error preparing packfile from multi-pack-index")); p = m->packs[pack_int_id]; @@ -301,14 +304,17 @@ static int nth_midxed_pack_entry(struct multi_pack_index *m, struct pack_entry * return 1; } -int fill_midx_entry(const struct object_id *oid, struct pack_entry *e, struct multi_pack_index *m) +int fill_midx_entry(struct repository * r, + const struct object_id *oid, + struct pack_entry *e, + struct multi_pack_index *m) { uint32_t pos; if (!bsearch_midx(oid, m, &pos)) return 0; - return nth_midxed_pack_entry(m, e, pos); + return nth_midxed_pack_entry(r, m, e, pos); } /* Match "foo.idx" against either "foo.pack" _or_ "foo.idx". */ @@ -1020,7 +1026,7 @@ static int compare_pair_pos_vs_id(const void *_a, const void *_b) display_progress(progress, _n); \ } while (0) -int verify_midx_file(const char *object_dir) +int verify_midx_file(struct repository *r, const char *object_dir) { struct pair_pos_vs_id *pairs = NULL; uint32_t i; @@ -1034,7 +1040,7 @@ int verify_midx_file(const char *object_dir) progress = start_progress(_("Looking for referenced packfiles"), m->num_packs); for (i = 0; i < m->num_packs; i++) { - if (prepare_midx_pack(m, i)) + if (prepare_midx_pack(r, m, i)) midx_report("failed to load pack in position %d", i); display_progress(progress, i + 1); @@ -1099,7 +1105,7 @@ int verify_midx_file(const char *object_dir) nth_midxed_object_oid(&oid, m, pairs[i].pos); - if (!fill_midx_entry(&oid, &e, m)) { + if (!fill_midx_entry(r, &oid, &e, m)) { midx_report(_("failed to load pack entry for oid[%d] = %s"), pairs[i].pos, oid_to_hex(&oid)); continue; diff --git a/midx.h b/midx.h index 26dd042d63..3eb29731f2 100644 --- a/midx.h +++ b/midx.h @@ -5,6 +5,7 @@ struct object_id; struct pack_entry; +struct repository; #define GIT_TEST_MULTI_PACK_INDEX "GIT_TEST_MULTI_PACK_INDEX" @@ -37,18 +38,18 @@ struct multi_pack_index { }; struct multi_pack_index *load_multi_pack_index(const char *object_dir, int local); -int prepare_midx_pack(struct multi_pack_index *m, uint32_t pack_int_id); +int prepare_midx_pack(struct repository *r, struct multi_pack_index *m, uint32_t pack_int_id); int bsearch_midx(const struct object_id *oid, struct multi_pack_index *m, uint32_t *result); struct object_id *nth_midxed_object_oid(struct object_id *oid, struct multi_pack_index *m, uint32_t n); -int fill_midx_entry(const struct object_id *oid, struct pack_entry *e, struct multi_pack_index *m); +int fill_midx_entry(struct repository *r, const struct object_id *oid, struct pack_entry *e, struct multi_pack_index *m); int midx_contains_pack(struct multi_pack_index *m, const char *idx_or_pack_name); int prepare_multi_pack_index_one(struct repository *r, const char *object_dir, int local); int write_midx_file(const char *object_dir); void clear_midx_file(struct repository *r); -int verify_midx_file(const char *object_dir); +int verify_midx_file(struct repository *r, const char *object_dir); void close_midx(struct multi_pack_index *m); diff --git a/packfile.c b/packfile.c index cdf6b6ec34..7b94a14726 100644 --- a/packfile.c +++ b/packfile.c @@ -1035,7 +1035,7 @@ struct packed_git *get_all_packs(struct repository *r) for (m = r->objects->multi_pack_index; m; m = m->next) { uint32_t i; for (i = 0; i < m->num_packs; i++) { - if (!prepare_midx_pack(m, i)) { + if (!prepare_midx_pack(r, m, i)) { m->packs[i]->next = p; p = m->packs[i]; } @@ -1998,7 +1998,7 @@ int find_pack_entry(struct repository *r, const struct object_id *oid, struct pa return 0; for (m = r->objects->multi_pack_index; m; m = m->next) { - if (fill_midx_entry(oid, e, m)) + if (fill_midx_entry(r, oid, e, m)) return 1; } From af96fe3392fb078cb5447bcb94f2ed8d79d0a4a8 Mon Sep 17 00:00:00 2001 From: Derrick Stolee Date: Mon, 29 Apr 2019 09:18:56 -0700 Subject: [PATCH 09/66] midx: add packs to packed_git linked list The multi-pack-index allows searching for objects across multiple packs using one object list. The original design gains many of these performance benefits by keeping the packs in the multi-pack-index out of the packed_git list. Unfortunately, this has one major drawback. If the multi-pack-index covers thousands of packs, and a command loads many of those packs, then we can hit the limit for open file descriptors. The close_one_pack() method is used to limit this resource, but it only looks at the packed_git list, and uses an LRU cache to prevent thrashing. Instead of complicating this close_one_pack() logic to include direct references to the multi-pack-index, simply add the packs opened by the multi-pack-index to the packed_git list. This immediately solves the file-descriptor limit problem, but requires some extra steps to avoid performance issues or other problems: 1. Create a multi_pack_index bit in the packed_git struct that is one if and only if the pack was loaded from a multi-pack-index. 2. Skip packs with the multi_pack_index bit when doing object lookups and abbreviations. These algorithms already check the multi-pack-index before the packed_git struct. This has a very small performance hit, as we need to walk more packed_git structs. This is acceptable, since these operations run binary search on the other packs, so this walk-and-ignore logic is very fast by comparison. 3. When closing a multi-pack-index file, do not close its packs, as those packs will be closed using close_all_packs(). In some cases, such as 'git repack', we run 'close_midx()' without also closing the packs, so we need to un-set the multi_pack_index bit in those packs. This is necessary, and caught by running t6501-freshen-objects.sh with GIT_TEST_MULTI_PACK_INDEX=1. To manually test this change, I inserted trace2 logging into close_pack_fd() and set pack_max_fds to 10, then ran 'git rev-list --all --objects' on a copy of the Git repo with 300+ pack-files and a multi-pack-index. The logs verified the packs are closed as we read them beyond the file descriptor limit. Signed-off-by: Derrick Stolee Signed-off-by: Junio C Hamano --- midx.c | 20 ++++++++++++++------ object-store.h | 9 ++------- packfile.c | 28 ++++++++-------------------- sha1-name.c | 6 ++++++ 4 files changed, 30 insertions(+), 33 deletions(-) diff --git a/midx.c b/midx.c index 8b8faec35a..e7e1fe4d65 100644 --- a/midx.c +++ b/midx.c @@ -192,10 +192,8 @@ void close_midx(struct multi_pack_index *m) m->fd = -1; for (i = 0; i < m->num_packs; i++) { - if (m->packs[i]) { - close_pack(m->packs[i]); - free(m->packs[i]); - } + if (m->packs[i]) + m->packs[i]->multi_pack_index = 0; } FREE_AND_NULL(m->packs); FREE_AND_NULL(m->pack_names); @@ -204,6 +202,7 @@ void close_midx(struct multi_pack_index *m) int prepare_midx_pack(struct repository *r, struct multi_pack_index *m, uint32_t pack_int_id) { struct strbuf pack_name = STRBUF_INIT; + struct packed_git *p; if (pack_int_id >= m->num_packs) die(_("bad pack-int-id: %u (%u total packs)"), @@ -215,9 +214,18 @@ int prepare_midx_pack(struct repository *r, struct multi_pack_index *m, uint32_t strbuf_addf(&pack_name, "%s/pack/%s", m->object_dir, m->pack_names[pack_int_id]); - m->packs[pack_int_id] = add_packed_git(pack_name.buf, pack_name.len, m->local); + p = add_packed_git(pack_name.buf, pack_name.len, m->local); strbuf_release(&pack_name); - return !m->packs[pack_int_id]; + + if (!p) + return 1; + + p->multi_pack_index = 1; + m->packs[pack_int_id] = p; + install_packed_git(r, p); + list_add_tail(&p->mru, &r->objects->packed_git_mru); + + return 0; } int bsearch_midx(const struct object_id *oid, struct multi_pack_index *m, uint32_t *result) diff --git a/object-store.h b/object-store.h index b086f5ecdb..7acbc7fffe 100644 --- a/object-store.h +++ b/object-store.h @@ -76,7 +76,8 @@ struct packed_git { pack_keep_in_core:1, freshened:1, do_not_close:1, - pack_promisor:1; + pack_promisor:1, + multi_pack_index:1; unsigned char hash[GIT_MAX_RAWSZ]; struct revindex_entry *revindex; /* something like ".git/objects/pack/xxxxx.pack" */ @@ -128,12 +129,6 @@ struct raw_object_store { /* A most-recently-used ordered version of the packed_git list. */ struct list_head packed_git_mru; - /* - * A linked list containing all packfiles, starting with those - * contained in the multi_pack_index. - */ - struct packed_git *all_packs; - /* * A fast, rough count of the number of objects in the repository. * These two fields are not meant for direct access. Use diff --git a/packfile.c b/packfile.c index 7b94a14726..060de420d1 100644 --- a/packfile.c +++ b/packfile.c @@ -994,8 +994,6 @@ static void prepare_packed_git(struct repository *r) } rearrange_packed_git(r); - r->objects->all_packs = NULL; - prepare_packed_git_mru(r); r->objects->packed_git_initialized = 1; } @@ -1026,26 +1024,16 @@ struct multi_pack_index *get_multi_pack_index(struct repository *r) struct packed_git *get_all_packs(struct repository *r) { + struct multi_pack_index *m; + prepare_packed_git(r); - - if (!r->objects->all_packs) { - struct packed_git *p = r->objects->packed_git; - struct multi_pack_index *m; - - for (m = r->objects->multi_pack_index; m; m = m->next) { - uint32_t i; - for (i = 0; i < m->num_packs; i++) { - if (!prepare_midx_pack(r, m, i)) { - m->packs[i]->next = p; - p = m->packs[i]; - } - } - } - - r->objects->all_packs = p; + for (m = r->objects->multi_pack_index; m; m = m->next) { + uint32_t i; + for (i = 0; i < m->num_packs; i++) + prepare_midx_pack(r, m, i); } - return r->objects->all_packs; + return r->objects->packed_git; } struct list_head *get_packed_git_mru(struct repository *r) @@ -2004,7 +1992,7 @@ int find_pack_entry(struct repository *r, const struct object_id *oid, struct pa list_for_each(pos, &r->objects->packed_git_mru) { struct packed_git *p = list_entry(pos, struct packed_git, mru); - if (fill_pack_entry(oid, e, p)) { + if (!p->multi_pack_index && fill_pack_entry(oid, e, p)) { list_move(&p->mru, &r->objects->packed_git_mru); return 1; } diff --git a/sha1-name.c b/sha1-name.c index 07c71a7567..42ac1c5bb6 100644 --- a/sha1-name.c +++ b/sha1-name.c @@ -157,6 +157,9 @@ static void unique_in_pack(struct packed_git *p, uint32_t num, i, first = 0; const struct object_id *current = NULL; + if (p->multi_pack_index) + return; + if (open_pack_index(p) || !p->num_objects) return; @@ -589,6 +592,9 @@ static void find_abbrev_len_for_pack(struct packed_git *p, struct object_id oid; const struct object_id *mad_oid; + if (p->multi_pack_index) + return; + if (open_pack_index(p) || !p->num_objects) return; From 98552f252ad4a86573d75665fc403f5e66056bb2 Mon Sep 17 00:00:00 2001 From: Josh Steadmon Date: Mon, 6 May 2019 14:36:58 -0700 Subject: [PATCH 10/66] commit-graph: fix memory leak Free the commit graph when verify_commit_graph_lite() reports an error. Credit to OSS-Fuzz for finding this leak. Signed-off-by: Josh Steadmon Signed-off-by: Junio C Hamano --- commit-graph.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/commit-graph.c b/commit-graph.c index 66865acbd7..4bce70d35c 100644 --- a/commit-graph.c +++ b/commit-graph.c @@ -267,8 +267,10 @@ struct commit_graph *parse_commit_graph(void *graph_map, int fd, last_chunk_offset = chunk_offset; } - if (verify_commit_graph_lite(graph)) + if (verify_commit_graph_lite(graph)) { + free(graph); return NULL; + } return graph; } From ce4c7bfc90d93bdbf979d756c03c52d2d6b286d4 Mon Sep 17 00:00:00 2001 From: Boxuan Li Date: Sun, 5 May 2019 16:16:33 +0800 Subject: [PATCH 11/66] t4253-am-keep-cr-dos: avoid using pipes The exit code of the upstream in a pipe is ignored thus we should avoid using it. By writing out the output of the git command to a file, we can test the exit codes of both the commands. Signed-off-by: Boxuan Li Signed-off-by: Junio C Hamano --- t/t4253-am-keep-cr-dos.sh | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/t/t4253-am-keep-cr-dos.sh b/t/t4253-am-keep-cr-dos.sh index 553fe3e88e..6e1b73ec3a 100755 --- a/t/t4253-am-keep-cr-dos.sh +++ b/t/t4253-am-keep-cr-dos.sh @@ -51,14 +51,16 @@ test_expect_success 'am with dos files without --keep-cr' ' test_expect_success 'am with dos files with --keep-cr' ' git checkout -b dosfiles-keep-cr initial && - git format-patch -k --stdout initial..master | git am --keep-cr -k -3 && + git format-patch -k --stdout initial..master >output && + git am --keep-cr -k -3 output && git diff --exit-code master ' test_expect_success 'am with dos files config am.keepcr' ' git config am.keepcr 1 && git checkout -b dosfiles-conf-keepcr initial && - git format-patch -k --stdout initial..master | git am -k -3 && + git format-patch -k --stdout initial..master >output && + git am -k -3 output && git diff --exit-code master ' From db864306cf10bb46077252d83987a9d9054e86ee Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?SZEDER=20G=C3=A1bor?= Date: Tue, 30 Apr 2019 14:37:24 +0200 Subject: [PATCH 12/66] ci: install 'libsvn-perl' instead of 'git-svn' MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Since e7e9f5e7a1 (travis-ci: enable Git SVN tests t91xx on Linux, 2016-05-19) some of our Travis CI build jobs install the 'git-svn' package, because it was a convenient way to install its dependencies, which are necessary to run our 'git-svn' tests (we don't actually need the 'git-svn' package itself). However, from those dependencies, namely the 'libsvn-perl', 'libyaml-perl', and 'libterm-readkey-perl' packages, only 'libsvn-perl' is necessary to run those tests, the others arent, not even to fulfill some prereqs. So update 'ci/install-dependencies.sh' to install only 'libsvn-perl' instead of 'git-svn' and its additional dependencies. Note that this change has more important implications than merely not installing three unnecessary packages, as it keeps our builds working with Travis CI's Xenial images. In our '.travis.yml' we never explicitly specified which Linux image we want to use to run our Linux build jobs, and so far they have been run on the default Ubuntu 14.04 Trusty image. However, 14.04 just reached its EOL, and Travis CI has already began the transition to use 16.04 Xenial as the default Linux build environment [1]. Alas, our Linux Clang and GCC build jobs can't simply 'apt-get install git-svn' in the current Xenial images [2], like they did in the Trusty images, and, consequently, fail. Installing only 'libsvn-perl' avoids this issue, while the 'git svn' tests are still run as they should. [1] https://blog.travis-ci.com/2019-04-15-xenial-default-build-environment [2] 'apt-get install git-svn' in the Xenial image fails with: The following packages have unmet dependencies: git-svn : Depends: git (< 1:2.7.4-.) E: Unable to correct problems, you have held broken packages. The reason is that both the Trusty and Xenial images contain the 'git' package installed from 'ppa:git-core/ppa', so it's considerably newer than the 'git' package in the corresponding standard Ubuntu package repositories. The difference is that the Trusty image still contains these third-party apt repositories, so the 'git-svn' package was installed from the same PPA, and its version matched the version of the already installed 'git' package. In the Xenial image, however, these third-party apt-repositories are removed (to reduce the risk of unrelated interference and faster 'apt-get update') [3], and the version of the 'git-svn' package coming from the standard Ubuntu package repositories doesn't match the much more recent version of the 'git' package installed from the PPA, resulting in this dependecy error. Adding back the 'ppa:git-core/ppa' package repository would solve this dependency issue as well, but since the troublesome package happens to be unnecessary, not installing it in the first place is better. [3] https://docs.travis-ci.com/user/reference/xenial/#third-party-apt-repositories-removed Signed-off-by: SZEDER Gábor Signed-off-by: Junio C Hamano --- ci/install-dependencies.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ci/install-dependencies.sh b/ci/install-dependencies.sh index d64667fcbf..bab7934b16 100755 --- a/ci/install-dependencies.sh +++ b/ci/install-dependencies.sh @@ -12,7 +12,7 @@ case "$jobname" in linux-clang|linux-gcc) sudo apt-add-repository -y "ppa:ubuntu-toolchain-r/test" sudo apt-get -q update - sudo apt-get -q -y install language-pack-is git-svn apache2 + sudo apt-get -q -y install language-pack-is libsvn-perl apache2 case "$jobname" in linux-gcc) sudo apt-get -q -y install gcc-8 From 1ac8d363eec3ec137af7ed832dc524b23f81ac64 Mon Sep 17 00:00:00 2001 From: Dustin Spicuzza Date: Mon, 29 Apr 2019 14:58:47 -0700 Subject: [PATCH 13/66] cvsexportcommit: force crlf translation When using cvsnt + msys + git, it seems like the output of cvs status had \r\n in it, and caused the command to fail. This fixes that. Signed-off-by: Dustin Spicuzza Signed-off-by: Johannes Schindelin Signed-off-by: Junio C Hamano --- git-cvsexportcommit.perl | 1 + 1 file changed, 1 insertion(+) diff --git a/git-cvsexportcommit.perl b/git-cvsexportcommit.perl index d13f02da95..fc00d5946a 100755 --- a/git-cvsexportcommit.perl +++ b/git-cvsexportcommit.perl @@ -431,6 +431,7 @@ END sub safe_pipe_capture { my @output; if (my $pid = open my $child, '-|') { + binmode($child, ":crlf"); @output = (<$child>); close $child or die join(' ',@_).": $! $?"; } else { From 397a46db786a53db5261a74232447f7b49574bba Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Mon, 25 Feb 2019 06:17:45 -0800 Subject: [PATCH 14/66] t5580: verify that alternates can be UNC paths On Windows, UNC paths are a very convenient way to share data, and alternates are all about sharing data. We fixed a bug where alternates specifying UNC paths were not handled properly, and it is high time that we add a regression test to ensure that this bug is not reintroduced. Signed-off-by: Johannes Schindelin Signed-off-by: Junio C Hamano --- t/t5580-clone-push-unc.sh | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/t/t5580-clone-push-unc.sh b/t/t5580-clone-push-unc.sh index 217adf3a63..b3c8a92450 100755 --- a/t/t5580-clone-push-unc.sh +++ b/t/t5580-clone-push-unc.sh @@ -62,4 +62,16 @@ test_expect_success MINGW 'remote nick cannot contain backslashes' ' test_i18ngrep ! "unable to access" err ' +test_expect_success 'unc alternates' ' + tree="$(git rev-parse HEAD:)" && + mkdir test-unc-alternate && + ( + cd test-unc-alternate && + git init && + test_must_fail git show $tree && + echo "$UNCPATH/.git/objects" >.git/objects/info/alternates && + git show $tree + ) +' + test_done From b9f0193b252b289a0188886aa6cf396510ccebc8 Mon Sep 17 00:00:00 2001 From: Tanushree Tumane Date: Wed, 27 Feb 2019 00:43:13 -0800 Subject: [PATCH 15/66] mingw: remove obsolete IPv6-related code To support IPv6, Git provided fall back functions for Windows versions that did not support IPv6. However, as Git dropped support for Windows XP and prior, those functions are not needed anymore. Remove those fallbacks by reverting fe3b2b7b827c (Enable support for IPv6 on MinGW, 2009-11-24) and using the functions directly (without 'ipv6_' prefix). Signed-off-by: Tanushree Tumane Signed-off-by: Johannes Schindelin Signed-off-by: Junio C Hamano --- compat/mingw.c | 178 +------------------------------------------------ compat/mingw.h | 8 --- 2 files changed, 3 insertions(+), 183 deletions(-) diff --git a/compat/mingw.c b/compat/mingw.c index 6b04514cdc..e51ee74087 100644 --- a/compat/mingw.c +++ b/compat/mingw.c @@ -1714,142 +1714,10 @@ int mingw_putenv(const char *namevalue) return result ? 0 : -1; } -/* - * Note, this isn't a complete replacement for getaddrinfo. It assumes - * that service contains a numerical port, or that it is null. It - * does a simple search using gethostbyname, and returns one IPv4 host - * if one was found. - */ -static int WSAAPI getaddrinfo_stub(const char *node, const char *service, - const struct addrinfo *hints, - struct addrinfo **res) -{ - struct hostent *h = NULL; - struct addrinfo *ai; - struct sockaddr_in *sin; - - if (node) { - h = gethostbyname(node); - if (!h) - return WSAGetLastError(); - } - - ai = xmalloc(sizeof(struct addrinfo)); - *res = ai; - ai->ai_flags = 0; - ai->ai_family = AF_INET; - ai->ai_socktype = hints ? hints->ai_socktype : 0; - switch (ai->ai_socktype) { - case SOCK_STREAM: - ai->ai_protocol = IPPROTO_TCP; - break; - case SOCK_DGRAM: - ai->ai_protocol = IPPROTO_UDP; - break; - default: - ai->ai_protocol = 0; - break; - } - ai->ai_addrlen = sizeof(struct sockaddr_in); - if (hints && (hints->ai_flags & AI_CANONNAME)) - ai->ai_canonname = h ? xstrdup(h->h_name) : NULL; - else - ai->ai_canonname = NULL; - - sin = xcalloc(1, ai->ai_addrlen); - sin->sin_family = AF_INET; - /* Note: getaddrinfo is supposed to allow service to be a string, - * which should be looked up using getservbyname. This is - * currently not implemented */ - if (service) - sin->sin_port = htons(atoi(service)); - if (h) - sin->sin_addr = *(struct in_addr *)h->h_addr; - else if (hints && (hints->ai_flags & AI_PASSIVE)) - sin->sin_addr.s_addr = INADDR_ANY; - else - sin->sin_addr.s_addr = INADDR_LOOPBACK; - ai->ai_addr = (struct sockaddr *)sin; - ai->ai_next = NULL; - return 0; -} - -static void WSAAPI freeaddrinfo_stub(struct addrinfo *res) -{ - free(res->ai_canonname); - free(res->ai_addr); - free(res); -} - -static int WSAAPI getnameinfo_stub(const struct sockaddr *sa, socklen_t salen, - char *host, DWORD hostlen, - char *serv, DWORD servlen, int flags) -{ - const struct sockaddr_in *sin = (const struct sockaddr_in *)sa; - if (sa->sa_family != AF_INET) - return EAI_FAMILY; - if (!host && !serv) - return EAI_NONAME; - - if (host && hostlen > 0) { - struct hostent *ent = NULL; - if (!(flags & NI_NUMERICHOST)) - ent = gethostbyaddr((const char *)&sin->sin_addr, - sizeof(sin->sin_addr), AF_INET); - - if (ent) - snprintf(host, hostlen, "%s", ent->h_name); - else if (flags & NI_NAMEREQD) - return EAI_NONAME; - else - snprintf(host, hostlen, "%s", inet_ntoa(sin->sin_addr)); - } - - if (serv && servlen > 0) { - struct servent *ent = NULL; - if (!(flags & NI_NUMERICSERV)) - ent = getservbyport(sin->sin_port, - flags & NI_DGRAM ? "udp" : "tcp"); - - if (ent) - snprintf(serv, servlen, "%s", ent->s_name); - else - snprintf(serv, servlen, "%d", ntohs(sin->sin_port)); - } - - return 0; -} - -static HMODULE ipv6_dll = NULL; -static void (WSAAPI *ipv6_freeaddrinfo)(struct addrinfo *res); -static int (WSAAPI *ipv6_getaddrinfo)(const char *node, const char *service, - const struct addrinfo *hints, - struct addrinfo **res); -static int (WSAAPI *ipv6_getnameinfo)(const struct sockaddr *sa, socklen_t salen, - char *host, DWORD hostlen, - char *serv, DWORD servlen, int flags); -/* - * gai_strerror is an inline function in the ws2tcpip.h header, so we - * don't need to try to load that one dynamically. - */ - -static void socket_cleanup(void) -{ - WSACleanup(); - if (ipv6_dll) - FreeLibrary(ipv6_dll); - ipv6_dll = NULL; - ipv6_freeaddrinfo = freeaddrinfo_stub; - ipv6_getaddrinfo = getaddrinfo_stub; - ipv6_getnameinfo = getnameinfo_stub; -} - static void ensure_socket_initialization(void) { WSADATA wsa; static int initialized = 0; - const char *libraries[] = { "ws2_32.dll", "wship6.dll", NULL }; - const char **name; if (initialized) return; @@ -1858,35 +1726,7 @@ static void ensure_socket_initialization(void) die("unable to initialize winsock subsystem, error %d", WSAGetLastError()); - for (name = libraries; *name; name++) { - ipv6_dll = LoadLibraryExA(*name, NULL, - LOAD_LIBRARY_SEARCH_SYSTEM32); - if (!ipv6_dll) - continue; - - ipv6_freeaddrinfo = (void (WSAAPI *)(struct addrinfo *)) - GetProcAddress(ipv6_dll, "freeaddrinfo"); - ipv6_getaddrinfo = (int (WSAAPI *)(const char *, const char *, - const struct addrinfo *, - struct addrinfo **)) - GetProcAddress(ipv6_dll, "getaddrinfo"); - ipv6_getnameinfo = (int (WSAAPI *)(const struct sockaddr *, - socklen_t, char *, DWORD, - char *, DWORD, int)) - GetProcAddress(ipv6_dll, "getnameinfo"); - if (!ipv6_freeaddrinfo || !ipv6_getaddrinfo || !ipv6_getnameinfo) { - FreeLibrary(ipv6_dll); - ipv6_dll = NULL; - } else - break; - } - if (!ipv6_freeaddrinfo || !ipv6_getaddrinfo || !ipv6_getnameinfo) { - ipv6_freeaddrinfo = freeaddrinfo_stub; - ipv6_getaddrinfo = getaddrinfo_stub; - ipv6_getnameinfo = getnameinfo_stub; - } - - atexit(socket_cleanup); + atexit((void(*)(void)) WSACleanup); initialized = 1; } @@ -1904,24 +1744,12 @@ struct hostent *mingw_gethostbyname(const char *host) return gethostbyname(host); } -void mingw_freeaddrinfo(struct addrinfo *res) -{ - ipv6_freeaddrinfo(res); -} - +#undef getaddrinfo int mingw_getaddrinfo(const char *node, const char *service, const struct addrinfo *hints, struct addrinfo **res) { ensure_socket_initialization(); - return ipv6_getaddrinfo(node, service, hints, res); -} - -int mingw_getnameinfo(const struct sockaddr *sa, socklen_t salen, - char *host, DWORD hostlen, char *serv, DWORD servlen, - int flags) -{ - ensure_socket_initialization(); - return ipv6_getnameinfo(sa, salen, host, hostlen, serv, servlen, flags); + return getaddrinfo(node, service, hints, res); } int mingw_socket(int domain, int type, int protocol) diff --git a/compat/mingw.h b/compat/mingw.h index 4d73f8aa9d..593bdbffe6 100644 --- a/compat/mingw.h +++ b/compat/mingw.h @@ -295,18 +295,10 @@ int mingw_gethostname(char *host, int namelen); struct hostent *mingw_gethostbyname(const char *host); #define gethostbyname mingw_gethostbyname -void mingw_freeaddrinfo(struct addrinfo *res); -#define freeaddrinfo mingw_freeaddrinfo - int mingw_getaddrinfo(const char *node, const char *service, const struct addrinfo *hints, struct addrinfo **res); #define getaddrinfo mingw_getaddrinfo -int mingw_getnameinfo(const struct sockaddr *sa, socklen_t salen, - char *host, DWORD hostlen, char *serv, DWORD servlen, - int flags); -#define getnameinfo mingw_getnameinfo - int mingw_socket(int domain, int type, int protocol); #define socket mingw_socket From d4907720a2fc2d096fa0d904f7ee92b0fbc302bf Mon Sep 17 00:00:00 2001 From: Chris Mayo Date: Sat, 4 May 2019 19:45:07 +0100 Subject: [PATCH 16/66] notes: correct documentation of format_display_notes() 'flags' parameter was replaced by 'raw' in commit: 76141e2e62 ("format_note(): simplify API", 2012-10-17) Signed-off-by: Chris Mayo Signed-off-by: Junio C Hamano --- notes.h | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/notes.h b/notes.h index 414bc6855a..76337f2384 100644 --- a/notes.h +++ b/notes.h @@ -276,12 +276,10 @@ void init_display_notes(struct display_notes_opt *opt); /* * Append notes for the given 'object_sha1' from all trees set up by - * init_display_notes() to 'sb'. The 'flags' are a bitwise - * combination of + * init_display_notes() to 'sb'. * - * - NOTES_SHOW_HEADER: add a 'Notes (refname):' header - * - * - NOTES_INDENT: indent the notes by 4 places + * If 'raw' is false the note will be indented by 4 places and + * a 'Notes (refname):' header added. * * You *must* call init_display_notes() before using this function. */ From 9bb81452ffce9481f7613868ab9c8be998529bd6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86var=20Arnfj=C3=B6r=C3=B0=20Bjarmason?= Date: Tue, 7 May 2019 12:54:29 +0200 Subject: [PATCH 17/66] perf README: correct docs for 3c8f12c96c regression MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Since 3c8f12c96c ("test-lib: reorder and include GIT-BUILD-OPTIONS a lot earlier", 2012-06-24) the suggested advice of overriding GIT_BUILD_DIR has not worked. We've printed a hard error like this given e.g. GIT_BUILD_DIR=/home/avar/g/git: /bin-wrappers/git is not executable; using GIT_EXEC_PATH error: You haven't built things yet, have you? Let's just suggest that the user run other gits via the "run" script. That'll do the right thing for setting the path to the other git, and running the "aggregate.perl" scripts afterwards will work. As an aside, if setting GIT_BUILD_DIR had still worked, then the MODERN_GIT feature/fix added in 1a0962dee5 ("t/perf: fix regression in testing older versions of git", 2016-06-22) would have broke. Signed-off-by: Ævar Arnfjörð Bjarmason --- t/perf/README | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/t/perf/README b/t/perf/README index be12090c38..c7b70e2d28 100644 --- a/t/perf/README +++ b/t/perf/README @@ -45,7 +45,7 @@ call the aggregation script to summarize the results: $ ./p0001-rev-list.sh [...] - $ GIT_BUILD_DIR=/path/to/other/git ./p0001-rev-list.sh + $ ./run /path/to/other/git -- ./p0001-rev-list.sh [...] $ ./aggregate.perl . /path/to/other/git ./p0001-rev-list.sh From c43b7e6089629b05d5af6995e99d5149ae5c3621 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86var=20Arnfj=C3=B6r=C3=B0=20Bjarmason?= Date: Tue, 7 May 2019 12:54:30 +0200 Subject: [PATCH 18/66] perf aggregate: remove GIT_TEST_INSTALLED from --codespeed MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Remove the setting of the "environment" from the --codespeed output. I don't think this is useful, and it helps with a later refactoring where we GIT_TEST_INSTALLED stop munging/reading GIT_TEST_INSTALLED in the perf tests in so many places. This was added in 05eb1c37ed ("perf/aggregate: implement codespeed JSON output", 2018-01-05), but since the "run" scripts uses "GIT_TEST_INSTALLED" internally this was only ever useful for one-off runs of a single revision as all the "environment" values would be ones for whatever directory the "run" script ran last. Let's instead fall back on the "uname -r" case, which is the sort of thing the environment should be set to, not something that duplicates other parts of the codpseed output. For setting the "environment" to something custom the perf.repoName variable can be used. See 19cf57a92e ("perf/run: read GIT_PERF_REPO_NAME from perf.repoName", 2018-01-05). Signed-off-by: Ævar Arnfjörð Bjarmason --- t/perf/aggregate.perl | 3 --- 1 file changed, 3 deletions(-) diff --git a/t/perf/aggregate.perl b/t/perf/aggregate.perl index 494907a892..f6518339dc 100755 --- a/t/perf/aggregate.perl +++ b/t/perf/aggregate.perl @@ -312,9 +312,6 @@ sub print_codespeed_results { $environment = $reponame; } elsif (exists $ENV{GIT_PERF_REPO_NAME} and $ENV{GIT_PERF_REPO_NAME} ne "") { $environment = $ENV{GIT_PERF_REPO_NAME}; - } elsif (exists $ENV{GIT_TEST_INSTALLED} and $ENV{GIT_TEST_INSTALLED} ne "") { - $environment = $ENV{GIT_TEST_INSTALLED}; - $environment =~ s|/bin-wrappers$||; } else { $environment = `uname -r`; chomp $environment; From 90e38154eeb57f05ac099c42f4b369c8dad9ad2c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86var=20Arnfj=C3=B6r=C3=B0=20Bjarmason?= Date: Tue, 7 May 2019 12:54:31 +0200 Subject: [PATCH 19/66] perf-lib.sh: make "./run " use the correct gits MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fix a really bad regression in 0baf78e7bc ("perf-lib.sh: rely on test-lib.sh for --tee handling", 2019-03-15). Since that change all runs of different of git have used the git found in the user's $PATH, e.g. /usr/bin/git instead of the we just built and wanted to performance test. The problem starts with GIT_TEST_INSTALLED not working like our non-perf tests with the "run" script. I.e. you can't run performance tests against a given installed git. Instead we expect to use it ourselves to point GIT_TEST_INSTALLED to the we just built. However, we had been relying on '$(cd "$GIT_TEST_INSTALLED" && pwd)' to resolve that relative $GIT_TEST_INSTALLED to an absolute path *before* test-lib.sh was loaded, in cases where it was e.g. "build//bin-wrappers" and we wanted "build/...". This change post-dates another proposed solution by a few days[1], I didn't notice that version when I initially wrote this. I'm doing the most minimal thing to solve the regression here, a follow-up change will move this result prefix selection logic entirely into the "run" script. This makes e.g. these cases all work: ./run . $PWD/../../ origin/master origin/next HEAD -- As well as just a plain one-off: ./run And, since we're passing down the new GIT_PERF_DIR_MYDIR_REL we make sure the bug relating to aggregate.perl not finding our files as described in 0baf78e7bc doesn't happen again. What *doesn't* work is setting GIT_TEST_INSTALLED to a relative path, this will subtly fail in test-lib.sh. This has always been the case even before 0baf78e7bc, and as documented in t/README the GIT_TEST_INSTALLED variable should be set to an absolute path (needs to be set "to the bindir", which is always absolute), and the "perf" framework expects to munge it itself. Perhaps that should be dealt with in the future to allow manually setting GIT_TEST_INSTALLED, but as a preceding commit showed the user can just use the "run" script, which'll also pick the right output directory for the test results as expected by aggregate.perl. 1. https://public-inbox.org/git/20190502222409.GA15631@sigill.intra.peff.net/ Signed-off-by: Ævar Arnfjörð Bjarmason --- t/perf/perf-lib.sh | 4 ++++ t/perf/run | 8 ++++++-- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/t/perf/perf-lib.sh b/t/perf/perf-lib.sh index 169f92eae3..b15ee1d262 100644 --- a/t/perf/perf-lib.sh +++ b/t/perf/perf-lib.sh @@ -32,6 +32,10 @@ TEST_NO_MALLOC_CHECK=t if test -z "$GIT_TEST_INSTALLED"; then perf_results_prefix= else + if test -n "$GIT_PERF_DIR_MYDIR_REL" + then + GIT_TEST_INSTALLED=$GIT_PERF_DIR_MYDIR_REL + fi perf_results_prefix=$(printf "%s" "${GIT_TEST_INSTALLED%/bin-wrappers}" | tr -c "[a-zA-Z0-9]" "[_*]")"." GIT_TEST_INSTALLED=$ABSOLUTE_GIT_TEST_INSTALLED fi diff --git a/t/perf/run b/t/perf/run index 9aaa733c77..0a7c8744ab 100755 --- a/t/perf/run +++ b/t/perf/run @@ -91,10 +91,14 @@ run_dirs_helper () { if test "$mydir" = .; then unset GIT_TEST_INSTALLED else - GIT_TEST_INSTALLED="$mydir/bin-wrappers" + GIT_PERF_DIR_MYDIR_REL=$mydir + GIT_PERF_DIR_MYDIR_ABS=$(cd $mydir && pwd) + export GIT_PERF_DIR_MYDIR_REL GIT_PERF_DIR_MYDIR_ABS + + GIT_TEST_INSTALLED="$GIT_PERF_DIR_MYDIR_ABS/bin-wrappers" # Older versions of git lacked bin-wrappers; fallback to the # files in the root. - test -d "$GIT_TEST_INSTALLED" || GIT_TEST_INSTALLED=$mydir + test -d "$GIT_TEST_INSTALLED" || GIT_TEST_INSTALLED=$GIT_PERF_DIR_MYDIR_ABS export GIT_TEST_INSTALLED fi run_one_dir "$@" From df0f5021951bd0ae5e7db0d89fd7e5c141c334be Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86var=20Arnfj=C3=B6r=C3=B0=20Bjarmason?= Date: Tue, 7 May 2019 12:54:32 +0200 Subject: [PATCH 20/66] perf-lib.sh: remove GIT_TEST_INSTALLED from perf-lib.sh MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Follow-up my preceding change which fixed the immediate "./run " regression in 0baf78e7bc ("perf-lib.sh: rely on test-lib.sh for --tee handling", 2019-03-15) and entirely get rid of GIT_TEST_INSTALLED from perf-lib.sh (and aggregate.perl). As noted in that change the dance we're doing with GIT_TEST_INSTALLED perf-lib.sh isn't necessary, but there I was doing the most minimal set of changes to quickly fix a regression. But it's much simpler to never deal with the "GIT_TEST_INSTALLED" we were setting in perf-lib.sh at all. Instead the run_dirs_helper() sets the previously inferred $PERF_RESULTS_PREFIX directly. Setting this at the callsite that's already best positioned to exhaustively know about all the different cases we need to handle where PERF_RESULTS_PREFIX isn't what we want already (the empty string) makes the most sense. In one-off cases like: ./run ./p0000-perf-lib-sanity.sh ./p0000-perf-lib-sanity.sh We'll just do the right thing because PERF_RESULTS_PREFIX will be empty, and test-lib.sh takes care of finding where our git is. Any refactoring of this code needs to change both the shell code and the Perl code in aggregate.perl, because when running e.g.: ./run ../../ -- The "../../" path to a relative bindir needs to be munged to a filename containing the results, and critically aggregate.perl does not get passed the path to those aggregations, just "../..". Let's fix cases where aggregate.perl would print e.g. ".." in its report output for this, and "git" for "/home/avar/g/git", i.e. it would always pick the last element. Now'll always print the full path instead. This also makes the code sturdier, e.g. you can feed "../.." to "./run" and then an absolute path to the aggregate.perl script, as long as the absolute path and "../.." resolved to the same directory printing the aggregation will work. Also simplify the "[_*]" on the RHS of "tr -c", we're trimming everything to "_", so we don't need that. Signed-off-by: Ævar Arnfjörð Bjarmason --- t/perf/aggregate.perl | 10 ++++++---- t/perf/perf-lib.sh | 15 +-------------- t/perf/run | 43 ++++++++++++++++++++++++++++++------------- 3 files changed, 37 insertions(+), 31 deletions(-) diff --git a/t/perf/aggregate.perl b/t/perf/aggregate.perl index f6518339dc..c8f4a78903 100755 --- a/t/perf/aggregate.perl +++ b/t/perf/aggregate.perl @@ -6,6 +6,7 @@ use warnings; use JSON; use Getopt::Long; use Git; +use Cwd qw(realpath); sub get_times { my $name = shift; @@ -103,13 +104,14 @@ while (scalar @ARGV) { if (! -d $arg) { my $rev = Git::command_oneline(qw(rev-parse --verify), $arg); $dir = "build/".$rev; + } elsif ($arg eq '.') { + $dir = '.'; } else { - $arg =~ s{/*$}{}; - $dir = $arg; - $dirabbrevs{$dir} = $dir; + $dir = realpath($arg); + $dirnames{$dir} = $dir; } push @dirs, $dir; - $dirnames{$dir} = $arg; + $dirnames{$dir} ||= $arg; my $prefix = $dir; $prefix =~ tr/^a-zA-Z0-9/_/c; $prefixes{$dir} = $prefix . '.'; diff --git a/t/perf/perf-lib.sh b/t/perf/perf-lib.sh index b15ee1d262..9cdccba222 100644 --- a/t/perf/perf-lib.sh +++ b/t/perf/perf-lib.sh @@ -21,25 +21,12 @@ # because it will change our working directory. TEST_DIRECTORY=$(pwd)/.. TEST_OUTPUT_DIRECTORY=$(pwd) -ABSOLUTE_GIT_TEST_INSTALLED=$( - test -n "$GIT_TEST_INSTALLED" && cd "$GIT_TEST_INSTALLED" && pwd) TEST_NO_CREATE_REPO=t TEST_NO_MALLOC_CHECK=t . ../test-lib.sh -if test -z "$GIT_TEST_INSTALLED"; then - perf_results_prefix= -else - if test -n "$GIT_PERF_DIR_MYDIR_REL" - then - GIT_TEST_INSTALLED=$GIT_PERF_DIR_MYDIR_REL - fi - perf_results_prefix=$(printf "%s" "${GIT_TEST_INSTALLED%/bin-wrappers}" | tr -c "[a-zA-Z0-9]" "[_*]")"." - GIT_TEST_INSTALLED=$ABSOLUTE_GIT_TEST_INSTALLED -fi - # Variables from test-lib that are normally internal to the tests; we # need to export them for test_perf subshells export TEST_DIRECTORY TRASH_DIRECTORY GIT_BUILD_DIR GIT_TEST_CMP @@ -183,7 +170,7 @@ test_wrapper_ () { base=$(basename "$0" .sh) echo "$test_count" >>"$perf_results_dir"/$base.subtests echo "$1" >"$perf_results_dir"/$base.$test_count.descr - base="$perf_results_dir"/"$perf_results_prefix$(basename "$0" .sh)"."$test_count" + base="$perf_results_dir"/"$PERF_RESULTS_PREFIX$(basename "$0" .sh)"."$test_count" "$test_wrapper_func_" "$@" fi diff --git a/t/perf/run b/t/perf/run index 0a7c8744ab..85b7bd31d5 100755 --- a/t/perf/run +++ b/t/perf/run @@ -70,6 +70,22 @@ build_git_rev () { ) || die "failed to build revision '$mydir'" } +set_git_test_installed () { + mydir=$1 + + mydir_abs=$(cd $mydir && pwd) + mydir_abs_wrappers="$mydir_abs_wrappers/bin-wrappers" + if test -d "$mydir_abs_wrappers" + then + GIT_TEST_INSTALLED=$mydir_abs_wrappers + else + # Older versions of git lacked bin-wrappers; + # fallback to the files in the root. + GIT_TEST_INSTALLED=$mydir_abs + fi + export GIT_TEST_INSTALLED +} + run_dirs_helper () { mydir=${1%/} shift @@ -79,7 +95,16 @@ run_dirs_helper () { if test $# -gt 0 -a "$1" = --; then shift fi - if [ ! -d "$mydir" ]; then + + PERF_RESULTS_PREFIX= + if test "$mydir" = "." + then + unset GIT_TEST_INSTALLED + elif test -d "$mydir" + then + PERF_RESULTS_PREFIX=$(cd $mydir && printf "%s" "$(pwd)" | tr -c "[a-zA-Z0-9]" "_"). + set_git_test_installed "$mydir" + else rev=$(git rev-parse --verify "$mydir" 2>/dev/null) || die "'$mydir' is neither a directory nor a valid revision" if [ ! -d build/$rev ]; then @@ -87,20 +112,12 @@ run_dirs_helper () { fi build_git_rev $rev "$mydir" mydir=build/$rev - fi - if test "$mydir" = .; then - unset GIT_TEST_INSTALLED - else - GIT_PERF_DIR_MYDIR_REL=$mydir - GIT_PERF_DIR_MYDIR_ABS=$(cd $mydir && pwd) - export GIT_PERF_DIR_MYDIR_REL GIT_PERF_DIR_MYDIR_ABS - GIT_TEST_INSTALLED="$GIT_PERF_DIR_MYDIR_ABS/bin-wrappers" - # Older versions of git lacked bin-wrappers; fallback to the - # files in the root. - test -d "$GIT_TEST_INSTALLED" || GIT_TEST_INSTALLED=$GIT_PERF_DIR_MYDIR_ABS - export GIT_TEST_INSTALLED + PERF_RESULTS_PREFIX=build_$rev. + set_git_test_installed "$mydir" fi + export PERF_RESULTS_PREFIX + run_one_dir "$@" } From fab80ee79ddf59a5d00812005bef0fa3acf5b6bf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86var=20Arnfj=C3=B6r=C3=B0=20Bjarmason?= Date: Tue, 7 May 2019 12:54:33 +0200 Subject: [PATCH 21/66] perf tests: add "bindir" prefix to git tree test results MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Change the output file names in test-results/ to be "test-results/bindir_" rather than just "test-results/". This is for consistency with the "build_" directories we have for built revisions, i.e. "test-results/build_". There's no user-visible functional changes here, it just makes it easier to see at a glance what "test-results" files are of what "type" as they're all explicitly grouped together now, and to grep this code to find both the run_dirs_helper() implementation and its corresponding aggregate.perl code. Note that we already guarantee that the rest of the PERF_RESULTS_PREFIX is an absolute path, and since it'll start with e.g. "/" which we munge to "_" we'll up with a readable string like "bindir_home_avar[...]". Signed-off-by: Ævar Arnfjörð Bjarmason --- t/perf/aggregate.perl | 4 +++- t/perf/run | 2 +- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/t/perf/aggregate.perl b/t/perf/aggregate.perl index c8f4a78903..b951747e08 100755 --- a/t/perf/aggregate.perl +++ b/t/perf/aggregate.perl @@ -100,6 +100,7 @@ usage() unless $rc; while (scalar @ARGV) { my $arg = $ARGV[0]; my $dir; + my $prefix = ''; last if -f $arg or $arg eq "--"; if (! -d $arg) { my $rev = Git::command_oneline(qw(rev-parse --verify), $arg); @@ -109,10 +110,11 @@ while (scalar @ARGV) { } else { $dir = realpath($arg); $dirnames{$dir} = $dir; + $prefix .= 'bindir'; } push @dirs, $dir; $dirnames{$dir} ||= $arg; - my $prefix = $dir; + $prefix .= $dir; $prefix =~ tr/^a-zA-Z0-9/_/c; $prefixes{$dir} = $prefix . '.'; shift @ARGV; diff --git a/t/perf/run b/t/perf/run index 85b7bd31d5..cd3882b117 100755 --- a/t/perf/run +++ b/t/perf/run @@ -102,7 +102,7 @@ run_dirs_helper () { unset GIT_TEST_INSTALLED elif test -d "$mydir" then - PERF_RESULTS_PREFIX=$(cd $mydir && printf "%s" "$(pwd)" | tr -c "[a-zA-Z0-9]" "_"). + PERF_RESULTS_PREFIX=bindir$(cd $mydir && printf "%s" "$(pwd)" | tr -c "[a-zA-Z0-9]" "_"). set_git_test_installed "$mydir" else rev=$(git rev-parse --verify "$mydir" 2>/dev/null) || From 82b7eb231d146678317fd4cf63a646c4233976e0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86var=20Arnfj=C3=B6r=C3=B0=20Bjarmason?= Date: Tue, 7 May 2019 12:54:34 +0200 Subject: [PATCH 22/66] perf-lib.sh: forbid the use of GIT_TEST_INSTALLED MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit As noted in preceding commits setting GIT_TEST_INSTALLED has never been supported or documented, and as noted in an earlier t/perf/README change to the extent that it's been documented nobody's notices that the example hasn't worked since 3c8f12c96c ("test-lib: reorder and include GIT-BUILD-OPTIONS a lot earlier", 2012-06-24). We could directly support GIT_TEST_INSTALLED for invocations without the "run" script, such as: GIT_TEST_INSTALLED=../../ ./p0000-perf-lib-sanity.sh GIT_TEST_INSTALLED=/home/avar/g/git ./p0000-perf-lib-sanity.sh But while not having this "error" will "work", it won't write the the resulting "test-results/*" files to the right place, and thus a subsequent call to aggregate.perl won't work as expected. Let's just tell the user that they need to use the "run" script, which'll correctly deal with this and set the right PERF_RESULTS_PREFIX. If someone's in desperate need of bypassing "run" for whatever reason they can trivially do so by setting "PERF_SET_GIT_TEST_INSTALLED", but not we won't have people who expect GIT_TEST_INSTALLED to just work wondering why their aggregation doesn't work, even though they're running the right "git". Signed-off-by: Ævar Arnfjörð Bjarmason --- t/perf/perf-lib.sh | 11 +++++++++++ t/perf/run | 2 ++ 2 files changed, 13 insertions(+) diff --git a/t/perf/perf-lib.sh b/t/perf/perf-lib.sh index 9cdccba222..b58a43ea43 100644 --- a/t/perf/perf-lib.sh +++ b/t/perf/perf-lib.sh @@ -27,6 +27,17 @@ TEST_NO_MALLOC_CHECK=t . ../test-lib.sh +if test -n "$GIT_TEST_INSTALLED" -a -z "$PERF_SET_GIT_TEST_INSTALLED" +then + error "Do not use GIT_TEST_INSTALLED with the perf tests. + +Instead use: + + ./run -- + +See t/perf/README for details." +fi + # Variables from test-lib that are normally internal to the tests; we # need to export them for test_perf subshells export TEST_DIRECTORY TRASH_DIRECTORY GIT_BUILD_DIR GIT_TEST_CMP diff --git a/t/perf/run b/t/perf/run index cd3882b117..c7b86104e1 100755 --- a/t/perf/run +++ b/t/perf/run @@ -84,6 +84,8 @@ set_git_test_installed () { GIT_TEST_INSTALLED=$mydir_abs fi export GIT_TEST_INSTALLED + PERF_SET_GIT_TEST_INSTALLED=true + export PERF_SET_GIT_TEST_INSTALLED } run_dirs_helper () { From dc76852df2f7bc5a6c79a796954a81f5af284c34 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Tue, 7 May 2019 13:10:20 +0200 Subject: [PATCH 23/66] fsmonitor: demonstrate that it is not refreshed after discard_index() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This one is tricky. When `core.fsmonitor` is set, a `refresh_index()` will not perform a full scan of files that might be modified, but will query the fsmonitor and refresh only the ones that have been actually touched. Due to implementation details, the fsmonitor is queried in `refresh_cache_ent()`, but of course it only has to be queried once, so we set a flag when we did that. But when the index was discarded, we did not re-set that flag. So far, this is only covered by our test suite when running with GIT_TEST_FSMONITOR=$PWD/t7519/fsmonitor-all, and only due to the way the built-in stash interacts with the recursive merge machinery. Let's introduce a straight-forward regression test for this. We simply extend the "read & discard index" loop in `test-tool read-cache` to optionally refresh the index, report on a given file's status, and then modify that file. Due to the bug described above, only the first refresh will actually query the fsmonitor; subsequent loop iterations will not. This problem was reported by Ævar Arnfjörð Bjarmason. Signed-off-by: Johannes Schindelin Signed-off-by: Junio C Hamano --- t/helper/test-read-cache.c | 24 +++++++++++++++++++++++- t/t7519-status-fsmonitor.sh | 8 ++++++++ 2 files changed, 31 insertions(+), 1 deletion(-) diff --git a/t/helper/test-read-cache.c b/t/helper/test-read-cache.c index d674c88ba0..7e79b555de 100644 --- a/t/helper/test-read-cache.c +++ b/t/helper/test-read-cache.c @@ -1,14 +1,36 @@ #include "test-tool.h" #include "cache.h" +#include "config.h" int cmd__read_cache(int argc, const char **argv) { - int i, cnt = 1; + int i, cnt = 1, namelen; + const char *name = NULL; + + if (argc > 1 && skip_prefix(argv[1], "--print-and-refresh=", &name)) { + namelen = strlen(name); + argc--; + argv++; + } + if (argc == 2) cnt = strtol(argv[1], NULL, 0); setup_git_directory(); + git_config(git_default_config, NULL); for (i = 0; i < cnt; i++) { read_cache(); + if (name) { + int pos; + + refresh_index(&the_index, REFRESH_QUIET, + NULL, NULL, NULL); + pos = index_name_pos(&the_index, name, namelen); + if (pos < 0) + die("%s not in index", name); + printf("%s is%s up to date\n", name, + ce_uptodate(the_index.cache[pos]) ? "" : " not"); + write_file(name, "%d\n", i); + } discard_cache(); } return 0; diff --git a/t/t7519-status-fsmonitor.sh b/t/t7519-status-fsmonitor.sh index 3e0a61db23..afd8fa7532 100755 --- a/t/t7519-status-fsmonitor.sh +++ b/t/t7519-status-fsmonitor.sh @@ -346,4 +346,12 @@ test_expect_success UNTRACKED_CACHE 'ignore .git changes when invalidating UNTR' test_cmp before after ' +test_expect_failure 'discard_index() also discards fsmonitor info' ' + test_config core.fsmonitor "$TEST_DIRECTORY/t7519/fsmonitor-all" && + test_might_fail git update-index --refresh && + test-tool read-cache --print-and-refresh=tracked 2 >actual && + printf "tracked is%s up to date\n" "" " not" >expect && + test_cmp expect actual +' + test_done From 398a3b0899dd8a440d4adbcbda38362e3f8359b1 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Tue, 7 May 2019 13:10:21 +0200 Subject: [PATCH 24/66] fsmonitor: force a refresh after the index was discarded With this change, the `index_state` struct becomes the new home for the flag that says whether the fsmonitor hook has been run, i.e. it is now per-index. It also gets re-set when the index is discarded, fixing the bug demonstrated by the "test_expect_failure" test added in the preceding commit. In that case fsmonitor-enabled Git would miss updates under certain circumstances, see that preceding commit for details. Signed-off-by: Johannes Schindelin Signed-off-by: Junio C Hamano --- cache.h | 3 ++- fsmonitor.c | 5 ++--- read-cache.c | 1 + t/t7519-status-fsmonitor.sh | 2 +- 4 files changed, 6 insertions(+), 5 deletions(-) diff --git a/cache.h b/cache.h index 27fe635f62..06ca755c93 100644 --- a/cache.h +++ b/cache.h @@ -338,7 +338,8 @@ struct index_state { struct cache_time timestamp; unsigned name_hash_initialized : 1, initialized : 1, - drop_cache_tree : 1; + drop_cache_tree : 1, + fsmonitor_has_run_once : 1; struct hashmap name_hash; struct hashmap dir_hash; struct object_id oid; diff --git a/fsmonitor.c b/fsmonitor.c index 665bd2d425..1dee0aded1 100644 --- a/fsmonitor.c +++ b/fsmonitor.c @@ -129,7 +129,6 @@ static void fsmonitor_refresh_callback(struct index_state *istate, const char *n void refresh_fsmonitor(struct index_state *istate) { - static int has_run_once = 0; struct strbuf query_result = STRBUF_INIT; int query_success = 0; size_t bol; /* beginning of line */ @@ -137,9 +136,9 @@ void refresh_fsmonitor(struct index_state *istate) char *buf; int i; - if (!core_fsmonitor || has_run_once) + if (!core_fsmonitor || istate->fsmonitor_has_run_once) return; - has_run_once = 1; + istate->fsmonitor_has_run_once = 1; trace_printf_key(&trace_fsmonitor, "refresh fsmonitor"); /* diff --git a/read-cache.c b/read-cache.c index 0e0c93edc9..b298c7f535 100644 --- a/read-cache.c +++ b/read-cache.c @@ -2307,6 +2307,7 @@ int discard_index(struct index_state *istate) free_name_hash(istate); cache_tree_free(&(istate->cache_tree)); istate->initialized = 0; + istate->fsmonitor_has_run_once = 0; FREE_AND_NULL(istate->cache); istate->cache_alloc = 0; discard_split_index(istate); diff --git a/t/t7519-status-fsmonitor.sh b/t/t7519-status-fsmonitor.sh index afd8fa7532..81a375fa0f 100755 --- a/t/t7519-status-fsmonitor.sh +++ b/t/t7519-status-fsmonitor.sh @@ -346,7 +346,7 @@ test_expect_success UNTRACKED_CACHE 'ignore .git changes when invalidating UNTR' test_cmp before after ' -test_expect_failure 'discard_index() also discards fsmonitor info' ' +test_expect_success 'discard_index() also discards fsmonitor info' ' test_config core.fsmonitor "$TEST_DIRECTORY/t7519/fsmonitor-all" && test_might_fail git update-index --refresh && test-tool read-cache --print-and-refresh=tracked 2 >actual && From 5c3d5a38231c2c8c5414232cfbb662b6610662b1 Mon Sep 17 00:00:00 2001 From: Mike Hommey Date: Wed, 8 May 2019 08:03:54 +0900 Subject: [PATCH 25/66] Make fread/fwrite-like functions in http.c more like fread/fwrite. The fread/fwrite-like functions in http.c, namely fread_buffer, fwrite_buffer, fwrite_null, fwrite_sha1_file all return the multiplication of the size and number of items they are being given. Practically speaking, it doesn't matter, because in all contexts where those functions are used, size is 1. But those functions being similar to fread and fwrite (the curl API is designed around being able to use fread and fwrite directly), it might be preferable to make them behave like fread and fwrite, which, from the fread/fwrite manual page, is: On success, fread() and fwrite() return the number of items read or written. This number equals the number of bytes transferred only when size is 1. If an error occurs, or the end of the file is reached, the return value is a short item count (or zero). Signed-off-by: Mike Hommey Signed-off-by: Junio C Hamano --- http.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/http.c b/http.c index 98fb06df0b..27aa0a3192 100644 --- a/http.c +++ b/http.c @@ -176,7 +176,7 @@ size_t fread_buffer(char *ptr, size_t eltsize, size_t nmemb, void *buffer_) memcpy(ptr, buffer->buf.buf + buffer->posn, size); buffer->posn += size; - return size; + return size / eltsize; } #ifndef NO_CURL_IOCTL @@ -204,12 +204,12 @@ size_t fwrite_buffer(char *ptr, size_t eltsize, size_t nmemb, void *buffer_) struct strbuf *buffer = buffer_; strbuf_add(buffer, ptr, size); - return size; + return nmemb; } size_t fwrite_null(char *ptr, size_t eltsize, size_t nmemb, void *strbuf) { - return eltsize * nmemb; + return nmemb; } static void closedown_active_slot(struct active_request_slot *slot) @@ -2319,14 +2319,14 @@ static size_t fwrite_sha1_file(char *ptr, size_t eltsize, size_t nmemb, BUG("curl_easy_getinfo for HTTP code failed: %s", curl_easy_strerror(c)); if (slot->http_code >= 300) - return size; + return nmemb; } do { ssize_t retval = xwrite(freq->localfile, (char *) ptr + posn, size - posn); if (retval < 0) - return posn; + return posn / eltsize; posn += retval; } while (posn < size); @@ -2339,7 +2339,7 @@ static size_t fwrite_sha1_file(char *ptr, size_t eltsize, size_t nmemb, the_hash_algo->update_fn(&freq->c, expn, sizeof(expn) - freq->stream.avail_out); } while (freq->stream.avail_in && freq->zret == Z_OK); - return size; + return nmemb; } struct http_object_request *new_http_object_request(const char *base_url, From d9ef573837f4054b26f792c0ffd7cbb662cb38a6 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Tue, 7 May 2019 18:30:46 -0400 Subject: [PATCH 26/66] t/lib-httpd: pass LSAN_OPTIONS through apache Just as we instruct Apache to pass through ASAN_OPTIONS (so that server-side Git programs it spawns will respect our options while running the tests), we should do the same with LSAN_OPTIONS. Otherwise trying to collect a list of leaks like: export LSAN_OPTIONS=exitcode=0:log_path=/tmp/lsan make SANITIZE=leak test won't work for http tests (the server-side programs won't log their leaks to the right place, and they'll prematurely die, producing a spurious test failure). Signed-off-by: Jeff King Acked-by: Josh Steadmon Signed-off-by: Junio C Hamano --- t/lib-httpd/apache.conf | 1 + 1 file changed, 1 insertion(+) diff --git a/t/lib-httpd/apache.conf b/t/lib-httpd/apache.conf index 06a81b54c7..5c1c86c193 100644 --- a/t/lib-httpd/apache.conf +++ b/t/lib-httpd/apache.conf @@ -76,6 +76,7 @@ PassEnv GIT_VALGRIND PassEnv GIT_VALGRIND_OPTIONS PassEnv GNUPGHOME PassEnv ASAN_OPTIONS +PassEnv LSAN_OPTIONS PassEnv GIT_TRACE PassEnv GIT_CONFIG_NOSYSTEM PassEnv GIT_TEST_SIDEBAND_ALL From c871fbee2b7aaa3458f422a6d69a321cbfd9808a Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Tue, 7 May 2019 14:51:30 -0700 Subject: [PATCH 27/66] t6500(mingw): use the Windows PID of the shell In Git for Windows, we use the MSYS2 Bash which inherits a non-standard PID model from Cygwin's POSIX emulation layer: every MSYS2 process has a regular Windows PID, and in addition it has an MSYS2 PID (which corresponds to a shadow process that emulates Unix-style signal handling). With the upgrade to the MSYS2 runtime v3.x, this shadow process cannot be accessed via `OpenProcess()` any longer, and therefore t6500 thought incorrectly that the process referenced in `gc.pid` (which is not actually a real `gc` process in this context, but the current shell) no longer exists. Let's fix this by making sure that the Windows PID is written into `gc.pid` in this test script so that `git.exe` is able to understand that that process does indeed still exist. Signed-off-by: Johannes Schindelin Signed-off-by: Junio C Hamano --- t/t6500-gc.sh | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/t/t6500-gc.sh b/t/t6500-gc.sh index 4684d06552..53258d45a1 100755 --- a/t/t6500-gc.sh +++ b/t/t6500-gc.sh @@ -162,7 +162,15 @@ test_expect_success 'background auto gc respects lock for all operations' ' # now fake a concurrent gc that holds the lock; we can use our # shell pid so that it looks valid. hostname=$(hostname || echo unknown) && - printf "$$ %s" "$hostname" >.git/gc.pid && + shell_pid=$$ && + if test_have_prereq MINGW && test -f /proc/$shell_pid/winpid + then + # In Git for Windows, Bash (actually, the MSYS2 runtime) has a + # different idea of PIDs than git.exe (actually Windows). Use + # the Windows PID in this case. + shell_pid=$(cat /proc/$shell_pid/winpid) + fi && + printf "%d %s" "$shell_pid" "$hostname" >.git/gc.pid && # our gc should exit zero without doing anything run_and_wait_for_auto_gc && From bcb4edf7af7f10878dd75ccfc3fc0f7596f2d658 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Wed, 8 May 2019 03:07:54 -0400 Subject: [PATCH 28/66] coccicheck: make batch size of 0 mean "unlimited" If you have the memory to handle it, the ideal case is to run a single spatch invocation with all of the source files. But the only way to do so now is to pick an arbitrarily large batch size. Let's make "0" do this, which is a little friendlier (and doesn't otherwise have a useful meaning). Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- Makefile | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/Makefile b/Makefile index daba958b8f..9cea614523 100644 --- a/Makefile +++ b/Makefile @@ -1176,6 +1176,7 @@ SP_EXTRA_FLAGS = # For the 'coccicheck' target; setting SPATCH_BATCH_SIZE higher will # usually result in less CPU usage at the cost of higher peak memory. +# Setting it to 0 will feed all files in a single spatch invocation. SPATCH_FLAGS = --all-includes --patch . SPATCH_BATCH_SIZE = 1 @@ -2792,7 +2793,12 @@ endif %.cocci.patch: %.cocci $(COCCI_SOURCES) @echo ' ' SPATCH $<; \ - if ! echo $(COCCI_SOURCES) | xargs -n $(SPATCH_BATCH_SIZE) \ + if test $(SPATCH_BATCH_SIZE) = 0; then \ + limit=; \ + else \ + limit='-n $(SPATCH_BATCH_SIZE)'; \ + fi; \ + if ! echo $(COCCI_SOURCES) | xargs $$limit \ $(SPATCH) --sp-file $< $(SPATCH_FLAGS) \ >$@+ 2>$@.log; \ then \ From 1a8787144ddec30f29e177cdae6bd7cea3ba88fe Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nguy=E1=BB=85n=20Th=C3=A1i=20Ng=E1=BB=8Dc=20Duy?= Date: Tue, 7 May 2019 16:50:37 +0700 Subject: [PATCH 29/66] submodule--helper: add a missing \n MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This is a complete line. We're not expecting the next function to add anything to the same line. Signed-off-by: Nguyễn Thái Ngọc Duy Signed-off-by: Junio C Hamano --- builtin/submodule--helper.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c index b80fc4ba3d..163a6b8b99 100644 --- a/builtin/submodule--helper.c +++ b/builtin/submodule--helper.c @@ -1301,7 +1301,7 @@ static int add_possible_reference_from_superproject( die(_("submodule '%s' cannot add alternate: %s"), sas->submodule_name, err.buf); case SUBMODULE_ALTERNATE_ERROR_INFO: - fprintf(stderr, _("submodule '%s' cannot add alternate: %s"), + fprintf_ln(stderr, _("submodule '%s' cannot add alternate: %s"), sas->submodule_name, err.buf); case SUBMODULE_ALTERNATE_ERROR_IGNORE: ; /* nothing */ From 6804ba3a58ff2999533af28ca5af3901e30fd1f2 Mon Sep 17 00:00:00 2001 From: "Chris. Webster" Date: Wed, 31 Oct 2018 23:58:00 +0900 Subject: [PATCH 30/66] diff-highlight: use correct /dev/null for UNIX and Windows Use File::Spec->devnull() for output redirection to avoid messages when Windows version of Perl is first in path. The message 'The system cannot find the path specified.' is displayed each time git is run to get colors. Signed-off-by: Chris. Webster Signed-off-by: Junio C Hamano --- contrib/diff-highlight/DiffHighlight.pm | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/contrib/diff-highlight/DiffHighlight.pm b/contrib/diff-highlight/DiffHighlight.pm index 536754583b..7440aa1c46 100644 --- a/contrib/diff-highlight/DiffHighlight.pm +++ b/contrib/diff-highlight/DiffHighlight.pm @@ -4,6 +4,11 @@ use 5.008; use warnings FATAL => 'all'; use strict; +# Use the correct value for both UNIX and Windows (/dev/null vs nul) +use File::Spec; + +my $NULL = File::Spec->devnull(); + # Highlight by reversing foreground and background. You could do # other things like bold or underline if you prefer. my @OLD_HIGHLIGHT = ( @@ -134,7 +139,7 @@ sub highlight_stdin { # fallback, which means we will work even if git can't be run. sub color_config { my ($key, $default) = @_; - my $s = `git config --get-color $key 2>/dev/null`; + my $s = `git config --get-color $key 2>$NULL`; return length($s) ? $s : $default; } From a54b2ab345c4e50e2adfbdeab7d716f5be77a6b3 Mon Sep 17 00:00:00 2001 From: Denton Liu Date: Wed, 8 May 2019 15:16:41 -0400 Subject: [PATCH 31/66] tag: fix typo in nested tagging hint In eea9c1e78f (tag: advise on nested tags, 2019-04-04), tag was taught to hint at the user if a nested tag is made. However, this message had a typo and it said "The object referred to by your new is...", which was missing a "tag" after "new". Fix this message by adding the "tag". Signed-off-by: Denton Liu Signed-off-by: Junio C Hamano --- builtin/tag.c | 2 +- t/t7004-tag.sh | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/builtin/tag.c b/builtin/tag.c index 32948fade0..16b59c45cf 100644 --- a/builtin/tag.c +++ b/builtin/tag.c @@ -207,7 +207,7 @@ struct create_tag_options { }; static const char message_advice_nested_tag[] = - N_("You have created a nested tag. The object referred to by your new is\n" + N_("You have created a nested tag. The object referred to by your new tag is\n" "already a tag. If you meant to tag the object that it points to, use:\n" "\n" "\tgit tag -f %s %s^{}"); diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh index e285686662..6aeeb279a0 100755 --- a/t/t7004-tag.sh +++ b/t/t7004-tag.sh @@ -1702,7 +1702,7 @@ test_expect_success '--points-at finds annotated tags of tags' ' test_expect_success 'recursive tagging should give advice' ' sed -e "s/|$//" <<-EOF >expect && - hint: You have created a nested tag. The object referred to by your new is + hint: You have created a nested tag. The object referred to by your new tag is hint: already a tag. If you meant to tag the object that it points to, use: hint: | hint: git tag -f nested annotated-v4.0^{} From f3a3a021c716b46ed35e6b7171bbff4d8042da68 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Wed, 8 May 2019 14:52:41 -0700 Subject: [PATCH 32/66] difftool --no-index: error out on --dir-diff (and don't crash) In `--no-index` mode, we now no longer require a worktree nor a repository. But some code paths in `difftool` expect those to be present. The most notable such code path is the `--dir-diff` one: we use the existing checkout machinery to copy the files, and that machinery looks up replacement refs, looks at alternate ODBs, wants to use the worktree path, etc. Rather than running into segmentation faults, let's die with an informative error message. Signed-off-by: Johannes Schindelin Signed-off-by: Junio C Hamano --- builtin/difftool.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/builtin/difftool.c b/builtin/difftool.c index 4fff1e83f9..5704a76088 100644 --- a/builtin/difftool.c +++ b/builtin/difftool.c @@ -735,7 +735,8 @@ int cmd_difftool(int argc, const char **argv, const char *prefix) setup_work_tree(); setenv(GIT_DIR_ENVIRONMENT, absolute_path(get_git_dir()), 1); setenv(GIT_WORK_TREE_ENVIRONMENT, absolute_path(get_git_work_tree()), 1); - } + } else if (dir_diff) + die(_("--dir-diff is incompatible with --no-index")); if (use_gui_tool && diff_gui_tool && *diff_gui_tool) setenv("GIT_DIFF_TOOL", diff_gui_tool, 1); From 598b6c3a9270e7bd2379fc6084a6ddf694c0f8ea Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C4=B0smail=20D=C3=B6nmez?= Date: Wed, 8 May 2019 04:30:58 -0700 Subject: [PATCH 33/66] mingw: do not let ld strip relocations MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This is the first step for enabling ASLR (Address Space Layout Randomization) support. We want to enable ASLR for better protection against exploiting security holes in Git: it makes it harder to attack software by making code addresses unpredictable. The problem fixed by this commit is that `ld.exe` seems to be stripping relocations which in turn will break ASLR support. We just make sure it's not stripping the main executable entry. Signed-off-by: İsmail Dönmez Signed-off-by: Johannes Schindelin Signed-off-by: Junio C Hamano --- config.mak.uname | 2 ++ 1 file changed, 2 insertions(+) diff --git a/config.mak.uname b/config.mak.uname index d916d1dc7a..01b390c043 100644 --- a/config.mak.uname +++ b/config.mak.uname @@ -578,10 +578,12 @@ else ifeq (MINGW32,$(MSYSTEM)) prefix = /mingw32 HOST_CPU = i686 + BASIC_LDFLAGS += -Wl,--pic-executable,-e,_mainCRTStartup endif ifeq (MINGW64,$(MSYSTEM)) prefix = /mingw64 HOST_CPU = x86_64 + BASIC_LDFLAGS += -Wl,--pic-executable,-e,mainCRTStartup else COMPAT_CFLAGS += -D_USE_32BIT_TIME_T BASIC_LDFLAGS += -Wl,--large-address-aware From ce6a158561f906bfd48ab7a9c7a4c48134844e85 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C4=B0smail=20D=C3=B6nmez?= Date: Wed, 8 May 2019 04:30:59 -0700 Subject: [PATCH 34/66] mingw: enable DEP and ASLR MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Enable DEP (Data Execution Prevention) and ASLR (Address Space Layout Randomization) support. This applies to both 32bit and 64bit builds and makes it substantially harder to exploit security holes in Git by offering a much more unpredictable attack surface. ASLR interferes with GDB's ability to set breakpoints. A similar issue holds true when compiling with -O2 (in which case single-stepping is messed up because GDB cannot map the code back to the original source code properly). Therefore we simply enable ASLR only when an optimization flag is present in the CFLAGS, using it as an indicator that the developer does not want to debug in GDB anyway. Signed-off-by: İsmail Dönmez Signed-off-by: Johannes Schindelin Signed-off-by: Junio C Hamano --- config.mak.uname | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/config.mak.uname b/config.mak.uname index 01b390c043..6f92f4746e 100644 --- a/config.mak.uname +++ b/config.mak.uname @@ -575,6 +575,12 @@ else ifneq ($(shell expr "$(uname_R)" : '1\.'),2) # MSys2 prefix = /usr/ + # Enable DEP + BASIC_LDFLAGS += -Wl,--nxcompat + # Enable ASLR (unless debugging) + ifneq (,$(findstring -O,$(filter-out -O0 -Og,$(CFLAGS)))) + BASIC_LDFLAGS += -Wl,--dynamicbase + endif ifeq (MINGW32,$(MSYSTEM)) prefix = /mingw32 HOST_CPU = i686 From 142997d4895b2b39ade3560da774f181f0131086 Mon Sep 17 00:00:00 2001 From: Eric Sunshine Date: Fri, 10 May 2019 20:18:53 -0400 Subject: [PATCH 35/66] check-non-portable-shell: support Perl versions older than 5.10 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit For thoroughness when checking for one-shot environment variable assignments at shell function call sites, check-non-portable-shell stitches together incomplete lines (those ending with backslash). This allows it to correctly flag such undesirable usage even when the variable assignment and function call are split across lines, for example: FOO=bar \ func where 'func' is a shell function. The stitching is accomplished like this: while (<>) { chomp; # stitch together incomplete lines (those ending with "\") while (s/\\$//) { $_ .= readline; chomp; } # detect unportable/undesirable shell constructs ... } Although this implementation is well supported in reasonably modern Perl versions (5.10 and later), it fails with older versions (such as Perl 5.8 shipped with ancient Mac OS 10.5). In particular, in older Perl versions, 'readline' is not connected to the file handle associated with the "magic" while (<>) {...} construct, so 'readline' throws a "readline() on unopened filehandle" error. Work around this problem by dropping readline() and instead incorporating the stitching of incomplete lines directly into the existing while (<>) {...} loop. Helped-by: Ævar Arnfjörð Bjarmason Signed-off-by: Eric Sunshine Signed-off-by: Junio C Hamano --- t/check-non-portable-shell.pl | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/t/check-non-portable-shell.pl b/t/check-non-portable-shell.pl index 166d64d4a2..38bfeebd88 100755 --- a/t/check-non-portable-shell.pl +++ b/t/check-non-portable-shell.pl @@ -27,14 +27,14 @@ for my $i (@ARGV) { close $f; } +my $line = ''; while (<>) { chomp; + $line .= $_; # stitch together incomplete lines (those ending with "\") - while (s/\\$//) { - $_ .= readline; - chomp; - } + next if $line =~ s/\\$//; + $_ = $line; /\bcp\s+-a/ and err 'cp -a is not portable'; /\bsed\s+-[^efn]\s+/ and err 'sed option not portable (use only -n, -e, -f)'; /\becho\s+-[neE]/ and err 'echo with option is not portable (use printf)'; @@ -48,6 +48,7 @@ while (<>) { /\bexport\s+[A-Za-z0-9_]*=/ and err '"export FOO=bar" is not portable (use FOO=bar && export FOO)'; /^\s*([A-Z0-9_]+=(\w+|(["']).*?\3)\s+)+(\w+)/ and exists($func{$4}) and err '"FOO=bar shell_func" assignment extends beyond "shell_func"'; + $line = ''; # this resets our $. for each file close ARGV if eof; } From 7fef4e3e78bfd103282452b8714b359c8a9ab3bb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86var=20Arnfj=C3=B6r=C3=B0=20Bjarmason?= Date: Fri, 10 May 2019 15:37:38 +0200 Subject: [PATCH 36/66] trace2: fix up a missing "leave" entry point MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fix a trivial bug that's been here since the shared/do_write_index tracing was added in 42fee7a388 ("trace2:data: add trace2 instrumentation to index read/write", 2019-02-22). We should have enter/leave points, not two enter/enter points. This resulted in an "enter" event without a corresponding "leave" event. Signed-off-by: Ævar Arnfjörð Bjarmason Acked-by: Derrick Stolee Signed-off-by: Junio C Hamano --- read-cache.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/read-cache.c b/read-cache.c index eee8351d8e..49329a1fb1 100644 --- a/read-cache.c +++ b/read-cache.c @@ -3127,7 +3127,7 @@ static int write_shared_index(struct index_state *istate, trace2_region_enter_printf("index", "shared/do_write_index", the_repository, "%s", (*temp)->filename.buf); ret = do_write_index(si->base, *temp, 1); - trace2_region_enter_printf("index", "shared/do_write_index", + trace2_region_leave_printf("index", "shared/do_write_index", the_repository, "%s", (*temp)->filename.buf); if (ret) From 225a777af9b802e1f8516031b5fbf08e4d5cb48a Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Fri, 10 May 2019 13:23:14 -0700 Subject: [PATCH 37/66] status: fix display of rebase -ir's `label` command The argument of a `label` command does *not* want to be turned into an abbreviated SHA-1. Signed-off-by: Johannes Schindelin Signed-off-by: Junio C Hamano --- wt-status.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/wt-status.c b/wt-status.c index f4fa982638..f9fba796fb 100644 --- a/wt-status.c +++ b/wt-status.c @@ -1214,7 +1214,9 @@ static void abbrev_sha1_in_line(struct strbuf *line) int i; if (starts_with(line->buf, "exec ") || - starts_with(line->buf, "x ")) + starts_with(line->buf, "x ") || + starts_with(line->buf, "label ") || + starts_with(line->buf, "l ")) return; split = strbuf_split_max(line, ' ', 3); From 04b7e86e4854fecc8cca1d3c08af5cfe771e9091 Mon Sep 17 00:00:00 2001 From: Derrick Stolee Date: Fri, 10 May 2019 12:44:26 -0700 Subject: [PATCH 38/66] trace2: add variable description to git.txt MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Documentation/technical/api-trace2.txt contains the full details of the trace2 API and the GIT_TR2* environment variables. However, most environment variables are included in Documentation/git.txt, including the GIT_TRACE* variables. Add a brief description of the GIT_TR2* variables with links to the full technical details. The biggest difference from the original variables is that we can specify a Unix Domain Socket. Mention this difference, but leave the details to the technical documents. Reported-by: Szeder Gábor Signed-off-by: Derrick Stolee Signed-off-by: Junio C Hamano --- Documentation/git.txt | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/Documentation/git.txt b/Documentation/git.txt index 6d1f2fd9ae..72adfcc5e2 100644 --- a/Documentation/git.txt +++ b/Documentation/git.txt @@ -660,6 +660,27 @@ of clones and fetches. When a curl trace is enabled (see `GIT_TRACE_CURL` above), do not dump data (that is, only dump info lines and headers). +`GIT_TR2`:: + Enables more detailed trace messages from the "trace2" library. + Output from `GIT_TR2` is a simple text-based format for human + readability. ++ +The `GIT_TR2` variables can take many values. Any value available to +the `GIT_TRACE` variables is also available to `GIT_TR2`. The `GIT_TR2` +variables can also specify a Unix Domain Socket. See +link:technical/api-trace2.html[Trace2 documentation] for full details. + +`GIT_TR2_EVENT`:: + This setting writes a JSON-based format that is suited for machine + interpretation. See link:technical/api-trace2.html[Trace2 documentation] + for full details. + +`GIT_TR2_PERF`:: + In addition to the text-based messages available in `GIT_TR2`, this + setting writes a column-based format for understanding nesting + regions. See link:technical/api-trace2.html[Trace2 documentation] + for full details. + `GIT_REDACT_COOKIES`:: This can be set to a comma-separated list of strings. When a curl trace is enabled (see `GIT_TRACE_CURL` above), whenever a "Cookies:" header From 05fb8726cccc74908853c166248ff9b6abdafae5 Mon Sep 17 00:00:00 2001 From: Denton Liu Date: Mon, 29 Apr 2019 02:21:08 -0400 Subject: [PATCH 39/66] mergetool: use get_merge_tool function In git-mergetool, the logic for getting which merge tool to use is duplicated in git-mergetool--lib, except for the fact that it needs to know whether the tool was guessed or not. Rewrite `get_merge_tool` to return whether or not the tool was guessed through the return code and make git-mergetool call this function instead of duplicating the logic. Note that 1 was chosen to be the return code of when a tool is guessed because it seems like a slightly more abnormal condition than getting a tool that's explicitly specified but this is completely arbitrary. Also, let `$GIT_MERGETOOL_GUI` be set to determine whether or not the guitool will be selected. This change is not completely backwards compatible as there may be external users of git-mergetool--lib. However, only one user, git-diffall[1], was found from searching GitHub and Google, and this tool is superseded by `git difftool --dir-diff` anyway. It seems very unlikely that there exists an external caller that would take into account the return code of `get_merge_tool` as it would always return 0 before this change so this change probably does not affect any external users. [1]: https://github.com/thenigan/git-diffall Signed-off-by: Denton Liu Signed-off-by: Junio C Hamano --- Documentation/git-mergetool--lib.txt | 4 +++- git-difftool--helper.sh | 2 +- git-mergetool--lib.sh | 5 ++++- git-mergetool.sh | 12 ++++-------- 4 files changed, 12 insertions(+), 11 deletions(-) diff --git a/Documentation/git-mergetool--lib.txt b/Documentation/git-mergetool--lib.txt index 055550b2bc..4da9d24096 100644 --- a/Documentation/git-mergetool--lib.txt +++ b/Documentation/git-mergetool--lib.txt @@ -28,7 +28,9 @@ to define the operation mode for the functions listed below. FUNCTIONS --------- get_merge_tool:: - returns a merge tool. + returns a merge tool. the return code is 1 if we returned a guessed + merge tool, else 0. '$GIT_MERGETOOL_GUI' may be set to 'true' to + search for the appropriate guitool. get_merge_tool_cmd:: returns the custom command for a merge tool. diff --git a/git-difftool--helper.sh b/git-difftool--helper.sh index 7bfb6737df..46af3e60b7 100755 --- a/git-difftool--helper.sh +++ b/git-difftool--helper.sh @@ -71,7 +71,7 @@ then then merge_tool="$GIT_DIFF_TOOL" else - merge_tool="$(get_merge_tool)" || exit + merge_tool="$(get_merge_tool)" fi fi diff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh index 83bf52494c..b928179a2e 100644 --- a/git-mergetool--lib.sh +++ b/git-mergetool--lib.sh @@ -403,14 +403,17 @@ get_merge_tool_path () { } get_merge_tool () { + is_guessed=false # Check if a merge tool has been configured - merge_tool=$(get_configured_merge_tool) + merge_tool=$(get_configured_merge_tool $GIT_MERGETOOL_GUI) # Try to guess an appropriate merge tool if no tool has been set. if test -z "$merge_tool" then merge_tool=$(guess_merge_tool) || exit + is_guessed=true fi echo "$merge_tool" + test "$is_guessed" = false } mergetool_find_win32_cmd () { diff --git a/git-mergetool.sh b/git-mergetool.sh index 01b9ad59b2..88fa6a914a 100755 --- a/git-mergetool.sh +++ b/git-mergetool.sh @@ -389,7 +389,7 @@ print_noop_and_exit () { main () { prompt=$(git config --bool mergetool.prompt) - gui_tool=false + GIT_MERGETOOL_GUI=false guessed_merge_tool=false orderfile= @@ -416,10 +416,10 @@ main () { esac ;; --no-gui) - gui_tool=false + GIT_MERGETOOL_GUI=false ;; -g|--gui) - gui_tool=true + GIT_MERGETOOL_GUI=true ;; -y|--no-prompt) prompt=false @@ -449,12 +449,8 @@ main () { if test -z "$merge_tool" then - # Check if a merge tool has been configured - merge_tool=$(get_configured_merge_tool $gui_tool) - # Try to guess an appropriate merge tool if no tool has been set. - if test -z "$merge_tool" + if ! merge_tool=$(get_merge_tool) then - merge_tool=$(guess_merge_tool) || exit guessed_merge_tool=true fi fi From 884630b2e2634969656faabc7f4e33cab2e32b35 Mon Sep 17 00:00:00 2001 From: Denton Liu Date: Mon, 29 Apr 2019 02:21:11 -0400 Subject: [PATCH 40/66] mergetool--lib: create gui_mode function Before, in `get_configured_merge_tool`, we would test the value of the first argument directly, which corresponded to whether we were using guitool. However, since `$GIT_MERGETOOL_GUI` is available as an environment variable, create the `gui_mode` function which increases the clarify of functions which use it. While we're at it, add a space before `()` in function definitions to fix the style. Signed-off-by: Denton Liu Signed-off-by: Junio C Hamano --- git-mergetool--lib.sh | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh index b928179a2e..4ca170c8a7 100644 --- a/git-mergetool--lib.sh +++ b/git-mergetool--lib.sh @@ -80,14 +80,18 @@ show_tool_names () { } } -diff_mode() { +diff_mode () { test "$TOOL_MODE" = diff } -merge_mode() { +merge_mode () { test "$TOOL_MODE" = merge } +gui_mode () { + test "$GIT_MERGETOOL_GUI" = true +} + translate_merge_tool_path () { echo "$1" } @@ -350,8 +354,7 @@ guess_merge_tool () { } get_configured_merge_tool () { - # If first argument is true, find the guitool instead - if test "$1" = true + if gui_mode then gui_prefix=gui fi @@ -405,7 +408,7 @@ get_merge_tool_path () { get_merge_tool () { is_guessed=false # Check if a merge tool has been configured - merge_tool=$(get_configured_merge_tool $GIT_MERGETOOL_GUI) + merge_tool=$(get_configured_merge_tool) # Try to guess an appropriate merge tool if no tool has been set. if test -z "$merge_tool" then From 60aced3dfa68df60952fed28c4ae63a5bbda0275 Mon Sep 17 00:00:00 2001 From: Denton Liu Date: Mon, 29 Apr 2019 02:21:14 -0400 Subject: [PATCH 41/66] mergetool: fallback to tool when guitool unavailable In git-difftool, if the tool is called with --gui but `diff.guitool` is not set, it falls back to `diff.tool`. Make git-mergetool also fallback from `merge.guitool` to `merge.tool` if the former is undefined. If git-difftool, when called with `--gui`, were to use `get_configured_mergetool` in a future patch, it would also get the fallback behavior in the following precedence: 1. diff.guitool 2. merge.guitool 3. diff.tool 4. merge.tool The behavior for when difftool or mergetool are called without `--gui` should be identical with or without this patch. Note that the search loop could be written as sections="merge" keys="tool" if diff_mode then sections="diff $sections" fi if gui_mode then keys="guitool $keys" fi merge_tool=$( IFS=' ' for key in $keys do for section in $sections do selected=$(git config $section.$key) if test -n "$selected" then echo "$selected" return fi done done) which would make adding a mode in the future much easier. However, adding a new mode will likely never happen as it is highly discouraged so, as a result, it is written in its current form so that it is more readable for future readers. Signed-off-by: Denton Liu Signed-off-by: Junio C Hamano --- Documentation/git-mergetool.txt | 4 +++- git-mergetool--lib.sh | 35 ++++++++++++++++++++++++--------- t/t7610-mergetool.sh | 19 ++++++++++++++++++ 3 files changed, 48 insertions(+), 10 deletions(-) diff --git a/Documentation/git-mergetool.txt b/Documentation/git-mergetool.txt index 0c7975a050..6b14702e78 100644 --- a/Documentation/git-mergetool.txt +++ b/Documentation/git-mergetool.txt @@ -83,7 +83,9 @@ success of the resolution after the custom tool has exited. --gui:: When 'git-mergetool' is invoked with the `-g` or `--gui` option the default merge tool will be read from the configured - `merge.guitool` variable instead of `merge.tool`. + `merge.guitool` variable instead of `merge.tool`. If + `merge.guitool` is not set, we will fallback to the tool + configured under `merge.tool`. --no-gui:: This overrides a previous `-g` or `--gui` setting and reads the diff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh index 4ca170c8a7..696eb49160 100644 --- a/git-mergetool--lib.sh +++ b/git-mergetool--lib.sh @@ -354,19 +354,36 @@ guess_merge_tool () { } get_configured_merge_tool () { - if gui_mode - then - gui_prefix=gui - fi - - # Diff mode first tries diff.(gui)tool and falls back to merge.(gui)tool. - # Merge mode only checks merge.(gui)tool + keys= if diff_mode then - merge_tool=$(git config diff.${gui_prefix}tool || git config merge.${gui_prefix}tool) + if gui_mode + then + keys="diff.guitool merge.guitool diff.tool merge.tool" + else + keys="diff.tool merge.tool" + fi else - merge_tool=$(git config merge.${gui_prefix}tool) + if gui_mode + then + keys="merge.guitool merge.tool" + else + keys="merge.tool" + fi fi + + merge_tool=$( + IFS=' ' + for key in $keys + do + selected=$(git config $key) + if test -n "$selected" + then + echo "$selected" + return + fi + done) + if test -n "$merge_tool" && ! valid_tool "$merge_tool" then echo >&2 "git config option $TOOL_MODE.${gui_prefix}tool set to unknown tool: $merge_tool" diff --git a/t/t7610-mergetool.sh b/t/t7610-mergetool.sh index dad607e186..5b61c10a9c 100755 --- a/t/t7610-mergetool.sh +++ b/t/t7610-mergetool.sh @@ -167,6 +167,25 @@ test_expect_success 'gui mergetool' ' git commit -m "branch1 resolved with mergetool" ' +test_expect_success 'gui mergetool without merge.guitool set falls back to merge.tool' ' + test_when_finished "git reset --hard" && + git checkout -b test$test_count branch1 && + git submodule update -N && + test_must_fail git merge master && + ( yes "" | git mergetool --gui both ) && + ( yes "" | git mergetool -g file1 file1 ) && + ( yes "" | git mergetool --gui file2 "spaced name" ) && + ( yes "" | git mergetool --gui subdir/file3 ) && + ( yes "d" | git mergetool --gui file11 ) && + ( yes "d" | git mergetool --gui file12 ) && + ( yes "l" | git mergetool --gui submod ) && + test "$(cat file1)" = "master updated" && + test "$(cat file2)" = "master new" && + test "$(cat subdir/file3)" = "master new sub" && + test "$(cat submod/bar)" = "branch1 submodule" && + git commit -m "branch1 resolved with mergetool" +' + test_expect_success 'mergetool crlf' ' test_when_finished "git reset --hard" && # This test_config line must go after the above reset line so that From 7f978d7d10a87c4a56ea3101b936cddb25bbe2c6 Mon Sep 17 00:00:00 2001 From: Denton Liu Date: Mon, 29 Apr 2019 02:21:17 -0400 Subject: [PATCH 42/66] difftool: make --gui, --tool and --extcmd mutually exclusive In git-difftool, these options specify which tool to ultimately run. As a result, they are logically conflicting. Explicitly disallow these options from being used together. Signed-off-by: Denton Liu Signed-off-by: Junio C Hamano --- builtin/difftool.c | 3 +++ t/t7800-difftool.sh | 8 ++++++++ 2 files changed, 11 insertions(+) diff --git a/builtin/difftool.c b/builtin/difftool.c index 544b0e8639..481aa64af5 100644 --- a/builtin/difftool.c +++ b/builtin/difftool.c @@ -730,6 +730,9 @@ int cmd_difftool(int argc, const char **argv, const char *prefix) setenv(GIT_DIR_ENVIRONMENT, absolute_path(get_git_dir()), 1); setenv(GIT_WORK_TREE_ENVIRONMENT, absolute_path(get_git_work_tree()), 1); + if (use_gui_tool + !!difftool_cmd + !!extcmd > 1) + die(_("--gui, --tool and --extcmd are mutually exclusive")); + if (use_gui_tool && diff_gui_tool && *diff_gui_tool) setenv("GIT_DIFF_TOOL", diff_gui_tool, 1); else if (difftool_cmd) { diff --git a/t/t7800-difftool.sh b/t/t7800-difftool.sh index 562bd215a5..833e175bf9 100755 --- a/t/t7800-difftool.sh +++ b/t/t7800-difftool.sh @@ -705,4 +705,12 @@ test_expect_success SYMLINKS 'difftool --dir-diff handles modified symlinks' ' test_cmp expect actual ' +test_expect_success 'difftool --gui, --tool and --extcmd are mutually exclusive' ' + difftool_test_setup && + test_must_fail git difftool --gui --tool=test-tool && + test_must_fail git difftool --gui --extcmd=cat && + test_must_fail git difftool --tool=test-tool --extcmd=cat && + test_must_fail git difftool --gui --tool=test-tool --extcmd=cat +' + test_done From 6c22d715e7b4067a6865ff3fbceab991d0042c12 Mon Sep 17 00:00:00 2001 From: Denton Liu Date: Mon, 29 Apr 2019 02:21:20 -0400 Subject: [PATCH 43/66] difftool: fallback on merge.guitool In git-difftool.txt, it says 'git difftool' falls back to 'git mergetool' config variables when the difftool equivalents have not been defined. However, when `diff.guitool` is missing, it doesn't fallback to anything. Make git-difftool fallback to `merge.guitool` when `diff.guitool` is missing. Signed-off-by: Denton Liu Signed-off-by: Junio C Hamano --- Documentation/git-difftool.txt | 4 +++- builtin/difftool.c | 10 ++-------- t/t7800-difftool.sh | 16 ++++++++++++++++ 3 files changed, 21 insertions(+), 9 deletions(-) diff --git a/Documentation/git-difftool.txt b/Documentation/git-difftool.txt index 96c26e6aa8..484c485fd0 100644 --- a/Documentation/git-difftool.txt +++ b/Documentation/git-difftool.txt @@ -90,7 +90,9 @@ instead. `--no-symlinks` is the default on Windows. When 'git-difftool' is invoked with the `-g` or `--gui` option the default diff tool will be read from the configured `diff.guitool` variable instead of `diff.tool`. The `--no-gui` - option can be used to override this setting. + option can be used to override this setting. If `diff.guitool` + is not set, we will fallback in the order of `merge.guitool`, + `diff.tool`, `merge.tool` until a tool is found. --[no-]trust-exit-code:: 'git-difftool' invokes a diff tool individually on each file. diff --git a/builtin/difftool.c b/builtin/difftool.c index 481aa64af5..2205de1214 100644 --- a/builtin/difftool.c +++ b/builtin/difftool.c @@ -23,7 +23,6 @@ #include "object-store.h" #include "dir.h" -static char *diff_gui_tool; static int trust_exit_code; static const char *const builtin_difftool_usage[] = { @@ -33,11 +32,6 @@ static const char *const builtin_difftool_usage[] = { static int difftool_config(const char *var, const char *value, void *cb) { - if (!strcmp(var, "diff.guitool")) { - diff_gui_tool = xstrdup(value); - return 0; - } - if (!strcmp(var, "difftool.trustexitcode")) { trust_exit_code = git_config_bool(var, value); return 0; @@ -733,8 +727,8 @@ int cmd_difftool(int argc, const char **argv, const char *prefix) if (use_gui_tool + !!difftool_cmd + !!extcmd > 1) die(_("--gui, --tool and --extcmd are mutually exclusive")); - if (use_gui_tool && diff_gui_tool && *diff_gui_tool) - setenv("GIT_DIFF_TOOL", diff_gui_tool, 1); + if (use_gui_tool) + setenv("GIT_MERGETOOL_GUI", "true", 1); else if (difftool_cmd) { if (*difftool_cmd) setenv("GIT_DIFF_TOOL", difftool_cmd, 1); diff --git a/t/t7800-difftool.sh b/t/t7800-difftool.sh index 833e175bf9..48b74c0434 100755 --- a/t/t7800-difftool.sh +++ b/t/t7800-difftool.sh @@ -279,11 +279,27 @@ test_expect_success 'difftool + mergetool config variables' ' echo branch >expect && git difftool --no-prompt branch >actual && test_cmp expect actual && + git difftool --gui --no-prompt branch >actual && + test_cmp expect actual && # set merge.tool to something bogus, diff.tool to test-tool test_config merge.tool bogus-tool && test_config diff.tool test-tool && git difftool --no-prompt branch >actual && + test_cmp expect actual && + git difftool --gui --no-prompt branch >actual && + test_cmp expect actual && + + # set merge.tool, diff.tool to something bogus, merge.guitool to test-tool + test_config diff.tool bogus-tool && + test_config merge.guitool test-tool && + git difftool --gui --no-prompt branch >actual && + test_cmp expect actual && + + # set merge.tool, diff.tool, merge.guitool to something bogus, diff.guitool to test-tool + test_config merge.guitool bogus-tool && + test_config diff.guitool test-tool && + git difftool --gui --no-prompt branch >actual && test_cmp expect actual ' From 567fce1e10e77b39203504ad36a273d447ec031a Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Mon, 13 May 2019 15:43:17 -0700 Subject: [PATCH 44/66] parse-options: adjust `parse_opt_unknown_cb()`s declared return type In f41179f16ba2 (parse-options: avoid magic return codes, 2019-01-27), the signature of the low-level parse-opt callback function was changed to return an `enum`. And while the implementations were changed, one declaration was left unchanged, still claiming to return `int`. This can potentially lead to problems, as compilers are free to choose any integral type for an `enum` as long as it can represent all declared values. Signed-off-by: Johannes Schindelin Signed-off-by: Junio C Hamano --- parse-options.h | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/parse-options.h b/parse-options.h index bd00cf0049..cd756833a9 100644 --- a/parse-options.h +++ b/parse-options.h @@ -286,7 +286,9 @@ int parse_opt_commit(const struct option *, const char *, int); int parse_opt_tertiary(const struct option *, const char *, int); int parse_opt_string_list(const struct option *, const char *, int); int parse_opt_noop_cb(const struct option *, const char *, int); -int parse_opt_unknown_cb(struct parse_opt_ctx_t *ctx, const struct option *, const char *, int); +enum parse_opt_result parse_opt_unknown_cb(struct parse_opt_ctx_t *ctx, + const struct option *, + const char *, int); int parse_opt_passthru(const struct option *, const char *, int); int parse_opt_passthru_argv(const struct option *, const char *, int); From 4125f78222749cb8fc91115abec3ac83e5dfb194 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86var=20Arnfj=C3=B6r=C3=B0=20Bjarmason?= Date: Tue, 14 May 2019 00:17:01 +0200 Subject: [PATCH 45/66] sha1dc: update from upstream MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Update sha1dc from the latest version by the upstream maintainer[1]. See 07a20f569b ("Makefile: fix unaligned loads in sha1dc with UBSan", 2019-03-12) for the last update. This fixes an issue where HP-UX IA64 was wrongly detected as a Little-endian instead of a Big-endian system, see [2] and [3]. 1. https://github.com/cr-marcstevens/sha1collisiondetection/commit/855827c583bc30645ba427885caa40c5b81764d2 2. https://public-inbox.org/git/603989bd-f86d-c61d-c6f5-fb6748a65ba9@siemens.com/ 3. https://github.com/cr-marcstevens/sha1collisiondetection/pull/50 Signed-off-by: Ævar Arnfjörð Bjarmason Signed-off-by: Junio C Hamano --- sha1collisiondetection | 2 +- sha1dc/sha1.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/sha1collisiondetection b/sha1collisiondetection index 16033998da..855827c583 160000 --- a/sha1collisiondetection +++ b/sha1collisiondetection @@ -1 +1 @@ -Subproject commit 16033998da4b273aebd92c84b1e1b12e4aaf7009 +Subproject commit 855827c583bc30645ba427885caa40c5b81764d2 diff --git a/sha1dc/sha1.c b/sha1dc/sha1.c index 5931cf25d5..9d3cf81d4d 100644 --- a/sha1dc/sha1.c +++ b/sha1dc/sha1.c @@ -93,7 +93,7 @@ #define SHA1DC_BIGENDIAN /* Not under GCC-alike or glibc or *BSD or newlib or */ -#elif (defined(_AIX)) +#elif (defined(_AIX) || defined(__hpux)) /* * Defines Big Endian on a whitelist of OSs that are known to be Big From 336ad8424cb55905f78ff5e496890be3db13c357 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Tue, 14 May 2019 02:19:15 -0700 Subject: [PATCH 46/66] stash: document stash.useBuiltin The stash.useBuiltin variable introduced in 90a462725e ("stash: optionally use the scripted version again", 2019-02-25) was turned on by default, but had no documentation. Let's document it so that users who run into any stability issues with the C rewrite know there's an escape hatch, and spell out that the user should please report the bug when they have to turn off the built-in stash. Signed-off-by: Johannes Schindelin Signed-off-by: Junio C Hamano --- Documentation/config/stash.txt | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/Documentation/config/stash.txt b/Documentation/config/stash.txt index c583d46d6b..7710758efb 100644 --- a/Documentation/config/stash.txt +++ b/Documentation/config/stash.txt @@ -1,3 +1,18 @@ +stash.useBuiltin:: + Set to `false` to use the legacy shell script implementation of + linkgit:git-stash[1]. Is `true` by default, which means use + the built-in rewrite of it in C. ++ +The C rewrite is first included with Git version 2.22 (and Git for Windows +version 2.19). This option serves an an escape hatch to re-enable the +legacy version in case any bugs are found in the rewrite. This option and +the shell script version of linkgit:git-stash[1] will be removed in some +future release. ++ +If you find some reason to set this option to `false`, other than +one-off testing, you should report the behavior difference as a bug in +Git (see https://git-scm.com/community for details). + stash.showPatch:: If this is set to true, the `git stash show` command without an option will show the stash entry in patch form. Defaults to false. From 9dde06de130463c20f8b603ed3a3ffe10347f2f1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlo=20Marcelo=20Arenas=20Bel=C3=B3n?= Date: Tue, 14 May 2019 14:11:17 -0700 Subject: [PATCH 47/66] http-push: prevent format overflow warning with gcc >= 9 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In function 'finish_request', inlined from 'process_response' at http-push.c:248:2: http-push.c:587:4: warning: '%s' directive argument is null [-Wformat-overflow=] 587 | fprintf(stderr, "Unable to get pack file %s\n%s", | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 588 | request->url, curl_errorstr); | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~ request->url is needed for the error message if there was a failure during fetch but was being cleared unnecessarily earlier. note that the leak is prevented by calling release_request unconditionally at the end. Signed-off-by: Carlo Marcelo Arenas Belón Suggested-by: Eric Sunshine Signed-off-by: Junio C Hamano --- http-push.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/http-push.c b/http-push.c index b22c7caea0..f56cd91d44 100644 --- a/http-push.c +++ b/http-push.c @@ -525,8 +525,8 @@ static void finish_request(struct transfer_request *request) if (request->headers != NULL) curl_slist_free_all(request->headers); - /* URL is reused for MOVE after PUT */ - if (request->state != RUN_PUT) { + /* URL is reused for MOVE after PUT and used during FETCH */ + if (request->state != RUN_PUT && request->state != RUN_FETCH_PACKED) { FREE_AND_NULL(request->url); } From 581d2fd9f2d66ec5cb1859e0b6aef7c459a0d3a9 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Tue, 14 May 2019 09:54:55 -0400 Subject: [PATCH 48/66] get_oid: handle NULL repo->index When get_oid() and its helpers see an index name like ":.gitmodules", they try to load the index on demand, like: if (repo->index->cache) repo_read_index(repo); However, that misses the case when "repo->index" itself is NULL; we'll segfault in the conditional. This never happens with the_repository; there we always point its index field to &the_index. But a submodule repository may have a NULL index field until somebody calls repo_read_index(). This bug is triggered by t7411, but it was hard to notice because it's in an expect_failure block. That test was added by 2b1257e463 (t/helper: add test-submodule-nested-repo-config, 2018-10-25). Back then we had no easy way to access the .gitmodules blob of a submodule repo, so we expected (and got) an error message to that effect. Later, d9b8b8f896 (submodule-config.c: use repo_get_oid for reading .gitmodules, 2019-04-16) started looking in the correct repo, which is when we started triggering the segfault. With this fix, the test starts passing (once we clean it up as its comment instructs). Note that as far as I know, this bug could not be triggered outside of the test suite. It requires resolving an index name in a submodule, and all of the code paths (aside from test-tool) which do that either load the index themselves, or always pass the_repository. Ultimately it comes from 3a7a698e93 (sha1-name.c: remove implicit dependency on the_index, 2019-01-12), which replaced a check of "the_index.cache" with "repo->index->cache". So even if there is another way to trigger it, it wouldn't affect any versions before then. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- sha1-name.c | 2 +- t/t7411-submodule-config.sh | 8 ++------ 2 files changed, 3 insertions(+), 7 deletions(-) diff --git a/sha1-name.c b/sha1-name.c index 775a73d8ad..455e9fb1ea 100644 --- a/sha1-name.c +++ b/sha1-name.c @@ -1837,7 +1837,7 @@ static enum get_oid_result get_oid_with_context_1(struct repository *repo, if (flags & GET_OID_RECORD_PATH) oc->path = xstrdup(cp); - if (!repo->index->cache) + if (!repo->index || !repo->index->cache) repo_read_index(repo); pos = index_name_pos(repo->index, cp, namelen); if (pos < 0) diff --git a/t/t7411-submodule-config.sh b/t/t7411-submodule-config.sh index fcc0fb82d8..ad28e93880 100755 --- a/t/t7411-submodule-config.sh +++ b/t/t7411-submodule-config.sh @@ -243,18 +243,14 @@ test_expect_success 'reading nested submodules config' ' ) ' -# When this test eventually passes, before turning it into -# test_expect_success, remember to replace the test_i18ngrep below with -# a "test_must_be_empty warning" to be sure that the warning is actually -# removed from the code. -test_expect_failure 'reading nested submodules config when .gitmodules is not in the working tree' ' +test_expect_success 'reading nested submodules config when .gitmodules is not in the working tree' ' test_when_finished "git -C super/submodule checkout .gitmodules" && (cd super && echo "./nested_submodule" >expect && rm submodule/.gitmodules && test-tool submodule-nested-repo-config \ submodule submodule.nested_submodule.url >actual 2>warning && - test_i18ngrep "nested submodules without %s in the working tree are not supported yet" warning && + test_must_be_empty warning && test_cmp expect actual ) ' From abd0f289839f6029983a83707f43029c4a2ebf07 Mon Sep 17 00:00:00 2001 From: Todd Zullinger Date: Tue, 14 May 2019 21:36:33 -0400 Subject: [PATCH 49/66] test-lib: try harder to ensure a working jgit The JGIT prereq uses `type jgit` to determine whether jgit is present. While this is usually sufficient, it won't help if the jgit found is badly broken. This wastes time running tests which fail due to no fault of our own. Use `jgit --version` instead, to guard against cases where jgit is present on the system, but will fail to run, e.g. because of some JRE issue, or missing Java dependencies. Checking that it gets far enough to process the '--version' argument isn't perfect, but seems to be good enough in practice. It's also consistent with how we detect some other dependencies, see e.g. the CURL and UNZIP prerequisites. Signed-off-by: Todd Zullinger Signed-off-by: Junio C Hamano --- 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 8665b0a9b6..044f271bef 100644 --- a/t/test-lib.sh +++ b/t/test-lib.sh @@ -1476,7 +1476,7 @@ test_lazy_prereq NOT_ROOT ' ' test_lazy_prereq JGIT ' - type jgit + jgit --version ' # SANITY is about "can you correctly predict what the filesystem would From 56bea280365238f0313f177b66fda6f87049d094 Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Wed, 15 May 2019 10:42:35 +0900 Subject: [PATCH 50/66] pkt-line: drop 'const'-ness of a param to set_packet_header() The function's definition has a paramter of type "int" qualified as "const". The fact that the incoming parameter is used as read-only in the fuction is an implementation detail that the callers should not have to be told in the prototype declaring it (and "const" there has no effect, as C passes parameters by value). The prototype defined for the function in pkt-line.h lacked the matching "const" for this reason, but apparently some compilers (e.g. MS Visual C 2017) complain about the parameter type mismatch. Let's squelch it by removing the "const" that is pointless in the definition of a small and trivial function like this, which would not help optimizing compilers nor reading humans that much. Noticed-by: Johannes Schindelin Helped-by: Jeff King Signed-off-by: Junio C Hamano --- pkt-line.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkt-line.c b/pkt-line.c index c9ed780d0b..a0e87b1e81 100644 --- a/pkt-line.c +++ b/pkt-line.c @@ -119,7 +119,7 @@ void packet_buf_delim(struct strbuf *buf) strbuf_add(buf, "0001", 4); } -void set_packet_header(char *buf, const int size) +void set_packet_header(char *buf, int size) { static char hexchar[] = "0123456789abcdef"; From 8bcd8f4cea9d819da690a545d65dd562409d633a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jean-No=C3=ABl=20Avila?= Date: Fri, 17 May 2019 21:26:19 +0200 Subject: [PATCH 51/66] diff: fix mistake in translatable strings MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Jean-Noël Avila Signed-off-by: Junio C Hamano --- diff.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/diff.c b/diff.c index 4d3cf83a27..6e87f95645 100644 --- a/diff.c +++ b/diff.c @@ -5453,13 +5453,13 @@ static void prep_parse_options(struct diff_options *options) N_("equivalent to --word-diff=color --word-diff-regex="), PARSE_OPT_NONEG | PARSE_OPT_OPTARG, diff_opt_color_words), OPT_CALLBACK_F(0, "color-moved", options, N_(""), - N_("move lines of code are colored differently"), + N_("moved lines of code are colored differently"), PARSE_OPT_OPTARG, diff_opt_color_moved), OPT_CALLBACK_F(0, "color-moved-ws", options, N_(""), N_("how white spaces are ignored in --color-moved"), 0, diff_opt_color_moved_ws), - OPT_GROUP(N_("Diff other options")), + OPT_GROUP(N_("Other diff options")), OPT_CALLBACK_F(0, "relative", options, N_(""), N_("when run from subdir, exclude changes outside and show relative paths"), PARSE_OPT_NONEG | PARSE_OPT_OPTARG, From aa25c82427ae70aebf3b8f970f2afd54e9a2a8c6 Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Sun, 19 May 2019 16:46:42 +0900 Subject: [PATCH 52/66] Git 2.22-rc1 Signed-off-by: Junio C Hamano --- Documentation/RelNotes/2.22.0.txt | 76 +++++++++++++++++++++++++++++++ GIT-VERSION-GEN | 2 +- 2 files changed, 77 insertions(+), 1 deletion(-) diff --git a/Documentation/RelNotes/2.22.0.txt b/Documentation/RelNotes/2.22.0.txt index 114f147fd6..cdfdabe7d9 100644 --- a/Documentation/RelNotes/2.22.0.txt +++ b/Documentation/RelNotes/2.22.0.txt @@ -99,6 +99,18 @@ UI, Workflows & Features repositories now; also the pathname hash-cache is created by default to avoid making crappy deltas when repacking. + * "git branch new A...B" and "git checkout -b new A...B" have been + taught that in their contexts, the notation A...B means "the merge + base between these two commits", just like "git checkout A...B" + detaches HEAD at that commit. + + * Update "git difftool" and "git mergetool" so that the combinations + of {diff,merge}.{tool,guitool} configuration variables serve as + fallback settings of each other in a sensible order. + + * The "--dir-diff" mode of "git difftool" is not useful in "--no-index" + mode; they are now explicitly marked as mutually incompatible. + Performance, Internal Implementation, Development Support etc. @@ -178,6 +190,27 @@ Performance, Internal Implementation, Development Support etc. * The internal implementation of "git rebase -i" has been updated to avoid forking a separate "rebase--interactive" process. + * Allow DEP and ASLR for Windows build to for security hardening. + + * Performance test framework has been broken and measured the version + of Git that happens to be on $PATH, not the specified one to + measure, for a while, which has been corrected. + + * Optionally "make coccicheck" can feed multiple source files to + spatch, gaining performance while spending more memory. + + * Attempt to use an abbreviated option in "git clone --recurs" is + responded by a request to disambiguate between --recursive and + --recurse-submodules, which is bad because these two are synonyms. + The parse-options API has been extended to define such synonyms + more easily and not produce an unnecessary failure. + + * A pair of private functions in http.c that had names similar to + fread/fwrite did not return the number of elements, which was found + to be confusing. + + * Update collision-detecting SHA-1 code to build properly on HP-UX. + Fixes since v2.21 ----------------- @@ -474,6 +507,41 @@ Fixes since v2.21 files used by these commands in such a situation. (merge 4a72486de9 pw/clean-sequencer-state-upon-final-commit later to maint). + * On a filesystem like HFS+, the names of the refs stored as filesystem + entities may become different from what the end-user expects, just + like files in the working tree get "renamed". Work around the + mismatch by paying attention to the core.precomposeUnicode + configuration. + (merge 8e712ef6fc en/unicode-in-refnames later to maint). + + * The code to generate the multi-pack idx file was not prepared to + see too many packfiles and ran out of open file descriptor, which + has been corrected. + + * To run tests for Git SVN, our scripts for CI used to install the + git-svn package (in the hope that it would bring in the right + dependencies). This has been updated to install the more direct + dependency, namely, libsvn-perl. + (merge db864306cf sg/ci-libsvn-perl later to maint). + + * "git cvsexportcommit" running on msys did not expect cvsnt showed + "cvs status" output with CRLF line endings. + + * The fsmonitor interface got out of sync after the in-core index + file gets discarded, which has been corrected. + (merge 398a3b0899 js/fsmonitor-refresh-after-discarding-index later to maint). + + * "git status" did not know that the "label" instruction in the + todo-list "rebase -i -r" uses should not be shown as a hex object + name. + + * A prerequiste check in the test suite to see if a working jgit is + available was made more robust. + (merge abd0f28983 tz/test-lib-check-working-jgit later to maint). + + * The codepath to parse : that obtains the object name for an + indexed object has been made more robust. + * Code cleanup, docfix, build fix, etc. (merge 11f470aee7 jc/test-yes-doc later to maint). (merge 90503a240b js/doc-symref-in-proto-v1 later to maint). @@ -510,3 +578,11 @@ Fixes since v2.21 (merge d8083e4180 km/t3000-retitle later to maint). (merge 9e4cbccbd7 tz/git-svn-doc-markup-fix later to maint). (merge da9ca955a7 jk/ls-files-doc-markup-fix later to maint). + (merge 6804ba3a58 cw/diff-highlight later to maint). + (merge 1a8787144d nd/submodule-helper-incomplete-line-fix later to maint). + (merge d9ef573837 jk/apache-lsan later to maint). + (merge c871fbee2b js/t6500-use-windows-pid-on-mingw later to maint). + (merge ce4c7bfc90 bl/t4253-exit-code-from-format-patch later to maint). + (merge 397a46db78 js/t5580-unc-alternate-test later to maint). + (merge d4907720a2 cm/notes-comment-fix later to maint). + (merge 9dde06de13 cb/http-push-null-in-message-fix later to maint). diff --git a/GIT-VERSION-GEN b/GIT-VERSION-GEN index 8a5009caf9..777a79e2b9 100755 --- a/GIT-VERSION-GEN +++ b/GIT-VERSION-GEN @@ -1,7 +1,7 @@ #!/bin/sh GVF=GIT-VERSION-FILE -DEF_VER=v2.22.0-rc0 +DEF_VER=v2.22.0-rc1 LF=' ' From 8e9fe16c878d15f7a007dbe943e4480787381913 Mon Sep 17 00:00:00 2001 From: Emily Shaffer Date: Wed, 1 May 2019 13:32:17 -0700 Subject: [PATCH 53/66] gitsubmodules: align html and nroff lists There appears to be a bug in the toolchain generating manpages from lettered lists. When a list is enumerated with letters, the resulting nroff shows numbers instead. Mostly this is harmless, but in the case of gitsubmodules, the paragraph following the list refers back to each bullet by letter. As a result, reading this documentation via `man gitsubmodules` is hard to parse - readers must infer that a bug exists and a refers to 1, b refers to 2, and c refers to 3 in the list above. The problem specifically was introduced in ad47194; previously rather than generating numerated lists the bulleted area was entirely monospaced in HTML and shown in plaintext in nroff. The bug seems to exist in docbook-xml - I've reported it on May 1 via the docbook-apps mail list - but for now it may make more sense to just work around the issue. Signed-off-by: Emily Shaffer Reviewed-by: Jonathan Nieder Signed-off-by: Junio C Hamano --- Documentation/gitsubmodules.txt | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/Documentation/gitsubmodules.txt b/Documentation/gitsubmodules.txt index 57999e9f36..0a890205b8 100644 --- a/Documentation/gitsubmodules.txt +++ b/Documentation/gitsubmodules.txt @@ -169,15 +169,15 @@ ACTIVE SUBMODULES A submodule is considered active, - a. if `submodule..active` is set to `true` + 1. if `submodule..active` is set to `true` + or - b. if the submodule's path matches the pathspec in `submodule.active` + 2. if the submodule's path matches the pathspec in `submodule.active` + or - c. if `submodule..url` is set. + 3. if `submodule..url` is set. and these are evaluated in this order. @@ -193,11 +193,11 @@ For example: url = https://example.org/baz In the above config only the submodule 'bar' and 'baz' are active, -'bar' due to (a) and 'baz' due to (c). 'foo' is inactive because -(a) takes precedence over (c) +'bar' due to (1) and 'baz' due to (3). 'foo' is inactive because +(1) takes precedence over (3) -Note that (c) is a historical artefact and will be ignored if the -(a) and (b) specify that the submodule is not active. In other words, +Note that (3) is a historical artefact and will be ignored if the +(1) and (2) specify that the submodule is not active. In other words, if we have a `submodule..active` set to `false` or if the submodule's path is excluded in the pathspec in `submodule.active`, the url doesn't matter whether it is present or not. This is illustrated in From e4b75d6a1d3105958c307e526ae6759e86f7f837 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?SZEDER=20G=C3=A1bor?= Date: Sun, 19 May 2019 16:43:08 +0200 Subject: [PATCH 54/66] trace2: rename environment variables to GIT_TRACE2* MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit For an environment variable that is supposed to be set by users, the GIT_TR2* env vars are just too unclear, inconsistent, and ugly. Most of the established GIT_* environment variables don't use abbreviations, and in case of the few that do (GIT_DIR, GIT_COMMON_DIR, GIT_DIFF_OPTS) it's quite obvious what the abbreviations (DIR and OPTS) stand for. But what does TR stand for? Track, traditional, trailer, transaction, transfer, transformation, transition, translation, transplant, transport, traversal, tree, trigger, truncate, trust, or ...?! The trace2 facility, as the '2' suffix in its name suggests, is supposed to eventually supercede Git's original trace facility. It's reasonable to expect that the corresponding environment variables follow suit, and after the original GIT_TRACE variables they are called GIT_TRACE2; there is no such thing is 'GIT_TR'. All trace2-specific config variables are, very sensibly, in the 'trace2' section, not in 'tr2'. OTOH, we don't gain anything at all by omitting the last three characters of "trace" from the names of these environment variables. So let's rename all GIT_TR2* environment variables to GIT_TRACE2*, before they make their way into a stable release. Signed-off-by: SZEDER Gábor Signed-off-by: Junio C Hamano --- Documentation/config/trace2.txt | 18 +++++----- Documentation/git.txt | 14 ++++---- Documentation/technical/api-trace2.txt | 46 +++++++++++++------------- t/t0001-init.sh | 2 +- t/t0210-trace2-normal.sh | 24 +++++++------- t/t0211-trace2-perf.sh | 20 +++++------ t/t0212-trace2-event.sh | 16 ++++----- t/t0212/parse_events.perl | 2 +- trace2.h | 2 +- trace2/tr2_cmd_name.c | 2 +- trace2/tr2_sid.c | 2 +- trace2/tr2_sysenv.c | 20 +++++------ trace2/tr2_sysenv.h | 2 +- 13 files changed, 85 insertions(+), 85 deletions(-) diff --git a/Documentation/config/trace2.txt b/Documentation/config/trace2.txt index a5f409c1c1..2edbfb02fe 100644 --- a/Documentation/config/trace2.txt +++ b/Documentation/config/trace2.txt @@ -4,17 +4,17 @@ command line arguments are not respected. trace2.normalTarget:: This variable controls the normal target destination. - It may be overridden by the `GIT_TR2` environment variable. + It may be overridden by the `GIT_TRACE2` environment variable. The following table shows possible values. trace2.perfTarget:: This variable controls the performance target destination. - It may be overridden by the `GIT_TR2_PERF` environment variable. + It may be overridden by the `GIT_TRACE2_PERF` environment variable. The following table shows possible values. trace2.eventTarget:: This variable controls the event target destination. - It may be overridden by the `GIT_TR2_EVENT` environment variable. + It may be overridden by the `GIT_TRACE2_EVENT` environment variable. The following table shows possible values. + include::../trace2-target-values.txt[] @@ -22,22 +22,22 @@ include::../trace2-target-values.txt[] trace2.normalBrief:: Boolean. When true `time`, `filename`, and `line` fields are omitted from normal output. May be overridden by the - `GIT_TR2_BRIEF` environment variable. Defaults to false. + `GIT_TRACE2_BRIEF` environment variable. Defaults to false. trace2.perfBrief:: Boolean. When true `time`, `filename`, and `line` fields are omitted from PERF output. May be overridden by the - `GIT_TR2_PERF_BRIEF` environment variable. Defaults to false. + `GIT_TRACE2_PERF_BRIEF` environment variable. Defaults to false. trace2.eventBrief:: Boolean. When true `time`, `filename`, and `line` fields are omitted from event output. May be overridden by the - `GIT_TR2_EVENT_BRIEF` environment variable. Defaults to false. + `GIT_TRACE2_EVENT_BRIEF` environment variable. Defaults to false. trace2.eventNesting:: Integer. Specifies desired depth of nested regions in the event output. Regions deeper than this value will be - omitted. May be overridden by the `GIT_TR2_EVENT_NESTING` + omitted. May be overridden by the `GIT_TRACE2_EVENT_NESTING` environment variable. Defaults to 2. trace2.configParams:: @@ -45,7 +45,7 @@ trace2.configParams:: settings that should be recorded in the trace2 output. For example, `core.*,remote.*.url` would cause the trace2 output to contain events listing each configured remote. - May be overridden by the `GIT_TR2_CONFIG_PARAMS` environment + May be overridden by the `GIT_TRACE2_CONFIG_PARAMS` environment variable. Unset by default. trace2.destinationDebug:: @@ -53,4 +53,4 @@ trace2.destinationDebug:: trace target destination cannot be opened for writing. By default, these errors are suppressed and tracing is silently disabled. May be overridden by the - `GIT_TR2_DST_DEBUG` environment variable. + `GIT_TRACE2_DST_DEBUG` environment variable. diff --git a/Documentation/git.txt b/Documentation/git.txt index 72adfcc5e2..fcf81e3acf 100644 --- a/Documentation/git.txt +++ b/Documentation/git.txt @@ -660,23 +660,23 @@ of clones and fetches. When a curl trace is enabled (see `GIT_TRACE_CURL` above), do not dump data (that is, only dump info lines and headers). -`GIT_TR2`:: +`GIT_TRACE2`:: Enables more detailed trace messages from the "trace2" library. - Output from `GIT_TR2` is a simple text-based format for human + Output from `GIT_TRACE2` is a simple text-based format for human readability. + -The `GIT_TR2` variables can take many values. Any value available to -the `GIT_TRACE` variables is also available to `GIT_TR2`. The `GIT_TR2` +The `GIT_TRACE2` variables can take many values. Any value available to +the `GIT_TRACE` variables is also available to `GIT_TRACE2`. The `GIT_TRACE2` variables can also specify a Unix Domain Socket. See link:technical/api-trace2.html[Trace2 documentation] for full details. -`GIT_TR2_EVENT`:: +`GIT_TRACE2_EVENT`:: This setting writes a JSON-based format that is suited for machine interpretation. See link:technical/api-trace2.html[Trace2 documentation] for full details. -`GIT_TR2_PERF`:: - In addition to the text-based messages available in `GIT_TR2`, this +`GIT_TRACE2_PERF`:: + In addition to the text-based messages available in `GIT_TRACE2`, this setting writes a column-based format for understanding nesting regions. See link:technical/api-trace2.html[Trace2 documentation] for full details. diff --git a/Documentation/technical/api-trace2.txt b/Documentation/technical/api-trace2.txt index 9e585b8e79..23c3cc7a37 100644 --- a/Documentation/technical/api-trace2.txt +++ b/Documentation/technical/api-trace2.txt @@ -23,7 +23,7 @@ formats in the future. This might be used to define a binary format, for example. Trace2 is controlled using `trace2.*` config values in the system and -global config files and `GIT_TR2*` environment variables. Trace2 does +global config files and `GIT_TRACE2*` environment variables. Trace2 does not read from repo local or worktree config files or respect `-c` command line config settings. @@ -42,7 +42,7 @@ config setting. For example ------------ -$ export GIT_TR2=~/log.normal +$ export GIT_TRACE2=~/log.normal $ git version git version 2.20.1.155.g426c96fcdb ------------ @@ -71,13 +71,13 @@ $ cat ~/log.normal The performance format target (PERF) is a column-based format to replace GIT_TRACE_PERFORMANCE and is suitable for development and testing, possibly to complement tools like gprof. This format is -enabled with the `GIT_TR2_PERF` environment variable or the +enabled with the `GIT_TRACE2_PERF` environment variable or the `trace2.perfTarget` system or global config setting. For example ------------ -$ export GIT_TR2_PERF=~/log.perf +$ export GIT_TRACE2_PERF=~/log.perf $ git version git version 2.20.1.155.g426c96fcdb ------------ @@ -104,14 +104,14 @@ $ cat ~/log.perf === The Event Format Target The event format target is a JSON-based format of event data suitable -for telemetry analysis. This format is enabled with the `GIT_TR2_EVENT` +for telemetry analysis. This format is enabled with the `GIT_TRACE2_EVENT` environment variable or the `trace2.eventTarget` system or global config setting. For example ------------ -$ export GIT_TR2_EVENT=~/log.event +$ export GIT_TRACE2_EVENT=~/log.event $ git version git version 2.20.1.155.g426c96fcdb ------------ @@ -273,7 +273,7 @@ significantly affects program performance or behavior, such as Emits a "def_param" messages for "important" configuration settings. + -The environment variable `GIT_TR2_CONFIG_PARAMS` or the `trace2.configParams` +The environment variable `GIT_TRACE2_CONFIG_PARAMS` or the `trace2.configParams` config value can be set to a list of patterns of important configuration settings, for example: `core.*,remote.*.url`. This function will iterate over all config @@ -465,7 +465,7 @@ Events are written as lines of the form: Note that this may contain embedded LF or CRLF characters that are not escaped, so the event may spill across multiple lines. -If `GIT_TR2_BRIEF` or `trace2.normalBrief` is true, the `time`, `filename`, +If `GIT_TRACE2_BRIEF` or `trace2.normalBrief` is true, the `time`, `filename`, and `line` fields are omitted. This target is intended to be more of a summary (like GIT_TRACE) and @@ -533,7 +533,7 @@ This field is in anticipation of in-proc submodules in the future. 15:33:33.532712 wt-status.c:2331 | d0 | main | region_leave | r1 | 0.127568 | 0.001504 | status | label:print ------------ -If `GIT_TR2_PERF_BRIEF` or `trace2.perfBrief` is true, the `time`, `file`, +If `GIT_TRACE2_PERF_BRIEF` or `trace2.perfBrief` is true, the `time`, `file`, and `line` fields are omitted. ------------ @@ -598,7 +598,7 @@ The following key/value pairs are common to all events: `"repo":`:: when present, is the integer repo-id as described previously. -If `GIT_TR2_EVENT_BRIEF` or `trace2.eventBrief` is true, the `file` +If `GIT_TRACE2_EVENT_BRIEF` or `trace2.eventBrief` is true, the `file` and `line` fields are omitted from all events and the `time` field is only present on the "start" and "atexit" events. @@ -911,7 +911,7 @@ visited. The `category` field may be used in a future enhancement to do category-based filtering. + -`GIT_TR2_EVENT_NESTING` or `trace2.eventNesting` can be used to +`GIT_TRACE2_EVENT_NESTING` or `trace2.eventNesting` can be used to filter deeply nested regions and data events. It defaults to "2". `"region_leave"`:: @@ -1039,8 +1039,8 @@ rev-list, and gc. This example also shows that fetch took 5.199 seconds and of that 4.932 was in ssh. + ---------------- -$ export GIT_TR2_BRIEF=1 -$ export GIT_TR2=~/log.normal +$ export GIT_TRACE2_BRIEF=1 +$ export GIT_TRACE2=~/log.normal $ git fetch origin ... ---------------- @@ -1075,8 +1075,8 @@ its name as "gc", it also reports the hierarchy as "fetch/gc". indented for clarity.) + ---------------- -$ export GIT_TR2_BRIEF=1 -$ export GIT_TR2=~/log.normal +$ export GIT_TRACE2_BRIEF=1 +$ export GIT_TRACE2=~/log.normal $ git fetch origin ... ---------------- @@ -1134,8 +1134,8 @@ In this example, scanning for untracked files ran from +0.012568 to +0.027149 (since the process started) and took 0.014581 seconds. + ---------------- -$ export GIT_TR2_PERF_BRIEF=1 -$ export GIT_TR2_PERF=~/log.perf +$ export GIT_TRACE2_PERF_BRIEF=1 +$ export GIT_TRACE2_PERF=~/log.perf $ git status ... @@ -1180,8 +1180,8 @@ static enum path_treatment read_directory_recursive(struct dir_struct *dir, We can further investigate the time spent scanning for untracked files. + ---------------- -$ export GIT_TR2_PERF_BRIEF=1 -$ export GIT_TR2_PERF=~/log.perf +$ export GIT_TRACE2_PERF_BRIEF=1 +$ export GIT_TRACE2_PERF=~/log.perf $ git status ... $ cat ~/log.perf @@ -1236,8 +1236,8 @@ int read_index_from(struct index_state *istate, const char *path, This example shows that the index contained 3552 entries. + ---------------- -$ export GIT_TR2_PERF_BRIEF=1 -$ export GIT_TR2_PERF=~/log.perf +$ export GIT_TRACE2_PERF_BRIEF=1 +$ export GIT_TRACE2_PERF=~/log.perf $ git status ... $ cat ~/log.perf @@ -1310,8 +1310,8 @@ Data events are tagged with the active thread name. They are used to report the per-thread parameters. + ---------------- -$ export GIT_TR2_PERF_BRIEF=1 -$ export GIT_TR2_PERF=~/log.perf +$ export GIT_TRACE2_PERF_BRIEF=1 +$ export GIT_TRACE2_PERF=~/log.perf $ git status ... $ cat ~/log.perf diff --git a/t/t0001-init.sh b/t/t0001-init.sh index 1f462204ea..77a224aafb 100755 --- a/t/t0001-init.sh +++ b/t/t0001-init.sh @@ -93,7 +93,7 @@ test_expect_success 'No extra GIT_* on alias scripts' ' sed -n \ -e "/^GIT_PREFIX=/d" \ -e "/^GIT_TEXTDOMAINDIR=/d" \ - -e "/^GIT_TR2_PARENT/d" \ + -e "/^GIT_TRACE2_PARENT/d" \ -e "/^GIT_/s/=.*//p" | sort EOF diff --git a/t/t0210-trace2-normal.sh b/t/t0210-trace2-normal.sh index 71194a3623..ce7574edb1 100755 --- a/t/t0210-trace2-normal.sh +++ b/t/t0210-trace2-normal.sh @@ -4,9 +4,9 @@ test_description='test trace2 facility (normal target)' . ./test-lib.sh # Turn off any inherited trace2 settings for this test. -sane_unset GIT_TR2 GIT_TR2_PERF GIT_TR2_EVENT -sane_unset GIT_TR2_BRIEF -sane_unset GIT_TR2_CONFIG_PARAMS +sane_unset GIT_TRACE2 GIT_TRACE2_PERF GIT_TRACE2_EVENT +sane_unset GIT_TRACE2_BRIEF +sane_unset GIT_TRACE2_CONFIG_PARAMS # Add t/helper directory to PATH so that we can use a relative # path to run nested instances of test-tool.exe (see 004child). @@ -27,12 +27,12 @@ V=$(git version | sed -e 's/^git version //') && export V # to whatever filtering that target decides to do). # This script tests the normal target in isolation. # -# Defer setting GIT_TR2 until the actual command line we want to test +# Defer setting GIT_TRACE2 until the actual command line we want to test # because hidden git and test-tool commands run by the test harness # can contaminate our output. # Enable "brief" feature which turns off " : " prefix. -GIT_TR2_BRIEF=1 && export GIT_TR2_BRIEF +GIT_TRACE2_BRIEF=1 && export GIT_TRACE2_BRIEF # Basic tests of the trace2 normal stream. Since this stream is used # primarily with printf-style debugging/tracing, we do limited testing @@ -54,7 +54,7 @@ GIT_TR2_BRIEF=1 && export GIT_TR2_BRIEF test_expect_success 'normal stream, return code 0' ' test_when_finished "rm trace.normal actual expect" && - GIT_TR2="$(pwd)/trace.normal" test-tool trace2 001return 0 && + GIT_TRACE2="$(pwd)/trace.normal" test-tool trace2 001return 0 && perl "$TEST_DIRECTORY/t0210/scrub_normal.perl" actual && cat >expect <<-EOF && version $V @@ -68,7 +68,7 @@ test_expect_success 'normal stream, return code 0' ' test_expect_success 'normal stream, return code 1' ' test_when_finished "rm trace.normal actual expect" && - test_must_fail env GIT_TR2="$(pwd)/trace.normal" test-tool trace2 001return 1 && + test_must_fail env GIT_TRACE2="$(pwd)/trace.normal" test-tool trace2 001return 1 && perl "$TEST_DIRECTORY/t0210/scrub_normal.perl" actual && cat >expect <<-EOF && version $V @@ -83,7 +83,7 @@ test_expect_success 'normal stream, return code 1' ' test_expect_success 'automatic filename' ' test_when_finished "rm -r traces actual expect" && mkdir traces && - GIT_TR2="$(pwd)/traces" test-tool trace2 001return 0 && + GIT_TRACE2="$(pwd)/traces" test-tool trace2 001return 0 && perl "$TEST_DIRECTORY/t0210/scrub_normal.perl" <"$(ls traces/*)" >actual && cat >expect <<-EOF && version $V @@ -101,7 +101,7 @@ test_expect_success 'automatic filename' ' test_expect_success 'normal stream, exit code 0' ' test_when_finished "rm trace.normal actual expect" && - GIT_TR2="$(pwd)/trace.normal" test-tool trace2 002exit 0 && + GIT_TRACE2="$(pwd)/trace.normal" test-tool trace2 002exit 0 && perl "$TEST_DIRECTORY/t0210/scrub_normal.perl" actual && cat >expect <<-EOF && version $V @@ -115,7 +115,7 @@ test_expect_success 'normal stream, exit code 0' ' test_expect_success 'normal stream, exit code 1' ' test_when_finished "rm trace.normal actual expect" && - test_must_fail env GIT_TR2="$(pwd)/trace.normal" test-tool trace2 002exit 1 && + test_must_fail env GIT_TRACE2="$(pwd)/trace.normal" test-tool trace2 002exit 1 && perl "$TEST_DIRECTORY/t0210/scrub_normal.perl" actual && cat >expect <<-EOF && version $V @@ -133,7 +133,7 @@ test_expect_success 'normal stream, exit code 1' ' test_expect_success 'normal stream, error event' ' test_when_finished "rm trace.normal actual expect" && - GIT_TR2="$(pwd)/trace.normal" test-tool trace2 003error "hello world" "this is a test" && + GIT_TRACE2="$(pwd)/trace.normal" test-tool trace2 003error "hello world" "this is a test" && perl "$TEST_DIRECTORY/t0210/scrub_normal.perl" actual && cat >expect <<-EOF && version $V @@ -147,7 +147,7 @@ test_expect_success 'normal stream, error event' ' test_cmp expect actual ' -sane_unset GIT_TR2_BRIEF +sane_unset GIT_TRACE2_BRIEF # Now test without environment variables and get all Trace2 settings # from the global config. diff --git a/t/t0211-trace2-perf.sh b/t/t0211-trace2-perf.sh index b501e867af..2c3ad6e8c1 100755 --- a/t/t0211-trace2-perf.sh +++ b/t/t0211-trace2-perf.sh @@ -4,9 +4,9 @@ test_description='test trace2 facility (perf target)' . ./test-lib.sh # Turn off any inherited trace2 settings for this test. -sane_unset GIT_TR2 GIT_TR2_PERF GIT_TR2_EVENT -sane_unset GIT_TR2_PERF_BRIEF -sane_unset GIT_TR2_CONFIG_PARAMS +sane_unset GIT_TRACE2 GIT_TRACE2_PERF GIT_TRACE2_EVENT +sane_unset GIT_TRACE2_PERF_BRIEF +sane_unset GIT_TRACE2_CONFIG_PARAMS # Add t/helper directory to PATH so that we can use a relative # path to run nested instances of test-tool.exe (see 004child). @@ -27,13 +27,13 @@ V=$(git version | sed -e 's/^git version //') && export V # to whatever filtering that target decides to do). # Test each target independently. # -# Defer setting GIT_TR2_PERF until the actual command we want to +# Defer setting GIT_TRACE2_PERF until the actual command we want to # test because hidden git and test-tool commands in the test # harness can contaminate our output. # Enable "brief" feature which turns off the prefix: # " : | | " -GIT_TR2_PERF_BRIEF=1 && export GIT_TR2_PERF_BRIEF +GIT_TRACE2_PERF_BRIEF=1 && export GIT_TRACE2_PERF_BRIEF # Repeat some of the t0210 tests using the perf target stream instead of # the normal stream. @@ -46,7 +46,7 @@ GIT_TR2_PERF_BRIEF=1 && export GIT_TR2_PERF_BRIEF test_expect_success 'perf stream, return code 0' ' test_when_finished "rm trace.perf actual expect" && - GIT_TR2_PERF="$(pwd)/trace.perf" test-tool trace2 001return 0 && + GIT_TRACE2_PERF="$(pwd)/trace.perf" test-tool trace2 001return 0 && perl "$TEST_DIRECTORY/t0211/scrub_perf.perl" actual && cat >expect <<-EOF && d0|main|version|||||$V @@ -60,7 +60,7 @@ test_expect_success 'perf stream, return code 0' ' test_expect_success 'perf stream, return code 1' ' test_when_finished "rm trace.perf actual expect" && - test_must_fail env GIT_TR2_PERF="$(pwd)/trace.perf" test-tool trace2 001return 1 && + test_must_fail env GIT_TRACE2_PERF="$(pwd)/trace.perf" test-tool trace2 001return 1 && perl "$TEST_DIRECTORY/t0211/scrub_perf.perl" actual && cat >expect <<-EOF && d0|main|version|||||$V @@ -78,7 +78,7 @@ test_expect_success 'perf stream, return code 1' ' test_expect_success 'perf stream, error event' ' test_when_finished "rm trace.perf actual expect" && - GIT_TR2_PERF="$(pwd)/trace.perf" test-tool trace2 003error "hello world" "this is a test" && + GIT_TRACE2_PERF="$(pwd)/trace.perf" test-tool trace2 003error "hello world" "this is a test" && perl "$TEST_DIRECTORY/t0211/scrub_perf.perl" actual && cat >expect <<-EOF && d0|main|version|||||$V @@ -124,7 +124,7 @@ test_expect_success 'perf stream, error event' ' test_expect_success 'perf stream, child processes' ' test_when_finished "rm trace.perf actual expect" && - GIT_TR2_PERF="$(pwd)/trace.perf" test-tool trace2 004child test-tool trace2 004child test-tool trace2 001return 0 && + GIT_TRACE2_PERF="$(pwd)/trace.perf" test-tool trace2 004child test-tool trace2 004child test-tool trace2 001return 0 && perl "$TEST_DIRECTORY/t0211/scrub_perf.perl" actual && cat >expect <<-EOF && d0|main|version|||||$V @@ -150,7 +150,7 @@ test_expect_success 'perf stream, child processes' ' test_cmp expect actual ' -sane_unset GIT_TR2_PERF_BRIEF +sane_unset GIT_TRACE2_PERF_BRIEF # Now test without environment variables and get all Trace2 settings # from the global config. diff --git a/t/t0212-trace2-event.sh b/t/t0212-trace2-event.sh index 59adae8123..ff5b9cc729 100755 --- a/t/t0212-trace2-event.sh +++ b/t/t0212-trace2-event.sh @@ -4,9 +4,9 @@ test_description='test trace2 facility' . ./test-lib.sh # Turn off any inherited trace2 settings for this test. -sane_unset GIT_TR2 GIT_TR2_PERF GIT_TR2_EVENT -sane_unset GIT_TR2_BARE -sane_unset GIT_TR2_CONFIG_PARAMS +sane_unset GIT_TRACE2 GIT_TRACE2_PERF GIT_TRACE2_EVENT +sane_unset GIT_TRACE2_BARE +sane_unset GIT_TRACE2_CONFIG_PARAMS perl -MJSON::PP -e 0 >/dev/null 2>&1 && test_set_prereq JSON_PP @@ -29,7 +29,7 @@ V=$(git version | sed -e 's/^git version //') && export V # to whatever filtering that target decides to do). # Test each target independently. # -# Defer setting GIT_TR2_PERF until the actual command we want to +# Defer setting GIT_TRACE2_PERF until the actual command we want to # test because hidden git and test-tool commands in the test # harness can contaminate our output. @@ -42,7 +42,7 @@ V=$(git version | sed -e 's/^git version //') && export V test_expect_success JSON_PP 'event stream, error event' ' test_when_finished "rm trace.event actual expect" && - GIT_TR2_EVENT="$(pwd)/trace.event" test-tool trace2 003error "hello world" "this is a test" && + GIT_TRACE2_EVENT="$(pwd)/trace.event" test-tool trace2 003error "hello world" "this is a test" && perl "$TEST_DIRECTORY/t0212/parse_events.perl" actual && sed -e "s/^|//" >expect <<-EOF && |VAR1 = { @@ -79,7 +79,7 @@ test_expect_success JSON_PP 'event stream, error event' ' test_expect_success JSON_PP 'event stream, return code 0' ' test_when_finished "rm trace.event actual expect" && - GIT_TR2_EVENT="$(pwd)/trace.event" test-tool trace2 004child test-tool trace2 004child test-tool trace2 001return 0 && + GIT_TRACE2_EVENT="$(pwd)/trace.event" test-tool trace2 004child test-tool trace2 004child test-tool trace2 001return 0 && perl "$TEST_DIRECTORY/t0212/parse_events.perl" actual && sed -e "s/^|//" >expect <<-EOF && |VAR1 = { @@ -168,7 +168,7 @@ test_expect_success JSON_PP 'event stream, list config' ' test_when_finished "rm trace.event actual expect" && git config --local t0212.abc 1 && git config --local t0212.def "hello world" && - GIT_TR2_EVENT="$(pwd)/trace.event" GIT_TR2_CONFIG_PARAMS="t0212.*" test-tool trace2 001return 0 && + GIT_TRACE2_EVENT="$(pwd)/trace.event" GIT_TRACE2_CONFIG_PARAMS="t0212.*" test-tool trace2 001return 0 && perl "$TEST_DIRECTORY/t0212/parse_events.perl" actual && sed -e "s/^|//" >expect <<-EOF && |VAR1 = { @@ -201,7 +201,7 @@ test_expect_success JSON_PP 'event stream, list config' ' test_expect_success JSON_PP 'basic trace2_data' ' test_when_finished "rm trace.event actual expect" && - GIT_TR2_EVENT="$(pwd)/trace.event" test-tool trace2 006data test_category k1 v1 test_category k2 v2 && + GIT_TRACE2_EVENT="$(pwd)/trace.event" test-tool trace2 006data test_category k1 v1 test_category k2 v2 && perl "$TEST_DIRECTORY/t0212/parse_events.perl" actual && sed -e "s/^|//" >expect <<-EOF && |VAR1 = { diff --git a/t/t0212/parse_events.perl b/t/t0212/parse_events.perl index a2776ba216..6584bb5634 100644 --- a/t/t0212/parse_events.perl +++ b/t/t0212/parse_events.perl @@ -26,7 +26,7 @@ use Getopt::Long; # The version of the trace2 event target format that we understand. # This is reported in the 'version' event in the 'evt' field. -# It comes from the GIT_TR2_EVENT_VERSION macro in trace2/tr2_tgt_event.c +# It comes from the GIT_TRACE2_EVENT_VERSION macro in trace2/tr2_tgt_event.c my $evt_version = '1'; my $show_children = 1; diff --git a/trace2.h b/trace2.h index f189ef5984..050bf3c8c1 100644 --- a/trace2.h +++ b/trace2.h @@ -130,7 +130,7 @@ void trace2_cmd_alias_fl(const char *file, int line, const char *alias, * list of patterns configured important. For example: * git config --system trace2.configParams 'core.*,remote.*.url' * or: - * GIT_TR2_CONFIG_PARAMS=core.*,remote.*.url" + * GIT_TRACE2_CONFIG_PARAMS=core.*,remote.*.url" * * Note: this routine does a read-only iteration on the config data * (using read_early_config()), so it must not be called until enough diff --git a/trace2/tr2_cmd_name.c b/trace2/tr2_cmd_name.c index e999592b4c..dd313204f5 100644 --- a/trace2/tr2_cmd_name.c +++ b/trace2/tr2_cmd_name.c @@ -1,7 +1,7 @@ #include "cache.h" #include "trace2/tr2_cmd_name.h" -#define TR2_ENVVAR_PARENT_NAME "GIT_TR2_PARENT_NAME" +#define TR2_ENVVAR_PARENT_NAME "GIT_TRACE2_PARENT_NAME" static struct strbuf tr2cmdname_hierarchy = STRBUF_INIT; diff --git a/trace2/tr2_sid.c b/trace2/tr2_sid.c index 5047095478..6948fd4108 100644 --- a/trace2/tr2_sid.c +++ b/trace2/tr2_sid.c @@ -2,7 +2,7 @@ #include "trace2/tr2_tbuf.h" #include "trace2/tr2_sid.h" -#define TR2_ENVVAR_PARENT_SID "GIT_TR2_PARENT_SID" +#define TR2_ENVVAR_PARENT_SID "GIT_TRACE2_PARENT_SID" static struct strbuf tr2sid_buf = STRBUF_INIT; static int tr2sid_nr_git_parents; diff --git a/trace2/tr2_sysenv.c b/trace2/tr2_sysenv.c index 9025b86303..5958cfc424 100644 --- a/trace2/tr2_sysenv.c +++ b/trace2/tr2_sysenv.c @@ -21,33 +21,33 @@ struct tr2_sysenv_entry { * The strings in this table are constant and must match the published * config and environment variable names as described in the documentation. * - * We do not define entries for the GIT_TR2_PARENT_* environment + * We do not define entries for the GIT_TRACE2_PARENT_* environment * variables because they are transient and used to pass information * from parent to child git processes, rather than settings. */ /* clang-format off */ static struct tr2_sysenv_entry tr2_sysenv_settings[] = { - [TR2_SYSENV_CFG_PARAM] = { "GIT_TR2_CONFIG_PARAMS", + [TR2_SYSENV_CFG_PARAM] = { "GIT_TRACE2_CONFIG_PARAMS", "trace2.configparams" }, - [TR2_SYSENV_DST_DEBUG] = { "GIT_TR2_DST_DEBUG", + [TR2_SYSENV_DST_DEBUG] = { "GIT_TRACE2_DST_DEBUG", "trace2.destinationdebug" }, - [TR2_SYSENV_NORMAL] = { "GIT_TR2", + [TR2_SYSENV_NORMAL] = { "GIT_TRACE2", "trace2.normaltarget" }, - [TR2_SYSENV_NORMAL_BRIEF] = { "GIT_TR2_BRIEF", + [TR2_SYSENV_NORMAL_BRIEF] = { "GIT_TRACE2_BRIEF", "trace2.normalbrief" }, - [TR2_SYSENV_EVENT] = { "GIT_TR2_EVENT", + [TR2_SYSENV_EVENT] = { "GIT_TRACE2_EVENT", "trace2.eventtarget" }, - [TR2_SYSENV_EVENT_BRIEF] = { "GIT_TR2_EVENT_BRIEF", + [TR2_SYSENV_EVENT_BRIEF] = { "GIT_TRACE2_EVENT_BRIEF", "trace2.eventbrief" }, - [TR2_SYSENV_EVENT_NESTING] = { "GIT_TR2_EVENT_NESTING", + [TR2_SYSENV_EVENT_NESTING] = { "GIT_TRACE2_EVENT_NESTING", "trace2.eventnesting" }, - [TR2_SYSENV_PERF] = { "GIT_TR2_PERF", + [TR2_SYSENV_PERF] = { "GIT_TRACE2_PERF", "trace2.perftarget" }, - [TR2_SYSENV_PERF_BRIEF] = { "GIT_TR2_PERF_BRIEF", + [TR2_SYSENV_PERF_BRIEF] = { "GIT_TRACE2_PERF_BRIEF", "trace2.perfbrief" }, }; /* clang-format on */ diff --git a/trace2/tr2_sysenv.h b/trace2/tr2_sysenv.h index 369b20bd87..8dd82a7a56 100644 --- a/trace2/tr2_sysenv.h +++ b/trace2/tr2_sysenv.h @@ -7,7 +7,7 @@ * * Note that this set does not contain any of the transient * environment variables used to pass information from parent - * to child git processes, such "GIT_TR2_PARENT_SID". + * to child git processes, such "GIT_TRACE2_PARENT_SID". */ enum tr2_sysenv_variable { TR2_SYSENV_CFG_PARAM = 0, From 4e0d3aa18a60db8bf3448d4fceeed9c8ac8cc597 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?SZEDER=20G=C3=A1bor?= Date: Sun, 19 May 2019 16:43:09 +0200 Subject: [PATCH 55/66] trace2: document the supported values of GIT_TRACE2* env variables MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The descriptions of the GIT_TRACE2* environment variables link to the technical docs for further details on the supported values. However, a link like this only really works if the docs are viewed in a browser and the full documentation is available. OTOH, in 'man git' there are no links to conveniently click on, and distro-shipped git packages tend to include only the man pages, while the technical docs and the docs in html format are in a separate 'git-doc' package. So let's describe the supported values to make the manpage more self-contained, but still keep the references to the technical docs because the details of the SID, and the JSON and perf output formats are definitely beyond the scope of 'man git'. Signed-off-by: SZEDER Gábor Signed-off-by: Junio C Hamano --- Documentation/git.txt | 43 +++++++++++++++++++++++++++++++++++-------- 1 file changed, 35 insertions(+), 8 deletions(-) diff --git a/Documentation/git.txt b/Documentation/git.txt index fcf81e3acf..6ddc1e2ca6 100644 --- a/Documentation/git.txt +++ b/Documentation/git.txt @@ -665,21 +665,48 @@ of clones and fetches. Output from `GIT_TRACE2` is a simple text-based format for human readability. + -The `GIT_TRACE2` variables can take many values. Any value available to -the `GIT_TRACE` variables is also available to `GIT_TRACE2`. The `GIT_TRACE2` -variables can also specify a Unix Domain Socket. See -link:technical/api-trace2.html[Trace2 documentation] for full details. +If this variable is set to "1", "2" or "true" (comparison +is case insensitive), trace messages will be printed to +stderr. ++ +If the variable is set to an integer value greater than 2 +and lower than 10 (strictly) then Git will interpret this +value as an open file descriptor and will try to write the +trace messages into this file descriptor. ++ +Alternatively, if the variable is set to an absolute path +(starting with a '/' character), Git will interpret this +as a file path and will try to append the trace messages +to it. If the path already exists and is a directory, the +trace messages will be written to files (one per process) +in that directory, named according to the last component +of the SID and an optional counter (to avoid filename +collisions). ++ +In addition, if the variable is set to +`af_unix:[:]`, Git will try +to open the path as a Unix Domain Socket. The socket type +can be either `stream` or `dgram`. ++ +Unsetting the variable, or setting it to empty, "0" or +"false" (case insensitive) disables trace messages. ++ +See link:technical/api-trace2.html[Trace2 documentation] +for full details. + `GIT_TRACE2_EVENT`:: This setting writes a JSON-based format that is suited for machine - interpretation. See link:technical/api-trace2.html[Trace2 documentation] - for full details. + interpretation. + See `GIT_TRACE2` for available trace output options and + link:technical/api-trace2.html[Trace2 documentation] for full details. `GIT_TRACE2_PERF`:: In addition to the text-based messages available in `GIT_TRACE2`, this setting writes a column-based format for understanding nesting - regions. See link:technical/api-trace2.html[Trace2 documentation] - for full details. + regions. + See `GIT_TRACE2` for available trace output options and + link:technical/api-trace2.html[Trace2 documentation] for full details. `GIT_REDACT_COOKIES`:: This can be set to a comma-separated list of strings. When a curl trace From 1aed1a5f25ad300dbdc48f0943ca5c3bed952e6a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?SZEDER=20G=C3=A1bor?= Date: Sun, 19 May 2019 16:45:46 +0200 Subject: [PATCH 56/66] progress: avoid empty line when breaking the progress line MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Since commit 545dc345eb (progress: break too long progress bar lines, 2019-04-12) when splitting a too long progress line, sometimes it looks as if a superfluous empty line were added between the title line and the counters. To make sure that the previously displayed progress line is completely covered up when writing the new, shorter title line, we calculate how many characters need to be overwritten with spaces. Alas, this calculation doesn't account for the newline character at the end of the new title line, and resulted in printing one more space than strictly necessary. This extra space character doesn't matter, if the length of the previous progress line was shorter than the width of the terminal. However, if the previous line matched the terminal width, then this extra space made the new line longer, effectively adding that empty line after the title line. Fix this off-by-one to avoid that spurious empty line. Signed-off-by: SZEDER Gábor Signed-off-by: Junio C Hamano --- progress.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/progress.c b/progress.c index 2d8022a622..a4da26c2aa 100644 --- a/progress.c +++ b/progress.c @@ -127,7 +127,7 @@ static void display(struct progress *progress, uint64_t n, const char *done) (int) clear_len, eol); } else if (!done && cols < progress_line_len) { clear_len = progress->title_len + 1 < cols ? - cols - progress->title_len : 0; + cols - progress->title_len - 1 : 0; fprintf(stderr, "%s:%*s\n %s%s", progress->title, (int) clear_len, "", counters_sb->buf, eol); From 4c785c0edcd09222a812244db04c6fd725e512f3 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Tue, 21 May 2019 10:50:20 -0700 Subject: [PATCH 57/66] rebase: replace incorrect logical negation by correct bitwise one In bff014dac7d9 (builtin rebase: support the `verbose` and `diffstat` options, 2018-09-04), we added a line that wanted to remove the `REBASE_DIFFSTAT` bit from the flags, but it used an incorrect negation. Found by Coverity. Signed-off-by: Johannes Schindelin Signed-off-by: Junio C Hamano --- builtin/rebase.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/builtin/rebase.c b/builtin/rebase.c index b5c99ec10c..58607a2019 100644 --- a/builtin/rebase.c +++ b/builtin/rebase.c @@ -653,7 +653,7 @@ static int rebase_config(const char *var, const char *value, void *data) if (git_config_bool(var, value)) opts->flags |= REBASE_DIFFSTAT; else - opts->flags &= !REBASE_DIFFSTAT; + opts->flags &= ~REBASE_DIFFSTAT; return 0; } From 5fdae9d3be45ce17e22b5f7a742cae3dfe687516 Mon Sep 17 00:00:00 2001 From: Jeff Hostetler Date: Tue, 21 May 2019 12:33:59 -0700 Subject: [PATCH 58/66] trace2: fix tracing when NO_PTHREADS is defined Teach trace2 TLS code to not rely on pthread_getspecific() when NO_PTHREADS is defined. Instead, always assume the context data of the main thread. Signed-off-by: Jeff Hostetler Signed-off-by: Junio C Hamano --- trace2/tr2_tls.c | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/trace2/tr2_tls.c b/trace2/tr2_tls.c index 8e65b0361d..e0c4529f73 100644 --- a/trace2/tr2_tls.c +++ b/trace2/tr2_tls.c @@ -47,7 +47,12 @@ struct tr2tls_thread_ctx *tr2tls_create_self(const char *thread_name) struct tr2tls_thread_ctx *tr2tls_get_self(void) { - struct tr2tls_thread_ctx *ctx = pthread_getspecific(tr2tls_key); + struct tr2tls_thread_ctx *ctx; + + if (!HAVE_THREADS) + return tr2tls_thread_main; + + ctx = pthread_getspecific(tr2tls_key); /* * If the thread-proc did not call trace2_thread_start(), we won't @@ -62,9 +67,10 @@ struct tr2tls_thread_ctx *tr2tls_get_self(void) int tr2tls_is_main_thread(void) { - struct tr2tls_thread_ctx *ctx = pthread_getspecific(tr2tls_key); + if (!HAVE_THREADS) + return 1; - return ctx == tr2tls_thread_main; + return pthread_getspecific(tr2tls_key) == tr2tls_thread_main; } void tr2tls_unset_self(void) From 5b204b7df3ed7e97040d40db9d7c31a0a645af15 Mon Sep 17 00:00:00 2001 From: Jonathan Tan Date: Wed, 22 May 2019 13:08:22 -0700 Subject: [PATCH 59/66] fetch-pack: send server options after command Currently, if any server options are specified during a protocol v2 fetch, server options will be sent before "command=fetch". Write server options to the request buffer in send_fetch_request() so that the components of the request are sent in the correct order. The protocol documentation states that the command must come first. The Git server implementation in serve.c (see process_request() in that file) tolerates any order of command and capability, which is perhaps why we haven't noticed this. This was noticed when testing against a JGit server implementation, which follows the documentation in this regard. Signed-off-by: Jonathan Tan Acked-by: Jonathan Nieder Signed-off-by: Junio C Hamano --- fetch-pack.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fetch-pack.c b/fetch-pack.c index 812be15d7e..46651b16bc 100644 --- a/fetch-pack.c +++ b/fetch-pack.c @@ -1111,7 +1111,7 @@ static int send_fetch_request(struct fetch_negotiator *negotiator, int fd_out, server_supports_v2("server-option", 1)) { int i; for (i = 0; i < args->server_options->nr; i++) - packet_write_fmt(fd_out, "server-option=%s", + packet_buf_write(&req_buf, "server-option=%s", args->server_options->items[i].string); } From db4a3f26c373fd73688991fbeb76b2d00b9a470c Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Tue, 28 May 2019 05:42:14 -0700 Subject: [PATCH 60/66] tests: mark a couple more test cases as requiring `rebase -p` The `--preserve-merges` option has been deprecated, and as a consequence we started to mark test cases that require that option to be supported, in preparation for removing that support eventually. Since we marked those test cases, a couple more crept into the test suite, and with this patch, we mark them, too. Signed-off-by: Johannes Schindelin Signed-off-by: Junio C Hamano --- t/t3422-rebase-incompatible-options.sh | 5 +++-- t/t3427-rebase-subtree.sh | 15 ++++++++++----- 2 files changed, 13 insertions(+), 7 deletions(-) diff --git a/t/t3422-rebase-incompatible-options.sh b/t/t3422-rebase-incompatible-options.sh index bb78a6ec86..a5868ea152 100755 --- a/t/t3422-rebase-incompatible-options.sh +++ b/t/t3422-rebase-incompatible-options.sh @@ -65,12 +65,13 @@ test_rebase_am_only --ignore-whitespace test_rebase_am_only --committer-date-is-author-date test_rebase_am_only -C4 -test_expect_success '--preserve-merges incompatible with --signoff' ' +test_expect_success REBASE_P '--preserve-merges incompatible with --signoff' ' git checkout B^0 && test_must_fail git rebase --preserve-merges --signoff A ' -test_expect_success '--preserve-merges incompatible with --rebase-merges' ' +test_expect_success REBASE_P \ + '--preserve-merges incompatible with --rebase-merges' ' git checkout B^0 && test_must_fail git rebase --preserve-merges --rebase-merges A ' diff --git a/t/t3427-rebase-subtree.sh b/t/t3427-rebase-subtree.sh index 3780877e4e..d8640522a0 100755 --- a/t/t3427-rebase-subtree.sh +++ b/t/t3427-rebase-subtree.sh @@ -38,7 +38,8 @@ test_expect_success 'setup' ' ' # FAILURE: Does not preserve master4. -test_expect_failure 'Rebase -Xsubtree --preserve-merges --onto commit 4' ' +test_expect_failure REBASE_P \ + 'Rebase -Xsubtree --preserve-merges --onto commit 4' ' reset_rebase && git checkout -b rebase-preserve-merges-4 master && git filter-branch --prune-empty -f --subdirectory-filter files_subtree && @@ -48,7 +49,8 @@ test_expect_failure 'Rebase -Xsubtree --preserve-merges --onto commit 4' ' ' # FAILURE: Does not preserve master5. -test_expect_failure 'Rebase -Xsubtree --preserve-merges --onto commit 5' ' +test_expect_failure REBASE_P \ + 'Rebase -Xsubtree --preserve-merges --onto commit 5' ' reset_rebase && git checkout -b rebase-preserve-merges-5 master && git filter-branch --prune-empty -f --subdirectory-filter files_subtree && @@ -58,7 +60,8 @@ test_expect_failure 'Rebase -Xsubtree --preserve-merges --onto commit 5' ' ' # FAILURE: Does not preserve master4. -test_expect_failure 'Rebase -Xsubtree --keep-empty --preserve-merges --onto commit 4' ' +test_expect_failure REBASE_P \ + 'Rebase -Xsubtree --keep-empty --preserve-merges --onto commit 4' ' reset_rebase && git checkout -b rebase-keep-empty-4 master && git filter-branch --prune-empty -f --subdirectory-filter files_subtree && @@ -68,7 +71,8 @@ test_expect_failure 'Rebase -Xsubtree --keep-empty --preserve-merges --onto comm ' # FAILURE: Does not preserve master5. -test_expect_failure 'Rebase -Xsubtree --keep-empty --preserve-merges --onto commit 5' ' +test_expect_failure REBASE_P \ + 'Rebase -Xsubtree --keep-empty --preserve-merges --onto commit 5' ' reset_rebase && git checkout -b rebase-keep-empty-5 master && git filter-branch --prune-empty -f --subdirectory-filter files_subtree && @@ -78,7 +82,8 @@ test_expect_failure 'Rebase -Xsubtree --keep-empty --preserve-merges --onto comm ' # FAILURE: Does not preserve Empty. -test_expect_failure 'Rebase -Xsubtree --keep-empty --preserve-merges --onto empty commit' ' +test_expect_failure REBASE_P \ + 'Rebase -Xsubtree --keep-empty --preserve-merges --onto empty commit' ' reset_rebase && git checkout -b rebase-keep-empty-empty master && git filter-branch --prune-empty -f --subdirectory-filter files_subtree && From 7401ab92f8c9c9f7c81763d6f7da8a63a258eff2 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Tue, 28 May 2019 05:42:15 -0700 Subject: [PATCH 61/66] docs: say that `--rebase=preserve` is deprecated As of Git v2.22.0, the `--preserve-merges` backend of `git rebase` will be officially deprecated in favor of the `--rebase-merges` backend. Consequently, `git pull --rebase=preserve` will also be deprected. State this explicitly. Signed-off-by: Johannes Schindelin Signed-off-by: Junio C Hamano --- Documentation/git-pull.txt | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/Documentation/git-pull.txt b/Documentation/git-pull.txt index 118d9d86f7..a5e9501a0a 100644 --- a/Documentation/git-pull.txt +++ b/Documentation/git-pull.txt @@ -112,8 +112,9 @@ When set to `merges`, rebase using `git rebase --rebase-merges` so that the local merge commits are included in the rebase (see linkgit:git-rebase[1] for details). + -When set to preserve, rebase with the `--preserve-merges` option passed -to `git rebase` so that locally created merge commits will not be flattened. +When set to `preserve` (deprecated in favor of `merges`), rebase with the +`--preserve-merges` option passed to `git rebase` so that locally created +merge commits will not be flattened. + When false, merge the current branch into the upstream branch. + From 7948b49ac7be06dd926b5a86e70f7f40f3041899 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Tue, 28 May 2019 05:42:16 -0700 Subject: [PATCH 62/66] rebase docs: recommend `-r` over `-p` The `--preserve-merges` option is now deprecated in favor of `--rebase-merges`; Let's stop recommending the former. Signed-off-by: Johannes Schindelin Signed-off-by: Junio C Hamano --- Documentation/git-rebase.txt | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt index 63047375b6..274870fed4 100644 --- a/Documentation/git-rebase.txt +++ b/Documentation/git-rebase.txt @@ -670,7 +670,8 @@ $ git rebase -i HEAD~5 And move the first patch to the end of the list. -You might want to preserve merges, if you have a history like this: +You might want to recreate merge commits, e.g. if you have a history +like this: ------------------ X @@ -684,7 +685,7 @@ Suppose you want to rebase the side branch starting at "A" to "Q". Make sure that the current HEAD is "B", and call ----------------------------- -$ git rebase -i -p --onto Q O +$ git rebase -i -r --onto Q O ----------------------------- Reordering and editing commits usually creates untested intermediate From 7f125ff9094bfebbb381afa19380943a088c8fcb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nguy=E1=BB=85n=20Th=C3=A1i=20Ng=E1=BB=8Dc=20Duy?= Date: Wed, 29 May 2019 16:11:14 +0700 Subject: [PATCH 63/66] diff-parseopt: correct variable types that are used by parseopt MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Most number-related OPT_ macros store the value in an 'int' variable. Many of the variables in 'struct diff_options' have a different type, but during the conversion to using parse_options() I failed to notice and correct. The problem was reported on s360x which is a big-endian architechture. The variable to store '-w' option in this case is xdl_opts, 'long' type, 8 bytes. But since parse_options() assumes 'int' (4 bytes), it will store bits in the wrong part of xdl_opts. The problem was found on little-endian platforms because parse_options() will accidentally store at the right part of xdl_opts. There aren't much to say about the type change (except that 'int' for xdl_opts should still be big enough, since Windows' long is the same size as 'int' and nobody has complained so far). Some safety checks may be implemented in the future to prevent class of bugs. Reported-by: Todd Zullinger Signed-off-by: Nguyễn Thái Ngọc Duy Signed-off-by: Junio C Hamano --- diff.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/diff.h b/diff.h index d9ad73f0e1..7b66bf1b80 100644 --- a/diff.h +++ b/diff.h @@ -169,7 +169,7 @@ struct diff_options { const char *prefix; int prefix_length; const char *stat_sep; - long xdl_opts; + int xdl_opts; /* see Documentation/diff-options.txt */ char **anchors; From 8ef05193bcdda6a28cd41fa3ecd8f356061d49e0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nguy=E1=BB=85n=20Th=C3=A1i=20Ng=E1=BB=8Dc=20Duy?= Date: Wed, 29 May 2019 16:11:15 +0700 Subject: [PATCH 64/66] diff-parseopt: restore -U (no argument) behavior MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Before d473e2e0e8 (diff.c: convert -U|--unified, 2019-01-27), -U and --unified are implemented with a custom parser opt_arg() in diff.c. I didn't check this code carefully and not realize that it's the equivalent of PARSE_OPT_NONEG | PARSE_OPT_OPTARG. In other words, if -U is specified without any argument, the option should be accepted, and the default value should be used. Without PARSE_OPT_OPTARG, parse_options() will reject this case and cause a regression. Reported-by: Bryan Turner Signed-off-by: Nguyễn Thái Ngọc Duy Signed-off-by: Junio C Hamano --- diff.c | 10 +++++---- t/t4013-diff-various.sh | 2 ++ t/t4013/diff.diff_-U1_initial..side | 29 ++++++++++++++++++++++++++ t/t4013/diff.diff_-U2_initial..side | 31 ++++++++++++++++++++++++++++ t/t4013/diff.diff_-U_initial..side | 32 +++++++++++++++++++++++++++++ 5 files changed, 100 insertions(+), 4 deletions(-) create mode 100644 t/t4013/diff.diff_-U1_initial..side create mode 100644 t/t4013/diff.diff_-U2_initial..side create mode 100644 t/t4013/diff.diff_-U_initial..side diff --git a/diff.c b/diff.c index 4bc9df7362..0abab4a0b1 100644 --- a/diff.c +++ b/diff.c @@ -4875,9 +4875,11 @@ static int diff_opt_unified(const struct option *opt, BUG_ON_OPT_NEG(unset); - options->context = strtol(arg, &s, 10); - if (*s) - return error(_("%s expects a numerical value"), "--unified"); + if (arg) { + options->context = strtol(arg, &s, 10); + if (*s) + return error(_("%s expects a numerical value"), "--unified"); + } enable_patch_output(&options->output_format); return 0; @@ -4895,7 +4897,7 @@ static void prep_parse_options(struct diff_options *options) DIFF_FORMAT_PATCH, DIFF_FORMAT_NO_OUTPUT), OPT_CALLBACK_F('U', "unified", options, N_(""), N_("generate diffs with lines context"), - PARSE_OPT_NONEG, diff_opt_unified), + PARSE_OPT_NONEG | PARSE_OPT_OPTARG, diff_opt_unified), OPT_BOOL('W', "function-context", &options->flags.funccontext, N_("generate diffs with lines context")), OPT_BIT_F(0, "raw", &options->output_format, diff --git a/t/t4013-diff-various.sh b/t/t4013-diff-various.sh index 7d985ff6b1..3c4a3dfcc1 100755 --- a/t/t4013-diff-various.sh +++ b/t/t4013-diff-various.sh @@ -330,6 +330,8 @@ format-patch --inline --stdout initial..master^^ format-patch --stdout --cover-letter -n initial..master^ diff --abbrev initial..side +diff -U initial..side +diff -U1 initial..side diff -r initial..side diff --stat initial..side diff -r --stat initial..side diff --git a/t/t4013/diff.diff_-U1_initial..side b/t/t4013/diff.diff_-U1_initial..side new file mode 100644 index 0000000000..b69f8f048a --- /dev/null +++ b/t/t4013/diff.diff_-U1_initial..side @@ -0,0 +1,29 @@ +$ git diff -U1 initial..side +diff --git a/dir/sub b/dir/sub +index 35d242b..7289e35 100644 +--- a/dir/sub ++++ b/dir/sub +@@ -2 +2,3 @@ A + B ++1 ++2 +diff --git a/file0 b/file0 +index 01e79c3..f4615da 100644 +--- a/file0 ++++ b/file0 +@@ -3 +3,4 @@ + 3 ++A ++B ++C +diff --git a/file3 b/file3 +new file mode 100644 +index 0000000..7289e35 +--- /dev/null ++++ b/file3 +@@ -0,0 +1,4 @@ ++A ++B ++1 ++2 +$ diff --git a/t/t4013/diff.diff_-U2_initial..side b/t/t4013/diff.diff_-U2_initial..side new file mode 100644 index 0000000000..8ffe04f203 --- /dev/null +++ b/t/t4013/diff.diff_-U2_initial..side @@ -0,0 +1,31 @@ +$ git diff -U2 initial..side +diff --git a/dir/sub b/dir/sub +index 35d242b..7289e35 100644 +--- a/dir/sub ++++ b/dir/sub +@@ -1,2 +1,4 @@ + A + B ++1 ++2 +diff --git a/file0 b/file0 +index 01e79c3..f4615da 100644 +--- a/file0 ++++ b/file0 +@@ -2,2 +2,5 @@ + 2 + 3 ++A ++B ++C +diff --git a/file3 b/file3 +new file mode 100644 +index 0000000..7289e35 +--- /dev/null ++++ b/file3 +@@ -0,0 +1,4 @@ ++A ++B ++1 ++2 +$ diff --git a/t/t4013/diff.diff_-U_initial..side b/t/t4013/diff.diff_-U_initial..side new file mode 100644 index 0000000000..c66c0dd5c6 --- /dev/null +++ b/t/t4013/diff.diff_-U_initial..side @@ -0,0 +1,32 @@ +$ git diff -U initial..side +diff --git a/dir/sub b/dir/sub +index 35d242b..7289e35 100644 +--- a/dir/sub ++++ b/dir/sub +@@ -1,2 +1,4 @@ + A + B ++1 ++2 +diff --git a/file0 b/file0 +index 01e79c3..f4615da 100644 +--- a/file0 ++++ b/file0 +@@ -1,3 +1,6 @@ + 1 + 2 + 3 ++A ++B ++C +diff --git a/file3 b/file3 +new file mode 100644 +index 0000000..7289e35 +--- /dev/null ++++ b/file3 +@@ -0,0 +1,4 @@ ++A ++B ++1 ++2 +$ From f7e68a08780e91d7c2f830f33457041407172b96 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nguy=E1=BB=85n=20Th=C3=A1i=20Ng=E1=BB=8Dc=20Duy?= Date: Wed, 29 May 2019 16:11:16 +0700 Subject: [PATCH 65/66] parse-options: check empty value in OPT_INTEGER and OPT_ABBREV MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When parsing the argument for OPT_INTEGER and OPT_ABBREV, we check if we can parse the entire argument to a number with "if (*s)". There is one missing check: if "arg" is empty to begin with, we fail to notice. This could happen with long option by writing like git diff --inter-hunk-context= blah blah Before 16ed6c97cc (diff-parseopt: convert --inter-hunk-context, 2019-03-24), --inter-hunk-context is handled by a custom parser opt_arg() and does detect this correctly. This restores the bahvior for --inter-hunk-context and make sure all other integer options are handled the same (sane) way. For OPT_ABBREV this is new behavior. But it makes it consistent with the rest. PS. OPT_MAGNITUDE has similar code but git_parse_ulong() does detect empty "arg". So it's good to go. Signed-off-by: Nguyễn Thái Ngọc Duy Signed-off-by: Junio C Hamano --- parse-options-cb.c | 3 +++ parse-options.c | 3 +++ 2 files changed, 6 insertions(+) diff --git a/parse-options-cb.c b/parse-options-cb.c index 2733393546..02293493b0 100644 --- a/parse-options-cb.c +++ b/parse-options-cb.c @@ -16,6 +16,9 @@ int parse_opt_abbrev_cb(const struct option *opt, const char *arg, int unset) if (!arg) { v = unset ? 0 : DEFAULT_ABBREV; } else { + if (!*arg) + return error(_("option `%s' expects a numerical value"), + opt->long_name); v = strtol(arg, (char **)&arg, 10); if (*arg) return error(_("option `%s' expects a numerical value"), diff --git a/parse-options.c b/parse-options.c index cec74522e5..c8ee221196 100644 --- a/parse-options.c +++ b/parse-options.c @@ -193,6 +193,9 @@ static enum parse_opt_result get_value(struct parse_opt_ctx_t *p, } if (get_arg(p, opt, flags, &arg)) return -1; + if (!*arg) + return error(_("%s expects a numerical value"), + optname(opt, flags)); *(int *)opt->value = strtol(arg, (char **)&s, 10); if (*s) return error(_("%s expects a numerical value"), From 874dd410cecbc2953f624ab6ab9fda10d1650870 Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Thu, 30 May 2019 10:56:29 -0700 Subject: [PATCH 66/66] Git 2.22-rc2 Signed-off-by: Junio C Hamano --- Documentation/RelNotes/2.22.0.txt | 2 ++ GIT-VERSION-GEN | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/Documentation/RelNotes/2.22.0.txt b/Documentation/RelNotes/2.22.0.txt index cdfdabe7d9..84e1ba6106 100644 --- a/Documentation/RelNotes/2.22.0.txt +++ b/Documentation/RelNotes/2.22.0.txt @@ -586,3 +586,5 @@ Fixes since v2.21 (merge 397a46db78 js/t5580-unc-alternate-test later to maint). (merge d4907720a2 cm/notes-comment-fix later to maint). (merge 9dde06de13 cb/http-push-null-in-message-fix later to maint). + (merge 4c785c0edc js/rebase-config-bitfix later to maint). + (merge 8e9fe16c87 es/doc-gitsubmodules-markup later to maint). diff --git a/GIT-VERSION-GEN b/GIT-VERSION-GEN index 777a79e2b9..7a228b2a9c 100755 --- a/GIT-VERSION-GEN +++ b/GIT-VERSION-GEN @@ -1,7 +1,7 @@ #!/bin/sh GVF=GIT-VERSION-FILE -DEF_VER=v2.22.0-rc1 +DEF_VER=v2.22.0-rc2 LF=' '