From 53ec551c87c731c5c4171e943842998bdfb5548e Mon Sep 17 00:00:00 2001 From: Jeff King Date: Mon, 27 Jan 2014 20:36:12 -0500 Subject: [PATCH 01/82] expand_user_path: do not look at NULL path We explicitly check for and handle the case that the incoming "path" variable is NULL, but before doing so we call strchrnul on it, leading to a potential segfault. We can fix this simply by moving the strchrnul call down; as a bonus, we can tighten the scope on the associated variable. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- path.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/path.c b/path.c index 6f2aa699ad63c2..3a5796156bd3d5 100644 --- a/path.c +++ b/path.c @@ -231,12 +231,12 @@ static struct passwd *getpw_str(const char *username, size_t len) char *expand_user_path(const char *path) { struct strbuf user_path = STRBUF_INIT; - const char *first_slash = strchrnul(path, '/'); const char *to_copy = path; if (path == NULL) goto return_null; if (path[0] == '~') { + const char *first_slash = strchrnul(path, '/'); const char *username = path + 1; size_t username_len = first_slash - username; if (username_len == 0) { From 67beb600563cf28186f44450e528df1ec4d524fd Mon Sep 17 00:00:00 2001 From: Jeff King Date: Mon, 27 Jan 2014 20:37:30 -0500 Subject: [PATCH 02/82] handle_path_include: don't look at NULL value When we see config like: [include] path the expand_user_path helper notices that the config value is empty, but we then dereference NULL while printing the error message (glibc will helpfully print "(null)" for us here, but we cannot rely on that). $ git -c include.path rev-parse error: Could not expand include path '(null)' fatal: unable to parse command-line config Instead of tweaking our message, let's actually use config_error_nonbool to match other config variables that expect a value: $ git -c include.path rev-parse error: Missing value for 'include.path' fatal: unable to parse command-line config Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- config.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/config.c b/config.c index 2bbf02d1e8c2c0..47ce8d8b8eadb9 100644 --- a/config.c +++ b/config.c @@ -37,8 +37,12 @@ static int handle_path_include(const char *path, struct config_include_data *inc { int ret = 0; struct strbuf buf = STRBUF_INIT; - char *expanded = expand_user_path(path); + char *expanded; + if (!path) + return config_error_nonbool("include.path"); + + expanded = expand_user_path(path); if (!expanded) return error("Could not expand include path '%s'", path); path = expanded; From a43219f2aaa39fbc53ba0aee2481e1cca3c0e649 Mon Sep 17 00:00:00 2001 From: David Sharp Date: Tue, 28 Jan 2014 13:21:00 -0800 Subject: [PATCH 03/82] rev-parse: check i before using argv[i] against argc The --prefix, --default, and --resolve-git-dir options to git-rev-parse require an argument, but when given no argument, the code uses the NULL read from argv[argc] without checking, leading to a segfault. Instead, check first and die() with an error message. Signed-off-by: David Sharp Signed-off-by: Junio C Hamano --- builtin/rev-parse.c | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/builtin/rev-parse.c b/builtin/rev-parse.c index c76b89dc5bcccb..068fd17ac91be6 100644 --- a/builtin/rev-parse.c +++ b/builtin/rev-parse.c @@ -523,15 +523,17 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix) continue; } if (!strcmp(arg, "--default")) { - def = argv[i+1]; - i++; + def = argv[++i]; + if (!def) + die("--default requires an argument"); continue; } if (!strcmp(arg, "--prefix")) { - prefix = argv[i+1]; + prefix = argv[++i]; + if (!prefix) + die("--prefix requires an argument"); startup_info->prefix = prefix; output_prefix = 1; - i++; continue; } if (!strcmp(arg, "--revs-only")) { @@ -703,9 +705,12 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix) continue; } if (!strcmp(arg, "--resolve-git-dir")) { - const char *gitdir = resolve_gitdir(argv[i+1]); + const char *gitdir = argv[++i]; if (!gitdir) - die("not a gitdir '%s'", argv[i+1]); + die("--resolve-git-dir requires an argument"); + gitdir = resolve_gitdir(gitdir); + if (!gitdir) + die("not a gitdir '%s'", argv[i]); puts(gitdir); continue; } From c4a7bce1b5d1ad18482a87cb80a6bddabb925f62 Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Thu, 6 Feb 2014 10:16:27 -0800 Subject: [PATCH 04/82] t0003: do not chdir the whole test process Moving to some other directory and letting the remainder of the test pieces to expect that they start there is a bad practice. The test that contains chdir itself may fail (or by mistake skipped via the GIT_SKIP_TESTS mechanism) in which case the remainder may operate on files in unexpected places. Signed-off-by: Junio C Hamano --- t/t0003-attributes.sh | 52 +++++++++++++++++++++++++------------------ 1 file changed, 30 insertions(+), 22 deletions(-) diff --git a/t/t0003-attributes.sh b/t/t0003-attributes.sh index febc45c9cc6f34..0554b130f299fd 100755 --- a/t/t0003-attributes.sh +++ b/t/t0003-attributes.sh @@ -197,39 +197,47 @@ test_expect_success 'root subdir attribute test' ' ' test_expect_success 'setup bare' ' - git clone --bare . bare.git && - cd bare.git + git clone --bare . bare.git ' test_expect_success 'bare repository: check that .gitattribute is ignored' ' ( - echo "f test=f" - echo "a/i test=a/i" - ) >.gitattributes && - attr_check f unspecified && - attr_check a/f unspecified && - attr_check a/c/f unspecified && - attr_check a/i unspecified && - attr_check subdir/a/i unspecified + cd bare.git && + ( + echo "f test=f" + echo "a/i test=a/i" + ) >.gitattributes && + attr_check f unspecified && + attr_check a/f unspecified && + attr_check a/c/f unspecified && + attr_check a/i unspecified && + attr_check subdir/a/i unspecified + ) ' test_expect_success 'bare repository: check that --cached honors index' ' - GIT_INDEX_FILE=../.git/index \ - git check-attr --cached --stdin --all <../stdin-all | - sort >actual && - test_cmp ../specified-all actual + ( + cd bare.git && + GIT_INDEX_FILE=../.git/index \ + git check-attr --cached --stdin --all <../stdin-all | + sort >actual && + test_cmp ../specified-all actual + ) ' test_expect_success 'bare repository: test info/attributes' ' ( - echo "f test=f" - echo "a/i test=a/i" - ) >info/attributes && - attr_check f f && - attr_check a/f f && - attr_check a/c/f f && - attr_check a/i a/i && - attr_check subdir/a/i unspecified + cd bare.git && + ( + echo "f test=f" + echo "a/i test=a/i" + ) >info/attributes && + attr_check f f && + attr_check a/f f && + attr_check a/c/f f && + attr_check a/i a/i && + attr_check subdir/a/i unspecified + ) ' test_done From cdbf623254fc281e42eb41e700ae785813983960 Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Thu, 6 Feb 2014 10:19:33 -0800 Subject: [PATCH 05/82] check-attr: move to the top of working tree when in non-bare repository Lasse Makholm noticed that running "git check-attr" from a place totally unrelated to $GIT_DIR and $GIT_WORK_TREE does not give expected results. I think it is because the command does not say it wants to call setup_work_tree(). We still need to support use cases where only a bare repository is involved, so unconditionally requiring a working tree would not work well. Instead, make a call only in a non-bare repository. We may want to see if we want to do a similar fix in the opposite direction to check-ignore. The command unconditionally requires a working tree, but it should be usable in a bare repository just like check-attr attempts to be. Signed-off-by: Junio C Hamano --- builtin/check-attr.c | 3 +++ t/t0003-attributes.sh | 10 ++++++++++ 2 files changed, 13 insertions(+) diff --git a/builtin/check-attr.c b/builtin/check-attr.c index 075d01d30c58d4..f29d6c33e90455 100644 --- a/builtin/check-attr.c +++ b/builtin/check-attr.c @@ -94,6 +94,9 @@ int cmd_check_attr(int argc, const char **argv, const char *prefix) struct git_attr_check *check; int cnt, i, doubledash, filei; + if (!is_bare_repository()) + setup_work_tree(); + git_config(git_default_config, NULL); argc = parse_options(argc, argv, prefix, check_attr_options, diff --git a/t/t0003-attributes.sh b/t/t0003-attributes.sh index 0554b130f299fd..6e6aef598836b6 100755 --- a/t/t0003-attributes.sh +++ b/t/t0003-attributes.sh @@ -196,6 +196,16 @@ test_expect_success 'root subdir attribute test' ' attr_check subdir/a/i unspecified ' +test_expect_success 'using --git-dir and --work-tree' ' + mkdir unreal real && + git init real && + echo "file test=in-real" >real/.gitattributes && + ( + cd unreal && + attr_check file in-real "--git-dir ../real/.git --work-tree ../real" + ) +' + test_expect_success 'setup bare' ' git clone --bare . bare.git ' From 3bb486e4397415e1855e5edbf28d8e50c3d45265 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Mon, 10 Feb 2014 09:39:48 -0500 Subject: [PATCH 06/82] tests: auto-set LIB_HTTPD_PORT from test name We set the default apache port for each of the httpd tests to the 4-digit test number of the test script. We want these to remain unique so that the tests do not conflict with each other when run in parallel. Instead of doing it manually in each test script, let's just set it from the test name at run time. This is simpler, and is one less thing to be updated when test scripts are renamed (e.g., when being re-rolled or when conflicting after being merged with another topic). Incidentally, this fixes a case where t5537 and t5538 used the same port number (5537), and could conflict with each other when run in parallel. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- t/lib-httpd.sh | 2 +- t/t5537-fetch-shallow.sh | 1 - t/t5538-push-shallow.sh | 1 - t/t5540-http-push.sh | 1 - t/t5541-http-push.sh | 1 - t/t5550-http-fetch.sh | 1 - t/t5551-http-fetch.sh | 1 - t/t5561-http-backend.sh | 1 - 8 files changed, 1 insertion(+), 8 deletions(-) diff --git a/t/lib-httpd.sh b/t/lib-httpd.sh index bfdff2a8c93091..b43162ea2a932d 100644 --- a/t/lib-httpd.sh +++ b/t/lib-httpd.sh @@ -64,7 +64,7 @@ case $(uname) in esac LIB_HTTPD_PATH=${LIB_HTTPD_PATH-"$DEFAULT_HTTPD_PATH"} -LIB_HTTPD_PORT=${LIB_HTTPD_PORT-'8111'} +LIB_HTTPD_PORT=${LIB_HTTPD_PORT-${this_test#t}} TEST_PATH="$TEST_DIRECTORY"/lib-httpd HTTPD_ROOT_PATH="$PWD"/httpd diff --git a/t/t5537-fetch-shallow.sh b/t/t5537-fetch-shallow.sh index b0fa7387cbe082..adf215a1937cf2 100755 --- a/t/t5537-fetch-shallow.sh +++ b/t/t5537-fetch-shallow.sh @@ -178,7 +178,6 @@ if test -n "$NO_CURL" -o -z "$GIT_TEST_HTTPD"; then test_done fi -LIB_HTTPD_PORT=${LIB_HTTPD_PORT-'5537'} . "$TEST_DIRECTORY"/lib-httpd.sh start_httpd diff --git a/t/t5538-push-shallow.sh b/t/t5538-push-shallow.sh index 0a6e40f144a767..8e54ac57462987 100755 --- a/t/t5538-push-shallow.sh +++ b/t/t5538-push-shallow.sh @@ -126,7 +126,6 @@ if test -n "$NO_CURL" -o -z "$GIT_TEST_HTTPD"; then test_done fi -LIB_HTTPD_PORT=${LIB_HTTPD_PORT-'5537'} . "$TEST_DIRECTORY"/lib-httpd.sh start_httpd diff --git a/t/t5540-http-push.sh b/t/t5540-http-push.sh index 5b0198cbc88ad5..8d7b3c57e31dd3 100755 --- a/t/t5540-http-push.sh +++ b/t/t5540-http-push.sh @@ -16,7 +16,6 @@ then fi LIB_HTTPD_DAV=t -LIB_HTTPD_PORT=${LIB_HTTPD_PORT-'5540'} . "$TEST_DIRECTORY"/lib-httpd.sh ROOT_PATH="$PWD" start_httpd diff --git a/t/t5541-http-push.sh b/t/t5541-http-push.sh index bfd241ea8ae4e5..73af16f481836d 100755 --- a/t/t5541-http-push.sh +++ b/t/t5541-http-push.sh @@ -12,7 +12,6 @@ if test -n "$NO_CURL"; then fi ROOT_PATH="$PWD" -LIB_HTTPD_PORT=${LIB_HTTPD_PORT-'5541'} . "$TEST_DIRECTORY"/lib-httpd.sh . "$TEST_DIRECTORY"/lib-terminal.sh start_httpd diff --git a/t/t5550-http-fetch.sh b/t/t5550-http-fetch.sh index 83926247142458..1a3a2b6c1a2055 100755 --- a/t/t5550-http-fetch.sh +++ b/t/t5550-http-fetch.sh @@ -8,7 +8,6 @@ if test -n "$NO_CURL"; then test_done fi -LIB_HTTPD_PORT=${LIB_HTTPD_PORT-'5550'} . "$TEST_DIRECTORY"/lib-httpd.sh start_httpd diff --git a/t/t5551-http-fetch.sh b/t/t5551-http-fetch.sh index a124efe1145990..e07eaf35f11889 100755 --- a/t/t5551-http-fetch.sh +++ b/t/t5551-http-fetch.sh @@ -8,7 +8,6 @@ if test -n "$NO_CURL"; then test_done fi -LIB_HTTPD_PORT=${LIB_HTTPD_PORT-'5551'} . "$TEST_DIRECTORY"/lib-httpd.sh start_httpd diff --git a/t/t5561-http-backend.sh b/t/t5561-http-backend.sh index b5d7fbc3815aed..d23fb023848352 100755 --- a/t/t5561-http-backend.sh +++ b/t/t5561-http-backend.sh @@ -8,7 +8,6 @@ if test -n "$NO_CURL"; then test_done fi -LIB_HTTPD_PORT=${LIB_HTTPD_PORT-'5561'} . "$TEST_DIRECTORY"/lib-httpd.sh start_httpd From a87679339c0abb67b0be8b8b1cc10483f28df038 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nguy=E1=BB=85n=20Th=C3=A1i=20Ng=E1=BB=8Dc=20Duy?= Date: Thu, 6 Feb 2014 22:10:34 +0700 Subject: [PATCH 07/82] test: rename http fetch and push test files MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Make clear which one is for dumb protocol, which one is for smart from their file name. Signed-off-by: Nguyễn Thái Ngọc Duy Signed-off-by: Junio C Hamano --- t/{t5540-http-push.sh => t5540-http-push-webdav.sh} | 0 t/{t5541-http-push.sh => t5541-http-push-smart.sh} | 0 t/{t5550-http-fetch.sh => t5550-http-fetch-dumb.sh} | 0 t/{t5551-http-fetch.sh => t5551-http-fetch-smart.sh} | 0 4 files changed, 0 insertions(+), 0 deletions(-) rename t/{t5540-http-push.sh => t5540-http-push-webdav.sh} (100%) rename t/{t5541-http-push.sh => t5541-http-push-smart.sh} (100%) rename t/{t5550-http-fetch.sh => t5550-http-fetch-dumb.sh} (100%) rename t/{t5551-http-fetch.sh => t5551-http-fetch-smart.sh} (100%) diff --git a/t/t5540-http-push.sh b/t/t5540-http-push-webdav.sh similarity index 100% rename from t/t5540-http-push.sh rename to t/t5540-http-push-webdav.sh diff --git a/t/t5541-http-push.sh b/t/t5541-http-push-smart.sh similarity index 100% rename from t/t5541-http-push.sh rename to t/t5541-http-push-smart.sh diff --git a/t/t5550-http-fetch.sh b/t/t5550-http-fetch-dumb.sh similarity index 100% rename from t/t5550-http-fetch.sh rename to t/t5550-http-fetch-dumb.sh diff --git a/t/t5551-http-fetch.sh b/t/t5551-http-fetch-smart.sh similarity index 100% rename from t/t5551-http-fetch.sh rename to t/t5551-http-fetch-smart.sh From 32752e966d3370f6b411b42fe3a301bbd46d3eae Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nguy=E1=BB=85n=20Th=C3=A1i=20Ng=E1=BB=8Dc=20Duy?= Date: Thu, 6 Feb 2014 22:10:36 +0700 Subject: [PATCH 08/82] pack-protocol.txt: clarify 'obj-id' in the last ACK after 'done' MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit It's introduced in 1bd8c8f (git-upload-pack: Support the multi_ack protocol - 2005-10-28) but probably better documented in the commit message of 78affc4 (Add multi_ack_detailed capability to fetch-pack/upload-pack - 2009-10-30). Signed-off-by: Nguyễn Thái Ngọc Duy Signed-off-by: Junio C Hamano --- Documentation/technical/pack-protocol.txt | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Documentation/technical/pack-protocol.txt b/Documentation/technical/pack-protocol.txt index c73b62f5e1ced7..39c64105a61a37 100644 --- a/Documentation/technical/pack-protocol.txt +++ b/Documentation/technical/pack-protocol.txt @@ -338,7 +338,8 @@ during a prior round. This helps to ensure that at least one common ancestor is found before we give up entirely. Once the 'done' line is read from the client, the server will either -send a final 'ACK obj-id' or it will send a 'NAK'. The server only sends +send a final 'ACK obj-id' or it will send a 'NAK'. 'obj-id' is the object +name of the last commit determined to be common. The server only sends ACK after 'done' if there is at least one common base and multi_ack or multi_ack_detailed is enabled. The server always sends NAK after 'done' if there is no common base found. From 087e347f2672b9a950d6da56179ca44eb7ded834 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nguy=E1=BB=85n=20Th=C3=A1i=20Ng=E1=BB=8Dc=20Duy?= Date: Thu, 6 Feb 2014 22:10:37 +0700 Subject: [PATCH 09/82] protocol-capabilities.txt: refer multi_ack_detailed back to pack-protocol.txt MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit pack-protocol.txt explains in detail how multi_ack_detailed works and what's the difference between no multi_ack, multi_ack and multi_ack_detailed. No need to repeat here. Signed-off-by: Nguyễn Thái Ngọc Duy Signed-off-by: Junio C Hamano --- Documentation/technical/protocol-capabilities.txt | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/Documentation/technical/protocol-capabilities.txt b/Documentation/technical/protocol-capabilities.txt index e3e792476e7a6b..cb40ebbd8baf13 100644 --- a/Documentation/technical/protocol-capabilities.txt +++ b/Documentation/technical/protocol-capabilities.txt @@ -69,6 +69,12 @@ ends. Without multi_ack the client would have sent that c-b-a chain anyway, interleaved with S-R-Q. +multi_ack_detailed +------------------ +This is an extension of multi_ack that permits client to better +understand the server's in-memory state. See pack-protocol.txt, +section "Packfile Negotiation" for more information. + thin-pack --------- From c9cd60f6fa064595e4c1a8a84b9a15cc016a09e1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nguy=E1=BB=85n=20Th=C3=A1i=20Ng=E1=BB=8Dc=20Duy?= Date: Thu, 6 Feb 2014 22:10:38 +0700 Subject: [PATCH 10/82] protocol-capabilities.txt: document no-done MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit See 3e63b21 (upload-pack: Implement no-done capability - 2011-03-14) and 761ecf0 (fetch-pack: Implement no-done capability - 2011-03-14) for more information. Signed-off-by: Nguyễn Thái Ngọc Duy Signed-off-by: Junio C Hamano --- Documentation/technical/protocol-capabilities.txt | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/Documentation/technical/protocol-capabilities.txt b/Documentation/technical/protocol-capabilities.txt index cb40ebbd8baf13..e1743438472466 100644 --- a/Documentation/technical/protocol-capabilities.txt +++ b/Documentation/technical/protocol-capabilities.txt @@ -75,6 +75,18 @@ This is an extension of multi_ack that permits client to better understand the server's in-memory state. See pack-protocol.txt, section "Packfile Negotiation" for more information. +no-done +------- +This capability should only be used with the smart HTTP protocol. If +multi_ack_detailed and no-done are both present, then the sender is +free to immediately send a pack following its first "ACK obj-id ready" +message. + +Without no-done in the smart HTTP protocol, the server session would +end and the client has to make another trip to send "done" before +the server can send the pack. no-done removes the last round and +thus slightly reduces latency. + thin-pack --------- From ff62eca7d1f9716e36550adedc6e8edc35ff9a15 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nguy=E1=BB=85n=20Th=C3=A1i=20Ng=E1=BB=8Dc=20Duy?= Date: Thu, 6 Feb 2014 22:10:39 +0700 Subject: [PATCH 11/82] fetch-pack: fix deepen shallow over smart http with no-done cap MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In smart http, upload-pack adds new shallow lines at the beginning of each rpc response. Only shallow lines from the first rpc call are useful. After that they are thrown away. It's designed this way because upload-pack is stateless and has no idea when its shallow lines are helpful or not. So after refs are negotiated with multi_ack_detailed and the server thinks it learned enough, it sends "ACK obj-id ready", terminates the rpc call and waits for the final rpc round. The client sends "done". The server sends another response, which also has shallow lines at the beginning, and the last "ACK obj-id" line. When no-done is active, the last round is cut out, the server sends "ACK obj-id ready" and "ACK obj-id" in the same rpc response. fetch-pack is updated to recognize this and not send "done". However it still tries to consume shallow lines, which are never sent. Update the code, make sure to skip consuming shallow lines when no-done is enabled. Reported-by: Jeff King Signed-off-by: Nguyễn Thái Ngọc Duy Signed-off-by: Junio C Hamano --- fetch-pack.c | 3 ++- t/t5537-fetch-shallow.sh | 30 ++++++++++++++++++++++++++++++ 2 files changed, 32 insertions(+), 1 deletion(-) diff --git a/fetch-pack.c b/fetch-pack.c index 90fdd49821a1d6..f061f1fe85ea20 100644 --- a/fetch-pack.c +++ b/fetch-pack.c @@ -439,7 +439,8 @@ static int find_common(struct fetch_pack_args *args, } strbuf_release(&req_buf); - consume_shallow_list(args, fd[0]); + if (!got_ready || !no_done) + consume_shallow_list(args, fd[0]); while (flushes || multi_ack) { int ack = get_ack(fd[0], result_sha1); if (ack) { diff --git a/t/t5537-fetch-shallow.sh b/t/t5537-fetch-shallow.sh index adf215a1937cf2..098f220bbee890 100755 --- a/t/t5537-fetch-shallow.sh +++ b/t/t5537-fetch-shallow.sh @@ -199,5 +199,35 @@ EOF ) ' +# This test is tricky. We need large enough "have"s that fetch-pack +# will put pkt-flush in between. Then we need a "have" the server +# does not have, it'll send "ACK %s ready" +test_expect_success 'no shallow lines after receiving ACK ready' ' + ( + cd shallow && + for i in $(test_seq 10) + do + git checkout --orphan unrelated$i && + test_commit unrelated$i && + git push -q "$HTTPD_DOCUMENT_ROOT_PATH/repo.git" \ + refs/heads/unrelated$i:refs/heads/unrelated$i && + git push -q ../clone/.git \ + refs/heads/unrelated$i:refs/heads/unrelated$i || + exit 1 + done && + git checkout master && + test_commit new && + git push "$HTTPD_DOCUMENT_ROOT_PATH/repo.git" master + ) && + ( + cd clone && + git checkout --orphan newnew && + test_commit new-too && + GIT_TRACE_PACKET="$TRASH_DIRECTORY/trace" git fetch --depth=2 && + grep "fetch-pack< ACK .* ready" ../trace && + ! grep "fetch-pack> done" ../trace + ) +' + stop_httpd test_done From 0232852b06cb000a3b1f5f48676c8b4d084f18ea Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nguy=E1=BB=85n=20Th=C3=A1i=20Ng=E1=BB=8Dc=20Duy?= Date: Thu, 13 Feb 2014 20:21:14 +0700 Subject: [PATCH 12/82] t5537: move http tests out to t5539 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit start_httpd is supposed to be at the beginning of the test file, not the middle of it. The "test_seq" line in "no shallow lines.." test is updated to compensate missing refs that are there in t5537, but not in the new t5539. Signed-off-by: Nguyễn Thái Ngọc Duy Signed-off-by: Junio C Hamano --- t/t5537-fetch-shallow.sh | 57 ------------------------ t/t5539-fetch-http-shallow.sh | 82 +++++++++++++++++++++++++++++++++++ 2 files changed, 82 insertions(+), 57 deletions(-) create mode 100755 t/t5539-fetch-http-shallow.sh diff --git a/t/t5537-fetch-shallow.sh b/t/t5537-fetch-shallow.sh index 098f220bbee890..3ae9092f5c2511 100755 --- a/t/t5537-fetch-shallow.sh +++ b/t/t5537-fetch-shallow.sh @@ -173,61 +173,4 @@ EOF ) ' -if test -n "$NO_CURL" -o -z "$GIT_TEST_HTTPD"; then - say 'skipping remaining tests, git built without http support' - test_done -fi - -. "$TEST_DIRECTORY"/lib-httpd.sh -start_httpd - -test_expect_success 'clone http repository' ' - git clone --bare --no-local shallow "$HTTPD_DOCUMENT_ROOT_PATH/repo.git" && - git clone $HTTPD_URL/smart/repo.git clone && - ( - cd clone && - git fsck && - git log --format=%s origin/master >actual && - cat <expect && -7 -6 -5 -4 -3 -EOF - test_cmp expect actual - ) -' - -# This test is tricky. We need large enough "have"s that fetch-pack -# will put pkt-flush in between. Then we need a "have" the server -# does not have, it'll send "ACK %s ready" -test_expect_success 'no shallow lines after receiving ACK ready' ' - ( - cd shallow && - for i in $(test_seq 10) - do - git checkout --orphan unrelated$i && - test_commit unrelated$i && - git push -q "$HTTPD_DOCUMENT_ROOT_PATH/repo.git" \ - refs/heads/unrelated$i:refs/heads/unrelated$i && - git push -q ../clone/.git \ - refs/heads/unrelated$i:refs/heads/unrelated$i || - exit 1 - done && - git checkout master && - test_commit new && - git push "$HTTPD_DOCUMENT_ROOT_PATH/repo.git" master - ) && - ( - cd clone && - git checkout --orphan newnew && - test_commit new-too && - GIT_TRACE_PACKET="$TRASH_DIRECTORY/trace" git fetch --depth=2 && - grep "fetch-pack< ACK .* ready" ../trace && - ! grep "fetch-pack> done" ../trace - ) -' - -stop_httpd test_done diff --git a/t/t5539-fetch-http-shallow.sh b/t/t5539-fetch-http-shallow.sh new file mode 100755 index 00000000000000..94553e10396843 --- /dev/null +++ b/t/t5539-fetch-http-shallow.sh @@ -0,0 +1,82 @@ +#!/bin/sh + +test_description='fetch/clone from a shallow clone over http' + +. ./test-lib.sh + +if test -n "$NO_CURL"; then + skip_all='skipping test, git built without http support' + test_done +fi + +. "$TEST_DIRECTORY"/lib-httpd.sh +start_httpd + +commit() { + echo "$1" >tracked && + git add tracked && + git commit -m "$1" +} + +test_expect_success 'setup shallow clone' ' + commit 1 && + commit 2 && + commit 3 && + commit 4 && + commit 5 && + commit 6 && + commit 7 && + git clone --no-local --depth=5 .git shallow && + git config --global transfer.fsckObjects true +' + +test_expect_success 'clone http repository' ' + git clone --bare --no-local shallow "$HTTPD_DOCUMENT_ROOT_PATH/repo.git" && + git clone $HTTPD_URL/smart/repo.git clone && + ( + cd clone && + git fsck && + git log --format=%s origin/master >actual && + cat <expect && +7 +6 +5 +4 +3 +EOF + test_cmp expect actual + ) +' + +# This test is tricky. We need large enough "have"s that fetch-pack +# will put pkt-flush in between. Then we need a "have" the server +# does not have, it'll send "ACK %s ready" +test_expect_success 'no shallow lines after receiving ACK ready' ' + ( + cd shallow && + for i in $(test_seq 15) + do + git checkout --orphan unrelated$i && + test_commit unrelated$i && + git push -q "$HTTPD_DOCUMENT_ROOT_PATH/repo.git" \ + refs/heads/unrelated$i:refs/heads/unrelated$i && + git push -q ../clone/.git \ + refs/heads/unrelated$i:refs/heads/unrelated$i || + exit 1 + done && + git checkout master && + test_commit new && + git push "$HTTPD_DOCUMENT_ROOT_PATH/repo.git" master + ) && + ( + cd clone && + git checkout --orphan newnew && + test_commit new-too && + GIT_TRACE_PACKET="$TRASH_DIRECTORY/trace" git fetch --depth=2 && + grep "fetch-pack< ACK .* ready" ../trace && + ! grep "fetch-pack> done" ../trace + ) +' + +stop_httpd +test_done From aba4727281612c3e24914691727e11e1f44a9aac Mon Sep 17 00:00:00 2001 From: Thomas Rast Date: Sun, 16 Feb 2014 17:52:34 +0100 Subject: [PATCH 13/82] diff: do not reuse_worktree_file for submodules MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The GIT_EXTERNAL_DIFF calling code attempts to reuse existing worktree files for the worktree side of diffs, for performance reasons. However, that code also tries to do the same with submodules. This results in calls to $GIT_EXTERNAL_DIFF where the old-file is a file of the form "Submodule commit $sha1", but the new-file is a directory in the worktree. Fix it by never reusing a worktree "file" in the submodule case. Reported-by: Grégory Pakosz Signed-off-by: Thomas Rast Signed-off-by: Junio C Hamano --- diff.c | 5 +++-- t/t4020-diff-external.sh | 30 +++++++++++++++++++++++++++++- 2 files changed, 32 insertions(+), 3 deletions(-) diff --git a/diff.c b/diff.c index 266112ca610445..a96992a1bdc2a7 100644 --- a/diff.c +++ b/diff.c @@ -2842,8 +2842,9 @@ static struct diff_tempfile *prepare_temp_file(const char *name, remove_tempfile_installed = 1; } - if (!one->sha1_valid || - reuse_worktree_file(name, one->sha1, 1)) { + if (!S_ISGITLINK(one->mode) && + (!one->sha1_valid || + reuse_worktree_file(name, one->sha1, 1))) { struct stat st; if (lstat(name, &st) < 0) { if (errno == ENOENT) diff --git a/t/t4020-diff-external.sh b/t/t4020-diff-external.sh index 2e7d73f0906e6a..295a563496dd4c 100755 --- a/t/t4020-diff-external.sh +++ b/t/t4020-diff-external.sh @@ -213,12 +213,13 @@ keep_only_cr () { } test_expect_success 'external diff with autocrlf = true' ' - git config core.autocrlf true && + test_config core.autocrlf true && GIT_EXTERNAL_DIFF=./fake-diff.sh git diff && test $(wc -l < crlfed.txt) = $(cat crlfed.txt | keep_only_cr | wc -c) ' test_expect_success 'diff --cached' ' + test_config core.autocrlf true && git add file && git update-index --assume-unchanged file && echo second >file && @@ -226,4 +227,31 @@ test_expect_success 'diff --cached' ' test_cmp "$TEST_DIRECTORY"/t4020/diff.NUL actual ' +test_expect_success 'clean up crlf leftovers' ' + git update-index --no-assume-unchanged file && + rm -f file* && + git reset --hard +' + +test_expect_success 'submodule diff' ' + git init sub && + ( cd sub && test_commit sub1 ) && + git add sub && + test_tick && + git commit -m "add submodule" && + ( cd sub && test_commit sub2 ) && + write_script gather_pre_post.sh <<-\EOF && + echo "$1 $4" # path, mode + cat "$2" # old file + cat "$5" # new file + EOF + GIT_EXTERNAL_DIFF=./gather_pre_post.sh git diff >actual && + cat >expected <<-EOF && + sub 160000 + Subproject commit $(git rev-parse HEAD:sub) + Subproject commit $(cd sub && git rev-parse HEAD) + EOF + test_cmp expected actual +' + test_done From b7756d41dcc6720e6005b663bcb6f8b2f2a66763 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nguy=E1=BB=85n=20Th=C3=A1i=20Ng=E1=BB=8Dc=20Duy?= Date: Sun, 16 Feb 2014 09:28:03 +0700 Subject: [PATCH 14/82] reset: optionally setup worktree and refresh index on --mixed MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Refreshing index requires work tree. So we have two options: always set up work tree (and refuse to reset if failing to do so), or make refreshing index optional. As refreshing index is not the main task, it makes more sense to make it optional. This allows us to still work in a bare repository to update what is in the index. Reported-by: Patrick Palka Signed-off-by: Nguyễn Thái Ngọc Duy Signed-off-by: Junio C Hamano --- builtin/reset.c | 7 ++++--- t/t7102-reset.sh | 11 +++++++++++ 2 files changed, 15 insertions(+), 3 deletions(-) diff --git a/builtin/reset.c b/builtin/reset.c index 60048030dd4d05..a9913443580846 100644 --- a/builtin/reset.c +++ b/builtin/reset.c @@ -320,7 +320,7 @@ int cmd_reset(int argc, const char **argv, const char *prefix) if (reset_type == NONE) reset_type = MIXED; /* by default */ - if (reset_type != SOFT && reset_type != MIXED) + if (reset_type != SOFT && (reset_type != MIXED || get_git_work_tree())) setup_work_tree(); if (reset_type == MIXED && is_bare_repository()) @@ -340,8 +340,9 @@ int cmd_reset(int argc, const char **argv, const char *prefix) int flags = quiet ? REFRESH_QUIET : REFRESH_IN_PORCELAIN; if (read_from_tree(&pathspec, sha1)) return 1; - refresh_index(&the_index, flags, NULL, NULL, - _("Unstaged changes after reset:")); + if (get_git_work_tree()) + refresh_index(&the_index, flags, NULL, NULL, + _("Unstaged changes after reset:")); } else { int err = reset_index(sha1, reset_type, quiet); if (reset_type == KEEP && !err) diff --git a/t/t7102-reset.sh b/t/t7102-reset.sh index 8d4b50d1b5816d..ee117e2e727d2b 100755 --- a/t/t7102-reset.sh +++ b/t/t7102-reset.sh @@ -535,4 +535,15 @@ test_expect_success 'reset with paths accepts tree' ' git diff HEAD --exit-code ' +test_expect_success 'reset --mixed sets up work tree' ' + git init mixed_worktree && + ( + cd mixed_worktree && + test_commit dummy + ) && + : >expect && + git --git-dir=mixed_worktree/.git --work-tree=mixed_worktree reset >actual && + test_cmp expect actual +' + test_done From 94eaa806519498de2ca59a424b013812d72d21c5 Mon Sep 17 00:00:00 2001 From: David Aguilar Date: Sun, 23 Feb 2014 19:12:35 -0800 Subject: [PATCH 15/82] difftool: support repositories with .git-files MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Modern versions of "git submodule" use .git-files to setup the submodule directory. When run in a "git submodule"-created repository "git difftool --dir-diff" dies with the following error: $ git difftool -d HEAD~ fatal: This operation must be run in a work tree diff --raw --no-abbrev -z HEAD~: command returned error: 128 core.worktree is relative to the .git directory but the logic in find_worktree() does not account for it. Use `git rev-parse --show-toplevel` to find the worktree so that the dir-diff feature works inside a submodule. Reported-by: Gábor Lipták Helped-by: Jens Lehmann Helped-by: John Keeping Signed-off-by: David Aguilar Signed-off-by: Junio C Hamano --- git-difftool.perl | 18 ++---------------- 1 file changed, 2 insertions(+), 16 deletions(-) diff --git a/git-difftool.perl b/git-difftool.perl index e57d3d1295a5ba..18ca61e8d0493b 100755 --- a/git-difftool.perl +++ b/git-difftool.perl @@ -39,24 +39,10 @@ sub usage sub find_worktree { - my ($repo) = @_; - # Git->repository->wc_path() does not honor changes to the working # tree location made by $ENV{GIT_WORK_TREE} or the 'core.worktree' # config variable. - my $worktree; - my $env_worktree = $ENV{GIT_WORK_TREE}; - my $core_worktree = Git::config('core.worktree'); - - if (defined($env_worktree) and (length($env_worktree) > 0)) { - $worktree = $env_worktree; - } elsif (defined($core_worktree) and (length($core_worktree) > 0)) { - $worktree = $core_worktree; - } else { - $worktree = $repo->wc_path(); - } - - return $worktree; + return Git::command_oneline('rev-parse', '--show-toplevel'); } sub print_tool_help @@ -418,7 +404,7 @@ sub dir_diff my $rc; my $error = 0; my $repo = Git->repository(); - my $workdir = find_worktree($repo); + my $workdir = find_worktree(); my ($a, $b, $tmpdir, @worktree) = setup_dir_diff($repo, $workdir, $symlinks); From 7d9a2819415663ee5f0676d06cdbb1368fdc02c7 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Mon, 24 Feb 2014 02:36:22 -0500 Subject: [PATCH 16/82] t4212: test bogus timestamps with git-log When t4212 was originally added by 9dbe7c3d (pretty: handle broken commit headers gracefully, 2013-04-17), it tested our handling of commits with broken ident lines in which the timestamps could not be parsed. It does so using a bogus line like "Name -<> 1234 -0000", because that simulates an error that was seen in the wild. Later, 03818a4 (split_ident: parse timestamp from end of line, 2013-10-14) made our parser smart enough to actually find the timestamp on such a line, and t4212 was adjusted to match. While it's nice that we handle this real-world case, this meant that we were not actually testing the bogus-timestamp case anymore. This patch adds a test with a totally incomprehensible timestamp to make sure we are testing the code path. Note that the behavior is slightly different between regular log output and "--format=%ad". In the former case, we produce a sentinel value and in the latter, we produce an empty string. While at first this seems unnecessarily inconsistent, it matches the original behavior given by 9dbe7c3d. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- t/t4212-log-corrupt.sh | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/t/t4212-log-corrupt.sh b/t/t4212-log-corrupt.sh index ec5099b83d7490..611b687a3cde08 100755 --- a/t/t4212-log-corrupt.sh +++ b/t/t4212-log-corrupt.sh @@ -39,4 +39,25 @@ test_expect_success 'git log --format with broken author email' ' test_cmp expect.err actual.err ' +munge_author_date () { + git cat-file commit "$1" >commit.orig && + sed "s/^\(author .*>\) [0-9]*/\1 $2/" commit.munge && + git hash-object -w -t commit commit.munge +} + +test_expect_success 'unparsable dates produce sentinel value' ' + commit=$(munge_author_date HEAD totally_bogus) && + echo "Date: Thu Jan 1 00:00:00 1970 +0000" >expect && + git log -1 $commit >actual.full && + grep Date actual && + test_cmp expect actual +' + +test_expect_success 'unparsable dates produce sentinel value (%ad)' ' + commit=$(munge_author_date HEAD totally_bogus) && + echo >expect && + git log -1 --format=%ad $commit >actual + test_cmp expect actual +' + test_done From d4b8de0420ffcc7a654ddc6c69a96d3c1b25b4fa Mon Sep 17 00:00:00 2001 From: Jeff King Date: Mon, 24 Feb 2014 02:39:04 -0500 Subject: [PATCH 17/82] fsck: report integer overflow in author timestamps When we check commit objects, we complain if commit->date is ULONG_MAX, which is an indication that we saw integer overflow when parsing it. However, we do not do any check at all for author lines, which also contain a timestamp. Let's actually check the timestamps on each ident line with strtoul. This catches both author and committer lines, and we can get rid of the now-redundant commit->date check. Note that like the existing check, we compare only against ULONG_MAX. Now that we are calling strtoul at the site of the check, we could be slightly more careful and also check that errno is set to ERANGE. However, this will make further refactoring in future patches a little harder, and it doesn't really matter in practice. For 32-bit systems, one would have to create a commit at the exact wrong second in 2038. But by the time we get close to that, all systems will hopefully have moved to 64-bit (and if they haven't, they have a real problem one second later). For 64-bit systems, by the time we get close to ULONG_MAX, all systems will hopefully have been consumed in the fiery wrath of our expanding Sun. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- fsck.c | 12 ++++++------ t/t1450-fsck.sh | 14 ++++++++++++++ 2 files changed, 20 insertions(+), 6 deletions(-) diff --git a/fsck.c b/fsck.c index 99c04976748428..760e072ccef1e2 100644 --- a/fsck.c +++ b/fsck.c @@ -245,6 +245,8 @@ static int fsck_tree(struct tree *item, int strict, fsck_error error_func) static int fsck_ident(char **ident, struct object *obj, fsck_error error_func) { + char *end; + if (**ident == '<') return error_func(obj, FSCK_ERROR, "invalid author/committer line - missing space before email"); *ident += strcspn(*ident, "<>\n"); @@ -264,10 +266,11 @@ static int fsck_ident(char **ident, struct object *obj, fsck_error error_func) (*ident)++; if (**ident == '0' && (*ident)[1] != ' ') return error_func(obj, FSCK_ERROR, "invalid author/committer line - zero-padded date"); - *ident += strspn(*ident, "0123456789"); - if (**ident != ' ') + if (strtoul(*ident, &end, 10) == ULONG_MAX) + return error_func(obj, FSCK_ERROR, "invalid author/committer line - date causes integer overflow"); + if (end == *ident || *end != ' ') return error_func(obj, FSCK_ERROR, "invalid author/committer line - bad date"); - (*ident)++; + *ident = end + 1; if ((**ident != '+' && **ident != '-') || !isdigit((*ident)[1]) || !isdigit((*ident)[2]) || @@ -287,9 +290,6 @@ static int fsck_commit(struct commit *commit, fsck_error error_func) int parents = 0; int err; - if (commit->date == ULONG_MAX) - return error_func(&commit->object, FSCK_ERROR, "invalid author/committer line"); - if (memcmp(buffer, "tree ", 5)) return error_func(&commit->object, FSCK_ERROR, "invalid format - expected 'tree' line"); if (get_sha1_hex(buffer+5, tree_sha1) || buffer[45] != '\n') diff --git a/t/t1450-fsck.sh b/t/t1450-fsck.sh index d730734fde8e4d..8c739c96135fe9 100755 --- a/t/t1450-fsck.sh +++ b/t/t1450-fsck.sh @@ -142,6 +142,20 @@ test_expect_success '> in name is reported' ' grep "error in commit $new" out ' +# date is 2^64 + 1 +test_expect_success 'integer overflow in timestamps is reported' ' + git cat-file commit HEAD >basis && + sed "s/^\\(author .*>\\) [0-9]*/\\1 18446744073709551617/" \ + bad-timestamp && + new=$(git hash-object -t commit -w --stdin out && + cat out && + grep "error in commit $new.*integer overflow" out +' + test_expect_success 'tag pointing to nonexistent' ' cat >invalid-tag <<-\EOF && object ffffffffffffffffffffffffffffffffffffffff From 7ca36d9398a85e7974d04f8fbd2c6adb088290e1 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Mon, 24 Feb 2014 02:39:45 -0500 Subject: [PATCH 18/82] date: check date overflow against time_t When we check whether a timestamp has overflowed, we check only against ULONG_MAX, meaning that strtoul has overflowed. However, we also feed these timestamps to system functions like gmtime, which expect a time_t. On many systems, time_t is actually smaller than "unsigned long" (e.g., because it is signed), and we would overflow when using these functions. We don't know the actual size or signedness of time_t, but we can easily check for truncation with a simple assignment. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- cache.h | 1 + date.c | 17 +++++++++++++++++ fsck.c | 2 +- 3 files changed, 19 insertions(+), 1 deletion(-) diff --git a/cache.h b/cache.h index bb71bf8a7f92ba..9a2c377b73f109 100644 --- a/cache.h +++ b/cache.h @@ -909,6 +909,7 @@ void datestamp(char *buf, int bufsize); unsigned long approxidate_careful(const char *, int *); unsigned long approxidate_relative(const char *date, const struct timeval *now); enum date_mode parse_date_format(const char *format); +int date_overflows(unsigned long date); #define IDENT_STRICT 1 #define IDENT_NO_DATE 2 diff --git a/date.c b/date.c index 57331ed406e239..2dae471dd12400 100644 --- a/date.c +++ b/date.c @@ -1085,3 +1085,20 @@ unsigned long approxidate_careful(const char *date, int *error_ret) gettimeofday(&tv, NULL); return approxidate_str(date, &tv, error_ret); } + +int date_overflows(unsigned long t) +{ + time_t sys; + + /* If we overflowed our unsigned long, that's bad... */ + if (t == ULONG_MAX) + return 1; + + /* + * ...but we also are going to feed the result to system + * functions that expect time_t, which is often "signed long". + * Make sure that we fit into time_t, as well. + */ + sys = t; + return t != sys || (t < 1) != (sys < 1); +} diff --git a/fsck.c b/fsck.c index 760e072ccef1e2..64bf279fd7e42d 100644 --- a/fsck.c +++ b/fsck.c @@ -266,7 +266,7 @@ static int fsck_ident(char **ident, struct object *obj, fsck_error error_func) (*ident)++; if (**ident == '0' && (*ident)[1] != ' ') return error_func(obj, FSCK_ERROR, "invalid author/committer line - zero-padded date"); - if (strtoul(*ident, &end, 10) == ULONG_MAX) + if (date_overflows(strtoul(*ident, &end, 10))) return error_func(obj, FSCK_ERROR, "invalid author/committer line - date causes integer overflow"); if (end == *ident || *end != ' ') return error_func(obj, FSCK_ERROR, "invalid author/committer line - bad date"); From 1dca155fe3fac29e847d2d8ff1087d892a129a9c Mon Sep 17 00:00:00 2001 From: Jeff King Date: Mon, 24 Feb 2014 02:46:37 -0500 Subject: [PATCH 19/82] log: handle integer overflow in timestamps If an ident line has a ridiculous date value like (2^64)+1, we currently just pass ULONG_MAX along to the date code, which can produce nonsensical dates. On systems with a signed long time_t (e.g., 64-bit glibc systems), this actually doesn't end up too bad. The ULONG_MAX is converted to -1, we apply the timezone field to that, and the result ends up somewhere between Dec 31, 1969 and Jan 1, 1970. However, there is still a few good reasons to detect the overflow explicitly: 1. On systems where "unsigned long" is smaller than time_t, we get a nonsensical date in the future. 2. Even where it would produce "Dec 31, 1969", it's easier to recognize "midnight Jan 1" as a consistent sentinel value for "we could not parse this". 3. Values which do not overflow strtoul but do overflow a signed time_t produce nonsensical values in the past. For example, on a 64-bit system with a signed long time_t, a timestamp of 18446744073000000000 produces a date in 1947. We also recognize overflow in the timezone field, which could produce nonsensical results. In this case we show the parsed date, but in UTC. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- pretty.c | 10 ++++++++-- t/t4212-log-corrupt.sh | 16 ++++++++++++++++ 2 files changed, 24 insertions(+), 2 deletions(-) diff --git a/pretty.c b/pretty.c index acbfceb5fea49e..4da9a682f3bd6f 100644 --- a/pretty.c +++ b/pretty.c @@ -401,8 +401,14 @@ static const char *show_ident_date(const struct ident_split *ident, if (ident->date_begin && ident->date_end) date = strtoul(ident->date_begin, NULL, 10); - if (ident->tz_begin && ident->tz_end) - tz = strtol(ident->tz_begin, NULL, 10); + if (date_overflows(date)) + date = 0; + else { + if (ident->tz_begin && ident->tz_end) + tz = strtol(ident->tz_begin, NULL, 10); + if (tz == LONG_MAX || tz == LONG_MIN) + tz = 0; + } return show_date(date, tz, mode); } diff --git a/t/t4212-log-corrupt.sh b/t/t4212-log-corrupt.sh index 611b687a3cde08..80542d624be288 100755 --- a/t/t4212-log-corrupt.sh +++ b/t/t4212-log-corrupt.sh @@ -60,4 +60,20 @@ test_expect_success 'unparsable dates produce sentinel value (%ad)' ' test_cmp expect actual ' +# date is 2^64 + 1 +test_expect_success 'date parser recognizes integer overflow' ' + commit=$(munge_author_date HEAD 18446744073709551617) && + echo "Thu Jan 1 00:00:00 1970 +0000" >expect && + git log -1 --format=%ad $commit >actual && + test_cmp expect actual +' + +# date is 2^64 - 2 +test_expect_success 'date parser recognizes time_t overflow' ' + commit=$(munge_author_date HEAD 18446744073709551614) && + echo "Thu Jan 1 00:00:00 1970 +0000" >expect && + git log -1 --format=%ad $commit >actual && + test_cmp expect actual +' + test_done From 2b15846dbfb31df10a69a4d56ae944a01563bc07 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Mon, 24 Feb 2014 02:49:05 -0500 Subject: [PATCH 20/82] log: do not segfault on gmtime errors Many code paths assume that show_date and show_ident_date cannot return NULL. For the most part, we handle missing or corrupt timestamps by showing the epoch time t=0. However, we might still return NULL if gmtime rejects the time_t we feed it, resulting in a segfault. Let's catch this case and just format t=0. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- date.c | 6 ++++-- t/t4212-log-corrupt.sh | 8 ++++++++ 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/date.c b/date.c index 2dae471dd12400..f64bbeb8237acc 100644 --- a/date.c +++ b/date.c @@ -184,8 +184,10 @@ const char *show_date(unsigned long time, int tz, enum date_mode mode) tz = local_tzoffset(time); tm = time_to_tm(time, tz); - if (!tm) - return NULL; + if (!tm) { + tm = time_to_tm(0, 0); + tz = 0; + } strbuf_reset(&timebuf); if (mode == DATE_SHORT) diff --git a/t/t4212-log-corrupt.sh b/t/t4212-log-corrupt.sh index 80542d624be288..85c6df4ec9834d 100755 --- a/t/t4212-log-corrupt.sh +++ b/t/t4212-log-corrupt.sh @@ -76,4 +76,12 @@ test_expect_success 'date parser recognizes time_t overflow' ' test_cmp expect actual ' +# date is within 2^63-1, but enough to choke glibc's gmtime +test_expect_success 'absurdly far-in-future dates produce sentinel' ' + commit=$(munge_author_date HEAD 999999999999999999) && + echo "Thu Jan 1 00:00:00 1970 +0000" >expect && + git log -1 --format=%ad $commit >actual && + test_cmp expect actual +' + test_done From 98b406f3ad6a6989a5b11c2a2582a9f539d66263 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Mon, 24 Feb 2014 03:59:03 -0500 Subject: [PATCH 21/82] remote: handle pushremote config in any order The remote we push can be defined either by remote.pushdefault or by branch.*.pushremote for the current branch. The order in which they appear in the config file should not matter to precedence (which should be to prefer the branch-specific config). The current code parses the config linearly and uses a single string to store both values, overwriting any previous value. Thus, config like: [branch "master"] pushremote = foo [remote] pushdefault = bar erroneously ends up pushing to "bar" from the master branch. We can fix this by storing both values and resolving the correct value after all config is read. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- remote.c | 7 ++++++- t/t5516-fetch-push.sh | 13 +++++++++++++ 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/remote.c b/remote.c index 9f1a8aa2c499ae..c0e95e2eb6dfb9 100644 --- a/remote.c +++ b/remote.c @@ -49,6 +49,7 @@ static int branches_nr; static struct branch *current_branch; static const char *default_remote_name; +static const char *branch_pushremote_name; static const char *pushremote_name; static int explicit_default_remote_name; @@ -352,7 +353,7 @@ static int handle_config(const char *key, const char *value, void *cb) } } else if (!strcmp(subkey, ".pushremote")) { if (branch == current_branch) - if (git_config_string(&pushremote_name, key, value)) + if (git_config_string(&branch_pushremote_name, key, value)) return -1; } else if (!strcmp(subkey, ".merge")) { if (!value) @@ -492,6 +493,10 @@ static void read_config(void) make_branch(head_ref + strlen("refs/heads/"), 0); } git_config(handle_config, NULL); + if (branch_pushremote_name) { + free((char *)pushremote_name); + pushremote_name = branch_pushremote_name; + } alias_all_urls(); } diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh index 99c32d75391bb0..084a8cbd91ae7d 100755 --- a/t/t5516-fetch-push.sh +++ b/t/t5516-fetch-push.sh @@ -536,6 +536,19 @@ test_expect_success 'push with config branch.*.pushremote' ' check_push_result down_repo $the_commit heads/master ' +test_expect_success 'branch.*.pushremote config order is irrelevant' ' + mk_test one_repo heads/master && + mk_test two_repo heads/master && + test_config remote.one.url one_repo && + test_config remote.two.url two_repo && + test_config branch.master.pushremote two_repo && + test_config remote.pushdefault one_repo && + test_config push.default matching && + git push && + check_push_result one_repo $the_first_commit heads/master && + check_push_result two_repo $the_commit heads/master +' + test_expect_success 'push with dry-run' ' mk_test testrepo heads/master && From 29d9af586ba3ee1e6df1dc070f42ea659ad77cc2 Mon Sep 17 00:00:00 2001 From: Brad King Date: Mon, 27 Jan 2014 09:45:06 -0500 Subject: [PATCH 22/82] t3030-merge-recursive: test known breakage with empty work tree Sometimes when working with a large repository it can be useful to try out a merge and only check out conflicting files to disk (for example as a speed optimization on a server). Until v1.7.7-rc1~28^2~20 (merge-recursive: When we detect we can skip an update, actually skip it, 2011-08-11), it was possible to do so with the following idiom: # Prepare a temporary index and empty work tree. GIT_INDEX_FILE="$PWD/tmp-$$-index" && export GIT_INDEX_FILE && GIT_WORK_TREE="$PWD/tmp-$$-work" && export GIT_WORK_TREE && mkdir "$GIT_WORK_TREE" && # Convince the index that our side is on disk. git read-tree -i -m $ours && git update-index --ignore-missing --refresh && # Merge their side into our side. bases=$(git merge-base --all $ours $theirs) && git merge-recursive $bases -- $ours $theirs && tree=$(git write-tree) Nowadays, that still works and the exit status is the same, but merge-recursive produces a diagnostic if "our" side renamed a file: error: addinfo_cache failed for path 'dst' Add a test to document this regression. Signed-off-by: Brad King Signed-off-by: Junio C Hamano --- t/t3030-merge-recursive.sh | 47 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 47 insertions(+) diff --git a/t/t3030-merge-recursive.sh b/t/t3030-merge-recursive.sh index 2f96100a5f655b..3db3bf66517919 100755 --- a/t/t3030-merge-recursive.sh +++ b/t/t3030-merge-recursive.sh @@ -257,6 +257,7 @@ test_expect_success 'setup 8' ' git add e && test_tick && git commit -m "rename a->e" && + c7=$(git rev-parse --verify HEAD) && git checkout rename-ln && git mv a e && test_ln_s_add e a && @@ -517,6 +518,52 @@ test_expect_success 'reset and bind merge' ' ' +test_expect_failure 'merge-recursive w/ empty work tree - ours has rename' ' + ( + GIT_WORK_TREE="$PWD/ours-has-rename-work" && + export GIT_WORK_TREE && + GIT_INDEX_FILE="$PWD/ours-has-rename-index" && + export GIT_INDEX_FILE && + mkdir "$GIT_WORK_TREE" && + git read-tree -i -m $c7 && + git update-index --ignore-missing --refresh && + git merge-recursive $c0 -- $c7 $c3 && + git ls-files -s >actual-files + ) 2>actual-err && + >expected-err && + cat >expected-files <<-EOF && + 100644 $o3 0 b/c + 100644 $o0 0 c + 100644 $o0 0 d/e + 100644 $o0 0 e + EOF + test_cmp expected-files actual-files && + test_cmp expected-err actual-err +' + +test_expect_success 'merge-recursive w/ empty work tree - theirs has rename' ' + ( + GIT_WORK_TREE="$PWD/theirs-has-rename-work" && + export GIT_WORK_TREE && + GIT_INDEX_FILE="$PWD/theirs-has-rename-index" && + export GIT_INDEX_FILE && + mkdir "$GIT_WORK_TREE" && + git read-tree -i -m $c3 && + git update-index --ignore-missing --refresh && + git merge-recursive $c0 -- $c3 $c7 && + git ls-files -s >actual-files + ) 2>actual-err && + >expected-err && + cat >expected-files <<-EOF && + 100644 $o3 0 b/c + 100644 $o0 0 c + 100644 $o0 0 d/e + 100644 $o0 0 e + EOF + test_cmp expected-files actual-files && + test_cmp expected-err actual-err +' + test_expect_success 'merge removes empty directories' ' git reset --hard master && From 2e2e7ec1ef567ac0a4ad8294ada15836661e6589 Mon Sep 17 00:00:00 2001 From: Brad King Date: Mon, 27 Jan 2014 09:45:07 -0500 Subject: [PATCH 23/82] read-cache.c: refactor --ignore-missing implementation Move lstat ENOENT handling from refresh_index to refresh_cache_ent and activate it with a new CE_MATCH_IGNORE_MISSING option. This will allow other call paths into refresh_cache_ent to use the feature. Signed-off-by: Brad King Signed-off-by: Junio C Hamano --- cache.h | 2 ++ read-cache.c | 8 +++++--- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/cache.h b/cache.h index 415d8830809cbb..87429db0e4c1e6 100644 --- a/cache.h +++ b/cache.h @@ -488,6 +488,8 @@ extern void *read_blob_data_from_index(struct index_state *, const char *, unsig #define CE_MATCH_RACY_IS_DIRTY 02 /* do stat comparison even if CE_SKIP_WORKTREE is true */ #define CE_MATCH_IGNORE_SKIP_WORKTREE 04 +/* ignore non-existent files during stat update */ +#define CE_MATCH_IGNORE_MISSING 0x08 extern int ie_match_stat(const struct index_state *, const struct cache_entry *, struct stat *, unsigned int); extern int ie_modified(const struct index_state *, const struct cache_entry *, struct stat *, unsigned int); diff --git a/read-cache.c b/read-cache.c index c3d5e3543fae75..bb38115a9dfb7d 100644 --- a/read-cache.c +++ b/read-cache.c @@ -1031,6 +1031,7 @@ static struct cache_entry *refresh_cache_ent(struct index_state *istate, int changed, size; int ignore_valid = options & CE_MATCH_IGNORE_VALID; int ignore_skip_worktree = options & CE_MATCH_IGNORE_SKIP_WORKTREE; + int ignore_missing = options & CE_MATCH_IGNORE_MISSING; if (ce_uptodate(ce)) return ce; @@ -1050,6 +1051,8 @@ static struct cache_entry *refresh_cache_ent(struct index_state *istate, } if (lstat(ce->name, &st) < 0) { + if (ignore_missing && errno == ENOENT) + return ce; if (err) *err = errno; return NULL; @@ -1126,7 +1129,8 @@ int refresh_index(struct index_state *istate, unsigned int flags, const char **p int ignore_submodules = (flags & REFRESH_IGNORE_SUBMODULES) != 0; int first = 1; int in_porcelain = (flags & REFRESH_IN_PORCELAIN); - unsigned int options = really ? CE_MATCH_IGNORE_VALID : 0; + unsigned int options = ((really ? CE_MATCH_IGNORE_VALID : 0) | + (not_new ? CE_MATCH_IGNORE_MISSING : 0)); const char *modified_fmt; const char *deleted_fmt; const char *typechange_fmt; @@ -1175,8 +1179,6 @@ int refresh_index(struct index_state *istate, unsigned int flags, const char **p if (!new) { const char *fmt; - if (not_new && cache_errno == ENOENT) - continue; if (really && cache_errno == EINVAL) { /* If we are doing --really-refresh that * means the index is not valid anymore. From 257627268ad19cb616ad3feb6ca8171d400df287 Mon Sep 17 00:00:00 2001 From: Brad King Date: Mon, 27 Jan 2014 09:45:08 -0500 Subject: [PATCH 24/82] read-cache.c: extend make_cache_entry refresh flag with options Convert the make_cache_entry boolean 'refresh' argument to a more general 'refresh_options' argument. Pass the value through to the underlying refresh_cache_ent call. Add option CE_MATCH_REFRESH to enable stat refresh. Update call sites to use the new signature. Signed-off-by: Brad King Signed-off-by: Junio C Hamano --- cache.h | 4 +++- merge-recursive.c | 3 ++- read-cache.c | 21 +++++++++++---------- 3 files changed, 16 insertions(+), 12 deletions(-) diff --git a/cache.h b/cache.h index 87429db0e4c1e6..18ecd779143347 100644 --- a/cache.h +++ b/cache.h @@ -477,7 +477,7 @@ extern int remove_file_from_index(struct index_state *, const char *path); #define ADD_CACHE_IMPLICIT_DOT 32 /* internal to "git add -u/-A" */ extern int add_to_index(struct index_state *, const char *path, struct stat *, int flags); extern int add_file_to_index(struct index_state *, const char *path, int flags); -extern struct cache_entry *make_cache_entry(unsigned int mode, const unsigned char *sha1, const char *path, int stage, int refresh); +extern struct cache_entry *make_cache_entry(unsigned int mode, const unsigned char *sha1, const char *path, int stage, unsigned int refresh_options); extern int ce_same_name(const struct cache_entry *a, const struct cache_entry *b); extern int index_name_is_other(const struct index_state *, const char *, int); extern void *read_blob_data_from_index(struct index_state *, const char *, unsigned long *); @@ -490,6 +490,8 @@ extern void *read_blob_data_from_index(struct index_state *, const char *, unsig #define CE_MATCH_IGNORE_SKIP_WORKTREE 04 /* ignore non-existent files during stat update */ #define CE_MATCH_IGNORE_MISSING 0x08 +/* enable stat refresh */ +#define CE_MATCH_REFRESH 0x10 extern int ie_match_stat(const struct index_state *, const struct cache_entry *, struct stat *, unsigned int); extern int ie_modified(const struct index_state *, const struct cache_entry *, struct stat *, unsigned int); diff --git a/merge-recursive.c b/merge-recursive.c index fc2f00176c3082..05311e411ce85e 100644 --- a/merge-recursive.c +++ b/merge-recursive.c @@ -201,7 +201,8 @@ static int add_cacheinfo(unsigned int mode, const unsigned char *sha1, const char *path, int stage, int refresh, int options) { struct cache_entry *ce; - ce = make_cache_entry(mode, sha1 ? sha1 : null_sha1, path, stage, refresh); + ce = make_cache_entry(mode, sha1 ? sha1 : null_sha1, path, stage, + (refresh ? CE_MATCH_REFRESH : 0 )); if (!ce) return error(_("addinfo_cache failed for path '%s'"), path); return add_cache_entry(ce, options); diff --git a/read-cache.c b/read-cache.c index bb38115a9dfb7d..9032550bdef3f6 100644 --- a/read-cache.c +++ b/read-cache.c @@ -15,7 +15,8 @@ #include "strbuf.h" #include "varint.h" -static struct cache_entry *refresh_cache_entry(struct cache_entry *ce, int really); +static struct cache_entry *refresh_cache_entry(struct cache_entry *ce, + unsigned int options); /* Mask for the name length in ce_flags in the on-disk index */ @@ -696,7 +697,7 @@ int add_file_to_index(struct index_state *istate, const char *path, int flags) struct cache_entry *make_cache_entry(unsigned int mode, const unsigned char *sha1, const char *path, int stage, - int refresh) + unsigned int refresh_options) { int size, len; struct cache_entry *ce; @@ -716,10 +717,7 @@ struct cache_entry *make_cache_entry(unsigned int mode, ce->ce_namelen = len; ce->ce_mode = create_ce_mode(mode); - if (refresh) - return refresh_cache_entry(ce, 0); - - return ce; + return refresh_cache_entry(ce, refresh_options); } int ce_same_name(const struct cache_entry *a, const struct cache_entry *b) @@ -1029,11 +1027,12 @@ static struct cache_entry *refresh_cache_ent(struct index_state *istate, struct stat st; struct cache_entry *updated; int changed, size; + int refresh = options & CE_MATCH_REFRESH; int ignore_valid = options & CE_MATCH_IGNORE_VALID; int ignore_skip_worktree = options & CE_MATCH_IGNORE_SKIP_WORKTREE; int ignore_missing = options & CE_MATCH_IGNORE_MISSING; - if (ce_uptodate(ce)) + if (!refresh || ce_uptodate(ce)) return ce; /* @@ -1129,7 +1128,8 @@ int refresh_index(struct index_state *istate, unsigned int flags, const char **p int ignore_submodules = (flags & REFRESH_IGNORE_SUBMODULES) != 0; int first = 1; int in_porcelain = (flags & REFRESH_IN_PORCELAIN); - unsigned int options = ((really ? CE_MATCH_IGNORE_VALID : 0) | + unsigned int options = (CE_MATCH_REFRESH | + (really ? CE_MATCH_IGNORE_VALID : 0) | (not_new ? CE_MATCH_IGNORE_MISSING : 0)); const char *modified_fmt; const char *deleted_fmt; @@ -1208,9 +1208,10 @@ int refresh_index(struct index_state *istate, unsigned int flags, const char **p return has_errors; } -static struct cache_entry *refresh_cache_entry(struct cache_entry *ce, int really) +static struct cache_entry *refresh_cache_entry(struct cache_entry *ce, + unsigned int options) { - return refresh_cache_ent(&the_index, ce, really, NULL, NULL); + return refresh_cache_ent(&the_index, ce, options, NULL, NULL); } From 6e2068ae48000a2dfdb2044bbb91073c11f6fbff Mon Sep 17 00:00:00 2001 From: Brad King Date: Mon, 27 Jan 2014 09:45:09 -0500 Subject: [PATCH 25/82] merge-recursive.c: tolerate missing files while refreshing index Teach add_cacheinfo to tell make_cache_entry to skip refreshing stat information when a file is missing from the work tree. We do not want the index to be stat-dirty after the merge but also do not want to fail when a file happens to be missing. This fixes the 'merge-recursive w/ empty work tree - ours has rename' case in t3030-merge-recursive. Suggested-by: Elijah Newren Signed-off-by: Brad King Signed-off-by: Junio C Hamano --- merge-recursive.c | 3 ++- t/t3030-merge-recursive.sh | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/merge-recursive.c b/merge-recursive.c index 05311e411ce85e..786dee7596d54d 100644 --- a/merge-recursive.c +++ b/merge-recursive.c @@ -202,7 +202,8 @@ static int add_cacheinfo(unsigned int mode, const unsigned char *sha1, { struct cache_entry *ce; ce = make_cache_entry(mode, sha1 ? sha1 : null_sha1, path, stage, - (refresh ? CE_MATCH_REFRESH : 0 )); + (refresh ? (CE_MATCH_REFRESH | + CE_MATCH_IGNORE_MISSING) : 0 )); if (!ce) return error(_("addinfo_cache failed for path '%s'"), path); return add_cache_entry(ce, options); diff --git a/t/t3030-merge-recursive.sh b/t/t3030-merge-recursive.sh index 3db3bf66517919..82e18548c39817 100755 --- a/t/t3030-merge-recursive.sh +++ b/t/t3030-merge-recursive.sh @@ -518,7 +518,7 @@ test_expect_success 'reset and bind merge' ' ' -test_expect_failure 'merge-recursive w/ empty work tree - ours has rename' ' +test_expect_success 'merge-recursive w/ empty work tree - ours has rename' ' ( GIT_WORK_TREE="$PWD/ours-has-rename-work" && export GIT_WORK_TREE && From 429bb40abdb5b42ffdde5b1a58f9a37da723d179 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nguy=E1=BB=85n=20Th=C3=A1i=20Ng=E1=BB=8Dc=20Duy?= Date: Fri, 24 Jan 2014 20:40:28 +0700 Subject: [PATCH 26/82] pathspec: convert some match_pathspec_depth() to ce_path_match() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This helps reduce the number of match_pathspec_depth() call sites and show how match_pathspec_depth() is used. Signed-off-by: Nguyễn Thái Ngọc Duy Signed-off-by: Junio C Hamano --- builtin/checkout.c | 3 +-- builtin/commit.c | 2 +- builtin/grep.c | 2 +- builtin/rm.c | 2 +- builtin/update-index.c | 3 ++- cache.h | 2 -- diff-lib.c | 5 +++-- dir.h | 7 +++++++ pathspec.c | 2 +- preload-index.c | 3 ++- read-cache.c | 8 +------- resolve-undo.c | 2 +- revision.c | 3 ++- wt-status.c | 2 +- 14 files changed, 24 insertions(+), 22 deletions(-) diff --git a/builtin/checkout.c b/builtin/checkout.c index 5df3837e3102e2..ada51fa70ff59b 100644 --- a/builtin/checkout.c +++ b/builtin/checkout.c @@ -297,8 +297,7 @@ static int checkout_paths(const struct checkout_opts *opts, * match_pathspec() for _all_ entries when * opts->source_tree != NULL. */ - if (match_pathspec_depth(&opts->pathspec, ce->name, ce_namelen(ce), - 0, ps_matched)) + if (ce_path_match(ce, &opts->pathspec, ps_matched)) ce->ce_flags |= CE_MATCHED; } diff --git a/builtin/commit.c b/builtin/commit.c index 3767478c6ddb02..26b2986abe679b 100644 --- a/builtin/commit.c +++ b/builtin/commit.c @@ -234,7 +234,7 @@ static int list_paths(struct string_list *list, const char *with_tree, if (ce->ce_flags & CE_UPDATE) continue; - if (!match_pathspec_depth(pattern, ce->name, ce_namelen(ce), 0, m)) + if (!ce_path_match(ce, pattern, m)) continue; item = string_list_insert(list, ce->name); if (ce_skip_worktree(ce)) diff --git a/builtin/grep.c b/builtin/grep.c index 63f86032d91f00..3d924c25a574b5 100644 --- a/builtin/grep.c +++ b/builtin/grep.c @@ -379,7 +379,7 @@ static int grep_cache(struct grep_opt *opt, const struct pathspec *pathspec, int const struct cache_entry *ce = active_cache[nr]; if (!S_ISREG(ce->ce_mode)) continue; - if (!match_pathspec_depth(pathspec, ce->name, ce_namelen(ce), 0, NULL)) + if (!ce_path_match(ce, pathspec, NULL)) continue; /* * If CE_VALID is on, we assume worktree file and its cache entry diff --git a/builtin/rm.c b/builtin/rm.c index 3a0e0eaab7d1fd..05642184c5d89a 100644 --- a/builtin/rm.c +++ b/builtin/rm.c @@ -308,7 +308,7 @@ int cmd_rm(int argc, const char **argv, const char *prefix) for (i = 0; i < active_nr; i++) { const struct cache_entry *ce = active_cache[i]; - if (!match_pathspec_depth(&pathspec, ce->name, ce_namelen(ce), 0, seen)) + if (!ce_path_match(ce, &pathspec, seen)) continue; ALLOC_GROW(list.entry, list.nr + 1, list.alloc); list.entry[list.nr].name = ce->name; diff --git a/builtin/update-index.c b/builtin/update-index.c index e3a10d706d4068..aaa6f78f162931 100644 --- a/builtin/update-index.c +++ b/builtin/update-index.c @@ -12,6 +12,7 @@ #include "resolve-undo.h" #include "parse-options.h" #include "pathspec.h" +#include "dir.h" /* * Default to not allowing changes to the list of files. The @@ -564,7 +565,7 @@ static int do_reupdate(int ac, const char **av, struct cache_entry *old = NULL; int save_nr; - if (ce_stage(ce) || !ce_path_match(ce, &pathspec)) + if (ce_stage(ce) || !ce_path_match(ce, &pathspec, NULL)) continue; if (has_head) old = read_one_ent(NULL, head_sha1, diff --git a/cache.h b/cache.h index dc040fb1aa99b7..aa6dbf6956b5a0 100644 --- a/cache.h +++ b/cache.h @@ -501,8 +501,6 @@ extern void *read_blob_data_from_index(struct index_state *, const char *, unsig extern int ie_match_stat(const struct index_state *, const struct cache_entry *, struct stat *, unsigned int); extern int ie_modified(const struct index_state *, const struct cache_entry *, struct stat *, unsigned int); -extern int ce_path_match(const struct cache_entry *ce, const struct pathspec *pathspec); - #define HASH_WRITE_OBJECT 1 #define HASH_FORMAT_CHECK 2 extern int index_fd(unsigned char *sha1, int fd, struct stat *st, enum object_type type, const char *path, unsigned flags); diff --git a/diff-lib.c b/diff-lib.c index 346cac651da725..2eddc66bbd5cf3 100644 --- a/diff-lib.c +++ b/diff-lib.c @@ -11,6 +11,7 @@ #include "unpack-trees.h" #include "refs.h" #include "submodule.h" +#include "dir.h" /* * diff-files @@ -108,7 +109,7 @@ int run_diff_files(struct rev_info *revs, unsigned int option) if (diff_can_quit_early(&revs->diffopt)) break; - if (!ce_path_match(ce, &revs->prune_data)) + if (!ce_path_match(ce, &revs->prune_data, NULL)) continue; if (ce_stage(ce)) { @@ -438,7 +439,7 @@ static int oneway_diff(const struct cache_entry * const *src, if (tree == o->df_conflict_entry) tree = NULL; - if (ce_path_match(idx ? idx : tree, &revs->prune_data)) { + if (ce_path_match(idx ? idx : tree, &revs->prune_data, NULL)) { do_oneway_diff(o, idx, tree); if (diff_can_quit_early(&revs->diffopt)) { o->exiting_early = 1; diff --git a/dir.h b/dir.h index 9b7e4e77d8b11b..42793e582ffc8b 100644 --- a/dir.h +++ b/dir.h @@ -205,4 +205,11 @@ extern int git_fnmatch(const struct pathspec_item *item, const char *pattern, const char *string, int prefix); +static inline int ce_path_match(const struct cache_entry *ce, + const struct pathspec *pathspec, + char *seen) +{ + return match_pathspec_depth(pathspec, ce->name, ce_namelen(ce), 0, seen); +} + #endif diff --git a/pathspec.c b/pathspec.c index 6cb9fd3014a424..80430999553d8f 100644 --- a/pathspec.c +++ b/pathspec.c @@ -33,7 +33,7 @@ void add_pathspec_matches_against_index(const struct pathspec *pathspec, return; for (i = 0; i < active_nr; i++) { const struct cache_entry *ce = active_cache[i]; - match_pathspec_depth(pathspec, ce->name, ce_namelen(ce), 0, seen); + ce_path_match(ce, pathspec, seen); } } diff --git a/preload-index.c b/preload-index.c index 8c44ceb2c71593..968ee25eae13f8 100644 --- a/preload-index.c +++ b/preload-index.c @@ -3,6 +3,7 @@ */ #include "cache.h" #include "pathspec.h" +#include "dir.h" #ifdef NO_PTHREADS static void preload_index(struct index_state *index, @@ -53,7 +54,7 @@ static void *preload_thread(void *_data) continue; if (ce_uptodate(ce)) continue; - if (!ce_path_match(ce, &p->pathspec)) + if (!ce_path_match(ce, &p->pathspec, NULL)) continue; if (threaded_has_symlink_leading_path(&cache, ce->name, ce_namelen(ce))) continue; diff --git a/read-cache.c b/read-cache.c index 33dd676ccbbd24..23eb2513c139fa 100644 --- a/read-cache.c +++ b/read-cache.c @@ -728,11 +728,6 @@ int ce_same_name(const struct cache_entry *a, const struct cache_entry *b) return ce_namelen(b) == len && !memcmp(a->name, b->name, len); } -int ce_path_match(const struct cache_entry *ce, const struct pathspec *pathspec) -{ - return match_pathspec_depth(pathspec, ce->name, ce_namelen(ce), 0, NULL); -} - /* * We fundamentally don't like some paths: we don't want * dot or dot-dot anywhere, and for obvious reasons don't @@ -1149,8 +1144,7 @@ int refresh_index(struct index_state *istate, unsigned int flags, if (ignore_submodules && S_ISGITLINK(ce->ce_mode)) continue; - if (pathspec && - !match_pathspec_depth(pathspec, ce->name, ce_namelen(ce), 0, seen)) + if (pathspec && !ce_path_match(ce, pathspec, seen)) filtered = 1; if (ce_stage(ce)) { diff --git a/resolve-undo.c b/resolve-undo.c index c09b00664e6892..67d1543141bf33 100644 --- a/resolve-undo.c +++ b/resolve-undo.c @@ -182,7 +182,7 @@ void unmerge_index(struct index_state *istate, const struct pathspec *pathspec) for (i = 0; i < istate->cache_nr; i++) { const struct cache_entry *ce = istate->cache[i]; - if (!match_pathspec_depth(pathspec, ce->name, ce_namelen(ce), 0, NULL)) + if (!ce_path_match(ce, pathspec, NULL)) continue; i = unmerge_index_entry_at(istate, i); } diff --git a/revision.c b/revision.c index a0df72f32c100a..f40ccf14269a33 100644 --- a/revision.c +++ b/revision.c @@ -16,6 +16,7 @@ #include "line-log.h" #include "mailmap.h" #include "commit-slab.h" +#include "dir.h" volatile show_early_output_fn_t show_early_output; @@ -1400,7 +1401,7 @@ static void prepare_show_merge(struct rev_info *revs) const struct cache_entry *ce = active_cache[i]; if (!ce_stage(ce)) continue; - if (ce_path_match(ce, &revs->prune_data)) { + if (ce_path_match(ce, &revs->prune_data, NULL)) { prune_num++; prune = xrealloc(prune, sizeof(*prune) * prune_num); prune[prune_num-2] = ce->name; diff --git a/wt-status.c b/wt-status.c index 4e5581005936a1..295c09e3fc9180 100644 --- a/wt-status.c +++ b/wt-status.c @@ -510,7 +510,7 @@ static void wt_status_collect_changes_initial(struct wt_status *s) struct wt_status_change_data *d; const struct cache_entry *ce = active_cache[i]; - if (!ce_path_match(ce, &s->pathspec)) + if (!ce_path_match(ce, &s->pathspec, NULL)) continue; it = string_list_insert(&s->change, ce->name); d = it->util; From ebb32893bad46bf5edae881552672a47dd2684b8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nguy=E1=BB=85n=20Th=C3=A1i=20Ng=E1=BB=8Dc=20Duy?= Date: Fri, 24 Jan 2014 20:40:29 +0700 Subject: [PATCH 27/82] pathspec: convert some match_pathspec_depth() to dir_path_match() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This helps reduce the number of match_pathspec_depth() call sites and show how m_p_d() is used. And it usage is: - match against an index entry (ce_path_match or match_pathspec_depth in ls-files) - match against a dir_entry from read_directory (dir_path_match and match_pathspec_depth in clean.c, which will be converted later) - resolve-undo (rerere.c and ls-files.c) Signed-off-by: Nguyễn Thái Ngọc Duy Signed-off-by: Junio C Hamano --- builtin/add.c | 3 +-- builtin/grep.c | 4 +--- builtin/ls-files.c | 2 +- dir.h | 7 +++++++ wt-status.c | 4 ++-- 5 files changed, 12 insertions(+), 8 deletions(-) diff --git a/builtin/add.c b/builtin/add.c index 2a2722fa10aa98..672adc01ffc07f 100644 --- a/builtin/add.c +++ b/builtin/add.c @@ -208,8 +208,7 @@ static char *prune_directory(struct dir_struct *dir, struct pathspec *pathspec, i = dir->nr; while (--i >= 0) { struct dir_entry *entry = *src++; - if (match_pathspec_depth(pathspec, entry->name, entry->len, - prefix, seen)) + if (dir_path_match(entry, pathspec, prefix, seen)) *dst++ = entry; else if (flag & WARN_IMPLICIT_DOT) /* diff --git a/builtin/grep.c b/builtin/grep.c index 3d924c25a574b5..69ac2d8797ec32 100644 --- a/builtin/grep.c +++ b/builtin/grep.c @@ -524,9 +524,7 @@ static int grep_directory(struct grep_opt *opt, const struct pathspec *pathspec, fill_directory(&dir, pathspec); for (i = 0; i < dir.nr; i++) { - const char *name = dir.entries[i]->name; - int namelen = strlen(name); - if (!match_pathspec_depth(pathspec, name, namelen, 0, NULL)) + if (!dir_path_match(dir.entries[i], pathspec, 0, NULL)) continue; hit |= grep_file(opt, dir.entries[i]->name); if (hit && opt->status_only) diff --git a/builtin/ls-files.c b/builtin/ls-files.c index e1cf6d8547d4aa..e238608bda9564 100644 --- a/builtin/ls-files.c +++ b/builtin/ls-files.c @@ -64,7 +64,7 @@ static void show_dir_entry(const char *tag, struct dir_entry *ent) if (len >= ent->len) die("git ls-files: internal error - directory entry not superset of prefix"); - if (!match_pathspec_depth(&pathspec, ent->name, ent->len, len, ps_matched)) + if (!dir_path_match(ent, &pathspec, len, ps_matched)) return; fputs(tag, stdout); diff --git a/dir.h b/dir.h index 42793e582ffc8b..65f54b606f853a 100644 --- a/dir.h +++ b/dir.h @@ -212,4 +212,11 @@ static inline int ce_path_match(const struct cache_entry *ce, return match_pathspec_depth(pathspec, ce->name, ce_namelen(ce), 0, seen); } +static inline int dir_path_match(const struct dir_entry *ent, + const struct pathspec *pathspec, + int prefix, char *seen) +{ + return match_pathspec_depth(pathspec, ent->name, ent->len, prefix, seen); +} + #endif diff --git a/wt-status.c b/wt-status.c index 295c09e3fc9180..a452407dad5194 100644 --- a/wt-status.c +++ b/wt-status.c @@ -552,7 +552,7 @@ static void wt_status_collect_untracked(struct wt_status *s) for (i = 0; i < dir.nr; i++) { struct dir_entry *ent = dir.entries[i]; if (cache_name_is_other(ent->name, ent->len) && - match_pathspec_depth(&s->pathspec, ent->name, ent->len, 0, NULL)) + dir_path_match(ent, &s->pathspec, 0, NULL)) string_list_insert(&s->untracked, ent->name); free(ent); } @@ -560,7 +560,7 @@ static void wt_status_collect_untracked(struct wt_status *s) for (i = 0; i < dir.ignored_nr; i++) { struct dir_entry *ent = dir.ignored[i]; if (cache_name_is_other(ent->name, ent->len) && - match_pathspec_depth(&s->pathspec, ent->name, ent->len, 0, NULL)) + dir_path_match(ent, &s->pathspec, 0, NULL)) string_list_insert(&s->ignored, ent->name); free(ent); } From 854b09592ce9a497f56f35d973c4abe43af84cd1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nguy=E1=BB=85n=20Th=C3=A1i=20Ng=E1=BB=8Dc=20Duy?= Date: Fri, 24 Jan 2014 20:40:30 +0700 Subject: [PATCH 28/82] pathspec: rename match_pathspec_depth() to match_pathspec() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A long time ago, for some reason I was not happy with match_pathspec(). I created a better version, match_pathspec_depth() that was suppose to replace match_pathspec() eventually. match_pathspec() has finally been gone since 6 months ago. Use the shorter name for match_pathspec_depth(). Signed-off-by: Nguyễn Thái Ngọc Duy Signed-off-by: Junio C Hamano --- builtin/clean.c | 4 ++-- builtin/ls-files.c | 6 ++++-- builtin/ls-tree.c | 2 +- dir.c | 20 ++++++++++---------- dir.h | 10 +++++----- rerere.c | 4 ++-- t/t6131-pathspec-icase.sh | 6 +++--- 7 files changed, 27 insertions(+), 25 deletions(-) diff --git a/builtin/clean.c b/builtin/clean.c index 2f26297142fde8..f59c753a4644c3 100644 --- a/builtin/clean.c +++ b/builtin/clean.c @@ -961,8 +961,8 @@ int cmd_clean(int argc, const char **argv, const char *prefix) die_errno("Cannot lstat '%s'", ent->name); if (pathspec.nr) - matches = match_pathspec_depth(&pathspec, ent->name, - len, 0, NULL); + matches = match_pathspec(&pathspec, ent->name, + len, 0, NULL); if (S_ISDIR(st.st_mode)) { if (remove_directories || (matches == MATCHED_EXACTLY)) { diff --git a/builtin/ls-files.c b/builtin/ls-files.c index e238608bda9564..02db0e1ae82fb4 100644 --- a/builtin/ls-files.c +++ b/builtin/ls-files.c @@ -139,7 +139,8 @@ static void show_ce_entry(const char *tag, const struct cache_entry *ce) if (len >= ce_namelen(ce)) die("git ls-files: internal error - cache entry not superset of prefix"); - if (!match_pathspec_depth(&pathspec, ce->name, ce_namelen(ce), len, ps_matched)) + if (!match_pathspec(&pathspec, ce->name, ce_namelen(ce), + len, ps_matched)) return; if (tag && *tag && show_valid_bit && @@ -195,7 +196,8 @@ static void show_ru_info(void) len = strlen(path); if (len < max_prefix_len) continue; /* outside of the prefix */ - if (!match_pathspec_depth(&pathspec, path, len, max_prefix_len, ps_matched)) + if (!match_pathspec(&pathspec, path, len, + max_prefix_len, ps_matched)) continue; /* uninterested */ for (i = 0; i < 3; i++) { if (!ui->mode[i]) diff --git a/builtin/ls-tree.c b/builtin/ls-tree.c index 65ec9318461461..51184dfa2efa46 100644 --- a/builtin/ls-tree.c +++ b/builtin/ls-tree.c @@ -171,7 +171,7 @@ int cmd_ls_tree(int argc, const char **argv, const char *prefix) * show_recursive() rolls its own matching code and is * generally ignorant of 'struct pathspec'. The magic mask * cannot be lifted until it is converted to use - * match_pathspec_depth() or tree_entry_interesting() + * match_pathspec() or tree_entry_interesting() */ parse_pathspec(&pathspec, PATHSPEC_GLOB | PATHSPEC_ICASE, PATHSPEC_PREFER_CWD, diff --git a/dir.c b/dir.c index b35b6330f850f6..442a548f25d8c5 100644 --- a/dir.c +++ b/dir.c @@ -218,7 +218,7 @@ static int match_pathspec_item(const struct pathspec_item *item, int prefix, * The normal call pattern is: * 1. prefix = common_prefix_len(ps); * 2. prune something, or fill_directory - * 3. match_pathspec_depth() + * 3. match_pathspec() * * 'prefix' at #1 may be shorter than the command's prefix and * it's ok for #2 to match extra files. Those extras will be @@ -282,10 +282,10 @@ static int match_pathspec_item(const struct pathspec_item *item, int prefix, * pathspec did not match any names, which could indicate that the * user mistyped the nth pathspec. */ -static int match_pathspec_depth_1(const struct pathspec *ps, - const char *name, int namelen, - int prefix, char *seen, - int exclude) +static int do_match_pathspec(const struct pathspec *ps, + const char *name, int namelen, + int prefix, char *seen, + int exclude) { int i, retval = 0; @@ -350,15 +350,15 @@ static int match_pathspec_depth_1(const struct pathspec *ps, return retval; } -int match_pathspec_depth(const struct pathspec *ps, - const char *name, int namelen, - int prefix, char *seen) +int match_pathspec(const struct pathspec *ps, + const char *name, int namelen, + int prefix, char *seen) { int positive, negative; - positive = match_pathspec_depth_1(ps, name, namelen, prefix, seen, 0); + positive = do_match_pathspec(ps, name, namelen, prefix, seen, 0); if (!(ps->magic & PATHSPEC_EXCLUDE) || !positive) return positive; - negative = match_pathspec_depth_1(ps, name, namelen, prefix, seen, 1); + negative = do_match_pathspec(ps, name, namelen, prefix, seen, 1); return negative ? 0 : positive; } diff --git a/dir.h b/dir.h index 65f54b606f853a..c31ed9af785abb 100644 --- a/dir.h +++ b/dir.h @@ -132,9 +132,9 @@ struct dir_struct { extern int simple_length(const char *match); extern int no_wildcard(const char *string); extern char *common_prefix(const struct pathspec *pathspec); -extern int match_pathspec_depth(const struct pathspec *pathspec, - const char *name, int namelen, - int prefix, char *seen); +extern int match_pathspec(const struct pathspec *pathspec, + const char *name, int namelen, + int prefix, char *seen); extern int within_depth(const char *name, int namelen, int depth, int max_depth); extern int fill_directory(struct dir_struct *dir, const struct pathspec *pathspec); @@ -209,14 +209,14 @@ static inline int ce_path_match(const struct cache_entry *ce, const struct pathspec *pathspec, char *seen) { - return match_pathspec_depth(pathspec, ce->name, ce_namelen(ce), 0, seen); + return match_pathspec(pathspec, ce->name, ce_namelen(ce), 0, seen); } static inline int dir_path_match(const struct dir_entry *ent, const struct pathspec *pathspec, int prefix, char *seen) { - return match_pathspec_depth(pathspec, ent->name, ent->len, prefix, seen); + return match_pathspec(pathspec, ent->name, ent->len, prefix, seen); } #endif diff --git a/rerere.c b/rerere.c index 1f2d21a72f56a6..34a21c431bef5c 100644 --- a/rerere.c +++ b/rerere.c @@ -672,8 +672,8 @@ int rerere_forget(struct pathspec *pathspec) find_conflict(&conflict); for (i = 0; i < conflict.nr; i++) { struct string_list_item *it = &conflict.items[i]; - if (!match_pathspec_depth(pathspec, it->string, strlen(it->string), - 0, NULL)) + if (!match_pathspec(pathspec, it->string, + strlen(it->string), 0, NULL)) continue; rerere_forget_one_path(it->string, &merge_rr); } diff --git a/t/t6131-pathspec-icase.sh b/t/t6131-pathspec-icase.sh index a7c7ff5f4938c1..39fc3f6769be1c 100755 --- a/t/t6131-pathspec-icase.sh +++ b/t/t6131-pathspec-icase.sh @@ -69,7 +69,7 @@ test_expect_success 'tree_entry_interesting matches :(icase)bar with empty prefi test_cmp expect actual ' -test_expect_success 'match_pathspec_depth matches :(icase)bar' ' +test_expect_success 'match_pathspec matches :(icase)bar' ' cat <<-EOF >expect && BAR bAr @@ -79,7 +79,7 @@ test_expect_success 'match_pathspec_depth matches :(icase)bar' ' test_cmp expect actual ' -test_expect_success 'match_pathspec_depth matches :(icase)bar with prefix' ' +test_expect_success 'match_pathspec matches :(icase)bar with prefix' ' cat <<-EOF >expect && fOo/BAR fOo/bAr @@ -89,7 +89,7 @@ test_expect_success 'match_pathspec_depth matches :(icase)bar with prefix' ' test_cmp expect actual ' -test_expect_success 'match_pathspec_depth matches :(icase)bar with empty prefix' ' +test_expect_success 'match_pathspec matches :(icase)bar with empty prefix' ' cat <<-EOF >expect && bar fOo/BAR From 42b0874a7ef66f9bd561c66df6e989f58d0393b9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nguy=E1=BB=85n=20Th=C3=A1i=20Ng=E1=BB=8Dc=20Duy?= Date: Fri, 24 Jan 2014 20:40:31 +0700 Subject: [PATCH 29/82] dir.c: prepare match_pathspec_item for taking more flags MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Nguyễn Thái Ngọc Duy Signed-off-by: Junio C Hamano --- dir.c | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/dir.c b/dir.c index 442a548f25d8c5..124b4341099fcc 100644 --- a/dir.c +++ b/dir.c @@ -195,6 +195,8 @@ int within_depth(const char *name, int namelen, return 1; } +#define DO_MATCH_EXCLUDE 1 + /* * Does 'match' match the given name? * A match is found if @@ -208,7 +210,7 @@ int within_depth(const char *name, int namelen, * It returns 0 when there is no match. */ static int match_pathspec_item(const struct pathspec_item *item, int prefix, - const char *name, int namelen) + const char *name, int namelen, unsigned flags) { /* name/namelen has prefix cut off by caller */ const char *match = item->match + prefix; @@ -285,9 +287,9 @@ static int match_pathspec_item(const struct pathspec_item *item, int prefix, static int do_match_pathspec(const struct pathspec *ps, const char *name, int namelen, int prefix, char *seen, - int exclude) + unsigned flags) { - int i, retval = 0; + int i, retval = 0, exclude = flags & DO_MATCH_EXCLUDE; GUARD_PATHSPEC(ps, PATHSPEC_FROMTOP | @@ -327,7 +329,8 @@ static int do_match_pathspec(const struct pathspec *ps, */ if (seen && ps->items[i].magic & PATHSPEC_EXCLUDE) seen[i] = MATCHED_FNMATCH; - how = match_pathspec_item(ps->items+i, prefix, name, namelen); + how = match_pathspec_item(ps->items+i, prefix, name, + namelen, flags); if (ps->recursive && (ps->magic & PATHSPEC_MAXDEPTH) && ps->max_depth != -1 && @@ -355,10 +358,14 @@ int match_pathspec(const struct pathspec *ps, int prefix, char *seen) { int positive, negative; - positive = do_match_pathspec(ps, name, namelen, prefix, seen, 0); + unsigned flags = 0; + positive = do_match_pathspec(ps, name, namelen, + prefix, seen, flags); if (!(ps->magic & PATHSPEC_EXCLUDE) || !positive) return positive; - negative = do_match_pathspec(ps, name, namelen, prefix, seen, 1); + negative = do_match_pathspec(ps, name, namelen, + prefix, seen, + flags | DO_MATCH_EXCLUDE); return negative ? 0 : positive; } From 68690fdd0b78762eb6387d7a437b588d15b6cf47 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nguy=E1=BB=85n=20Th=C3=A1i=20Ng=E1=BB=8Dc=20Duy?= Date: Fri, 24 Jan 2014 20:40:32 +0700 Subject: [PATCH 30/82] match_pathspec: match pathspec "foo/" against directory "foo" MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Currently we do support matching pathspec "foo/" against directory "foo". That is because match_pathspec() has no way to tell "foo" is a directory and matching "foo/" against _file_ "foo" is wrong. The callers can now tell match_pathspec if "foo" is a directory, we could make an exception for this case. Code is not executed though because no callers pass the flag yet. Signed-off-by: Nguyễn Thái Ngọc Duy Signed-off-by: Junio C Hamano --- dir.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/dir.c b/dir.c index 124b4341099fcc..5359d75e8baed3 100644 --- a/dir.c +++ b/dir.c @@ -196,6 +196,7 @@ int within_depth(const char *name, int namelen, } #define DO_MATCH_EXCLUDE 1 +#define DO_MATCH_DIRECTORY 2 /* * Does 'match' match the given name? @@ -259,7 +260,11 @@ static int match_pathspec_item(const struct pathspec_item *item, int prefix, if (match[matchlen-1] == '/' || name[matchlen] == '/') return MATCHED_RECURSIVELY; - } + } else if ((flags & DO_MATCH_DIRECTORY) && + match[matchlen - 1] == '/' && + namelen == matchlen - 1 && + !ps_strncmp(item, match, name, namelen)) + return MATCHED_EXACTLY; if (item->nowildcard_len < item->len && !git_fnmatch(item, match, name, From ae8d0824217bdf97c69ead49568cd03fc140627b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nguy=E1=BB=85n=20Th=C3=A1i=20Ng=E1=BB=8Dc=20Duy?= Date: Fri, 24 Jan 2014 20:40:33 +0700 Subject: [PATCH 31/82] pathspec: pass directory indicator to match_pathspec_item() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This patch activates the DO_MATCH_DIRECTORY code in m_p_i(), which makes "git diff HEAD submodule/" and "git diff HEAD submodule" produce the same output. Previously only the version without trailing slash returns the difference (if any). That's the effect of new ce_path_match(). dir_path_match() is not executed by the new tests. And it should not introduce regressions. Previously if path "dir/" is passed in with pathspec "dir/", they obviously match. With new dir_path_match(), the path becomes _directory_ "dir" vs pathspec "dir/", which is not executed by the old code path in m_p_i(). The new code path is executed and produces the same result. The other case is pathspec "dir" and path "dir/" is now turned to "dir" (with DO_MATCH_DIRECTORY). Still the same result before or after the patch. So why change? Because of the next patch about clean.c. Signed-off-by: Nguyễn Thái Ngọc Duy Signed-off-by: Junio C Hamano --- builtin/clean.c | 2 +- builtin/ls-files.c | 5 +++-- dir.c | 4 ++-- dir.h | 10 +++++++--- rerere.c | 2 +- t/t4010-diff-pathspec.sh | 6 ++++++ 6 files changed, 20 insertions(+), 9 deletions(-) diff --git a/builtin/clean.c b/builtin/clean.c index f59c753a4644c3..4c9680acb6ded6 100644 --- a/builtin/clean.c +++ b/builtin/clean.c @@ -962,7 +962,7 @@ int cmd_clean(int argc, const char **argv, const char *prefix) if (pathspec.nr) matches = match_pathspec(&pathspec, ent->name, - len, 0, NULL); + len, 0, NULL, 0); if (S_ISDIR(st.st_mode)) { if (remove_directories || (matches == MATCHED_EXACTLY)) { diff --git a/builtin/ls-files.c b/builtin/ls-files.c index 02db0e1ae82fb4..47c38808a26a46 100644 --- a/builtin/ls-files.c +++ b/builtin/ls-files.c @@ -140,7 +140,8 @@ static void show_ce_entry(const char *tag, const struct cache_entry *ce) die("git ls-files: internal error - cache entry not superset of prefix"); if (!match_pathspec(&pathspec, ce->name, ce_namelen(ce), - len, ps_matched)) + len, ps_matched, + S_ISDIR(ce->ce_mode) || S_ISGITLINK(ce->ce_mode))) return; if (tag && *tag && show_valid_bit && @@ -197,7 +198,7 @@ static void show_ru_info(void) if (len < max_prefix_len) continue; /* outside of the prefix */ if (!match_pathspec(&pathspec, path, len, - max_prefix_len, ps_matched)) + max_prefix_len, ps_matched, 0)) continue; /* uninterested */ for (i = 0; i < 3; i++) { if (!ui->mode[i]) diff --git a/dir.c b/dir.c index 5359d75e8baed3..98bb50fbabb69d 100644 --- a/dir.c +++ b/dir.c @@ -360,10 +360,10 @@ static int do_match_pathspec(const struct pathspec *ps, int match_pathspec(const struct pathspec *ps, const char *name, int namelen, - int prefix, char *seen) + int prefix, char *seen, int is_dir) { int positive, negative; - unsigned flags = 0; + unsigned flags = is_dir ? DO_MATCH_DIRECTORY : 0; positive = do_match_pathspec(ps, name, namelen, prefix, seen, flags); if (!(ps->magic & PATHSPEC_EXCLUDE) || !positive) diff --git a/dir.h b/dir.h index c31ed9af785abb..55e53456afab4c 100644 --- a/dir.h +++ b/dir.h @@ -134,7 +134,7 @@ extern int no_wildcard(const char *string); extern char *common_prefix(const struct pathspec *pathspec); extern int match_pathspec(const struct pathspec *pathspec, const char *name, int namelen, - int prefix, char *seen); + int prefix, char *seen, int is_dir); extern int within_depth(const char *name, int namelen, int depth, int max_depth); extern int fill_directory(struct dir_struct *dir, const struct pathspec *pathspec); @@ -209,14 +209,18 @@ static inline int ce_path_match(const struct cache_entry *ce, const struct pathspec *pathspec, char *seen) { - return match_pathspec(pathspec, ce->name, ce_namelen(ce), 0, seen); + return match_pathspec(pathspec, ce->name, ce_namelen(ce), 0, seen, + S_ISDIR(ce->ce_mode) || S_ISGITLINK(ce->ce_mode)); } static inline int dir_path_match(const struct dir_entry *ent, const struct pathspec *pathspec, int prefix, char *seen) { - return match_pathspec(pathspec, ent->name, ent->len, prefix, seen); + int has_trailing_dir = ent->len && ent->name[ent->len - 1] == '/'; + int len = has_trailing_dir ? ent->len - 1 : ent->len; + return match_pathspec(pathspec, ent->name, len, prefix, seen, + has_trailing_dir); } #endif diff --git a/rerere.c b/rerere.c index 34a21c431bef5c..d55aa8a01b4a4f 100644 --- a/rerere.c +++ b/rerere.c @@ -673,7 +673,7 @@ int rerere_forget(struct pathspec *pathspec) for (i = 0; i < conflict.nr; i++) { struct string_list_item *it = &conflict.items[i]; if (!match_pathspec(pathspec, it->string, - strlen(it->string), 0, NULL)) + strlen(it->string), 0, NULL, 0)) continue; rerere_forget_one_path(it->string, &merge_rr); } diff --git a/t/t4010-diff-pathspec.sh b/t/t4010-diff-pathspec.sh index 15a491295ed3b8..d30ff34be7ebaf 100755 --- a/t/t4010-diff-pathspec.sh +++ b/t/t4010-diff-pathspec.sh @@ -127,4 +127,10 @@ test_expect_success 'diff-tree ignores trailing slash on submodule path' ' test_cmp expect actual ' +test_expect_success 'diff-cache ignores trailing slash on submodule path' ' + git diff --name-only HEAD^ submod >expect && + git diff --name-only HEAD^ submod/ >actual && + test_cmp expect actual +' + test_done From 05b85022c9a76954a1a281a5dc5bcd32ad486b88 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nguy=E1=BB=85n=20Th=C3=A1i=20Ng=E1=BB=8Dc=20Duy?= Date: Fri, 24 Jan 2014 20:40:34 +0700 Subject: [PATCH 32/82] clean: replace match_pathspec() with dir_path_match() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This instance was left out when many match_pathspec() call sites that take input from dir_entry were converted to dir_path_match() because it passed a path with the trailing slash stripped out to match_pathspec() while the others did not. Stripping for all call sites back then would be a regression because match_pathspec() did not know how to match pathspec foo/ against _directory_ foo (the stripped version of path "foo/"). match_pathspec() knows how to do it now. And dir_path_match() strips the trailing slash also. Use the new function, because the stripping code is removed in the next patch. Signed-off-by: Nguyễn Thái Ngọc Duy Signed-off-by: Junio C Hamano --- builtin/clean.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/builtin/clean.c b/builtin/clean.c index 4c9680acb6ded6..5adb52d82a4024 100644 --- a/builtin/clean.c +++ b/builtin/clean.c @@ -961,8 +961,7 @@ int cmd_clean(int argc, const char **argv, const char *prefix) die_errno("Cannot lstat '%s'", ent->name); if (pathspec.nr) - matches = match_pathspec(&pathspec, ent->name, - len, 0, NULL, 0); + matches = dir_path_match(ent, &pathspec, 0, NULL); if (S_ISDIR(st.st_mode)) { if (remove_directories || (matches == MATCHED_EXACTLY)) { From 2e70c01799bf663cc1fa5f6e37e025000ffdc0b0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nguy=E1=BB=85n=20Th=C3=A1i=20Ng=E1=BB=8Dc=20Duy?= Date: Fri, 24 Jan 2014 20:40:35 +0700 Subject: [PATCH 33/82] clean: use cache_name_is_other() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit cmd_clean() has the exact same code of index_name_is_other(). Reduce code duplication. Signed-off-by: Nguyễn Thái Ngọc Duy Signed-off-by: Junio C Hamano --- builtin/clean.c | 21 ++------------------- 1 file changed, 2 insertions(+), 19 deletions(-) diff --git a/builtin/clean.c b/builtin/clean.c index 5adb52d82a4024..cb02a5330ac582 100644 --- a/builtin/clean.c +++ b/builtin/clean.c @@ -933,29 +933,12 @@ int cmd_clean(int argc, const char **argv, const char *prefix) for (i = 0; i < dir.nr; i++) { struct dir_entry *ent = dir.entries[i]; - int len, pos; int matches = 0; - const struct cache_entry *ce; struct stat st; const char *rel; - /* - * Remove the '/' at the end that directory - * walking adds for directory entries. - */ - len = ent->len; - if (len && ent->name[len-1] == '/') - len--; - pos = cache_name_pos(ent->name, len); - if (0 <= pos) - continue; /* exact match */ - pos = -pos - 1; - if (pos < active_nr) { - ce = active_cache[pos]; - if (ce_namelen(ce) == len && - !memcmp(ce->name, ent->name, len)) - continue; /* Yup, this one exists unmerged */ - } + if (!cache_name_is_other(ent->name, ent->len)) + continue; if (lstat(ent->name, &st)) die_errno("Cannot lstat '%s'", ent->name); From fceb907225745b91aa8b9e0ef2ea068b0351ea01 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nguy=E1=BB=85n=20Th=C3=A1i=20Ng=E1=BB=8Dc=20Duy?= Date: Sat, 25 Jan 2014 13:46:49 +0700 Subject: [PATCH 34/82] diff.c: move diffcore_skip_stat_unmatch core logic out for reuse later MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Nguyễn Thái Ngọc Duy Signed-off-by: Junio C Hamano --- diff.c | 49 ++++++++++++++++++++++++++++--------------------- 1 file changed, 28 insertions(+), 21 deletions(-) diff --git a/diff.c b/diff.c index 266112ca610445..1e46b575e1940d 100644 --- a/diff.c +++ b/diff.c @@ -4592,6 +4592,33 @@ static int diff_filespec_is_identical(struct diff_filespec *one, return !memcmp(one->data, two->data, one->size); } +static int diff_filespec_check_stat_unmatch(struct diff_filepair *p) +{ + /* + * 1. Entries that come from stat info dirtiness + * always have both sides (iow, not create/delete), + * one side of the object name is unknown, with + * the same mode and size. Keep the ones that + * do not match these criteria. They have real + * differences. + * + * 2. At this point, the file is known to be modified, + * with the same mode and size, and the object + * name of one side is unknown. Need to inspect + * the identical contents. + */ + if (!DIFF_FILE_VALID(p->one) || /* (1) */ + !DIFF_FILE_VALID(p->two) || + (p->one->sha1_valid && p->two->sha1_valid) || + (p->one->mode != p->two->mode) || + diff_populate_filespec(p->one, 1) || + diff_populate_filespec(p->two, 1) || + (p->one->size != p->two->size) || + !diff_filespec_is_identical(p->one, p->two)) /* (2) */ + return 1; + return 0; +} + static void diffcore_skip_stat_unmatch(struct diff_options *diffopt) { int i; @@ -4602,27 +4629,7 @@ static void diffcore_skip_stat_unmatch(struct diff_options *diffopt) for (i = 0; i < q->nr; i++) { struct diff_filepair *p = q->queue[i]; - /* - * 1. Entries that come from stat info dirtiness - * always have both sides (iow, not create/delete), - * one side of the object name is unknown, with - * the same mode and size. Keep the ones that - * do not match these criteria. They have real - * differences. - * - * 2. At this point, the file is known to be modified, - * with the same mode and size, and the object - * name of one side is unknown. Need to inspect - * the identical contents. - */ - if (!DIFF_FILE_VALID(p->one) || /* (1) */ - !DIFF_FILE_VALID(p->two) || - (p->one->sha1_valid && p->two->sha1_valid) || - (p->one->mode != p->two->mode) || - diff_populate_filespec(p->one, 1) || - diff_populate_filespec(p->two, 1) || - (p->one->size != p->two->size) || - !diff_filespec_is_identical(p->one, p->two)) /* (2) */ + if (diff_filespec_check_stat_unmatch(p)) diff_q(&outq, p); else { /* From f34b205f6cc2c78cbed03a1582422cb59e36f729 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nguy=E1=BB=85n=20Th=C3=A1i=20Ng=E1=BB=8Dc=20Duy?= Date: Sat, 25 Jan 2014 13:46:50 +0700 Subject: [PATCH 35/82] diff: do not quit early on stat-dirty files MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When QUICK is set (i.e. with --quiet) we try to do as little work as possible, stopping after seeing the first change. stat-dirty is considered a "change" but it may turn out not, if no actual content is changed. The actual content test is performed too late in the process and the shortcut may be taken prematurely, leading to incorrect return code. Assume we do "git diff --quiet". If we have a stat-dirty file "a" and a really dirty file "b". We break the loop in run_diff_files() and stop after "a" because we have got a "change". Later in diffcore_skip_stat_unmatch() we find out "a" is actually not changed. But there's nothing else in the diff queue, we incorrectly declare "no change", ignoring the fact that "b" is changed. This also happens to "git diff --quiet HEAD" when it hits diff_can_quit_early() in oneway_diff(). This patch does the content test earlier in order to keep going if "a" is unchanged. The test result is cached so that when diffcore_skip_stat_unmatch() is done in the end, we spend no cycles on re-testing "a". Reported-by: IWAMOTO Toshihiro Signed-off-by: Nguyễn Thái Ngọc Duy Signed-off-by: Junio C Hamano --- diff.c | 22 +++++++++++++++++----- diffcore.h | 2 ++ t/t4035-diff-quiet.sh | 6 ++++++ 3 files changed, 25 insertions(+), 5 deletions(-) diff --git a/diff.c b/diff.c index 1e46b575e1940d..691578b2c6f6d9 100644 --- a/diff.c +++ b/diff.c @@ -4594,6 +4594,11 @@ static int diff_filespec_is_identical(struct diff_filespec *one, static int diff_filespec_check_stat_unmatch(struct diff_filepair *p) { + if (p->done_skip_stat_unmatch) + return p->skip_stat_unmatch_result; + + p->done_skip_stat_unmatch = 1; + p->skip_stat_unmatch_result = 0; /* * 1. Entries that come from stat info dirtiness * always have both sides (iow, not create/delete), @@ -4615,8 +4620,8 @@ static int diff_filespec_check_stat_unmatch(struct diff_filepair *p) diff_populate_filespec(p->two, 1) || (p->one->size != p->two->size) || !diff_filespec_is_identical(p->one, p->two)) /* (2) */ - return 1; - return 0; + p->skip_stat_unmatch_result = 1; + return p->skip_stat_unmatch_result; } static void diffcore_skip_stat_unmatch(struct diff_options *diffopt) @@ -4792,6 +4797,7 @@ void diff_change(struct diff_options *options, unsigned old_dirty_submodule, unsigned new_dirty_submodule) { struct diff_filespec *one, *two; + struct diff_filepair *p; if (S_ISGITLINK(old_mode) && S_ISGITLINK(new_mode) && is_submodule_ignored(concatpath, options)) @@ -4818,10 +4824,16 @@ void diff_change(struct diff_options *options, fill_filespec(two, new_sha1, new_sha1_valid, new_mode); one->dirty_submodule = old_dirty_submodule; two->dirty_submodule = new_dirty_submodule; + p = diff_queue(&diff_queued_diff, one, two); - diff_queue(&diff_queued_diff, one, two); - if (!DIFF_OPT_TST(options, DIFF_FROM_CONTENTS)) - DIFF_OPT_SET(options, HAS_CHANGES); + if (DIFF_OPT_TST(options, DIFF_FROM_CONTENTS)) + return; + + if (DIFF_OPT_TST(options, QUICK) && options->skip_stat_unmatch && + !diff_filespec_check_stat_unmatch(p)) + return; + + DIFF_OPT_SET(options, HAS_CHANGES); } struct diff_filepair *diff_unmerge(struct diff_options *options, const char *path) diff --git a/diffcore.h b/diffcore.h index 1c16c8595b21c2..6b538bc2fbeb1b 100644 --- a/diffcore.h +++ b/diffcore.h @@ -70,6 +70,8 @@ struct diff_filepair { unsigned broken_pair : 1; unsigned renamed_pair : 1; unsigned is_unmerged : 1; + unsigned done_skip_stat_unmatch : 1; + unsigned skip_stat_unmatch_result : 1; }; #define DIFF_PAIR_UNMERGED(p) ((p)->is_unmerged) diff --git a/t/t4035-diff-quiet.sh b/t/t4035-diff-quiet.sh index 231412d1008e45..e8ae2a03fdcf5a 100755 --- a/t/t4035-diff-quiet.sh +++ b/t/t4035-diff-quiet.sh @@ -148,4 +148,10 @@ test_expect_success 'git diff --ignore-all-space, both files outside repo' ' ) ' +test_expect_success 'git diff --quiet ignores stat-change only entries' ' + test-chmtime +10 a && + echo modified >>b && + test_expect_code 1 git diff --quiet +' + test_done From 2d4c99339210d43a84841a71a50e88a050d9e21b Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Wed, 26 Feb 2014 14:18:54 -0800 Subject: [PATCH 36/82] stash pop: mention we did not drop the stash upon failing to apply Signed-off-by: Junio C Hamano --- git-stash.sh | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/git-stash.sh b/git-stash.sh index 1e541a21257c70..72941be4dab399 100755 --- a/git-stash.sh +++ b/git-stash.sh @@ -512,8 +512,14 @@ apply_stash () { pop_stash() { assert_stash_ref "$@" - apply_stash "$@" && - drop_stash "$@" + if apply_stash "$@" + then + drop_stash "$@" + else + status=$? + say "The stash is kept in case you need it again." + exit $status + fi } drop_stash () { From 0cc77c386cea7afebb54a5e7263ca37569ecfe7a Mon Sep 17 00:00:00 2001 From: Jeff King Date: Thu, 27 Feb 2014 05:56:31 -0500 Subject: [PATCH 37/82] shallow: use stat_validity to check for up-to-date file When we are about to write the shallow file, we check that it has not changed since we last read it. Instead of hand-rolling this, we can use stat_validity. This is built around the index stat-check, so it is more robust than just checking the mtime, as we do now (it uses the same check as we do for index files). The new code also handles the case of a shallow file appearing unexpectedly. With the current code, two simultaneous processes making us shallow (e.g., two "git fetch --depth=1" running at the same time in a non-shallow repository) can race to overwrite each other. As a bonus, we also remove a race in determining the stat information of what we read (we stat and then open, leaving a race window; instead we should open and then fstat the descriptor). Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- shallow.c | 24 +++++++----------------- 1 file changed, 7 insertions(+), 17 deletions(-) diff --git a/shallow.c b/shallow.c index bbc98b55c07969..1a6f022fa88bed 100644 --- a/shallow.c +++ b/shallow.c @@ -10,7 +10,7 @@ #include "commit-slab.h" static int is_shallow = -1; -static struct stat shallow_stat; +static struct stat_validity shallow_stat; static char *alternate_shallow_file; void set_alternate_shallow_file(const char *path, int override) @@ -52,12 +52,12 @@ int is_repository_shallow(void) * shallow file should be used. We could just open it and it * will likely fail. But let's do an explicit check instead. */ - if (!*path || - stat(path, &shallow_stat) || - (fp = fopen(path, "r")) == NULL) { + if (!*path || (fp = fopen(path, "r")) == NULL) { + stat_validity_clear(&shallow_stat); is_shallow = 0; return is_shallow; } + stat_validity_update(&shallow_stat, fileno(fp)); is_shallow = 1; while (fgets(buf, sizeof(buf), fp)) { @@ -137,21 +137,11 @@ struct commit_list *get_shallow_commits(struct object_array *heads, int depth, void check_shallow_file_for_update(void) { - struct stat st; - - if (!is_shallow) - return; - else if (is_shallow == -1) + if (is_shallow == -1) die("BUG: shallow must be initialized by now"); - if (stat(git_path("shallow"), &st)) - die("shallow file was removed during fetch"); - else if (st.st_mtime != shallow_stat.st_mtime -#ifdef USE_NSEC - || ST_MTIME_NSEC(st) != ST_MTIME_NSEC(shallow_stat) -#endif - ) - die("shallow file was changed during fetch"); + if (!stat_validity_check(&shallow_stat, git_path("shallow"))) + die("shallow file has changed since we read it"); } #define SEEN_ONLY 1 From 0179c945fce361c56b465e8a3f0fdf0962a816a1 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Thu, 27 Feb 2014 06:25:20 -0500 Subject: [PATCH 38/82] shallow: automatically clean up shallow tempfiles We sometimes write tempfiles of the form "shallow_XXXXXX" during fetch/push operations with shallow repositories. Under normal circumstances, we clean up the result when we are done. However, we do no take steps to clean up after ourselves when we exit due to die() or signal death. This patch teaches the tempfile creation code to register handlers to clean up after ourselves. To handle this, we change the ownership semantics of the filename returned by setup_temporary_shallow. It now keeps a copy of the filename itself, and returns only a const pointer to it. We can also do away with explicit tempfile removal in the callers. They all exit not long after finishing with the file, so they can rely on the auto-cleanup, simplifying the code. Note that we keep things simple and maintain only a single filename to be cleaned. This is sufficient for the current caller, but we future-proof it with a die("BUG"). Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- builtin/receive-pack.c | 16 ++++------------ commit.h | 2 +- fetch-pack.c | 11 ----------- shallow.c | 41 ++++++++++++++++++++++++++++++++++------- upload-pack.c | 7 +------ 5 files changed, 40 insertions(+), 37 deletions(-) diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c index 85bba356fab774..c3230817db4a76 100644 --- a/builtin/receive-pack.c +++ b/builtin/receive-pack.c @@ -828,14 +828,10 @@ static void execute_commands(struct command *commands, } } - if (shallow_update) { - if (!checked_connectivity) - error("BUG: run 'git fsck' for safety.\n" - "If there are errors, try to remove " - "the reported refs above"); - if (alt_shallow_file && *alt_shallow_file) - unlink(alt_shallow_file); - } + if (shallow_update && !checked_connectivity) + error("BUG: run 'git fsck' for safety.\n" + "If there are errors, try to remove " + "the reported refs above"); } static struct command *read_head_info(struct sha1_array *shallow) @@ -1087,10 +1083,6 @@ static void update_shallow_info(struct command *commands, cmd->skip_update = 1; } } - if (alt_shallow_file && *alt_shallow_file) { - unlink(alt_shallow_file); - alt_shallow_file = NULL; - } free(ref_status); } diff --git a/commit.h b/commit.h index 16d9c4351395ac..55631f191f2a01 100644 --- a/commit.h +++ b/commit.h @@ -209,7 +209,7 @@ extern int write_shallow_commits(struct strbuf *out, int use_pack_protocol, extern void setup_alternate_shallow(struct lock_file *shallow_lock, const char **alternate_shallow_file, const struct sha1_array *extra); -extern char *setup_temporary_shallow(const struct sha1_array *extra); +extern const char *setup_temporary_shallow(const struct sha1_array *extra); extern void advertise_shallow_grafts(int); struct shallow_info { diff --git a/fetch-pack.c b/fetch-pack.c index 90fdd49821a1d6..ae8550eb48ae1c 100644 --- a/fetch-pack.c +++ b/fetch-pack.c @@ -947,17 +947,6 @@ static void update_shallow(struct fetch_pack_args *args, if (!si->shallow || !si->shallow->nr) return; - if (alternate_shallow_file) { - /* - * The temporary shallow file is only useful for - * index-pack and unpack-objects because it may - * contain more roots than we want. Delete it. - */ - if (*alternate_shallow_file) - unlink(alternate_shallow_file); - free((char *)alternate_shallow_file); - } - if (args->cloning) { /* * remote is shallow, but this is a clone, there are diff --git a/shallow.c b/shallow.c index 1a6f022fa88bed..c7602ce3a2076f 100644 --- a/shallow.c +++ b/shallow.c @@ -8,6 +8,7 @@ #include "diff.h" #include "revision.h" #include "commit-slab.h" +#include "sigchain.h" static int is_shallow = -1; static struct stat_validity shallow_stat; @@ -206,27 +207,53 @@ int write_shallow_commits(struct strbuf *out, int use_pack_protocol, return write_shallow_commits_1(out, use_pack_protocol, extra, 0); } -char *setup_temporary_shallow(const struct sha1_array *extra) +static struct strbuf temporary_shallow = STRBUF_INIT; + +static void remove_temporary_shallow(void) +{ + if (temporary_shallow.len) { + unlink_or_warn(temporary_shallow.buf); + strbuf_reset(&temporary_shallow); + } +} + +static void remove_temporary_shallow_on_signal(int signo) +{ + remove_temporary_shallow(); + sigchain_pop(signo); + raise(signo); +} + +const char *setup_temporary_shallow(const struct sha1_array *extra) { + static int installed_handler; struct strbuf sb = STRBUF_INIT; int fd; + if (temporary_shallow.len) + die("BUG: attempt to create two temporary shallow files"); + if (write_shallow_commits(&sb, 0, extra)) { - struct strbuf path = STRBUF_INIT; - strbuf_addstr(&path, git_path("shallow_XXXXXX")); - fd = xmkstemp(path.buf); + strbuf_addstr(&temporary_shallow, git_path("shallow_XXXXXX")); + fd = xmkstemp(temporary_shallow.buf); + + if (!installed_handler) { + atexit(remove_temporary_shallow); + sigchain_push_common(remove_temporary_shallow_on_signal); + } + if (write_in_full(fd, sb.buf, sb.len) != sb.len) die_errno("failed to write to %s", - path.buf); + temporary_shallow.buf); close(fd); strbuf_release(&sb); - return strbuf_detach(&path, NULL); + return temporary_shallow.buf; } /* * is_repository_shallow() sees empty string as "no shallow * file". */ - return xstrdup(""); + return temporary_shallow.buf; } void setup_alternate_shallow(struct lock_file *shallow_lock, diff --git a/upload-pack.c b/upload-pack.c index 0c44f6b292563e..a3c52f691da5e5 100644 --- a/upload-pack.c +++ b/upload-pack.c @@ -81,7 +81,7 @@ static void create_pack_file(void) const char *argv[12]; int i, arg = 0; FILE *pipe_fd; - char *shallow_file = NULL; + const char *shallow_file = NULL; if (shallow_nr) { shallow_file = setup_temporary_shallow(NULL); @@ -242,11 +242,6 @@ static void create_pack_file(void) error("git upload-pack: git-pack-objects died with error."); goto fail; } - if (shallow_file) { - if (*shallow_file) - unlink(shallow_file); - free(shallow_file); - } /* flush the data */ if (0 <= buffered) { From f377e7a37c1b28359a228cf5bb43161a8a22b385 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Mart=C3=ADn=20Nieto?= Date: Thu, 27 Feb 2014 10:00:09 +0100 Subject: [PATCH 39/82] fetch: add a failing test for prunning with overlapping refspecs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When a remote has multiple fetch refspecs and these overlap in the target namespace, fetch may prune a remote-tracking branch which still exists in the remote. The test uses a popular form of this, by putting pull requests as stored in a popular hosting platform alongside "real" remote-tracking branches. The fetch command makes a decision of whether to prune based on the first matching refspec, which in this case is insufficient, as it covers the pull request names. This pair of refspecs does work as expected if the more "specific" refspec is the first in the list. Signed-off-by: Carlos Martín Nieto Signed-off-by: Junio C Hamano --- t/t5510-fetch.sh | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh index 07986d94bdf2ce..473e855c918546 100755 --- a/t/t5510-fetch.sh +++ b/t/t5510-fetch.sh @@ -113,6 +113,26 @@ test_expect_success 'fetch --prune with a namespace keeps other namespaces' ' git rev-parse origin/master ' +test_expect_failure 'fetch --prune handles overlapping refspecs' ' + cd "$D" && + git update-ref refs/pull/42/head master && + git clone . prune-overlapping && + cd prune-overlapping && + git config --add remote.origin.fetch refs/pull/*/head:refs/remotes/origin/pr/* && + + git fetch --prune origin && + git rev-parse origin/master && + git rev-parse origin/pr/42 && + + git config --unset-all remote.origin.fetch + git config remote.origin.fetch refs/pull/*/head:refs/remotes/origin/pr/* && + git config --add remote.origin.fetch refs/heads/*:refs/remotes/origin/* && + + git fetch --prune origin && + git rev-parse origin/master && + git rev-parse origin/pr/42 +' + test_expect_success 'fetch --prune --tags does not delete the remote-tracking branches' ' cd "$D" && git clone . prune-tags && From 16216b6ab1073b7aaa8225c32208758e6ea16629 Mon Sep 17 00:00:00 2001 From: Sandy Carter Date: Mon, 3 Mar 2014 09:55:53 -0500 Subject: [PATCH 40/82] i18n: proposed command missing leading dash Add missing leading dash to proposed commands in french output when using the command: git branch --set-upstream remotename/branchname and when upstream is gone Signed-off-by: Sandy Carter Signed-off-by: Junio C Hamano --- po/fr.po | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/po/fr.po b/po/fr.po index e10263f2b2ce90..0b9d59e8fa43dc 100644 --- a/po/fr.po +++ b/po/fr.po @@ -1075,7 +1075,7 @@ msgstr "Votre branche est basée sur '%s', mais la branche amont a disparu.\n" #: remote.c:1875 msgid " (use \"git branch --unset-upstream\" to fixup)\n" -msgstr " (utilisez \"git branch -unset-upstream\" pour corriger)\n" +msgstr " (utilisez \"git branch --unset-upstream\" pour corriger)\n" #: remote.c:1878 #, c-format @@ -3266,7 +3266,7 @@ msgstr " git branch -d %s\n" #: builtin/branch.c:1027 #, c-format msgid " git branch --set-upstream-to %s\n" -msgstr " git branch -set-upstream-to %s\n" +msgstr " git branch --set-upstream-to %s\n" #: builtin/bundle.c:47 #, c-format From fcfec8bd9a9bf0a0f36ba4aed85f5768988aa946 Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Wed, 5 Mar 2014 01:23:35 -0800 Subject: [PATCH 41/82] t7800: add a difftool test for .git-files Signed-off-by: David Aguilar Signed-off-by: Junio C Hamano --- t/t7800-difftool.sh | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/t/t7800-difftool.sh b/t/t7800-difftool.sh index 2418528487624b..5a193c500d282c 100755 --- a/t/t7800-difftool.sh +++ b/t/t7800-difftool.sh @@ -434,4 +434,18 @@ test_expect_success PERL 'difftool --no-symlinks detects conflict ' ' ) ' +test_expect_success PERL 'difftool properly honors gitlink and core.worktree' ' + git submodule add ./. submod/ule && + ( + cd submod/ule && + test_config diff.tool checktrees && + test_config difftool.checktrees.cmd '\'' + test -d "$LOCAL" && test -d "$REMOTE" && echo good + '\'' && + echo good >expect && + git difftool --tool=checktrees --dir-diff HEAD~ >actual && + test_cmp expect actual + ) +' + test_done From 3f419d45ef0dfc33dc301d9ae4737043c091291a Mon Sep 17 00:00:00 2001 From: Jeff King Date: Fri, 7 Mar 2014 12:15:01 -0500 Subject: [PATCH 42/82] show_ident_date: fix tz range check Commit 1dca155fe3fa (log: handle integer overflow in timestamps, 2014-02-24) tried to catch integer overflow coming from strtol() on the timezone field by comparing against LONG_MIN/LONG_MAX. However, the intermediate "tz" variable is an "int", which means it can never be LONG_MAX on LP64 systems; we would truncate the output from strtol before the comparison. Clang's -Wtautological-constant-out-of-range-compare notices this and rightly complains. Let's instead store the result of strtol in a long, and then compare it against INT_MIN/INT_MAX. This will catch overflow from strtol, and also overflow when we pass the result as an int to show_date. Reported-by: Eric Sunshine Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- pretty.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pretty.c b/pretty.c index 4da9a682f3bd6f..4d4c1e95625e57 100644 --- a/pretty.c +++ b/pretty.c @@ -397,7 +397,7 @@ static const char *show_ident_date(const struct ident_split *ident, enum date_mode mode) { unsigned long date = 0; - int tz = 0; + long tz = 0; if (ident->date_begin && ident->date_end) date = strtoul(ident->date_begin, NULL, 10); @@ -406,7 +406,7 @@ static const char *show_ident_date(const struct ident_split *ident, else { if (ident->tz_begin && ident->tz_end) tz = strtol(ident->tz_begin, NULL, 10); - if (tz == LONG_MAX || tz == LONG_MIN) + if (tz >= INT_MAX || tz <= INT_MIN) tz = 0; } return show_date(date, tz, mode); From cf424f5fd89bb5cd09b24c96633f8951b6fd7b54 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Mon, 10 Mar 2014 16:37:30 -0400 Subject: [PATCH 43/82] clean: respect pathspecs with "-d" git-clean uses read_directory to fill in a `struct dir` with potential hits. However, read_directory does not actually check against our pathspec. It uses a simplified version that may turn up false positives. As a result, we need to check that any hits match our pathspec. We do so reliably for non-directories. For directories, if "-d" is not given we check that the pathspec matched exactly (i.e., we are even stricter, and require an explicit "git clean foo" to clean "foo/"). But if "-d" is given, rather than relaxing the exact match to allow a recursive match, we do not check the pathspec at all. This regression was introduced in 113f10f (Make git-clean a builtin, 2007-11-11). Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- builtin/clean.c | 5 +++-- t/t7300-clean.sh | 16 ++++++++++++++++ 2 files changed, 19 insertions(+), 2 deletions(-) diff --git a/builtin/clean.c b/builtin/clean.c index 615cd57caf1d4c..857187ef3ba1f7 100644 --- a/builtin/clean.c +++ b/builtin/clean.c @@ -964,14 +964,15 @@ int cmd_clean(int argc, const char **argv, const char *prefix) matches = match_pathspec_depth(&pathspec, ent->name, len, 0, NULL); + if (pathspec.nr && !matches) + continue; + if (S_ISDIR(st.st_mode)) { if (remove_directories || (matches == MATCHED_EXACTLY)) { rel = relative_path(ent->name, prefix, &buf); string_list_append(&del_list, rel); } } else { - if (pathspec.nr && !matches) - continue; rel = relative_path(ent->name, prefix, &buf); string_list_append(&del_list, rel); } diff --git a/t/t7300-clean.sh b/t/t7300-clean.sh index 710be90489b2fd..74de814aec1465 100755 --- a/t/t7300-clean.sh +++ b/t/t7300-clean.sh @@ -511,4 +511,20 @@ test_expect_success SANITY 'git clean -d with an unreadable empty directory' ' ! test -d foo ' +test_expect_success 'git clean -d respects pathspecs (dir is prefix of pathspec)' ' + mkdir -p foo && + mkdir -p foobar && + git clean -df foobar && + test_path_is_dir foo && + test_path_is_missing foobar +' + +test_expect_success 'git clean -d respects pathspecs (pathspec is prefix of dir)' ' + mkdir -p foo && + mkdir -p foobar && + git clean -df foo && + test_path_is_missing foo && + test_path_is_dir foobar +' + test_done From 1f2e1088871e777355025a84f80a2d8b5cb04c06 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Mon, 10 Mar 2014 13:24:47 -0400 Subject: [PATCH 44/82] clean: simplify dir/not-dir logic When we get a list of paths from read_directory, we further prune it to create the final list of items to remove. The code paths for directories and non-directories repeat the same "add to list" code. This patch restructures the code so that we don't repeat ourselves. Also, by following a "if (condition) continue" pattern like the pathspec check above, it makes it more obvious that the conditional is about excluding directories under certain circumstances. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- builtin/clean.c | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/builtin/clean.c b/builtin/clean.c index 857187ef3ba1f7..4ec4fe2577dedb 100644 --- a/builtin/clean.c +++ b/builtin/clean.c @@ -967,15 +967,12 @@ int cmd_clean(int argc, const char **argv, const char *prefix) if (pathspec.nr && !matches) continue; - if (S_ISDIR(st.st_mode)) { - if (remove_directories || (matches == MATCHED_EXACTLY)) { - rel = relative_path(ent->name, prefix, &buf); - string_list_append(&del_list, rel); - } - } else { - rel = relative_path(ent->name, prefix, &buf); - string_list_append(&del_list, rel); - } + if (S_ISDIR(st.st_mode) && !remove_directories && + matches != MATCHED_EXACTLY) + continue; + + rel = relative_path(ent->name, prefix, &buf); + string_list_append(&del_list, rel); } if (interactive && del_list.nr > 0) From 3219bad94422084184987c9660361f3245d69f04 Mon Sep 17 00:00:00 2001 From: Benoit Pierre Date: Mon, 10 Mar 2014 19:49:31 +0100 Subject: [PATCH 45/82] merge hook tests: fix missing '&&' in test Signed-off-by: Benoit Pierre Signed-off-by: Junio C Hamano --- t/t7505-prepare-commit-msg-hook.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/t/t7505-prepare-commit-msg-hook.sh b/t/t7505-prepare-commit-msg-hook.sh index 357375151d79d0..1c95652dce4a61 100755 --- a/t/t7505-prepare-commit-msg-hook.sh +++ b/t/t7505-prepare-commit-msg-hook.sh @@ -174,7 +174,7 @@ test_expect_success 'with failing hook (merge)' ' git add file && rm -f "$HOOK" && git commit -m other && - write_script "$HOOK" <<-EOF + write_script "$HOOK" <<-EOF && exit 1 EOF git checkout - && From b7ae14148fc44223b4ffaff5ccbf3227a0af8f3c Mon Sep 17 00:00:00 2001 From: Benoit Pierre Date: Mon, 10 Mar 2014 19:49:32 +0100 Subject: [PATCH 46/82] merge hook tests: use 'test_must_fail' instead of '!' Signed-off-by: Benoit Pierre Signed-off-by: Junio C Hamano --- t/t7505-prepare-commit-msg-hook.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/t/t7505-prepare-commit-msg-hook.sh b/t/t7505-prepare-commit-msg-hook.sh index 1c95652dce4a61..5531abb0c4ed47 100755 --- a/t/t7505-prepare-commit-msg-hook.sh +++ b/t/t7505-prepare-commit-msg-hook.sh @@ -154,7 +154,7 @@ test_expect_success 'with failing hook' ' head=`git rev-parse HEAD` && echo "more" >> file && git add file && - ! GIT_EDITOR="\"\$FAKE_EDITOR\"" git commit -c $head + test_must_fail env GIT_EDITOR="\"\$FAKE_EDITOR\"" git commit -c $head ' @@ -163,7 +163,7 @@ test_expect_success 'with failing hook (--no-verify)' ' head=`git rev-parse HEAD` && echo "more" >> file && git add file && - ! GIT_EDITOR="\"\$FAKE_EDITOR\"" git commit --no-verify -c $head + test_must_fail env GIT_EDITOR="\"\$FAKE_EDITOR\"" git commit --no-verify -c $head ' From 89ccc1b09cf4004e6129c66def42b47206ed6b5f Mon Sep 17 00:00:00 2001 From: John Keeping Date: Sat, 8 Mar 2014 19:29:17 +0000 Subject: [PATCH 47/82] builtin/mv: fix out of bounds write When commit a88c915 (mv: move submodules using a gitfile, 2013-07-30) added the submodule_gitfile array, it was not added to the block that enlarges the arrays when we are moving a directory so that we do not have to worry about it being a directory when we perform the actual move. After this, the loop continues over the enlarged set of sources. Since we assume that submodule_gitfile has size argc, if any of the items in the source directory are submodules we are guaranteed to write beyond the end of submodule_gitfile. Fix this by realloc'ing submodule_gitfile at the same time as the other arrays. Reported-by: Guillaume Gelin Signed-off-by: John Keeping Signed-off-by: Junio C Hamano --- builtin/mv.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/builtin/mv.c b/builtin/mv.c index 21c46d1636e6e8..525807722417dd 100644 --- a/builtin/mv.c +++ b/builtin/mv.c @@ -179,6 +179,9 @@ int cmd_mv(int argc, const char **argv, const char *prefix) modes = xrealloc(modes, (argc + last - first) * sizeof(enum update_mode)); + submodule_gitfile = xrealloc(submodule_gitfile, + (argc + last - first) + * sizeof(char *)); } dst = add_slash(dst); @@ -192,6 +195,7 @@ int cmd_mv(int argc, const char **argv, const char *prefix) prefix_path(dst, dst_len, path + length + 1); modes[argc + j] = INDEX; + submodule_gitfile[argc + j] = NULL; } argc += last - first; } From d52cb5761a0e92e6051ea92edc7ae96fb6dca78f Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Wed, 12 Mar 2014 13:51:22 -0700 Subject: [PATCH 48/82] wt-status: make full label string to be subject to l10n Earlier in 3651e45c (wt-status: take the alignment burden off translators, 2013-11-05), we assumed that it is OK to make the string before the colon in a label string we give as the section header of various kinds of changes (e.g. "new file:") translatable. This assumption apparently does not hold for some languages, e.g. ones that want to have spaces around the colon. Also introduce a static label_width to avoid having to run strlen(padding) over and over. Signed-off-by: Junio C Hamano --- wt-status.c | 35 +++++++++++++++++------------------ 1 file changed, 17 insertions(+), 18 deletions(-) diff --git a/wt-status.c b/wt-status.c index 4e5581005936a1..9cf70287d9aae0 100644 --- a/wt-status.c +++ b/wt-status.c @@ -272,21 +272,21 @@ static const char *wt_status_diff_status_string(int status) { switch (status) { case DIFF_STATUS_ADDED: - return _("new file"); + return _("new file:"); case DIFF_STATUS_COPIED: - return _("copied"); + return _("copied:"); case DIFF_STATUS_DELETED: - return _("deleted"); + return _("deleted:"); case DIFF_STATUS_MODIFIED: - return _("modified"); + return _("modified:"); case DIFF_STATUS_RENAMED: - return _("renamed"); + return _("renamed:"); case DIFF_STATUS_TYPE_CHANGED: - return _("typechange"); + return _("typechange:"); case DIFF_STATUS_UNKNOWN: - return _("unknown"); + return _("unknown:"); case DIFF_STATUS_UNMERGED: - return _("unmerged"); + return _("unmerged:"); default: return NULL; } @@ -305,21 +305,21 @@ static void wt_status_print_change_data(struct wt_status *s, struct strbuf onebuf = STRBUF_INIT, twobuf = STRBUF_INIT; struct strbuf extra = STRBUF_INIT; static char *padding; + static int label_width; const char *what; int len; if (!padding) { - int width = 0; /* If DIFF_STATUS_* uses outside this range, we're in trouble */ for (status = 'A'; status <= 'Z'; status++) { what = wt_status_diff_status_string(status); len = what ? strlen(what) : 0; - if (len > width) - width = len; + if (len > label_width) + label_width = len; } - width += 2; /* colon and a space */ - padding = xmallocz(width); - memset(padding, ' ', width); + label_width += strlen(" "); + padding = xmallocz(label_width); + memset(padding, ' ', label_width); } one_name = two_name = it->string; @@ -355,14 +355,13 @@ static void wt_status_print_change_data(struct wt_status *s, what = wt_status_diff_status_string(status); if (!what) die(_("bug: unhandled diff status %c"), status); - /* 1 for colon, which is not part of "what" */ - len = strlen(padding) - (utf8_strwidth(what) + 1); + len = label_width - utf8_strwidth(what); assert(len >= 0); if (status == DIFF_STATUS_COPIED || status == DIFF_STATUS_RENAMED) - status_printf_more(s, c, "%s:%.*s%s -> %s", + status_printf_more(s, c, "%s%.*s%s -> %s", what, len, padding, one, two); else - status_printf_more(s, c, "%s:%.*s%s", + status_printf_more(s, c, "%s%.*s%s", what, len, padding, one); if (extra.len) { status_printf_more(s, color(WT_STATUS_HEADER, s), "%s", extra.buf); From 335e8250124aea055cbe4fb8a2268fa9e2f34439 Mon Sep 17 00:00:00 2001 From: Jonathan Nieder Date: Thu, 19 Dec 2013 11:43:19 -0800 Subject: [PATCH 49/82] wt-status: extract the code to compute width for labels Signed-off-by: Junio C Hamano --- wt-status.c | 22 +++++++++++++++------- 1 file changed, 15 insertions(+), 7 deletions(-) diff --git a/wt-status.c b/wt-status.c index 9cf70287d9aae0..db98c52d452f32 100644 --- a/wt-status.c +++ b/wt-status.c @@ -292,6 +292,19 @@ static const char *wt_status_diff_status_string(int status) } } +static int maxwidth(const char *(*label)(int), int minval, int maxval) +{ + int result = 0, i; + + for (i = minval; i <= maxval; i++) { + const char *s = label(i); + int len = s ? utf8_strwidth(s) : 0; + if (len > result) + result = len; + } + return result; +} + static void wt_status_print_change_data(struct wt_status *s, int change_type, struct string_list_item *it) @@ -310,13 +323,8 @@ static void wt_status_print_change_data(struct wt_status *s, int len; if (!padding) { - /* If DIFF_STATUS_* uses outside this range, we're in trouble */ - for (status = 'A'; status <= 'Z'; status++) { - what = wt_status_diff_status_string(status); - len = what ? strlen(what) : 0; - if (len > label_width) - label_width = len; - } + /* If DIFF_STATUS_* uses outside the range [A..Z], we're in trouble */ + label_width = maxwidth(wt_status_diff_status_string, 'A', 'Z'); label_width += strlen(" "); padding = xmallocz(label_width); memset(padding, ' ', label_width); From 8f17f5b22ae54ecc3dfdafe33d7697e1bf3949f6 Mon Sep 17 00:00:00 2001 From: Jonathan Nieder Date: Thu, 19 Dec 2013 11:43:19 -0800 Subject: [PATCH 50/82] wt-status: i18n of section labels The original code assumes that: (1) the number of bytes written is the width of a string, so they can line up; (2) the "how" string is always <= 19 bytes. Neither of which we should assume. Using the same approach as the earlier 3651e45c (wt-status: take the alignment burden off translators, 2013-11-05), compute the necessary column width to hold the longest label and use that for alignment. cf. http://bugs.debian.org/725777 Signed-off-by: Jonathan Nieder Helped-by: Sandy Carter Signed-off-by: Junio C Hamano --- wt-status.c | 66 ++++++++++++++++++++++++++++++++++++++--------------- 1 file changed, 47 insertions(+), 19 deletions(-) diff --git a/wt-status.c b/wt-status.c index db98c52d452f32..b1b018e54e9823 100644 --- a/wt-status.c +++ b/wt-status.c @@ -245,27 +245,26 @@ static void wt_status_print_trailer(struct wt_status *s) #define quote_path quote_path_relative -static void wt_status_print_unmerged_data(struct wt_status *s, - struct string_list_item *it) +static const char *wt_status_unmerged_status_string(int stagemask) { - const char *c = color(WT_STATUS_UNMERGED, s); - struct wt_status_change_data *d = it->util; - struct strbuf onebuf = STRBUF_INIT; - const char *one, *how = _("bug"); - - one = quote_path(it->string, s->prefix, &onebuf); - status_printf(s, color(WT_STATUS_HEADER, s), "\t"); - switch (d->stagemask) { - case 1: how = _("both deleted:"); break; - case 2: how = _("added by us:"); break; - case 3: how = _("deleted by them:"); break; - case 4: how = _("added by them:"); break; - case 5: how = _("deleted by us:"); break; - case 6: how = _("both added:"); break; - case 7: how = _("both modified:"); break; + switch (stagemask) { + case 1: + return _("both deleted:"); + case 2: + return _("added by us:"); + case 3: + return _("deleted by them:"); + case 4: + return _("added by them:"); + case 5: + return _("deleted by us:"); + case 6: + return _("both added:"); + case 7: + return _("both modified:"); + default: + die(_("bug: unhandled unmerged status %x"), stagemask); } - status_printf_more(s, c, "%-20s%s\n", how, one); - strbuf_release(&onebuf); } static const char *wt_status_diff_status_string(int status) @@ -305,6 +304,35 @@ static int maxwidth(const char *(*label)(int), int minval, int maxval) return result; } +static void wt_status_print_unmerged_data(struct wt_status *s, + struct string_list_item *it) +{ + const char *c = color(WT_STATUS_UNMERGED, s); + struct wt_status_change_data *d = it->util; + struct strbuf onebuf = STRBUF_INIT; + static char *padding; + static int label_width; + const char *one, *how; + int len; + + if (!padding) { + label_width = maxwidth(wt_status_unmerged_status_string, 1, 7); + label_width += strlen(" "); + if (label_width < 20) + label_width = 20; + padding = xmallocz(label_width); + memset(padding, ' ', label_width); + } + + one = quote_path(it->string, s->prefix, &onebuf); + status_printf(s, color(WT_STATUS_HEADER, s), "\t"); + + how = wt_status_unmerged_status_string(d->stagemask); + len = label_width - utf8_strwidth(how); + status_printf_more(s, c, "%s%.*s%s\n", how, len, padding, one); + strbuf_release(&onebuf); +} + static void wt_status_print_change_data(struct wt_status *s, int change_type, struct string_list_item *it) From c7cb333f60db9b8313da75c2075a3619202e905d Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Wed, 12 Mar 2014 13:43:51 -0700 Subject: [PATCH 51/82] wt-status: lift the artificual "at least 20 columns" floor When we show unmerged paths, we had an artificial 20 columns floor for the width of labels (e.g. "both deleted:") shown next to the pathnames. Depending on the locale, this may result in a label that is too wide when all the label strings are way shorter than 20 columns, or no-op when a label string is longer than 20 columns. Just drop the artificial floor. The screen real estate is better utilized this way when all the strings are shorter. Adjust the tests to this change. Signed-off-by: Junio C Hamano --- t/t7060-wtstatus.sh | 14 +++++++------- t/t7512-status-help.sh | 12 ++++++------ wt-status.c | 2 -- 3 files changed, 13 insertions(+), 15 deletions(-) diff --git a/t/t7060-wtstatus.sh b/t/t7060-wtstatus.sh index 7d467c034a27f5..741ec085767c94 100755 --- a/t/t7060-wtstatus.sh +++ b/t/t7060-wtstatus.sh @@ -38,7 +38,7 @@ You have unmerged paths. Unmerged paths: (use "git add/rm ..." as appropriate to mark resolution) - deleted by us: foo + deleted by us: foo no changes added to commit (use "git add" and/or "git commit -a") EOF @@ -142,8 +142,8 @@ You have unmerged paths. Unmerged paths: (use "git add/rm ..." as appropriate to mark resolution) - both added: conflict.txt - deleted by them: main.txt + both added: conflict.txt + deleted by them: main.txt no changes added to commit (use "git add" and/or "git commit -a") EOF @@ -175,9 +175,9 @@ You have unmerged paths. Unmerged paths: (use "git add/rm ..." as appropriate to mark resolution) - both deleted: main.txt - added by them: sub_master.txt - added by us: sub_second.txt + both deleted: main.txt + added by them: sub_master.txt + added by us: sub_second.txt no changes added to commit (use "git add" and/or "git commit -a") EOF @@ -203,7 +203,7 @@ Changes to be committed: Unmerged paths: (use "git rm ..." to mark resolution) - both deleted: main.txt + both deleted: main.txt Untracked files not listed (use -u option to show untracked files) EOF diff --git a/t/t7512-status-help.sh b/t/t7512-status-help.sh index 3cec57af1ee0ca..68ad2d7454d6cb 100755 --- a/t/t7512-status-help.sh +++ b/t/t7512-status-help.sh @@ -33,7 +33,7 @@ You have unmerged paths. Unmerged paths: (use "git add ..." to mark resolution) - both modified: main.txt + both modified: main.txt no changes added to commit (use "git add" and/or "git commit -a") EOF @@ -87,7 +87,7 @@ Unmerged paths: (use "git reset HEAD ..." to unstage) (use "git add ..." to mark resolution) - both modified: main.txt + both modified: main.txt no changes added to commit (use "git add" and/or "git commit -a") EOF @@ -146,7 +146,7 @@ Unmerged paths: (use "git reset HEAD ..." to unstage) (use "git add ..." to mark resolution) - both modified: main.txt + both modified: main.txt no changes added to commit (use "git add" and/or "git commit -a") EOF @@ -602,7 +602,7 @@ rebase in progress; onto $ONTO You are currently rebasing branch '\''statushints_disabled'\'' on '\''$ONTO'\''. Unmerged paths: - both modified: main.txt + both modified: main.txt no changes added to commit EOF @@ -636,7 +636,7 @@ You are currently cherry-picking commit $TO_CHERRY_PICK. Unmerged paths: (use "git add ..." to mark resolution) - both modified: main.txt + both modified: main.txt no changes added to commit (use "git add" and/or "git commit -a") EOF @@ -707,7 +707,7 @@ Unmerged paths: (use "git reset HEAD ..." to unstage) (use "git add ..." to mark resolution) - both modified: to-revert.txt + both modified: to-revert.txt no changes added to commit (use "git add" and/or "git commit -a") EOF diff --git a/wt-status.c b/wt-status.c index b1b018e54e9823..6f3ed67fc9a6af 100644 --- a/wt-status.c +++ b/wt-status.c @@ -318,8 +318,6 @@ static void wt_status_print_unmerged_data(struct wt_status *s, if (!padding) { label_width = maxwidth(wt_status_unmerged_status_string, 1, 7); label_width += strlen(" "); - if (label_width < 20) - label_width = 20; padding = xmallocz(label_width); memset(padding, ' ', label_width); } From f63272a35e03a9895c468d1a698dabaa4c3d9273 Mon Sep 17 00:00:00 2001 From: Michael Haggerty Date: Thu, 13 Mar 2014 10:19:07 +0100 Subject: [PATCH 52/82] checkout_entry(): use the strbuf throughout the function There is no need to break out the "buf" and "len" members into separate temporary variables. Rename path_buf to path and use path.buf and path.len directly. This makes it easier to reason about the data flow in the function. Signed-off-by: Michael Haggerty Signed-off-by: Junio C Hamano --- entry.c | 32 ++++++++++++++++---------------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/entry.c b/entry.c index fbb4863103417a..20d291918f540e 100644 --- a/entry.c +++ b/entry.c @@ -237,27 +237,25 @@ static int check_path(const char *path, int len, struct stat *st, int skiplen) int checkout_entry(struct cache_entry *ce, const struct checkout *state, char *topath) { - static struct strbuf path_buf = STRBUF_INIT; - char *path; + static struct strbuf path = STRBUF_INIT; struct stat st; - int len; if (topath) return write_entry(ce, topath, state, 1); - strbuf_reset(&path_buf); - strbuf_add(&path_buf, state->base_dir, state->base_dir_len); - strbuf_add(&path_buf, ce->name, ce_namelen(ce)); - path = path_buf.buf; - len = path_buf.len; + strbuf_reset(&path); + strbuf_add(&path, state->base_dir, state->base_dir_len); + strbuf_add(&path, ce->name, ce_namelen(ce)); - if (!check_path(path, len, &st, state->base_dir_len)) { + if (!check_path(path.buf, path.len, &st, state->base_dir_len)) { unsigned changed = ce_match_stat(ce, &st, CE_MATCH_IGNORE_VALID|CE_MATCH_IGNORE_SKIP_WORKTREE); if (!changed) return 0; if (!state->force) { if (!state->quiet) - fprintf(stderr, "%s already exists, no checkout\n", path); + fprintf(stderr, + "%s already exists, no checkout\n", + path.buf); return -1; } @@ -272,12 +270,14 @@ int checkout_entry(struct cache_entry *ce, if (S_ISGITLINK(ce->ce_mode)) return 0; if (!state->force) - return error("%s is a directory", path); - remove_subtree(path); - } else if (unlink(path)) - return error("unable to unlink old '%s' (%s)", path, strerror(errno)); + return error("%s is a directory", path.buf); + remove_subtree(path.buf); + } else if (unlink(path.buf)) + return error("unable to unlink old '%s' (%s)", + path.buf, strerror(errno)); } else if (state->not_new) return 0; - create_directories(path, len, state); - return write_entry(ce, path, state, 0); + + create_directories(path.buf, path.len, state); + return write_entry(ce, path.buf, state, 0); } From 2f29e0c6fa5d312c4e0675b0dd23d3126b9f55fa Mon Sep 17 00:00:00 2001 From: Michael Haggerty Date: Thu, 13 Mar 2014 10:19:08 +0100 Subject: [PATCH 53/82] entry.c: fix possible buffer overflow in remove_subtree() remove_subtree() manipulated path in a fixed-size buffer even though the length of the input, let alone the length of entries within the directory, were not known in advance. Change the function to take a strbuf argument and use that object as its scratch space. Signed-off-by: Michael Haggerty Signed-off-by: Junio C Hamano --- entry.c | 34 +++++++++++++++++----------------- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/entry.c b/entry.c index 20d291918f540e..fa0e5ec8ae2c6f 100644 --- a/entry.c +++ b/entry.c @@ -44,33 +44,33 @@ static void create_directories(const char *path, int path_len, free(buf); } -static void remove_subtree(const char *path) +static void remove_subtree(struct strbuf *path) { - DIR *dir = opendir(path); + DIR *dir = opendir(path->buf); struct dirent *de; - char pathbuf[PATH_MAX]; - char *name; + int origlen = path->len; if (!dir) - die_errno("cannot opendir '%s'", path); - strcpy(pathbuf, path); - name = pathbuf + strlen(path); - *name++ = '/'; + die_errno("cannot opendir '%s'", path->buf); while ((de = readdir(dir)) != NULL) { struct stat st; + if (is_dot_or_dotdot(de->d_name)) continue; - strcpy(name, de->d_name); - if (lstat(pathbuf, &st)) - die_errno("cannot lstat '%s'", pathbuf); + + strbuf_addch(path, '/'); + strbuf_addstr(path, de->d_name); + if (lstat(path->buf, &st)) + die_errno("cannot lstat '%s'", path->buf); if (S_ISDIR(st.st_mode)) - remove_subtree(pathbuf); - else if (unlink(pathbuf)) - die_errno("cannot unlink '%s'", pathbuf); + remove_subtree(path); + else if (unlink(path->buf)) + die_errno("cannot unlink '%s'", path->buf); + strbuf_setlen(path, origlen); } closedir(dir); - if (rmdir(path)) - die_errno("cannot rmdir '%s'", path); + if (rmdir(path->buf)) + die_errno("cannot rmdir '%s'", path->buf); } static int create_file(const char *path, unsigned int mode) @@ -271,7 +271,7 @@ int checkout_entry(struct cache_entry *ce, return 0; if (!state->force) return error("%s is a directory", path.buf); - remove_subtree(path.buf); + remove_subtree(&path); } else if (unlink(path.buf)) return error("unable to unlink old '%s' (%s)", path.buf, strerror(errno)); From 7e27173ef9152fe93d3fc42b60febbc782e4db16 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Fri, 14 Mar 2014 17:57:23 -0400 Subject: [PATCH 54/82] t/lib-terminal: make TTY a lazy prerequisite When lib-terminal.sh is sourced by a test script, we immediately set up the TTY prerequisite. We do so inside a test_expect_success, because that nicely isolates any generated output. However, this early test can interfere with a script that later wants to skip all tests (e.g., t5541 then goes on to set up the httpd server, and wants to skip_all if that fails). TAP output doesn't let us skip everything after we have already run at least one test. We could fix this by reordering the inclusion of lib-terminal.sh in t5541 to go after the httpd setup. That solves this case, but we might eventually hit a case with circular dependencies, where either lib-*.sh include might want to skip_all after the other has run a test. So instead, let's just remove the ordering constraint entirely by doing the setup inside a test_lazy_prereq construct, rather than in a regular test. We never cared about the test outcome anyway (it was written to always succeed). Note that in addition to setting up the prerequisite, the current test also defines test_terminal. Since we can't affect the environment from a lazy_prereq, we have to hoist that out. We previously depended on it _not_ being defined when the TTY prereq isn't set as a way to ensure that tests properly declare their dependency on TTY. However, we still cover the case (see the in-code comment for details). Reported-by: Jens Lehmann Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- t/lib-terminal.sh | 37 +++++++++++++++++++------------------ 1 file changed, 19 insertions(+), 18 deletions(-) diff --git a/t/lib-terminal.sh b/t/lib-terminal.sh index 737df289a1450b..920da3ab1896db 100644 --- a/t/lib-terminal.sh +++ b/t/lib-terminal.sh @@ -1,6 +1,20 @@ #!/bin/sh -test_expect_success PERL 'set up terminal for tests' ' +# Catch tests which should depend on TTY but forgot to. There's no need +# to aditionally check that the TTY prereq is set here. If the test declared +# it and we are running the test, then it must have been set. +test_terminal () { + if ! test_declared_prereq TTY + then + echo >&4 "test_terminal: need to declare TTY prerequisite" + return 127 + fi + perl "$TEST_DIRECTORY"/test-terminal.perl "$@" +} + +test_lazy_prereq TTY ' + test_have_prereq PERL && + # Reading from the pty master seems to get stuck _sometimes_ # on Mac OS X 10.5.0, using Perl 5.10.0 or 5.8.9. # @@ -15,21 +29,8 @@ test_expect_success PERL 'set up terminal for tests' ' # After 2000 iterations or so it hangs. # https://rt.cpan.org/Ticket/Display.html?id=65692 # - if test "$(uname -s)" = Darwin - then - : - elif - perl "$TEST_DIRECTORY"/test-terminal.perl \ - sh -c "test -t 1 && test -t 2" - then - test_set_prereq TTY && - test_terminal () { - if ! test_declared_prereq TTY - then - echo >&4 "test_terminal: need to declare TTY prerequisite" - return 127 - fi - perl "$TEST_DIRECTORY"/test-terminal.perl "$@" - } - fi + test "$(uname -s)" != Darwin && + + perl "$TEST_DIRECTORY"/test-terminal.perl \ + sh -c "test -t 1 && test -t 2" ' From fb8a4e8079ab8fc37e9cde32957c35637280ab8f Mon Sep 17 00:00:00 2001 From: "brian m. carlson" Date: Sat, 15 Mar 2014 18:56:52 +0000 Subject: [PATCH 55/82] mv: prevent mismatched data when ignoring errors. We shrink the source and destination arrays, but not the modes or submodule_gitfile arrays, resulting in potentially mismatched data. Shrink all the arrays at the same time to prevent this. Add tests to ensure the problem does not recur. Signed-off-by: brian m. carlson Signed-off-by: Junio C Hamano --- builtin/mv.c | 5 +++++ t/t7001-mv.sh | 13 ++++++++++++- 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/builtin/mv.c b/builtin/mv.c index 525807722417dd..45e57f307b7994 100644 --- a/builtin/mv.c +++ b/builtin/mv.c @@ -231,6 +231,11 @@ int cmd_mv(int argc, const char **argv, const char *prefix) memmove(destination + i, destination + i + 1, (argc - i) * sizeof(char *)); + memmove(modes + i, modes + i + 1, + (argc - i) * sizeof(enum update_mode)); + memmove(submodule_gitfile + i, + submodule_gitfile + i + 1, + (argc - i) * sizeof(char *)); i--; } } else diff --git a/t/t7001-mv.sh b/t/t7001-mv.sh index 3bfdfed1f7774e..4023b6ec48729d 100755 --- a/t/t7001-mv.sh +++ b/t/t7001-mv.sh @@ -294,7 +294,8 @@ test_expect_success 'setup submodule' ' git submodule add ./. sub && echo content >file && git add file && - git commit -m "added sub and file" + git commit -m "added sub and file" && + git branch submodule ' test_expect_success 'git mv cannot move a submodule in a file' ' @@ -442,4 +443,14 @@ test_expect_success 'mv --dry-run does not touch the submodule or .gitmodules' ' git diff-files --quiet -- sub .gitmodules ' +test_expect_success 'mv -k does not accidentally destroy submodules' ' + git checkout submodule && + mkdir dummy dest && + git mv -k dummy sub dest && + git status --porcelain >actual && + grep "^R sub -> dest/sub" actual && + git reset --hard && + git checkout . +' + test_done From 47be06602656ee9cac860f675d2c8d1f0deabdbe Mon Sep 17 00:00:00 2001 From: Uwe Storbeck Date: Sat, 15 Mar 2014 00:56:43 +0100 Subject: [PATCH 56/82] rebase -i: do not "echo" random user-supplied strings In some places we "echo" a string that comes from a commit log message, which may have a backslash sequence that is interpreted by the command (POSIX.1 allows this), most notably "dash"'s built-in 'echo'. A commit message which contains the string '\n' (or ends with the string '\c') may result in a garbage line in the todo list of an interactive rebase which causes the rebase to fail. To reproduce the behavior (with dash as /bin/sh): mkdir test && cd test && git init echo 1 >foo && git add foo git commit -m"this commit message ends with '\n'" echo 2 >foo && git commit -a --fixup HEAD git rebase -i --autosquash --root Now the editor opens with garbage in line 3 which has to be removed or the rebase fails. Signed-off-by: Uwe Storbeck Signed-off-by: Junio C Hamano --- git-rebase--interactive.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh index 43c19e0829ca72..43631b472311d0 100644 --- a/git-rebase--interactive.sh +++ b/git-rebase--interactive.sh @@ -739,7 +739,7 @@ rearrange_squash () { ;; esac done - echo "$sha1 $action $prefix $rest" + printf '%s %s %s %s\n' "$sha1" "$action" "$prefix" "$rest" # if it's a single word, try to resolve to a full sha1 and # emit a second copy. This allows us to match on both message # and on sha1 prefix From 7839632167bc6ceef20f28bd046f7001493b070f Mon Sep 17 00:00:00 2001 From: Jeff King Date: Fri, 14 Mar 2014 23:47:06 -0400 Subject: [PATCH 57/82] shallow: verify shallow file after taking lock Before writing the shallow file, we stat() the existing file to make sure it has not been updated since our operation began. However, we do not do so under a lock, so there is a possible race: 1. Process A takes the lock. 2. Process B calls check_shallow_file_for_update and finds no update. 3. Process A commits the lockfile. 4. Process B takes the lock, then overwrite's process A's changes. We can fix this by doing our check while we hold the lock. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- shallow.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/shallow.c b/shallow.c index c7602ce3a2076f..0b267b64117c5f 100644 --- a/shallow.c +++ b/shallow.c @@ -263,9 +263,9 @@ void setup_alternate_shallow(struct lock_file *shallow_lock, struct strbuf sb = STRBUF_INIT; int fd; - check_shallow_file_for_update(); fd = hold_lock_file_for_update(shallow_lock, git_path("shallow"), LOCK_DIE_ON_ERROR); + check_shallow_file_for_update(); if (write_shallow_commits(&sb, 0, extra)) { if (write_in_full(fd, sb.buf, sb.len) != sb.len) die_errno("failed to write to %s", @@ -310,9 +310,9 @@ void prune_shallow(int show_only) strbuf_release(&sb); return; } - check_shallow_file_for_update(); fd = hold_lock_file_for_update(&shallow_lock, git_path("shallow"), LOCK_DIE_ON_ERROR); + check_shallow_file_for_update(); if (write_shallow_commits_1(&sb, 0, NULL, SEEN_ONLY)) { if (write_in_full(fd, sb.buf, sb.len) != sb.len) die_errno("failed to write to %s", From de983a0a1817437fbb0db938ecf91f73023a5f87 Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Mon, 17 Mar 2014 15:08:36 -0700 Subject: [PATCH 58/82] index-pack: report error using the correct variable MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We feed a string pointer that is potentially NULL to die() when showing the message. Don't. Noticed-by: Nguyễn Thái Ngọc Duy Signed-off-by: Junio C Hamano --- builtin/index-pack.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/builtin/index-pack.c b/builtin/index-pack.c index 9e9eb4b74e8335..80c93741352915 100644 --- a/builtin/index-pack.c +++ b/builtin/index-pack.c @@ -1291,7 +1291,7 @@ static void final(const char *final_pack_name, const char *curr_pack_name, if (keep_fd < 0) { if (errno != EEXIST) die_errno(_("cannot write keep file '%s'"), - keep_name); + keep_name ? keep_name : name); } else { if (keep_msg_len > 0) { write_or_die(keep_fd, keep_msg, keep_msg_len); @@ -1299,7 +1299,7 @@ static void final(const char *final_pack_name, const char *curr_pack_name, } if (close(keep_fd) != 0) die_errno(_("cannot close written keep file '%s'"), - keep_name); + keep_name ? keep_name : name); report = "keep"; } } From 3c3e6f5645c5e661bc8c338e4e7a9252a14e429e Mon Sep 17 00:00:00 2001 From: Ramkumar Ramachandra Date: Sun, 16 Mar 2014 18:54:56 -0400 Subject: [PATCH 59/82] Documentation/merge-strategies: avoid hyphenated commands Replace git-pull and git-merge with the corresponding un-hyphenated versions. While at it, use ` to mark it up instead of '. Signed-off-by: Ramkumar Ramachandra Signed-off-by: Junio C Hamano --- Documentation/merge-strategies.txt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Documentation/merge-strategies.txt b/Documentation/merge-strategies.txt index 49a9a7d53f5836..81e349309c315e 100644 --- a/Documentation/merge-strategies.txt +++ b/Documentation/merge-strategies.txt @@ -1,10 +1,10 @@ MERGE STRATEGIES ---------------- -The merge mechanism ('git-merge' and 'git-pull' commands) allows the +The merge mechanism (`git merge` and `git pull` commands) allows the backend 'merge strategies' to be chosen with `-s` option. Some strategies can also take their own options, which can be passed by giving `-X