From 1b9b5c695e34051c80c9240fdb22e7c89ff0fd5f Mon Sep 17 00:00:00 2001 From: Stefan Beller Date: Tue, 16 Oct 2018 16:35:32 -0700 Subject: [PATCH 0001/1736] sha1_file: allow read_object to read objects in arbitrary repositories Allow read_object (a file local functon in sha1_file) to handle arbitrary repositories by passing the repository down to oid_object_info_extended. Signed-off-by: Stefan Beller Signed-off-by: Junio C Hamano --- sha1-file.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/sha1-file.c b/sha1-file.c index dd0b6aa873a954..b8ce21cbafd7a2 100644 --- a/sha1-file.c +++ b/sha1-file.c @@ -1361,7 +1361,9 @@ int oid_object_info(struct repository *r, return type; } -static void *read_object(const unsigned char *sha1, enum object_type *type, +static void *read_object(struct repository *r, + const unsigned char *sha1, + enum object_type *type, unsigned long *size) { struct object_id oid; @@ -1373,7 +1375,7 @@ static void *read_object(const unsigned char *sha1, enum object_type *type, hashcpy(oid.hash, sha1); - if (oid_object_info_extended(the_repository, &oid, &oi, 0) < 0) + if (oid_object_info_extended(r, &oid, &oi, 0) < 0) return NULL; return content; } @@ -1414,7 +1416,7 @@ void *read_object_file_extended(const struct object_id *oid, lookup_replace_object(the_repository, oid) : oid; errno = 0; - data = read_object(repl->hash, type, size); + data = read_object(the_repository, repl->hash, type, size); if (data) return data; @@ -1755,7 +1757,7 @@ int force_object_loose(const struct object_id *oid, time_t mtime) if (has_loose_object(oid)) return 0; - buf = read_object(oid->hash, &type, &len); + buf = read_object(the_repository, oid->hash, &type, &len); if (!buf) return error(_("cannot read sha1_file for %s"), oid_to_hex(oid)); hdrlen = xsnprintf(hdr, sizeof(hdr), "%s %lu", type_name(type), len) + 1; From 33b94066f27199071abaa4067d29a98bbbbdae11 Mon Sep 17 00:00:00 2001 From: Stefan Beller Date: Tue, 16 Oct 2018 16:35:33 -0700 Subject: [PATCH 0002/1736] packfile: allow has_packed_and_bad to handle arbitrary repositories has_packed_and_bad is not widely used, so just migrate it all at once. Signed-off-by: Stefan Beller Signed-off-by: Junio C Hamano --- packfile.c | 5 +++-- packfile.h | 2 +- sha1-file.c | 2 +- 3 files changed, 5 insertions(+), 4 deletions(-) diff --git a/packfile.c b/packfile.c index 841b36182fcd93..bc2e0f5043bed2 100644 --- a/packfile.c +++ b/packfile.c @@ -1131,12 +1131,13 @@ void mark_bad_packed_object(struct packed_git *p, const unsigned char *sha1) p->num_bad_objects++; } -const struct packed_git *has_packed_and_bad(const unsigned char *sha1) +const struct packed_git *has_packed_and_bad(struct repository *r, + const unsigned char *sha1) { struct packed_git *p; unsigned i; - for (p = the_repository->objects->packed_git; p; p = p->next) + for (p = r->objects->packed_git; p; p = p->next) for (i = 0; i < p->num_bad_objects; i++) if (hasheq(sha1, p->bad_object_sha1 + the_hash_algo->rawsz * i)) diff --git a/packfile.h b/packfile.h index 442625723dea4b..7a62d722317734 100644 --- a/packfile.h +++ b/packfile.h @@ -146,7 +146,7 @@ extern int packed_object_info(struct repository *r, off_t offset, struct object_info *); extern void mark_bad_packed_object(struct packed_git *p, const unsigned char *sha1); -extern const struct packed_git *has_packed_and_bad(const unsigned char *sha1); +extern const struct packed_git *has_packed_and_bad(struct repository *r, const unsigned char *sha1); /* * Iff a pack file in the given repository contains the object named by sha1, diff --git a/sha1-file.c b/sha1-file.c index b8ce21cbafd7a2..856e000ee1afda 100644 --- a/sha1-file.c +++ b/sha1-file.c @@ -1432,7 +1432,7 @@ void *read_object_file_extended(const struct object_id *oid, die(_("loose object %s (stored in %s) is corrupt"), oid_to_hex(repl), path); - if ((p = has_packed_and_bad(repl->hash)) != NULL) + if ((p = has_packed_and_bad(the_repository, repl->hash)) != NULL) die(_("packed object %s (stored in %s) is corrupt"), oid_to_hex(repl), p->pack_name); From 1ccf07cbb762b44746c49634e1c6fb945c9977df Mon Sep 17 00:00:00 2001 From: "brian m. carlson" Date: Mon, 22 Oct 2018 02:43:31 +0000 Subject: [PATCH 0003/1736] sha1-file: rename algorithm to "sha1" The transition plan anticipates us using a syntax such as "^{sha1}" for disambiguation. Since this is a syntax some people will be typing a lot, it makes sense to provide a short, easy-to-type syntax. Omitting the dash doesn't create any ambiguity; however, it does make the syntax shorter and easier to type, especially for touch typists. In addition, the transition plan already uses "sha1" in this context. Rename the name of SHA-1 implementation to "sha1". Note that this change creates no backwards compatibility concerns, since we haven't yet used this field in any configuration settings. Signed-off-by: brian m. carlson Signed-off-by: Junio C Hamano --- sha1-file.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sha1-file.c b/sha1-file.c index a4367b8f044c6e..e29825f259cbf0 100644 --- a/sha1-file.c +++ b/sha1-file.c @@ -97,7 +97,7 @@ const struct git_hash_algo hash_algos[GIT_HASH_NALGOS] = { NULL, }, { - "sha-1", + "sha1", /* "sha1", big-endian */ 0x73686131, GIT_SHA1_RAWSZ, From 2f90b9d9b4ba1f8e79318853301e8ef19ca02681 Mon Sep 17 00:00:00 2001 From: "brian m. carlson" Date: Mon, 22 Oct 2018 02:43:32 +0000 Subject: [PATCH 0004/1736] sha1-file: provide functions to look up hash algorithms There are several ways we might refer to a hash algorithm: by name, such as in the config file; by format ID, such as in a pack; or internally, by a pointer to the hash_algos array. Provide functions to look up hash algorithms based on these various forms and return the internal constant used for them. If conversion to another form is necessary, this internal constant can be used to look up the proper data in the hash_algos array. Signed-off-by: brian m. carlson Signed-off-by: Junio C Hamano --- hash.h | 13 +++++++++++++ sha1-file.c | 21 +++++++++++++++++++++ 2 files changed, 34 insertions(+) diff --git a/hash.h b/hash.h index 7c8238bc2ebfde..80881eea47b840 100644 --- a/hash.h +++ b/hash.h @@ -98,4 +98,17 @@ struct git_hash_algo { }; extern const struct git_hash_algo hash_algos[GIT_HASH_NALGOS]; +/* + * Return a GIT_HASH_* constant based on the name. Returns GIT_HASH_UNKNOWN if + * the name doesn't match a known algorithm. + */ +int hash_algo_by_name(const char *name); +/* Identical, except based on the format ID. */ +int hash_algo_by_id(uint32_t format_id); +/* Identical, except for a pointer to struct git_hash_algo. */ +static inline int hash_algo_by_ptr(const struct git_hash_algo *p) +{ + return p - hash_algos; +} + #endif diff --git a/sha1-file.c b/sha1-file.c index e29825f259cbf0..3a75d515eb981b 100644 --- a/sha1-file.c +++ b/sha1-file.c @@ -122,6 +122,27 @@ const char *empty_blob_oid_hex(void) return oid_to_hex_r(buf, the_hash_algo->empty_blob); } +int hash_algo_by_name(const char *name) +{ + int i; + if (!name) + return GIT_HASH_UNKNOWN; + for (i = 1; i < GIT_HASH_NALGOS; i++) + if (!strcmp(name, hash_algos[i].name)) + return i; + return GIT_HASH_UNKNOWN; +} + +int hash_algo_by_id(uint32_t format_id) +{ + int i; + for (i = 1; i < GIT_HASH_NALGOS; i++) + if (format_id == hash_algos[i].format_id) + return i; + return GIT_HASH_UNKNOWN; +} + + /* * This is meant to hold a *small* number of objects that you would * want read_sha1_file() to be able to return, but yet you do not want From bb05c9f7595274bcfabf775ea51b4765c503b055 Mon Sep 17 00:00:00 2001 From: Elijah Newren Date: Wed, 7 Nov 2018 20:40:22 -0800 Subject: [PATCH 0005/1736] t6042: add tests for consistency in file collision conflict handling Add testcases dealing with file collisions for the following types of conflicts: * add/add * rename/add * rename/rename(2to1) All these conflict types simplify down to two files "colliding" and should thus be handled similarly. This means that rename/add and rename/rename(2to1) conflicts need to be modified to behave the same as add/add conflicts currently do: the colliding files should be two-way merged (instead of the current behavior of writing the two colliding files out to separate temporary unique pathnames). Add testcases which check this; subsequent commits will fix the conflict handling to make these tests pass. Signed-off-by: Elijah Newren Signed-off-by: Junio C Hamano --- t/t6042-merge-rename-corner-cases.sh | 162 +++++++++++++++++++++++++++ 1 file changed, 162 insertions(+) diff --git a/t/t6042-merge-rename-corner-cases.sh b/t/t6042-merge-rename-corner-cases.sh index b97aca7fa263f7..b6fed2cb9a87fb 100755 --- a/t/t6042-merge-rename-corner-cases.sh +++ b/t/t6042-merge-rename-corner-cases.sh @@ -937,4 +937,166 @@ test_expect_failure 'mod6-check: chains of rename/rename(1to2) and rename/rename ) ' +test_conflicts_with_adds_and_renames() { + sideL=$1 + sideR=$2 + expect=$3 + + # Setup: + # L + # / \ + # master ? + # \ / + # R + # + # Where: + # Both L and R have files named 'three' which collide. Each of + # the colliding files could have been involved in a rename, in + # which case there was a file named 'one' or 'two' that was + # modified on the opposite side of history and renamed into the + # collision on this side of history. + # + # Questions: + # 1) The index should contain both a stage 2 and stage 3 entry + # for the colliding file. Does it? + # 2) When renames are involved, the content merges are clean, so + # the index should reflect the content merges, not merely the + # version of the colliding file from the prior commit. Does + # it? + # 3) There should be a file in the worktree named 'three' + # containing the two-way merged contents of the content-merged + # versions of 'three' from each of the two colliding + # files. Is it present? + # 4) There should not be any three~* files in the working + # tree + test_expect_success "setup simple $sideL/$sideR conflict" ' + test_create_repo simple_${sideL}_${sideR} && + ( + cd simple_${sideL}_${sideR} && + + # Create some related files now + for i in $(test_seq 1 10) + do + echo Random base content line $i + done >file_v1 && + cp file_v1 file_v2 && + echo modification >>file_v2 && + + cp file_v1 file_v3 && + echo more stuff >>file_v3 && + cp file_v3 file_v4 && + echo yet more stuff >>file_v4 && + + # Use a tag to record both these files for simple + # access, and clean out these untracked files + git tag file_v1 $(git hash-object -w file_v1) && + git tag file_v2 $(git hash-object -w file_v2) && + git tag file_v3 $(git hash-object -w file_v3) && + git tag file_v4 $(git hash-object -w file_v4) && + git clean -f && + + # Setup original commit (or merge-base), consisting of + # files named "one" and "two" if renames were involved. + touch irrelevant_file && + git add irrelevant_file && + if [ $sideL = "rename" ] + then + git show file_v1 >one && + git add one + fi && + if [ $sideR = "rename" ] + then + git show file_v3 >two && + git add two + fi && + test_tick && git commit -m initial && + + git branch L && + git branch R && + + # Handle the left side + git checkout L && + if [ $sideL = "rename" ] + then + git mv one three + else + git show file_v2 >three && + git add three + fi && + if [ $sideR = "rename" ] + then + git show file_v4 >two && + git add two + fi && + test_tick && git commit -m L && + + # Handle the right side + git checkout R && + if [ $sideL = "rename" ] + then + git show file_v2 >one && + git add one + fi && + if [ $sideR = "rename" ] + then + git mv two three + else + git show file_v4 >three && + git add three + fi && + test_tick && git commit -m R + ) + ' + + test_expect_$expect "check simple $sideL/$sideR conflict" ' + ( + cd simple_${sideL}_${sideR} && + + git checkout L^0 && + + # Merge must fail; there is a conflict + test_must_fail git merge -s recursive R^0 && + + # Make sure the index has the right number of entries + git ls-files -s >out && + test_line_count = 3 out && + git ls-files -u >out && + test_line_count = 2 out && + # Ensure we have the correct number of untracked files + git ls-files -o >out && + test_line_count = 1 out && + + # Nothing should have touched irrelevant_file + git rev-parse >actual \ + :0:irrelevant_file \ + :2:three \ + :3:three && + git rev-parse >expected \ + master:irrelevant_file \ + file_v2 \ + file_v4 && + test_cmp expected actual && + + # Make sure we have the correct merged contents for + # three + git show file_v1 >expected && + cat <<-\EOF >>expected && + <<<<<<< HEAD + modification + ======= + more stuff + yet more stuff + >>>>>>> R^0 + EOF + + test_cmp expected three + ) + ' +} + +test_conflicts_with_adds_and_renames rename rename failure +test_conflicts_with_adds_and_renames rename add failure +test_conflicts_with_adds_and_renames add rename failure +test_conflicts_with_adds_and_renames add add success + test_done From b28eeb3092fd12e24485fbadebfa57df457dd09a Mon Sep 17 00:00:00 2001 From: Elijah Newren Date: Wed, 7 Nov 2018 20:40:23 -0800 Subject: [PATCH 0006/1736] t6036, t6042: testcases for rename collision of already conflicting files When a single file is renamed, it can also be modified, yielding the possibility of that renamed file having content conflicts. If two different such files are renamed into the same location, then two-way merging those files may result in nested conflicts. Add a testcase that makes sure we get this case correct, and uses different lengths of conflict markers to differentiate between the different nestings. Also add another case with an extra (i.e. third) level of conflict markers due to using merge.conflictstyle=diff3 and the virtual merge base also having conflicts present. Signed-off-by: Elijah Newren Signed-off-by: Junio C Hamano --- t/t6036-recursive-corner-cases.sh | 194 +++++++++++++++++++++++++++ t/t6042-merge-rename-corner-cases.sh | 118 ++++++++++++++++ 2 files changed, 312 insertions(+) diff --git a/t/t6036-recursive-corner-cases.sh b/t/t6036-recursive-corner-cases.sh index e1cef58f2af96f..f229d7e47bbdcd 100755 --- a/t/t6036-recursive-corner-cases.sh +++ b/t/t6036-recursive-corner-cases.sh @@ -1402,4 +1402,198 @@ test_expect_failure 'check conflicting modes for regular file' ' ) ' +# Setup: +# L1---L2 +# / \ / \ +# master X ? +# \ / \ / +# R1---R2 +# +# Where: +# master has two files, named 'b' and 'a' +# branches L1 and R1 both modify each of the two files in conflicting ways +# +# L2 is a merge of R1 into L1; more on it later. +# R2 is a merge of L1 into R1; more on it later. +# +# X is an auto-generated merge-base used when merging L2 and R2. +# since X is a merge of L1 and R1, it has conflicting versions of each file +# +# More about L2 and R2: +# - both resolve the conflicts in 'b' and 'a' differently +# - L2 renames 'b' to 'm' +# - R2 renames 'a' to 'm' +# +# In the end, in file 'm' we have four different conflicting files (from +# two versions of 'b' and two of 'a'). In addition, if +# merge.conflictstyle is diff3, then the base version also has +# conflict markers of its own, leading to a total of three levels of +# conflict markers. This is a pretty weird corner case, but we just want +# to ensure that we handle it as well as practical. + +test_expect_success 'setup nested conflicts' ' + test_create_repo nested_conflicts && + ( + cd nested_conflicts && + + # Create some related files now + for i in $(test_seq 1 10) + do + echo Random base content line $i + done >initial && + + cp initial b_L1 && + cp initial b_R1 && + cp initial b_L2 && + cp initial b_R2 && + cp initial a_L1 && + cp initial a_R1 && + cp initial a_L2 && + cp initial a_R2 && + + test_write_lines b b_L1 >>b_L1 && + test_write_lines b b_R1 >>b_R1 && + test_write_lines b b_L2 >>b_L2 && + test_write_lines b b_R2 >>b_R2 && + test_write_lines a a_L1 >>a_L1 && + test_write_lines a a_R1 >>a_R1 && + test_write_lines a a_L2 >>a_L2 && + test_write_lines a a_R2 >>a_R2 && + + # Setup original commit (or merge-base), consisting of + # files named "b" and "a" + cp initial b && + cp initial a && + echo b >>b && + echo a >>a && + git add b a && + test_tick && git commit -m initial && + + git branch L && + git branch R && + + # Handle the left side + git checkout L && + mv -f b_L1 b && + mv -f a_L1 a && + git add b a && + test_tick && git commit -m "version L1 of files" && + git tag L1 && + + # Handle the right side + git checkout R && + mv -f b_R1 b && + mv -f a_R1 a && + git add b a && + test_tick && git commit -m "verson R1 of files" && + git tag R1 && + + # Create first merge on left side + git checkout L && + test_must_fail git merge R1 && + mv -f b_L2 b && + mv -f a_L2 a && + git add b a && + git mv b m && + test_tick && git commit -m "left merge, rename b->m" && + git tag L2 && + + # Create first merge on right side + git checkout R && + test_must_fail git merge L1 && + mv -f b_R2 b && + mv -f a_R2 a && + git add b a && + git mv a m && + test_tick && git commit -m "right merge, rename a->m" && + git tag R2 + ) +' + +test_expect_failure 'check nested conflicts' ' + ( + cd nested_conflicts && + + git clean -f && + git checkout L2^0 && + + # Merge must fail; there is a conflict + test_must_fail git -c merge.conflictstyle=diff3 merge -s recursive R2^0 && + + # Make sure the index has the right number of entries + git ls-files -s >out && + test_line_count = 2 out && + git ls-files -u >out && + test_line_count = 2 out && + # Ensure we have the correct number of untracked files + git ls-files -o >out && + test_line_count = 1 out && + + # Create a and b from virtual merge base X + git cat-file -p master:a >base && + git cat-file -p L1:a >ours && + git cat-file -p R1:a >theirs && + test_must_fail git merge-file --diff3 \ + -L "Temporary merge branch 1" \ + -L "merged common ancestors" \ + -L "Temporary merge branch 2" \ + ours \ + base \ + theirs && + sed -e "s/^\([<|=>]\)/\1\1/" ours >vmb_a && + + git cat-file -p master:b >base && + git cat-file -p L1:b >ours && + git cat-file -p R1:b >theirs && + test_must_fail git merge-file --diff3 \ + -L "Temporary merge branch 1" \ + -L "merged common ancestors" \ + -L "Temporary merge branch 2" \ + ours \ + base \ + theirs && + sed -e "s/^\([<|=>]\)/\1\1/" ours >vmb_b && + + # Compare :2:m to expected values + git cat-file -p L2:m >ours && + git cat-file -p R2:b >theirs && + test_must_fail git merge-file --diff3 \ + -L "HEAD:m" \ + -L "merged common ancestors:b" \ + -L "R2^0:b" \ + ours \ + vmb_b \ + theirs && + sed -e "s/^\([<|=>]\)/\1\1/" ours >m_stage_2 && + git cat-file -p :2:m >actual && + test_cmp m_stage_2 actual && + + # Compare :3:m to expected values + git cat-file -p L2:a >ours && + git cat-file -p R2:m >theirs && + test_must_fail git merge-file --diff3 \ + -L "HEAD:a" \ + -L "merged common ancestors:a" \ + -L "R2^0:m" \ + ours \ + vmb_a \ + theirs && + sed -e "s/^\([<|=>]\)/\1\1/" ours >m_stage_3 && + git cat-file -p :3:m >actual && + test_cmp m_stage_3 actual && + + # Compare m to expected contents + >empty && + cp -a m_stage_2 expected_final_m && + test_must_fail git merge-file --diff3 \ + -L "HEAD" \ + -L "merged common ancestors" \ + -L "R2^0" \ + expected_final_m \ + empty \ + m_stage_3 && + test_cmp expected_final_m m + ) +' + test_done diff --git a/t/t6042-merge-rename-corner-cases.sh b/t/t6042-merge-rename-corner-cases.sh index b6fed2cb9a87fb..23c3b6dffa220a 100755 --- a/t/t6042-merge-rename-corner-cases.sh +++ b/t/t6042-merge-rename-corner-cases.sh @@ -1099,4 +1099,122 @@ test_conflicts_with_adds_and_renames rename add failure test_conflicts_with_adds_and_renames add rename failure test_conflicts_with_adds_and_renames add add success +# Setup: +# L +# / \ +# master ? +# \ / +# R +# +# Where: +# master has two files, named 'one' and 'two'. +# branches L and R both modify 'one', in conflicting ways. +# branches L and R both modify 'two', in conflicting ways. +# branch L also renames 'one' to 'three'. +# branch R also renames 'two' to 'three'. +# +# So, we have four different conflicting files that all end up at path +# 'three'. +test_expect_success 'setup nested conflicts from rename/rename(2to1)' ' + test_create_repo nested_conflicts_from_rename_rename && + ( + cd nested_conflicts_from_rename_rename && + + # Create some related files now + for i in $(test_seq 1 10) + do + echo Random base content line $i + done >file_v1 && + + cp file_v1 file_v2 && + cp file_v1 file_v3 && + cp file_v1 file_v4 && + cp file_v1 file_v5 && + cp file_v1 file_v6 && + + echo one >>file_v1 && + echo uno >>file_v2 && + echo eins >>file_v3 && + + echo two >>file_v4 && + echo dos >>file_v5 && + echo zwei >>file_v6 && + + # Setup original commit (or merge-base), consisting of + # files named "one" and "two". + mv file_v1 one && + mv file_v4 two && + git add one two && + test_tick && git commit -m english && + + git branch L && + git branch R && + + # Handle the left side + git checkout L && + git mv one three && + mv -f file_v2 three && + mv -f file_v5 two && + git add two three && + test_tick && git commit -m spanish && + + # Handle the right side + git checkout R && + git mv two three && + mv -f file_v3 one && + mv -f file_v6 three && + git add one three && + test_tick && git commit -m german + ) +' + +test_expect_failure 'check nested conflicts from rename/rename(2to1)' ' + ( + cd nested_conflicts_from_rename_rename && + + git checkout L^0 && + + # Merge must fail; there is a conflict + test_must_fail git merge -s recursive R^0 && + + # Make sure the index has the right number of entries + git ls-files -s >out && + test_line_count = 2 out && + git ls-files -u >out && + test_line_count = 2 out && + # Ensure we have the correct number of untracked files + git ls-files -o >out && + test_line_count = 1 out && + + # Compare :2:three to expected values + git cat-file -p master:one >base && + git cat-file -p L:three >ours && + git cat-file -p R:one >theirs && + test_must_fail git merge-file \ + -L "HEAD:three" -L "" -L "R^0:one" \ + ours base theirs && + sed -e "s/^\([<=>]\)/\1\1/" ours >L-three && + git cat-file -p :2:three >expect && + test_cmp expect L-three && + + # Compare :2:three to expected values + git cat-file -p master:two >base && + git cat-file -p L:two >ours && + git cat-file -p R:three >theirs && + test_must_fail git merge-file \ + -L "HEAD:two" -L "" -L "R^0:three" \ + ours base theirs && + sed -e "s/^\([<=>]\)/\1\1/" ours >R-three && + git cat-file -p :3:three >expect && + test_cmp expect R-three && + + # Compare three to expected contents + >empty && + test_must_fail git merge-file \ + -L "HEAD" -L "" -L "R^0" \ + L-three empty R-three && + test_cmp three L-three + ) +' + test_done From b2a7942b8bcf397d9a5d3bb6671853755e14de60 Mon Sep 17 00:00:00 2001 From: Elijah Newren Date: Wed, 7 Nov 2018 20:40:24 -0800 Subject: [PATCH 0007/1736] merge-recursive: increase marker length with depth of recursion Later patches in this series will modify file collision conflict handling (e.g. from rename/add and rename/rename(2to1) conflicts) so that multiply nested conflict markers can arise even before considering conflicts in the virtual merge base. Including the virtual merge base will provide a way to get triply (or higher) nested conflict markers. This new way to get nested conflict markers will force the need for a more general mechanism to extend the length of conflict markers in order to differentiate between different nestings. Along with this change to conflict marker length handling, we want to make sure that we don't regress handling for other types of conflicts with nested conflict markers. Add a more involved testcase using merge.conflictstyle=diff3, where not only does the virtual merge base contain conflicts, but its virtual merge base does as well (i.e. a case with triply nested conflict markers). While there are multiple reasonable ways to handle nested conflict markers in the virtual merge base for this type of situation, the easiest approach that dovetails well with the new needs for the file collision conflict handling is to require that the length of the conflict markers increase with each subsequent nesting. Subsequent patches which change the rename/add and rename/rename(2to1) conflict handling will modify the extra_marker_size flag appropriately for their new needs. Signed-off-by: Elijah Newren Signed-off-by: Junio C Hamano --- ll-merge.c | 4 +- ll-merge.h | 1 + merge-recursive.c | 25 +++-- t/t6036-recursive-corner-cases.sh | 151 ++++++++++++++++++++++++++++++ 4 files changed, 172 insertions(+), 9 deletions(-) diff --git a/ll-merge.c b/ll-merge.c index 0e2800f7bb46bd..aabc1b5c2e29e2 100644 --- a/ll-merge.c +++ b/ll-merge.c @@ -384,7 +384,9 @@ int ll_merge(mmbuffer_t *result_buf, if (opts->virtual_ancestor) { if (driver->recursive) driver = find_ll_merge_driver(driver->recursive); - marker_size += 2; + } + if (opts->extra_marker_size) { + marker_size += opts->extra_marker_size; } return driver->fn(driver, result_buf, path, ancestor, ancestor_label, ours, our_label, theirs, their_label, diff --git a/ll-merge.h b/ll-merge.h index b72b19921e8f33..5b4e1585024592 100644 --- a/ll-merge.h +++ b/ll-merge.h @@ -11,6 +11,7 @@ struct ll_merge_options { unsigned virtual_ancestor : 1; unsigned variant : 2; /* favor ours, favor theirs, or union merge */ unsigned renormalize : 1; + unsigned extra_marker_size; long xdl_opts; }; diff --git a/merge-recursive.c b/merge-recursive.c index 73b57103865109..f795c92a69ac89 100644 --- a/merge-recursive.c +++ b/merge-recursive.c @@ -1058,7 +1058,8 @@ static int merge_3way(struct merge_options *o, const struct diff_filespec *a, const struct diff_filespec *b, const char *branch1, - const char *branch2) + const char *branch2, + const int extra_marker_size) { mmfile_t orig, src1, src2; struct ll_merge_options ll_opts = {0}; @@ -1066,6 +1067,7 @@ static int merge_3way(struct merge_options *o, int merge_status; ll_opts.renormalize = o->renormalize; + ll_opts.extra_marker_size = extra_marker_size; ll_opts.xdl_opts = o->xdl_opts; if (o->call_depth) { @@ -1300,6 +1302,7 @@ static int merge_mode_and_contents(struct merge_options *o, const char *filename, const char *branch1, const char *branch2, + const int extra_marker_size, struct merge_file_info *result) { if (o->branch1 != branch1) { @@ -1310,7 +1313,8 @@ static int merge_mode_and_contents(struct merge_options *o, */ return merge_mode_and_contents(o, one, b, a, filename, - branch2, branch1, result); + branch2, branch1, + extra_marker_size, result); } result->merge = 0; @@ -1351,7 +1355,8 @@ static int merge_mode_and_contents(struct merge_options *o, int ret = 0, merge_status; merge_status = merge_3way(o, &result_buf, one, a, b, - branch1, branch2); + branch1, branch2, + extra_marker_size); if ((merge_status < 0) || !result_buf.ptr) ret = err(o, _("Failed to execute internal merge")); @@ -1640,7 +1645,8 @@ static int handle_rename_rename_1to2(struct merge_options *o, struct diff_filespec other; struct diff_filespec *add; if (merge_mode_and_contents(o, one, a, b, one->path, - ci->branch1, ci->branch2, &mfi)) + ci->branch1, ci->branch2, + o->call_depth * 2, &mfi)) return -1; /* @@ -1707,9 +1713,11 @@ static int handle_rename_rename_2to1(struct merge_options *o, path_side_1_desc = xstrfmt("version of %s from %s", path, a->path); path_side_2_desc = xstrfmt("version of %s from %s", path, b->path); if (merge_mode_and_contents(o, a, c1, &ci->ren1_other, path_side_1_desc, - o->branch1, o->branch2, &mfi_c1) || + o->branch1, o->branch2, + o->call_depth * 2, &mfi_c1) || merge_mode_and_contents(o, b, &ci->ren2_other, c2, path_side_2_desc, - o->branch1, o->branch2, &mfi_c2)) + o->branch1, o->branch2, + o->call_depth * 2, &mfi_c2)) return -1; free(path_side_1_desc); free(path_side_2_desc); @@ -2755,7 +2763,7 @@ static int process_renames(struct merge_options *o, if (merge_mode_and_contents(o, &one, &a, &b, ren1_dst, branch1, branch2, - &mfi)) { + o->call_depth * 2, &mfi)) { clean_merge = -1; goto cleanup_and_return; } @@ -3052,7 +3060,8 @@ static int handle_content_merge(struct merge_options *o, df_conflict_remains = 1; } if (merge_mode_and_contents(o, &one, &a, &b, path, - o->branch1, o->branch2, &mfi)) + o->branch1, o->branch2, + o->call_depth * 2, &mfi)) return -1; /* diff --git a/t/t6036-recursive-corner-cases.sh b/t/t6036-recursive-corner-cases.sh index f229d7e47bbdcd..3091cbc06a332e 100755 --- a/t/t6036-recursive-corner-cases.sh +++ b/t/t6036-recursive-corner-cases.sh @@ -1596,4 +1596,155 @@ test_expect_failure 'check nested conflicts' ' ) ' +# Setup: +# L1---L2---L3 +# / \ / \ / \ +# master X1 X2 ? +# \ / \ / \ / +# R1---R2---R3 +# +# Where: +# master has one file named 'content' +# branches L1 and R1 both modify each of the two files in conflicting ways +# +# L (n>1) is a merge of R into L +# R (n>1) is a merge of L into R +# L and R resolve the conflicts differently. +# +# X is an auto-generated merge-base used when merging L and R. +# By construction, X1 has conflict markers due to conflicting versions. +# X2, due to using merge.conflictstyle=3, has nested conflict markers. +# +# So, merging R3 into L3 using merge.conflictstyle=3 should show the +# nested conflict markers from X2 in the base version -- that means we +# have three levels of conflict markers. Can we distinguish all three? + +test_expect_success 'setup virtual merge base with nested conflicts' ' + test_create_repo virtual_merge_base_has_nested_conflicts && + ( + cd virtual_merge_base_has_nested_conflicts && + + # Create some related files now + for i in $(test_seq 1 10) + do + echo Random base content line $i + done >content && + + # Setup original commit + git add content && + test_tick && git commit -m initial && + + git branch L && + git branch R && + + # Create L1 + git checkout L && + echo left >>content && + git add content && + test_tick && git commit -m "version L1 of content" && + git tag L1 && + + # Create R1 + git checkout R && + echo right >>content && + git add content && + test_tick && git commit -m "verson R1 of content" && + git tag R1 && + + # Create L2 + git checkout L && + test_must_fail git -c merge.conflictstyle=diff3 merge R1 && + git checkout L1 content && + test_tick && git commit -m "version L2 of content" && + git tag L2 && + + # Create R2 + git checkout R && + test_must_fail git -c merge.conflictstyle=diff3 merge L1 && + git checkout R1 content && + test_tick && git commit -m "version R2 of content" && + git tag R2 && + + # Create L3 + git checkout L && + test_must_fail git -c merge.conflictstyle=diff3 merge R2 && + git checkout L1 content && + test_tick && git commit -m "version L3 of content" && + git tag L3 && + + # Create R3 + git checkout R && + test_must_fail git -c merge.conflictstyle=diff3 merge L2 && + git checkout R1 content && + test_tick && git commit -m "version R3 of content" && + git tag R3 + ) +' + +test_expect_success 'check virtual merge base with nested conflicts' ' + ( + cd virtual_merge_base_has_nested_conflicts && + + git checkout L3^0 && + + # Merge must fail; there is a conflict + test_must_fail git -c merge.conflictstyle=diff3 merge -s recursive R3^0 && + + # Make sure the index has the right number of entries + git ls-files -s >out && + test_line_count = 3 out && + git ls-files -u >out && + test_line_count = 3 out && + # Ensure we have the correct number of untracked files + git ls-files -o >out && + test_line_count = 1 out && + + # Compare :[23]:content to expected values + git rev-parse L1:content R1:content >expect && + git rev-parse :2:content :3:content >actual && + test_cmp expect actual && + + # Imitate X1 merge base, except without long enough conflict + # markers because a subsequent sed will modify them. Put + # result into vmb. + git cat-file -p master:content >base && + git cat-file -p L:content >left && + git cat-file -p R:content >right && + cp left merged-once && + test_must_fail git merge-file --diff3 \ + -L "Temporary merge branch 1" \ + -L "merged common ancestors" \ + -L "Temporary merge branch 2" \ + merged-once \ + base \ + right && + sed -e "s/^\([<|=>]\)/\1\1\1/" merged-once >vmb && + + # Imitate X2 merge base, overwriting vmb. Note that we + # extend both sets of conflict markers to make them longer + # with the sed command. + cp left merged-twice && + test_must_fail git merge-file --diff3 \ + -L "Temporary merge branch 1" \ + -L "merged common ancestors" \ + -L "Temporary merge branch 2" \ + merged-twice \ + vmb \ + right && + sed -e "s/^\([<|=>]\)/\1\1\1/" merged-twice >vmb && + + # Compare :1:content to expected value + git cat-file -p :1:content >actual && + test_cmp vmb actual && + + # Determine expected content in final outer merge, compare to + # what the merge generated. + cp -f left expect && + test_must_fail git merge-file --diff3 \ + -L "HEAD" -L "merged common ancestors" -L "R3^0" \ + expect vmb right && + test_cmp expect content + ) +' + test_done From 37b65ce36bc8f79b5e8cf6f3addf586bd72f3352 Mon Sep 17 00:00:00 2001 From: Elijah Newren Date: Wed, 7 Nov 2018 20:40:25 -0800 Subject: [PATCH 0008/1736] merge-recursive: new function for better colliding conflict resolutions There are three conflict types that represent two (possibly entirely unrelated) files colliding at the same location: * add/add * rename/add * rename/rename(2to1) These three conflict types already share more similarity than might be immediately apparent from their description: (1) the handling of the rename variants already involves removing any entries from the index corresponding to the original file names[*], thus only leaving entries in the index for the colliding path; (2) likewise, any trace of the original file name in the working tree is also removed. So, in all three cases we're left with how to represent two colliding files in both the index and the working copy. [*] Technically, this isn't quite true because rename/rename(2to1) conflicts in the recursive (o->call_depth > 0) case do an "unrename" since about seven years ago. But even in that case, Junio felt compelled to explain that my decision to "unrename" wasn't necessarily the only or right answer -- search for "Comment from Junio" in t6036 for details. My initial motivation for looking at these three conflict types was that if the handling of these three conflict types is the same, at least in the limited set of cases where a renamed file is unmodified on the side of history where the file is not renamed, then a significant performance improvement for rename detection during merges is possible. However, while that served as motivation to look at these three types of conflicts, the actual goal of this new function is to try to improve the handling for all three cases, not to merely make them the same as each other in that special circumstance. === Handling the working tree === The previous behavior for these conflict types in regards to the working tree (assuming the file collision occurs at 'foo') was: * add/add does a two-way merge of the two files and records it as 'foo'. * rename/rename(2to1) records the two different files into two new uniquely named files (foo~HEAD and foo~$MERGE), while removing 'foo' from the working tree. * rename/add records the two different files into two different locations, recording the add at foo~$SIDE and, oddly, recording the rename at foo (why is the rename more important than the add?) So, the question for what to write to the working tree boils down to whether the two colliding files should be two-way merged and recorded in place, or recorded into separate files. As per discussion on the git mailing lit, two-way merging was deemed to always be preferred, as that makes these cases all more like content conflicts that users can handle from within their favorite editor, IDE, or merge tool. Note that since renames already involve a content merge, rename/add and rename/rename(2to1) conflicts could result in nested conflict markers. === Handling of the index === For a typical rename, unpack_trees() would set up the index in the following fashion: old_path new_path stage1: 5ca1ab1e 00000000 stage2: f005ba11 00000000 stage3: 00000000 b0a710ad And merge-recursive would rewrite this to new_path stage1: 5ca1ab1e stage2: f005ba11 stage3: b0a710ad Removing old_path from the index means the user won't have to `git rm old_path` manually every time a renamed path has a content conflict. It also means they can use `git checkout [--ours|--theirs|--conflict|-m] new_path`, `git diff [--ours|--theirs]` and various other commands that would be difficult otherwise. This strategy becomes a problem when we have a rename/add or rename/rename(2to1) conflict, however, because then we have only three slots to store blob sha1s and we need either four or six. Previously, this was handled by continuing to delete old_path from the index, and just outright ignoring any blob shas from old_path. That had the downside of deleting any trace of changes made to old_path on the other side of history. This function instead does a three-way content merge of the renamed file, and stores the blob sha1 for that at either stage2 or stage3 for new_path (depending on which side the rename came from). That has the advantage of bringing information about changes on both sides and still allows for easy resolution (no need to git rm old_path, etc.), but does have the downside that if the content merge had conflict markers, then what we store in the index is the sha1 of a blob with conflict markers. While that is a downside, it seems less problematic than the downsides of any obvious alternatives, and certainly makes more sense than the previous handling. Further, it has a precedent in that when we do recursive merges, we may accept a file with conflict markers as the resolution for the merge of the merge-bases, which will then show up in the index of the outer merge at stage 1 if a conflict exists at the outer level. Signed-off-by: Elijah Newren Signed-off-by: Junio C Hamano --- merge-recursive.c | 121 ++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 121 insertions(+) diff --git a/merge-recursive.c b/merge-recursive.c index f795c92a69ac89..24d979022e8e2d 100644 --- a/merge-recursive.c +++ b/merge-recursive.c @@ -1559,6 +1559,127 @@ static struct diff_filespec *filespec_from_entry(struct diff_filespec *target, return target; } +#if 0 // #if-0-ing avoids unused function warning; will make live in next commit +static int handle_file_collision(struct merge_options *o, + const char *collide_path, + const char *prev_path1, + const char *prev_path2, + const char *branch1, const char *branch2, + const struct object_id *a_oid, + unsigned int a_mode, + const struct object_id *b_oid, + unsigned int b_mode) +{ + struct merge_file_info mfi; + struct diff_filespec null, a, b; + char *alt_path = NULL; + const char *update_path = collide_path; + + /* + * In the recursive case, we just opt to undo renames + */ + if (o->call_depth && (prev_path1 || prev_path2)) { + /* Put first file (a_oid, a_mode) in its original spot */ + if (prev_path1) { + if (update_file(o, 1, a_oid, a_mode, prev_path1)) + return -1; + } else { + if (update_file(o, 1, a_oid, a_mode, collide_path)) + return -1; + } + + /* Put second file (b_oid, b_mode) in its original spot */ + if (prev_path2) { + if (update_file(o, 1, b_oid, b_mode, prev_path2)) + return -1; + } else { + if (update_file(o, 1, b_oid, b_mode, collide_path)) + return -1; + } + + /* Don't leave something at collision path if unrenaming both */ + if (prev_path1 && prev_path2) + remove_file(o, 1, collide_path, 0); + + return 0; + } + + /* Remove rename sources if rename/add or rename/rename(2to1) */ + if (prev_path1) + remove_file(o, 1, prev_path1, + o->call_depth || would_lose_untracked(prev_path1)); + if (prev_path2) + remove_file(o, 1, prev_path2, + o->call_depth || would_lose_untracked(prev_path2)); + + /* + * Remove the collision path, if it wouldn't cause dirty contents + * or an untracked file to get lost. We'll either overwrite with + * merged contents, or just write out to differently named files. + */ + if (was_dirty(o, collide_path)) { + output(o, 1, _("Refusing to lose dirty file at %s"), + collide_path); + update_path = alt_path = unique_path(o, collide_path, "merged"); + } else if (would_lose_untracked(collide_path)) { + /* + * Only way we get here is if both renames were from + * a directory rename AND user had an untracked file + * at the location where both files end up after the + * two directory renames. See testcase 10d of t6043. + */ + output(o, 1, _("Refusing to lose untracked file at " + "%s, even though it's in the way."), + collide_path); + update_path = alt_path = unique_path(o, collide_path, "merged"); + } else { + /* + * FIXME: It's possible that the two files are identical + * and that the current working copy happens to match, in + * which case we are unnecessarily touching the working + * tree file. It's not a likely enough scenario that I + * want to code up the checks for it and a better fix is + * available if we restructure how unpack_trees() and + * merge-recursive interoperate anyway, so punting for + * now... + */ + remove_file(o, 0, collide_path, 0); + } + + /* Store things in diff_filespecs for functions that need it */ + memset(&a, 0, sizeof(struct diff_filespec)); + memset(&b, 0, sizeof(struct diff_filespec)); + null.path = a.path = b.path = (char *)collide_path; + oidcpy(&null.oid, &null_oid); + null.mode = 0; + oidcpy(&a.oid, a_oid); + a.mode = a_mode; + a.oid_valid = 1; + oidcpy(&b.oid, b_oid); + b.mode = b_mode; + b.oid_valid = 1; + + if (merge_mode_and_contents(o, &null, &a, &b, collide_path, + branch1, branch2, o->call_depth * 2, &mfi)) + return -1; + mfi.clean &= !alt_path; + if (update_file(o, mfi.clean, &mfi.oid, mfi.mode, update_path)) + return -1; + if (!mfi.clean && !o->call_depth && + update_stages(o, collide_path, NULL, &a, &b)) + return -1; + free(alt_path); + /* + * FIXME: If both a & b both started with conflicts (only possible + * if they came from a rename/rename(2to1)), but had IDENTICAL + * contents including those conflicts, then in the next line we claim + * it was clean. If someone cares about this case, we should have the + * caller notify us if we started with conflicts. + */ + return mfi.clean; +} +#endif + static int handle_file(struct merge_options *o, struct diff_filespec *rename, int stage, From 7f8671656f31f5d1cc9d9748b7dc62b76a428cc5 Mon Sep 17 00:00:00 2001 From: Elijah Newren Date: Wed, 7 Nov 2018 20:40:26 -0800 Subject: [PATCH 0009/1736] merge-recursive: fix rename/add conflict handling This makes the rename/add conflict handling make use of the new handle_file_collision() function, which fixes several bugs and improves things for the rename/add case significantly. Previously, rename/add would: * Not leave any higher order stage entries in the index, making it appear as if there were no conflict. * Would place the rename file at the colliding path, and move the added file elsewhere, which combined with the lack of higher order stage entries felt really odd. It's not clear to me why the rename should take precedence over the add; if one should be moved out of the way, they both probably should. * In the recursive case, it would do a two way merge of the added file and the version of the renamed file on the renamed side, completely excluding modifications to the renamed file on the unrenamed side of history. Use the new handle_file_collision() to fix all of these issues. Signed-off-by: Elijah Newren Signed-off-by: Junio C Hamano --- merge-recursive.c | 137 +++++++++++++++++---------- t/t6036-recursive-corner-cases.sh | 24 ++--- t/t6042-merge-rename-corner-cases.sh | 4 +- 3 files changed, 101 insertions(+), 64 deletions(-) diff --git a/merge-recursive.c b/merge-recursive.c index 24d979022e8e2d..08050951681df3 100644 --- a/merge-recursive.c +++ b/merge-recursive.c @@ -186,6 +186,7 @@ static int oid_eq(const struct object_id *a, const struct object_id *b) enum rename_type { RENAME_NORMAL = 0, RENAME_VIA_DIR, + RENAME_ADD, RENAME_DELETE, RENAME_ONE_FILE_TO_ONE, RENAME_ONE_FILE_TO_TWO, @@ -228,6 +229,7 @@ static inline void setup_rename_conflict_info(enum rename_type rename_type, struct stage_data *src_entry1, struct stage_data *src_entry2) { + int ostage1 = 0, ostage2; struct rename_conflict_info *ci; /* @@ -264,18 +266,22 @@ static inline void setup_rename_conflict_info(enum rename_type rename_type, dst_entry2->rename_conflict_info = ci; } - if (rename_type == RENAME_TWO_FILES_TO_ONE) { - /* - * For each rename, there could have been - * modifications on the side of history where that - * file was not renamed. - */ - int ostage1 = o->branch1 == branch1 ? 3 : 2; - int ostage2 = ostage1 ^ 1; + /* + * For each rename, there could have been + * modifications on the side of history where that + * file was not renamed. + */ + if (rename_type == RENAME_ADD || + rename_type == RENAME_TWO_FILES_TO_ONE) { + ostage1 = o->branch1 == branch1 ? 3 : 2; ci->ren1_other.path = pair1->one->path; oidcpy(&ci->ren1_other.oid, &src_entry1->stages[ostage1].oid); ci->ren1_other.mode = src_entry1->stages[ostage1].mode; + } + + if (rename_type == RENAME_TWO_FILES_TO_ONE) { + ostage2 = ostage1 ^ 1; ci->ren2_other.path = pair2->one->path; oidcpy(&ci->ren2_other.oid, &src_entry2->stages[ostage2].oid); @@ -1559,7 +1565,6 @@ static struct diff_filespec *filespec_from_entry(struct diff_filespec *target, return target; } -#if 0 // #if-0-ing avoids unused function warning; will make live in next commit static int handle_file_collision(struct merge_options *o, const char *collide_path, const char *prev_path1, @@ -1575,6 +1580,20 @@ static int handle_file_collision(struct merge_options *o, char *alt_path = NULL; const char *update_path = collide_path; + /* + * It's easiest to get the correct things into stage 2 and 3, and + * to make sure that the content merge puts HEAD before the other + * branch if we just ensure that branch1 == o->branch1. So, simply + * flip arguments around if we don't have that. + */ + if (branch1 != o->branch1) { + return handle_file_collision(o, collide_path, + prev_path2, prev_path1, + branch2, branch1, + b_oid, b_mode, + a_oid, a_mode); + } + /* * In the recursive case, we just opt to undo renames */ @@ -1678,7 +1697,38 @@ static int handle_file_collision(struct merge_options *o, */ return mfi.clean; } -#endif + +static int handle_rename_add(struct merge_options *o, + struct rename_conflict_info *ci) +{ + /* a was renamed to c, and a separate c was added. */ + struct diff_filespec *a = ci->pair1->one; + struct diff_filespec *c = ci->pair1->two; + char *path = c->path; + char *prev_path_desc; + struct merge_file_info mfi; + + int other_stage = (ci->branch1 == o->branch1 ? 3 : 2); + + output(o, 1, _("CONFLICT (rename/add): " + "Rename %s->%s in %s. Added %s in %s"), + a->path, c->path, ci->branch1, + c->path, ci->branch2); + + prev_path_desc = xstrfmt("version of %s from %s", path, a->path); + if (merge_mode_and_contents(o, a, c, &ci->ren1_other, prev_path_desc, + o->branch1, o->branch2, + 1 + o->call_depth * 2, &mfi)) + return -1; + free(prev_path_desc); + + return handle_file_collision(o, + c->path, a->path, NULL, + ci->branch1, ci->branch2, + &mfi.oid, mfi.mode, + &ci->dst_entry1->stages[other_stage].oid, + ci->dst_entry1->stages[other_stage].mode); +} static int handle_file(struct merge_options *o, struct diff_filespec *rename, @@ -2860,47 +2910,23 @@ static int process_renames(struct merge_options *o, 0 /* update_wd */)) clean_merge = -1; } else if (!oid_eq(&dst_other.oid, &null_oid)) { - clean_merge = 0; - try_merge = 1; - output(o, 1, _("CONFLICT (rename/add): Rename %s->%s in %s. " - "%s added in %s"), - ren1_src, ren1_dst, branch1, - ren1_dst, branch2); - if (o->call_depth) { - struct merge_file_info mfi; - struct diff_filespec one, a, b; - - oidcpy(&one.oid, &null_oid); - one.mode = 0; - one.path = ren1->pair->two->path; - - oidcpy(&a.oid, &ren1->pair->two->oid); - a.mode = ren1->pair->two->mode; - a.path = one.path; - - oidcpy(&b.oid, &dst_other.oid); - b.mode = dst_other.mode; - b.path = one.path; - - if (merge_mode_and_contents(o, &one, &a, &b, ren1_dst, - branch1, branch2, - o->call_depth * 2, &mfi)) { - clean_merge = -1; - goto cleanup_and_return; - } - output(o, 1, _("Adding merged %s"), ren1_dst); - if (update_file(o, 0, &mfi.oid, - mfi.mode, ren1_dst)) - clean_merge = -1; - try_merge = 0; - } else { - char *new_path = unique_path(o, ren1_dst, branch2); - output(o, 1, _("Adding as %s instead"), new_path); - if (update_file(o, 0, &dst_other.oid, - dst_other.mode, new_path)) - clean_merge = -1; - free(new_path); - } + /* + * Probably not a clean merge, but it's + * premature to set clean_merge to 0 here, + * because if the rename merges cleanly and + * the merge exactly matches the newly added + * file, then the merge will be clean. + */ + setup_rename_conflict_info(RENAME_ADD, + ren1->pair, + NULL, + branch1, + branch2, + ren1->dst_entry, + NULL, + o, + ren1->src_entry, + NULL); } else try_merge = 1; @@ -3312,6 +3338,15 @@ static int process_entry(struct merge_options *o, conflict_info->branch2)) clean_merge = -1; break; + case RENAME_ADD: + /* + * Probably unclean merge, but if the renamed file + * merges cleanly and the result can then be + * two-way merged cleanly with the added file, I + * guess it's a clean merge? + */ + clean_merge = handle_rename_add(o, conflict_info); + break; case RENAME_DELETE: clean_merge = 0; if (handle_rename_delete(o, diff --git a/t/t6036-recursive-corner-cases.sh b/t/t6036-recursive-corner-cases.sh index 3091cbc06a332e..d6b7e568f5b5dc 100755 --- a/t/t6036-recursive-corner-cases.sh +++ b/t/t6036-recursive-corner-cases.sh @@ -185,7 +185,7 @@ test_expect_success 'setup differently handled merges of rename/add conflict' ' git branch B && git checkout -b C && echo 10 >>a && - echo "other content" >>new_a && + test_write_lines 0 1 2 3 4 5 6 7 foobar >new_a && git add a new_a && test_tick && git commit -m C && @@ -195,14 +195,14 @@ test_expect_success 'setup differently handled merges of rename/add conflict' ' git checkout B^0 && test_must_fail git merge C && - git clean -f && + git show :2:new_a >new_a && + git add new_a && test_tick && git commit -m D && git tag D && git checkout C^0 && test_must_fail git merge B && - rm new_a~HEAD new_a && - printf "Incorrectly merged content" >>new_a && + test_write_lines 0 1 2 3 4 5 6 7 bad_merge >new_a && git add -u && test_tick && git commit -m E && git tag E @@ -225,21 +225,23 @@ test_expect_success 'git detects differently handled merges conflict' ' test_line_count = 1 out && git rev-parse >expect \ - D:new_a E:new_a && + C:new_a D:new_a E:new_a && git rev-parse >actual \ - :2:new_a :3:new_a && + :1:new_a :2:new_a :3:new_a && test_cmp expect actual && - git cat-file -p C:new_a >ours && - git cat-file -p B:new_a >theirs && + # Test that the two-way merge in new_a is as expected + git cat-file -p D:new_a >ours && + git cat-file -p E:new_a >theirs && >empty && test_must_fail git merge-file \ - -L "Temporary merge branch 1" \ + -L "HEAD" \ -L "" \ - -L "Temporary merge branch 2" \ + -L "E^0" \ ours empty theirs && sed -e "s/^\([<=>]\)/\1\1\1/" ours >expect && - git cat-file -p :1:new_a >actual && + git hash-object new_a >actual && + git hash-object ours >expect && test_cmp expect actual ) ' diff --git a/t/t6042-merge-rename-corner-cases.sh b/t/t6042-merge-rename-corner-cases.sh index 23c3b6dffa220a..3e934ae89ef9e5 100755 --- a/t/t6042-merge-rename-corner-cases.sh +++ b/t/t6042-merge-rename-corner-cases.sh @@ -1095,8 +1095,8 @@ test_conflicts_with_adds_and_renames() { } test_conflicts_with_adds_and_renames rename rename failure -test_conflicts_with_adds_and_renames rename add failure -test_conflicts_with_adds_and_renames add rename failure +test_conflicts_with_adds_and_renames rename add success +test_conflicts_with_adds_and_renames add rename success test_conflicts_with_adds_and_renames add add success # Setup: From bbafc9c44ae7109f636772350934b3c5d747da44 Mon Sep 17 00:00:00 2001 From: Elijah Newren Date: Wed, 7 Nov 2018 20:40:27 -0800 Subject: [PATCH 0010/1736] merge-recursive: improve handling for rename/rename(2to1) conflicts This makes the rename/rename(2to1) conflicts use the new handle_file_collision() function. Since that function was based originally on the rename/rename(2to1) handling code, the main differences here are in what was added. In particular: * Instead of storing files at collide_path~HEAD and collide_path~MERGE, the files are two-way merged and recorded at collide_path. * Instead of recording the version of the renamed file that existed on the renamed side in the index (thus ignoring any changes that were made to the file on the side of history without the rename), we do a three-way content merge on the renamed path, then store that at either stage 2 or stage 3. * Note that since the content merge for each rename may have conflicts, and then we have to merge the two renamed files, we can end up with nested conflict markers. Signed-off-by: Elijah Newren Signed-off-by: Junio C Hamano --- merge-recursive.c | 104 ++++----------------------- t/t6036-recursive-corner-cases.sh | 12 +--- t/t6042-merge-rename-corner-cases.sh | 38 ++++++---- t/t6043-merge-rename-directories.sh | 83 ++++++++++++--------- 4 files changed, 89 insertions(+), 148 deletions(-) diff --git a/merge-recursive.c b/merge-recursive.c index 08050951681df3..ead6054a75b6bd 100644 --- a/merge-recursive.c +++ b/merge-recursive.c @@ -696,27 +696,6 @@ static int update_stages(struct merge_options *opt, const char *path, return 0; } -static int update_stages_for_stage_data(struct merge_options *opt, - const char *path, - const struct stage_data *stage_data) -{ - struct diff_filespec o, a, b; - - o.mode = stage_data->stages[1].mode; - oidcpy(&o.oid, &stage_data->stages[1].oid); - - a.mode = stage_data->stages[2].mode; - oidcpy(&a.oid, &stage_data->stages[2].oid); - - b.mode = stage_data->stages[3].mode; - oidcpy(&b.oid, &stage_data->stages[3].oid); - - return update_stages(opt, path, - is_null_oid(&o.oid) ? NULL : &o, - is_null_oid(&a.oid) ? NULL : &a, - is_null_oid(&b.oid) ? NULL : &b); -} - static void update_entry(struct stage_data *entry, struct diff_filespec *o, struct diff_filespec *a, @@ -1870,7 +1849,6 @@ static int handle_rename_rename_2to1(struct merge_options *o, char *path_side_2_desc; struct merge_file_info mfi_c1; struct merge_file_info mfi_c2; - int ret; output(o, 1, _("CONFLICT (rename/rename): " "Rename %s->%s in %s. " @@ -1878,81 +1856,22 @@ static int handle_rename_rename_2to1(struct merge_options *o, a->path, c1->path, ci->branch1, b->path, c2->path, ci->branch2); - remove_file(o, 1, a->path, o->call_depth || would_lose_untracked(a->path)); - remove_file(o, 1, b->path, o->call_depth || would_lose_untracked(b->path)); - path_side_1_desc = xstrfmt("version of %s from %s", path, a->path); path_side_2_desc = xstrfmt("version of %s from %s", path, b->path); if (merge_mode_and_contents(o, a, c1, &ci->ren1_other, path_side_1_desc, o->branch1, o->branch2, - o->call_depth * 2, &mfi_c1) || + 1 + o->call_depth * 2, &mfi_c1) || merge_mode_and_contents(o, b, &ci->ren2_other, c2, path_side_2_desc, o->branch1, o->branch2, - o->call_depth * 2, &mfi_c2)) + 1 + o->call_depth * 2, &mfi_c2)) return -1; free(path_side_1_desc); free(path_side_2_desc); - if (o->call_depth) { - /* - * If mfi_c1.clean && mfi_c2.clean, then it might make - * sense to do a two-way merge of those results. But, I - * think in all cases, it makes sense to have the virtual - * merge base just undo the renames; they can be detected - * again later for the non-recursive merge. - */ - remove_file(o, 0, path, 0); - ret = update_file(o, 0, &mfi_c1.oid, mfi_c1.mode, a->path); - if (!ret) - ret = update_file(o, 0, &mfi_c2.oid, mfi_c2.mode, - b->path); - } else { - char *new_path1 = unique_path(o, path, ci->branch1); - char *new_path2 = unique_path(o, path, ci->branch2); - output(o, 1, _("Renaming %s to %s and %s to %s instead"), - a->path, new_path1, b->path, new_path2); - if (was_dirty(o, path)) - output(o, 1, _("Refusing to lose dirty file at %s"), - path); - else if (would_lose_untracked(path)) - /* - * Only way we get here is if both renames were from - * a directory rename AND user had an untracked file - * at the location where both files end up after the - * two directory renames. See testcase 10d of t6043. - */ - output(o, 1, _("Refusing to lose untracked file at " - "%s, even though it's in the way."), - path); - else - remove_file(o, 0, path, 0); - ret = update_file(o, 0, &mfi_c1.oid, mfi_c1.mode, new_path1); - if (!ret) - ret = update_file(o, 0, &mfi_c2.oid, mfi_c2.mode, - new_path2); - /* - * unpack_trees() actually populates the index for us for - * "normal" rename/rename(2to1) situtations so that the - * correct entries are at the higher stages, which would - * make the call below to update_stages_for_stage_data - * unnecessary. However, if either of the renames came - * from a directory rename, then unpack_trees() will not - * have gotten the right data loaded into the index, so we - * need to do so now. (While it'd be tempting to move this - * call to update_stages_for_stage_data() to - * apply_directory_rename_modifications(), that would break - * our intermediate calls to would_lose_untracked() since - * those rely on the current in-memory index. See also the - * big "NOTE" in update_stages()). - */ - if (update_stages_for_stage_data(o, path, ci->dst_entry1)) - ret = -1; - - free(new_path2); - free(new_path1); - } - - return ret; + return handle_file_collision(o, path, a->path, b->path, + ci->branch1, ci->branch2, + &mfi_c1.oid, mfi_c1.mode, + &mfi_c2.oid, mfi_c2.mode); } /* @@ -3361,9 +3280,14 @@ static int process_entry(struct merge_options *o, clean_merge = -1; break; case RENAME_TWO_FILES_TO_ONE: - clean_merge = 0; - if (handle_rename_rename_2to1(o, conflict_info)) - clean_merge = -1; + /* + * Probably unclean merge, but if the two renamed + * files merge cleanly and the two resulting files + * can then be two-way merged cleanly, I guess it's + * a clean merge? + */ + clean_merge = handle_rename_rename_2to1(o, + conflict_info); break; default: entry->processed = 0; diff --git a/t/t6036-recursive-corner-cases.sh b/t/t6036-recursive-corner-cases.sh index d6b7e568f5b5dc..99cddb05af8411 100755 --- a/t/t6036-recursive-corner-cases.sh +++ b/t/t6036-recursive-corner-cases.sh @@ -64,15 +64,12 @@ test_expect_success 'merge simple rename+criss-cross with no modifications' ' git ls-files -u >out && test_line_count = 2 out && git ls-files -o >out && - test_line_count = 3 out && + test_line_count = 1 out && git rev-parse >expect \ - L2:three R2:three \ L2:three R2:three && git rev-parse >actual \ :2:three :3:three && - git hash-object >>actual \ - three~HEAD three~R2^0 && test_cmp expect actual ) ' @@ -140,15 +137,12 @@ test_expect_success 'merge criss-cross + rename merges with basic modification' git ls-files -u >out && test_line_count = 2 out && git ls-files -o >out && - test_line_count = 3 out && + test_line_count = 1 out && git rev-parse >expect \ - L2:three R2:three \ L2:three R2:three && git rev-parse >actual \ :2:three :3:three && - git hash-object >>actual \ - three~HEAD three~R2^0 && test_cmp expect actual ) ' @@ -1512,7 +1506,7 @@ test_expect_success 'setup nested conflicts' ' ) ' -test_expect_failure 'check nested conflicts' ' +test_expect_success 'check nested conflicts' ' ( cd nested_conflicts && diff --git a/t/t6042-merge-rename-corner-cases.sh b/t/t6042-merge-rename-corner-cases.sh index 3e934ae89ef9e5..b93139f628dfc7 100755 --- a/t/t6042-merge-rename-corner-cases.sh +++ b/t/t6042-merge-rename-corner-cases.sh @@ -464,17 +464,28 @@ test_expect_success 'handle rename/rename (2to1) conflict correctly' ' git ls-files -u c >out && test_line_count = 2 out && git ls-files -o >out && - test_line_count = 3 out && + test_line_count = 1 out && test_path_is_missing a && test_path_is_missing b && - test_path_is_file c~HEAD && - test_path_is_file c~C^0 && - git rev-parse >expect \ - C:a B:b && - git hash-object >actual \ - c~HEAD c~C^0 && + git rev-parse >expect \ + C:a B:b && + git rev-parse >actual \ + :2:c :3:c && + test_cmp expect actual && + + # Test that the two-way merge in new_a is as expected + git cat-file -p :2:c >>ours && + git cat-file -p :3:c >>theirs && + >empty && + test_must_fail git merge-file \ + -L "HEAD" \ + -L "" \ + -L "C^0" \ + ours empty theirs && + git hash-object c >actual && + git hash-object ours >expect && test_cmp expect actual ) ' @@ -940,7 +951,6 @@ test_expect_failure 'mod6-check: chains of rename/rename(1to2) and rename/rename test_conflicts_with_adds_and_renames() { sideL=$1 sideR=$2 - expect=$3 # Setup: # L @@ -1048,7 +1058,7 @@ test_conflicts_with_adds_and_renames() { ) ' - test_expect_$expect "check simple $sideL/$sideR conflict" ' + test_expect_success "check simple $sideL/$sideR conflict" ' ( cd simple_${sideL}_${sideR} && @@ -1094,10 +1104,10 @@ test_conflicts_with_adds_and_renames() { ' } -test_conflicts_with_adds_and_renames rename rename failure -test_conflicts_with_adds_and_renames rename add success -test_conflicts_with_adds_and_renames add rename success -test_conflicts_with_adds_and_renames add add success +test_conflicts_with_adds_and_renames rename rename +test_conflicts_with_adds_and_renames rename add +test_conflicts_with_adds_and_renames add rename +test_conflicts_with_adds_and_renames add add # Setup: # L @@ -1168,7 +1178,7 @@ test_expect_success 'setup nested conflicts from rename/rename(2to1)' ' ) ' -test_expect_failure 'check nested conflicts from rename/rename(2to1)' ' +test_expect_success 'check nested conflicts from rename/rename(2to1)' ' ( cd nested_conflicts_from_rename_rename && diff --git a/t/t6043-merge-rename-directories.sh b/t/t6043-merge-rename-directories.sh index 4a71f17edd69a2..fedaeafc55c9e6 100755 --- a/t/t6043-merge-rename-directories.sh +++ b/t/t6043-merge-rename-directories.sh @@ -278,7 +278,7 @@ test_expect_success '1d-check: Directory renames cause a rename/rename(2to1) con git ls-files -u >out && test_line_count = 2 out && git ls-files -o >out && - test_line_count = 3 out && + test_line_count = 1 out && git rev-parse >actual \ :0:x/b :0:x/c :0:x/d :0:x/e :0:x/m :0:x/n && @@ -293,15 +293,16 @@ test_expect_success '1d-check: Directory renames cause a rename/rename(2to1) con A:y/wham B:z/wham && test_cmp expect actual && - test_path_is_missing x/wham && - test_path_is_file x/wham~HEAD && - test_path_is_file x/wham~B^0 && - - git hash-object >actual \ - x/wham~HEAD x/wham~B^0 && - git rev-parse >expect \ - A:y/wham B:z/wham && - test_cmp expect actual + # Test that the two-way merge in x/wham is as expected + git cat-file -p :2:x/wham >expect && + git cat-file -p :3:x/wham >other && + >empty && + test_must_fail git merge-file \ + -L "HEAD" \ + -L "" \ + -L "B^0" \ + expect empty other && + test_cmp expect x/wham ) ' @@ -1670,7 +1671,7 @@ test_expect_success '7b-check: rename/rename(2to1), but only due to transitive r git ls-files -u >out && test_line_count = 2 out && git ls-files -o >out && - test_line_count = 3 out && + test_line_count = 1 out && git rev-parse >actual \ :0:y/b :0:y/c :2:y/d :3:y/d && @@ -1678,15 +1679,16 @@ test_expect_success '7b-check: rename/rename(2to1), but only due to transitive r O:z/b O:z/c O:w/d O:x/d && test_cmp expect actual && - test_path_is_missing y/d && - test_path_is_file y/d~HEAD && - test_path_is_file y/d~B^0 && - - git hash-object >actual \ - y/d~HEAD y/d~B^0 && - git rev-parse >expect \ - O:w/d O:x/d && - test_cmp expect actual + # Test that the two-way merge in y/d is as expected + git cat-file -p :2:y/d >expect && + git cat-file -p :3:y/d >other && + >empty && + test_must_fail git merge-file \ + -L "HEAD" \ + -L "" \ + -L "B^0" \ + expect empty other && + test_cmp expect y/d ) ' @@ -3165,7 +3167,7 @@ test_expect_success '10c-check: Overwrite untracked with dir rename/rename(1to2) # Commit O: z/{a,b,c_1}, x/{d,e,f_2} # Commit A: y/{a,b}, x/{d,e,f_2,wham_1} + untracked y/wham # Commit B: z/{a,b,c_1,wham_2}, y/{d,e} -# Expected: Failed Merge; y/{a,b,d,e} + untracked y/{wham,wham~B^0,wham~HEAD}+ +# Expected: Failed Merge; y/{a,b,d,e} + untracked y/{wham,wham~merged}+ # CONFLICT(rename/rename) z/c_1 vs x/f_2 -> y/wham # ERROR_MSG(Refusing to lose untracked file at y/wham) @@ -3219,7 +3221,7 @@ test_expect_success '10d-check: Delete untracked with dir rename/rename(2to1)' ' git ls-files -u >out && test_line_count = 2 out && git ls-files -o >out && - test_line_count = 4 out && + test_line_count = 3 out && git rev-parse >actual \ :0:y/a :0:y/b :0:y/d :0:y/e :2:y/wham :3:y/wham && @@ -3232,11 +3234,16 @@ test_expect_success '10d-check: Delete untracked with dir rename/rename(2to1)' ' echo important >expect && test_cmp expect y/wham && - git hash-object >actual \ - y/wham~B^0 y/wham~HEAD && - git rev-parse >expect \ - O:x/f O:z/c && - test_cmp expect actual + # Test that the two-way merge in y/wham~merged is as expected + git cat-file -p :2:y/wham >expect && + git cat-file -p :3:y/wham >other && + >empty && + test_must_fail git merge-file \ + -L "HEAD" \ + -L "" \ + -L "B^0" \ + expect empty other && + test_cmp expect y/wham~merged ) ' @@ -3689,7 +3696,7 @@ test_expect_success '11e-check: Avoid deleting not-uptodate with dir rename/rena # Commit O: z/{a,b}, x/{c_1,d_2} # Commit A: y/{a,b,wham_1}, x/d_2, except y/wham has uncommitted mods # Commit B: z/{a,b,wham_2}, x/c_1 -# Expected: Failed Merge; y/{a,b} + untracked y/{wham~B^0,wham~B^HEAD} + +# Expected: Failed Merge; y/{a,b} + untracked y/{wham~merged} + # y/wham with dirty changes from before merge + # CONFLICT(rename/rename) x/c vs x/d -> y/wham # ERROR_MSG(Refusing to lose dirty file at y/wham) @@ -3741,24 +3748,30 @@ test_expect_success '11f-check: Avoid deleting not-uptodate with dir rename/rena git ls-files -u >out && test_line_count = 2 out && git ls-files -o >out && - test_line_count = 4 out && + test_line_count = 3 out && test_seq 1 10 >expected && echo important >>expected && test_cmp expected y/wham && test_must_fail git rev-parse :1:y/wham && - git hash-object >actual \ - y/wham~B^0 y/wham~HEAD && - git rev-parse >expect \ - O:x/d O:x/c && - test_cmp expect actual && git rev-parse >actual \ :0:y/a :0:y/b :2:y/wham :3:y/wham && git rev-parse >expect \ O:z/a O:z/b O:x/c O:x/d && - test_cmp expect actual + test_cmp expect actual && + + # Test that the two-way merge in y/wham~merged is as expected + git cat-file -p :2:y/wham >expect && + git cat-file -p :3:y/wham >other && + >empty && + test_must_fail git merge-file \ + -L "HEAD" \ + -L "" \ + -L "B^0" \ + expect empty other && + test_cmp expect y/wham~merged ) ' From dcf2815098589b1fa0a01e7a8459c213581d4412 Mon Sep 17 00:00:00 2001 From: Elijah Newren Date: Wed, 7 Nov 2018 20:40:28 -0800 Subject: [PATCH 0011/1736] merge-recursive: use handle_file_collision for add/add conflicts This results in no-net change of behavior, it simply ensures that all file-collision conflict handling types are being handled the same by calling the same function. Signed-off-by: Elijah Newren Signed-off-by: Junio C Hamano --- merge-recursive.c | 29 +++++++++++++++++++++-------- 1 file changed, 21 insertions(+), 8 deletions(-) diff --git a/merge-recursive.c b/merge-recursive.c index ead6054a75b6bd..c78b347112c092 100644 --- a/merge-recursive.c +++ b/merge-recursive.c @@ -3355,14 +3355,27 @@ static int process_entry(struct merge_options *o, clean_merge = -1; } } else if (a_oid && b_oid) { - /* Case C: Added in both (check for same permissions) and */ - /* case D: Modified in both, but differently. */ - int is_dirty = 0; /* unpack_trees would have bailed if dirty */ - clean_merge = handle_content_merge(o, path, is_dirty, - o_oid, o_mode, - a_oid, a_mode, - b_oid, b_mode, - NULL); + if (!o_oid) { + /* Case C: Added in both (check for same permissions) */ + output(o, 1, + _("CONFLICT (add/add): Merge conflict in %s"), + path); + clean_merge = handle_file_collision(o, + path, NULL, NULL, + o->branch1, + o->branch2, + a_oid, a_mode, + b_oid, b_mode); + } else { + /* case D: Modified in both, but differently. */ + int is_dirty = 0; /* unpack_trees would have bailed if dirty */ + clean_merge = handle_content_merge(o, path, + is_dirty, + o_oid, o_mode, + a_oid, a_mode, + b_oid, b_mode, + NULL); + } } else if (!o_oid && !a_oid && !b_oid) { /* * this entry was deleted altogether. a_mode == 0 means From 48c9cb9d6d058bcf18e931a1ed0d88792bb506c9 Mon Sep 17 00:00:00 2001 From: Elijah Newren Date: Wed, 7 Nov 2018 20:40:29 -0800 Subject: [PATCH 0012/1736] merge-recursive: improve rename/rename(1to2)/add[/add] handling When we have a rename/rename(1to2) conflict, each of the renames can collide with a file addition. Each of these rename/add conflicts suffered from the same kinds of problems that normal rename/add suffered from. Make the code use handle_file_conflicts() as well so that we get all the same fixes and consistent behavior between the different conflict types. Signed-off-by: Elijah Newren Signed-off-by: Junio C Hamano --- merge-recursive.c | 154 +++++++++++++-------------- t/t6042-merge-rename-corner-cases.sh | 29 +++-- t/t6043-merge-rename-directories.sh | 24 +++-- 3 files changed, 113 insertions(+), 94 deletions(-) diff --git a/merge-recursive.c b/merge-recursive.c index c78b347112c092..59811116b6ddb2 100644 --- a/merge-recursive.c +++ b/merge-recursive.c @@ -1709,80 +1709,17 @@ static int handle_rename_add(struct merge_options *o, ci->dst_entry1->stages[other_stage].mode); } -static int handle_file(struct merge_options *o, - struct diff_filespec *rename, - int stage, - struct rename_conflict_info *ci) -{ - char *dst_name = rename->path; - struct stage_data *dst_entry; - const char *cur_branch, *other_branch; - struct diff_filespec other; - struct diff_filespec *add; - int ret; - - if (stage == 2) { - dst_entry = ci->dst_entry1; - cur_branch = ci->branch1; - other_branch = ci->branch2; - } else { - dst_entry = ci->dst_entry2; - cur_branch = ci->branch2; - other_branch = ci->branch1; - } - - add = filespec_from_entry(&other, dst_entry, stage ^ 1); - if (add) { - int ren_src_was_dirty = was_dirty(o, rename->path); - char *add_name = unique_path(o, rename->path, other_branch); - if (update_file(o, 0, &add->oid, add->mode, add_name)) - return -1; - - if (ren_src_was_dirty) { - output(o, 1, _("Refusing to lose dirty file at %s"), - rename->path); - } - /* - * Because the double negatives somehow keep confusing me... - * 1) update_wd iff !ren_src_was_dirty. - * 2) no_wd iff !update_wd - * 3) so, no_wd == !!ren_src_was_dirty == ren_src_was_dirty - */ - remove_file(o, 0, rename->path, ren_src_was_dirty); - dst_name = unique_path(o, rename->path, cur_branch); - } else { - if (dir_in_way(rename->path, !o->call_depth, 0)) { - dst_name = unique_path(o, rename->path, cur_branch); - output(o, 1, _("%s is a directory in %s adding as %s instead"), - rename->path, other_branch, dst_name); - } else if (!o->call_depth && - would_lose_untracked(rename->path)) { - dst_name = unique_path(o, rename->path, cur_branch); - output(o, 1, _("Refusing to lose untracked file at %s; " - "adding as %s instead"), - rename->path, dst_name); - } - } - if ((ret = update_file(o, 0, &rename->oid, rename->mode, dst_name))) - ; /* fall through, do allow dst_name to be released */ - else if (stage == 2) - ret = update_stages(o, rename->path, NULL, rename, add); - else - ret = update_stages(o, rename->path, NULL, add, rename); - - if (dst_name != rename->path) - free(dst_name); - - return ret; -} - static int handle_rename_rename_1to2(struct merge_options *o, struct rename_conflict_info *ci) { /* One file was renamed in both branches, but to different names. */ + struct merge_file_info mfi; + struct diff_filespec other; + struct diff_filespec *add; struct diff_filespec *one = ci->pair1->one; struct diff_filespec *a = ci->pair1->two; struct diff_filespec *b = ci->pair2->two; + char *path_desc; output(o, 1, _("CONFLICT (rename/rename): " "Rename \"%s\"->\"%s\" in branch \"%s\" " @@ -1790,15 +1727,16 @@ static int handle_rename_rename_1to2(struct merge_options *o, one->path, a->path, ci->branch1, one->path, b->path, ci->branch2, o->call_depth ? _(" (left unresolved)") : ""); - if (o->call_depth) { - struct merge_file_info mfi; - struct diff_filespec other; - struct diff_filespec *add; - if (merge_mode_and_contents(o, one, a, b, one->path, - ci->branch1, ci->branch2, - o->call_depth * 2, &mfi)) - return -1; + path_desc = xstrfmt("%s and %s, both renamed from %s", + a->path, b->path, one->path); + if (merge_mode_and_contents(o, one, a, b, path_desc, + ci->branch1, ci->branch2, + o->call_depth * 2, &mfi)) + return -1; + free(path_desc); + + if (o->call_depth) { /* * FIXME: For rename/add-source conflicts (if we could detect * such), this is wrong. We should instead find a unique @@ -1830,8 +1768,70 @@ static int handle_rename_rename_1to2(struct merge_options *o, } else remove_file_from_cache(b->path); - } else if (handle_file(o, a, 2, ci) || handle_file(o, b, 3, ci)) - return -1; + } else { + /* + * For each destination path, we need to see if there is a + * rename/add collision. If not, we can write the file out + * to the specified location. + */ + add = filespec_from_entry(&other, ci->dst_entry1, 2 ^ 1); + if (add) { + if (handle_file_collision(o, a->path, + NULL, NULL, + ci->branch1, ci->branch2, + &mfi.oid, mfi.mode, + &add->oid, add->mode) < 0) + return -1; + } else { + char *new_path = NULL; + if (dir_in_way(a->path, !o->call_depth, 0)) { + new_path = unique_path(o, a->path, ci->branch1); + output(o, 1, _("%s is a directory in %s adding " + "as %s instead"), + a->path, ci->branch2, new_path); + } else if (would_lose_untracked(a->path)) { + new_path = unique_path(o, a->path, ci->branch1); + output(o, 1, _("Refusing to lose untracked file" + " at %s; adding as %s instead"), + a->path, new_path); + } + + if (update_file(o, 0, &mfi.oid, mfi.mode, new_path ? new_path : a->path)) + return -1; + free(new_path); + if (update_stages(o, a->path, NULL, a, NULL)) + return -1; + } + + add = filespec_from_entry(&other, ci->dst_entry2, 3 ^ 1); + if (add) { + if (handle_file_collision(o, b->path, + NULL, NULL, + ci->branch1, ci->branch2, + &add->oid, add->mode, + &mfi.oid, mfi.mode) < 0) + return -1; + } else { + char *new_path = NULL; + if (dir_in_way(b->path, !o->call_depth, 0)) { + new_path = unique_path(o, b->path, ci->branch2); + output(o, 1, _("%s is a directory in %s adding " + "as %s instead"), + b->path, ci->branch1, new_path); + } else if (would_lose_untracked(b->path)) { + new_path = unique_path(o, b->path, ci->branch2); + output(o, 1, _("Refusing to lose untracked file" + " at %s; adding as %s instead"), + b->path, new_path); + } + + if (update_file(o, 0, &mfi.oid, mfi.mode, new_path ? new_path : b->path)) + return -1; + free(new_path); + if (update_stages(o, b->path, NULL, NULL, b)) + return -1; + } + } return 0; } diff --git a/t/t6042-merge-rename-corner-cases.sh b/t/t6042-merge-rename-corner-cases.sh index b93139f628dfc7..7cc34e75797b40 100755 --- a/t/t6042-merge-rename-corner-cases.sh +++ b/t/t6042-merge-rename-corner-cases.sh @@ -684,7 +684,7 @@ test_expect_success 'rename/rename/add-dest merge still knows about conflicting git ls-files -u c >out && test_line_count = 2 out && git ls-files -o >out && - test_line_count = 5 out && + test_line_count = 1 out && git rev-parse >expect \ A:a C:b B:b C:c B:c && @@ -692,14 +692,27 @@ test_expect_success 'rename/rename/add-dest merge still knows about conflicting :1:a :2:b :3:b :2:c :3:c && test_cmp expect actual && - git rev-parse >expect \ - C:c B:c C:b B:b && - git hash-object >actual \ - c~HEAD c~B\^0 b~HEAD b~B\^0 && - test_cmp expect actual && + # Record some contents for re-doing merges + git cat-file -p A:a >stuff && + git cat-file -p C:b >important_info && + git cat-file -p B:c >precious_data && + >empty && - test_path_is_missing b && - test_path_is_missing c + # Test the merge in b + test_must_fail git merge-file \ + -L "HEAD" \ + -L "" \ + -L "B^0" \ + important_info empty stuff && + test_cmp important_info b && + + # Test the merge in c + test_must_fail git merge-file \ + -L "HEAD" \ + -L "" \ + -L "B^0" \ + stuff empty precious_data && + test_cmp stuff c ) ' diff --git a/t/t6043-merge-rename-directories.sh b/t/t6043-merge-rename-directories.sh index fedaeafc55c9e6..5c01a0c14af320 100755 --- a/t/t6043-merge-rename-directories.sh +++ b/t/t6043-merge-rename-directories.sh @@ -1078,7 +1078,7 @@ test_expect_success '5c-check: Transitive rename would cause rename/rename/renam git ls-files -u >out && test_line_count = 6 out && git ls-files -o >out && - test_line_count = 3 out && + test_line_count = 1 out && git rev-parse >actual \ :0:y/b :0:y/c :0:y/e && @@ -1094,9 +1094,9 @@ test_expect_success '5c-check: Transitive rename would cause rename/rename/renam test_cmp expect actual && git hash-object >actual \ - w/d~HEAD w/d~B^0 z/d && + z/d && git rev-parse >expect \ - O:x/d B:w/d O:x/d && + O:x/d && test_cmp expect actual && test_path_is_missing x/d && test_path_is_file y/d && @@ -3672,7 +3672,7 @@ test_expect_success '11e-check: Avoid deleting not-uptodate with dir rename/rena git ls-files -u >out && test_line_count = 4 out && git ls-files -o >out && - test_line_count = 4 out && + test_line_count = 3 out && echo different >expected && echo mods >>expected && @@ -3684,11 +3684,17 @@ test_expect_success '11e-check: Avoid deleting not-uptodate with dir rename/rena O:z/a O:z/b O:x/d O:x/c O:x/c A:y/c O:x/c && test_cmp expect actual && - git hash-object >actual \ - y/c~B^0 y/c~HEAD && - git rev-parse >expect \ - O:x/c A:y/c && - test_cmp expect actual + # See if y/c~merged has expected contents; requires manually + # doing the expected file merge + git cat-file -p A:y/c >c1 && + git cat-file -p B:z/c >c2 && + >empty && + test_must_fail git merge-file \ + -L "HEAD" \ + -L "" \ + -L "B^0" \ + c1 empty c2 && + test_cmp c1 y/c~merged ) ' From b8cd1bb71306e489d73e5cf4ef75a6cdb4c223b7 Mon Sep 17 00:00:00 2001 From: Elijah Newren Date: Wed, 7 Nov 2018 20:40:30 -0800 Subject: [PATCH 0013/1736] t6036, t6043: increase code coverage for file collision handling Stolee's coverage reports found a few code blocks for file collision conflicts that had not previously been covered by testcases; add a few more testcases to cover those too. Signed-off-by: Elijah Newren Signed-off-by: Junio C Hamano --- t/t6036-recursive-corner-cases.sh | 51 +++++++++++++++++++++++++++++ t/t6043-merge-rename-directories.sh | 37 +++++++++++++++++++++ 2 files changed, 88 insertions(+) diff --git a/t/t6036-recursive-corner-cases.sh b/t/t6036-recursive-corner-cases.sh index 99cddb05af8411..b7488b00c04526 100755 --- a/t/t6036-recursive-corner-cases.sh +++ b/t/t6036-recursive-corner-cases.sh @@ -240,6 +240,57 @@ test_expect_success 'git detects differently handled merges conflict' ' ) ' +# Repeat the above testcase with precisely the same setup, other than with +# the two merge bases having different orderings of commit timestamps so +# that they are reversed in the order they are provided to merge-recursive, +# so that we can improve code coverage. +test_expect_success 'git detects differently handled merges conflict, swapped' ' + ( + cd rename-add && + + # Difference #1: Do cleanup from previous testrun + git reset --hard && + git clean -fdqx && + + # Difference #2: Change commit timestamps + btime=$(git log --no-walk --date=raw --format=%cd B | awk "{print \$1}") && + ctime=$(git log --no-walk --date=raw --format=%cd C | awk "{print \$1}") && + newctime=$(($btime+1)) && + git fast-export --no-data --all | sed -e s/$ctime/$newctime/ | git fast-import --force --quiet && + # End of differences; rest is copy-paste of last test + + git checkout D^0 && + test_must_fail git merge -s recursive E^0 && + + git ls-files -s >out && + test_line_count = 3 out && + git ls-files -u >out && + test_line_count = 3 out && + git ls-files -o >out && + test_line_count = 1 out && + + git rev-parse >expect \ + C:new_a D:new_a E:new_a && + git rev-parse >actual \ + :1:new_a :2:new_a :3:new_a && + test_cmp expect actual && + + # Test that the two-way merge in new_a is as expected + git cat-file -p D:new_a >ours && + git cat-file -p E:new_a >theirs && + >empty && + test_must_fail git merge-file \ + -L "HEAD" \ + -L "" \ + -L "E^0" \ + ours empty theirs && + sed -e "s/^\([<=>]\)/\1\1\1/" ours >expect && + git hash-object new_a >actual && + git hash-object ours >expect && + test_cmp expect actual + ) +' + # # criss-cross + modify/delete: # diff --git a/t/t6043-merge-rename-directories.sh b/t/t6043-merge-rename-directories.sh index 5c01a0c14af320..62c564707b9ff8 100755 --- a/t/t6043-merge-rename-directories.sh +++ b/t/t6043-merge-rename-directories.sh @@ -3163,6 +3163,43 @@ test_expect_success '10c-check: Overwrite untracked with dir rename/rename(1to2) ) ' +test_expect_success '10c-check: Overwrite untracked with dir rename/rename(1to2), other direction' ' + ( + cd 10c && + + git reset --hard && + git clean -fdqx && + + git checkout B^0 && + mkdir y && + echo important >y/c && + + test_must_fail git merge -s recursive A^0 >out 2>err && + test_i18ngrep "CONFLICT (rename/rename)" out && + test_i18ngrep "Refusing to lose untracked file at y/c; adding as y/c~HEAD instead" out && + + git ls-files -s >out && + test_line_count = 6 out && + git ls-files -u >out && + test_line_count = 3 out && + git ls-files -o >out && + test_line_count = 3 out && + + git rev-parse >actual \ + :0:y/a :0:y/b :0:x/d :1:x/c :3:w/c :2:y/c && + git rev-parse >expect \ + O:z/a O:z/b O:x/d O:x/c O:x/c O:x/c && + test_cmp expect actual && + + git hash-object y/c~HEAD >actual && + git rev-parse O:x/c >expect && + test_cmp expect actual && + + echo important >expect && + test_cmp expect y/c + ) +' + # Testcase 10d, Delete untracked w/ dir rename/rename(2to1) # Commit O: z/{a,b,c_1}, x/{d,e,f_2} # Commit A: y/{a,b}, x/{d,e,f_2,wham_1} + untracked y/wham From 80cee6e3210120a8a45fc4818e0049f482247b0f Mon Sep 17 00:00:00 2001 From: Derrick Stolee Date: Wed, 7 Nov 2018 20:40:31 -0800 Subject: [PATCH 0014/1736] merge-recursive: combine error handling In handle_rename_rename_1to2(), we have duplicated error handling around colliding paths. Specifically, when we want to write out the file and there is a directory or untracked file in the way, we need to create a temporary file to hold the contents. This has some special output to alert the user, and this output is duplicated for each side of the conflict. Simplify the call by generating this new path in a helper function. Signed-off-by: Derrick Stolee Signed-off-by: Elijah Newren Signed-off-by: Junio C Hamano --- merge-recursive.c | 53 ++++++++++++++++++++++++----------------------- 1 file changed, 27 insertions(+), 26 deletions(-) diff --git a/merge-recursive.c b/merge-recursive.c index 59811116b6ddb2..5e36bef162040f 100644 --- a/merge-recursive.c +++ b/merge-recursive.c @@ -1709,6 +1709,27 @@ static int handle_rename_add(struct merge_options *o, ci->dst_entry1->stages[other_stage].mode); } +static char *find_path_for_conflict(struct merge_options *o, + const char *path, + const char *branch1, + const char *branch2) +{ + char *new_path = NULL; + if (dir_in_way(path, !o->call_depth, 0)) { + new_path = unique_path(o, path, branch1); + output(o, 1, _("%s is a directory in %s adding " + "as %s instead"), + path, branch2, new_path); + } else if (would_lose_untracked(path)) { + new_path = unique_path(o, path, branch1); + output(o, 1, _("Refusing to lose untracked file" + " at %s; adding as %s instead"), + path, new_path); + } + + return new_path; +} + static int handle_rename_rename_1to2(struct merge_options *o, struct rename_conflict_info *ci) { @@ -1783,19 +1804,9 @@ static int handle_rename_rename_1to2(struct merge_options *o, &add->oid, add->mode) < 0) return -1; } else { - char *new_path = NULL; - if (dir_in_way(a->path, !o->call_depth, 0)) { - new_path = unique_path(o, a->path, ci->branch1); - output(o, 1, _("%s is a directory in %s adding " - "as %s instead"), - a->path, ci->branch2, new_path); - } else if (would_lose_untracked(a->path)) { - new_path = unique_path(o, a->path, ci->branch1); - output(o, 1, _("Refusing to lose untracked file" - " at %s; adding as %s instead"), - a->path, new_path); - } - + char *new_path = find_path_for_conflict(o, a->path, + ci->branch1, + ci->branch2); if (update_file(o, 0, &mfi.oid, mfi.mode, new_path ? new_path : a->path)) return -1; free(new_path); @@ -1812,19 +1823,9 @@ static int handle_rename_rename_1to2(struct merge_options *o, &mfi.oid, mfi.mode) < 0) return -1; } else { - char *new_path = NULL; - if (dir_in_way(b->path, !o->call_depth, 0)) { - new_path = unique_path(o, b->path, ci->branch2); - output(o, 1, _("%s is a directory in %s adding " - "as %s instead"), - b->path, ci->branch1, new_path); - } else if (would_lose_untracked(b->path)) { - new_path = unique_path(o, b->path, ci->branch2); - output(o, 1, _("Refusing to lose untracked file" - " at %s; adding as %s instead"), - b->path, new_path); - } - + char *new_path = find_path_for_conflict(o, b->path, + ci->branch2, + ci->branch1); if (update_file(o, 0, &mfi.oid, mfi.mode, new_path ? new_path : b->path)) return -1; free(new_path); From d73019feb44b9cb89d9967fb36eae809e8146ace Mon Sep 17 00:00:00 2001 From: Force Charlie Date: Thu, 8 Nov 2018 19:44:14 -0800 Subject: [PATCH 0015/1736] http: add support selecting http version Usually we don't need to set libcurl to choose which version of the HTTP protocol to use to communicate with a server. But different versions of libcurl, the default value is not the same. CURL >= 7.62.0: CURL_HTTP_VERSION_2TLS CURL < 7.62: CURL_HTTP_VERSION_1_1 In order to give users the freedom to control the HTTP version, we need to add a setting to choose which HTTP version to use. Signed-off-by: Force Charlie Signed-off-by: Junio C Hamano --- Documentation/config.txt | 9 +++++++++ http.c | 39 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 48 insertions(+) diff --git a/Documentation/config.txt b/Documentation/config.txt index 41a9ff2b6aa4eb..f3979421287191 100644 --- a/Documentation/config.txt +++ b/Documentation/config.txt @@ -1935,6 +1935,15 @@ http.saveCookies:: If set, store cookies received during requests to the file specified by http.cookieFile. Has no effect if http.cookieFile is unset. +http.version:: + Use the specified HTTP protocol version when communicating with a server. + If you want to force the default. The available and default version depend + on libcurl. Actually the possible values of + this option are: + + - HTTP/2 + - HTTP/1.1 + http.sslVersion:: The SSL version to use when negotiating an SSL connection, if you want to force the default. The available and default version diff --git a/http.c b/http.c index 3dc8c560d65e7a..bc3274804eab10 100644 --- a/http.c +++ b/http.c @@ -48,6 +48,7 @@ char curl_errorstr[CURL_ERROR_SIZE]; static int curl_ssl_verify = -1; static int curl_ssl_try; +static const char *curl_http_version = NULL; static const char *ssl_cert; static const char *ssl_cipherlist; static const char *ssl_version; @@ -284,6 +285,9 @@ static void process_curl_messages(void) static int http_options(const char *var, const char *value, void *cb) { + if (!strcmp("http.version", var)) { + return git_config_string(&curl_http_version, var, value); + } if (!strcmp("http.sslverify", var)) { curl_ssl_verify = git_config_bool(var, value); return 0; @@ -789,6 +793,31 @@ static long get_curl_allowed_protocols(int from_user) } #endif +#if LIBCURL_VERSION_NUM >=0x072f00 +static int get_curl_http_version_opt(const char *version_string, long *opt) +{ + int i; + static struct { + const char *name; + long opt_token; + } choice[] = { + { "HTTP/1.1", CURL_HTTP_VERSION_1_1 }, + { "HTTP/2", CURL_HTTP_VERSION_2 } + }; + + for (i = 0; i < ARRAY_SIZE(choice); i++) { + if (!strcmp(version_string, choice[i].name)) { + *opt = choice[i].opt_token; + return 0; + } + } + + warning("unknown value given to http.version: '%s'", version_string); + return -1; /* not found */ +} + +#endif + static CURL *get_curl_handle(void) { CURL *result = curl_easy_init(); @@ -806,6 +835,16 @@ static CURL *get_curl_handle(void) curl_easy_setopt(result, CURLOPT_SSL_VERIFYHOST, 2); } +#if LIBCURL_VERSION_NUM >= 0x072f00 // 7.47.0 + if (curl_http_version) { + long opt; + if (!get_curl_http_version_opt(curl_http_version, &opt)) { + /* Set request use http version */ + curl_easy_setopt(result, CURLOPT_HTTP_VERSION, opt); + } + } +#endif + #if LIBCURL_VERSION_NUM >= 0x070907 curl_easy_setopt(result, CURLOPT_NETRC, CURL_NETRC_OPTIONAL); #endif From 8aa8c1409774ba4fd718bfa9f7f667170aa3089b 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, 10 Nov 2018 06:16:00 +0100 Subject: [PATCH 0016/1736] git.c: mark more strings for translation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit One string is slightly updated to keep consistency with the rest: die() should begin with lowercase. Signed-off-by: Nguyễn Thái Ngọc Duy Signed-off-by: Junio C Hamano --- git.c | 30 +++++++++++++++--------------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/git.c b/git.c index adac132956e995..5fd30da09315ad 100644 --- a/git.c +++ b/git.c @@ -338,27 +338,27 @@ static int handle_alias(int *argcp, const char ***argv) if (ret >= 0) /* normal exit */ exit(ret); - die_errno("while expanding alias '%s': '%s'", - alias_command, alias_string + 1); + die_errno(_("while expanding alias '%s': '%s'"), + alias_command, alias_string + 1); } count = split_cmdline(alias_string, &new_argv); if (count < 0) - die("Bad alias.%s string: %s", alias_command, + die(_("bad alias.%s string: %s"), alias_command, split_cmdline_strerror(count)); option_count = handle_options(&new_argv, &count, &envchanged); if (envchanged) - die("alias '%s' changes environment variables.\n" - "You can use '!git' in the alias to do this", - alias_command); + die(_("alias '%s' changes environment variables.\n" + "You can use '!git' in the alias to do this"), + alias_command); memmove(new_argv - option_count, new_argv, count * sizeof(char *)); new_argv -= option_count; if (count < 1) - die("empty alias for %s", alias_command); + die(_("empty alias for %s"), alias_command); if (!strcmp(alias_command, new_argv[0])) - die("recursive alias: %s", alias_command); + die(_("recursive alias: %s"), alias_command); trace_argv_printf(new_argv, "trace: alias expansion: %s =>", @@ -409,7 +409,7 @@ static int run_builtin(struct cmd_struct *p, int argc, const char **argv) if (!help && get_super_prefix()) { if (!(p->option & SUPPORT_SUPER_PREFIX)) - die("%s doesn't support --super-prefix", p->cmd); + die(_("%s doesn't support --super-prefix"), p->cmd); } if (!help && p->option & NEED_WORK_TREE) @@ -433,11 +433,11 @@ static int run_builtin(struct cmd_struct *p, int argc, const char **argv) /* Check for ENOSPC and EIO errors.. */ if (fflush(stdout)) - die_errno("write failure on standard output"); + die_errno(_("write failure on standard output")); if (ferror(stdout)) - die("unknown write failure on standard output"); + die(_("unknown write failure on standard output")); if (fclose(stdout)) - die_errno("close failed on standard output"); + die_errno(_("close failed on standard output")); return 0; } @@ -648,7 +648,7 @@ static void execv_dashed_external(const char **argv) int status; if (get_super_prefix()) - die("%s doesn't support --super-prefix", argv[0]); + die(_("%s doesn't support --super-prefix"), argv[0]); if (use_pager == -1 && !is_builtin(argv[0])) use_pager = check_pager_config(argv[0]); @@ -760,7 +760,7 @@ int cmd_main(int argc, const char **argv) if (skip_prefix(cmd, "git-", &cmd)) { argv[0] = cmd; handle_builtin(argc, argv); - die("cannot handle %s as a builtin", cmd); + die(_("cannot handle %s as a builtin"), cmd); } /* Look for flags.. */ @@ -773,7 +773,7 @@ int cmd_main(int argc, const char **argv) } else { /* The user didn't specify a command; give them help */ commit_pager_choice(); - printf("usage: %s\n\n", git_usage_string); + printf(_("usage: %s\n\n"), git_usage_string); list_common_cmds_help(); printf("\n%s\n", _(git_more_info_string)); exit(1); From a7412ae1342b09f16d659d75da5d25f137800a60 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, 10 Nov 2018 06:16:01 +0100 Subject: [PATCH 0017/1736] alias.c: mark split_cmdline_strerror() strings for translation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This function can be part of translated messages. To make sure we don't have a sentence with mixed languages, mark the strings for translation, but only use translated strings in places we know we will output translated strings. Signed-off-by: Nguyễn Thái Ngọc Duy Signed-off-by: Junio C Hamano --- alias.c | 4 ++-- builtin/merge.c | 2 +- git.c | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/alias.c b/alias.c index a7e4e57130d461..c4715380205b5f 100644 --- a/alias.c +++ b/alias.c @@ -47,8 +47,8 @@ void list_aliases(struct string_list *list) #define SPLIT_CMDLINE_BAD_ENDING 1 #define SPLIT_CMDLINE_UNCLOSED_QUOTE 2 static const char *split_cmdline_errors[] = { - "cmdline ends with \\", - "unclosed quote" + N_("cmdline ends with \\"), + N_("unclosed quote") }; int split_cmdline(char *cmdline, const char ***argv) diff --git a/builtin/merge.c b/builtin/merge.c index 4aa60715980f2f..92ba7e1c6dc2a7 100644 --- a/builtin/merge.c +++ b/builtin/merge.c @@ -577,7 +577,7 @@ static void parse_branch_merge_options(char *bmo) argc = split_cmdline(bmo, &argv); if (argc < 0) die(_("Bad branch.%s.mergeoptions string: %s"), branch, - split_cmdline_strerror(argc)); + _(split_cmdline_strerror(argc))); REALLOC_ARRAY(argv, argc + 2); MOVE_ARRAY(argv + 1, argv, argc + 1); argc++; diff --git a/git.c b/git.c index 5fd30da09315ad..c7e122cfc1483b 100644 --- a/git.c +++ b/git.c @@ -344,7 +344,7 @@ static int handle_alias(int *argcp, const char ***argv) count = split_cmdline(alias_string, &new_argv); if (count < 0) die(_("bad alias.%s string: %s"), alias_command, - split_cmdline_strerror(count)); + _(split_cmdline_strerror(count))); option_count = handle_options(&new_argv, &count, &envchanged); if (envchanged) die(_("alias '%s' changes environment variables.\n" From c6e7965ddf950d764942ddc3c327103731354303 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, 10 Nov 2018 06:16:02 +0100 Subject: [PATCH 0018/1736] archive.c: mark more strings for translation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two messages also print extra information to be more useful Signed-off-by: Nguyễn Thái Ngọc Duy Signed-off-by: Junio C Hamano --- archive.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/archive.c b/archive.c index 9d16b7fadfc8d3..d8f6e1ce304c25 100644 --- a/archive.c +++ b/archive.c @@ -385,12 +385,12 @@ static void parse_treeish_arg(const char **argv, int refnamelen = colon - name; if (!dwim_ref(name, refnamelen, &oid, &ref)) - die("no such ref: %.*s", refnamelen, name); + die(_("no such ref: %.*s"), refnamelen, name); free(ref); } if (get_oid(name, &oid)) - die("Not a valid object name"); + die(_("not a valid object name: %s"), name); commit = lookup_commit_reference_gently(ar_args->repo, &oid, 1); if (commit) { @@ -403,7 +403,7 @@ static void parse_treeish_arg(const char **argv, tree = parse_tree_indirect(&oid); if (tree == NULL) - die("not a tree object"); + die(_("not a tree object: %s"), oid_to_hex(&oid)); if (prefix) { struct object_id tree_oid; @@ -413,7 +413,7 @@ static void parse_treeish_arg(const char **argv, err = get_tree_entry(&tree->object.oid, prefix, &tree_oid, &mode); if (err || !S_ISDIR(mode)) - die("current working directory is untracked"); + die(_("current working directory is untracked")); tree = parse_tree_indirect(&tree_oid); } From ad8f8f4aedc736b7ddbf802dd253f8682f4f5a3e 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, 10 Nov 2018 06:16:03 +0100 Subject: [PATCH 0019/1736] attr.c: mark more string for translation 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 --- attr.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/attr.c b/attr.c index 60d284796de728..3770bc1a11ea1a 100644 --- a/attr.c +++ b/attr.c @@ -372,8 +372,8 @@ static struct match_attr *parse_attr_line(const char *line, const char *src, if (strlen(ATTRIBUTE_MACRO_PREFIX) < namelen && starts_with(name, ATTRIBUTE_MACRO_PREFIX)) { if (!macro_ok) { - fprintf(stderr, "%s not allowed: %s:%d\n", - name, src, lineno); + fprintf_ln(stderr, _("%s not allowed: %s:%d"), + name, src, lineno); goto fail_return; } is_macro = 1; From 391408e532b9155eaad89f5f009549874d9a07e0 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, 10 Nov 2018 06:16:04 +0100 Subject: [PATCH 0020/1736] read-cache.c: turn die("internal error") to BUG() 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 --- read-cache.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/read-cache.c b/read-cache.c index d57958233e82df..0c37f4885ec95c 100644 --- a/read-cache.c +++ b/read-cache.c @@ -316,7 +316,7 @@ static int ce_match_stat_basic(const struct cache_entry *ce, struct stat *st) changed |= DATA_CHANGED; return changed; default: - die("internal error: ce_mode is %o", ce->ce_mode); + BUG("unsupported ce_mode: %o", ce->ce_mode); } changed |= match_stat_data(&ce->ce_stat_data, st); @@ -2356,14 +2356,14 @@ void validate_cache_entries(const struct index_state *istate) for (i = 0; i < istate->cache_nr; i++) { if (!istate) { - die("internal error: cache entry is not allocated from expected memory pool"); + BUG("cache entry is not allocated from expected memory pool"); } else if (!istate->ce_mem_pool || !mem_pool_contains(istate->ce_mem_pool, istate->cache[i])) { if (!istate->split_index || !istate->split_index->base || !istate->split_index->base->ce_mem_pool || !mem_pool_contains(istate->split_index->base->ce_mem_pool, istate->cache[i])) { - die("internal error: cache entry is not allocated from expected memory pool"); + BUG("cache entry is not allocated from expected memory pool"); } } } From 9d0a9e9089639d9bdcd7f616121e0683bb3d6e3e 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, 10 Nov 2018 06:16:05 +0100 Subject: [PATCH 0021/1736] read-cache.c: mark more strings for translation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit There are a couple other improvements on these strings as well: - add missing colon (as separator) - quote paths - provide more information on error messages - keep first word in lowercase Signed-off-by: Nguyễn Thái Ngọc Duy Signed-off-by: Junio C Hamano --- read-cache.c | 57 +++++++++++++++++++++++++------------------------ t/t1450-fsck.sh | 2 +- 2 files changed, 30 insertions(+), 29 deletions(-) diff --git a/read-cache.c b/read-cache.c index 0c37f4885ec95c..858befe73857f1 100644 --- a/read-cache.c +++ b/read-cache.c @@ -672,7 +672,8 @@ static struct cache_entry *create_alias_ce(struct index_state *istate, struct cache_entry *new_entry; if (alias->ce_flags & CE_ADDED) - die("Will not add file alias '%s' ('%s' already exists in index)", ce->name, alias->name); + die(_("will not add file alias '%s' ('%s' already exists in index)"), + ce->name, alias->name); /* Ok, create the new entry using the name of the existing alias */ len = ce_namelen(alias); @@ -687,7 +688,7 @@ void set_object_name_for_intent_to_add_entry(struct cache_entry *ce) { struct object_id oid; if (write_object_file("", 0, blob_type, &oid)) - die("cannot create an empty blob in the object database"); + die(_("cannot create an empty blob in the object database")); oidcpy(&ce->oid, &oid); } @@ -708,7 +709,7 @@ int add_to_index(struct index_state *istate, const char *path, struct stat *st, newflags |= HASH_RENORMALIZE; if (!S_ISREG(st_mode) && !S_ISLNK(st_mode) && !S_ISDIR(st_mode)) - return error("%s: can only add regular files, symbolic links or git-directories", path); + return error(_("%s: can only add regular files, symbolic links or git-directories"), path); namelen = strlen(path); if (S_ISDIR(st_mode)) { @@ -763,7 +764,7 @@ int add_to_index(struct index_state *istate, const char *path, struct stat *st, if (!intent_only) { if (index_path(istate, &ce->oid, path, st, newflags)) { discard_cache_entry(ce); - return error("unable to index file %s", path); + return error(_("unable to index file '%s'"), path); } } else set_object_name_for_intent_to_add_entry(ce); @@ -782,7 +783,7 @@ int add_to_index(struct index_state *istate, const char *path, struct stat *st, discard_cache_entry(ce); else if (add_index_entry(istate, ce, add_option)) { discard_cache_entry(ce); - return error("unable to add %s to index", path); + return error(_("unable to add '%s' to index"), path); } if (verbose && !was_same) printf("add '%s'\n", path); @@ -793,7 +794,7 @@ int add_file_to_index(struct index_state *istate, const char *path, int flags) { struct stat st; if (lstat(path, &st)) - die_errno("unable to stat '%s'", path); + die_errno(_("unable to stat '%s'"), path); return add_to_index(istate, path, &st, flags); } @@ -818,7 +819,7 @@ struct cache_entry *make_cache_entry(struct index_state *istate, int len; if (!verify_path(path, mode)) { - error("Invalid path '%s'", path); + error(_("invalid path '%s'"), path); return NULL; } @@ -844,7 +845,7 @@ struct cache_entry *make_transient_cache_entry(unsigned int mode, const struct o int len; if (!verify_path(path, mode)) { - error("Invalid path '%s'", path); + error(_("invalid path '%s'"), path); return NULL; } @@ -1297,12 +1298,12 @@ static int add_index_entry_with_check(struct index_state *istate, struct cache_e if (!ok_to_add) return -1; if (!verify_path(ce->name, ce->ce_mode)) - return error("Invalid path '%s'", ce->name); + return error(_("invalid path '%s'"), ce->name); if (!skip_df_check && check_file_directory_conflict(istate, ce, pos, ok_to_replace)) { if (!ok_to_replace) - return error("'%s' appears as both a file and as a directory", + return error(_("'%s' appears as both a file and as a directory"), ce->name); pos = index_name_stage_pos(istate, ce->name, ce_namelen(ce), ce_stage(ce)); pos = -pos-1; @@ -1676,10 +1677,10 @@ static int verify_hdr(const struct cache_header *hdr, unsigned long size) int hdr_version; if (hdr->hdr_signature != htonl(CACHE_SIGNATURE)) - return error("bad signature"); + return error(_("bad signature 0x%08x"), hdr->hdr_signature); hdr_version = ntohl(hdr->hdr_version); if (hdr_version < INDEX_FORMAT_LB || INDEX_FORMAT_UB < hdr_version) - return error("bad index version %d", hdr_version); + return error(_("bad index version %d"), hdr_version); if (!verify_index_checksum) return 0; @@ -1688,7 +1689,7 @@ static int verify_hdr(const struct cache_header *hdr, unsigned long size) the_hash_algo->update_fn(&c, hdr, size - the_hash_algo->rawsz); the_hash_algo->final_fn(hash, &c); if (!hasheq(hash, (unsigned char *)hdr + size - the_hash_algo->rawsz)) - return error("bad index file sha1 signature"); + return error(_("bad index file sha1 signature")); return 0; } @@ -1718,9 +1719,9 @@ static int read_index_extension(struct index_state *istate, break; default: if (*ext < 'A' || 'Z' < *ext) - return error("index uses %.4s extension, which we do not understand", + return error(_("index uses %.4s extension, which we do not understand"), ext); - fprintf(stderr, "ignoring %.4s extension\n", ext); + fprintf_ln(stderr, _("ignoring %.4s extension"), ext); break; } return 0; @@ -1767,7 +1768,7 @@ static struct cache_entry *create_from_disk(struct mem_pool *ce_mem_pool, extended_flags = get_be16(&ondisk2->flags2) << 16; /* We do not yet understand any bit out of CE_EXTENDED_FLAGS */ if (extended_flags & ~CE_EXTENDED_FLAGS) - die("Unknown index entry format %08x", extended_flags); + die(_("unknown index entry format 0x%08x"), extended_flags); flags |= extended_flags; name = ondisk2->name; } @@ -1840,13 +1841,13 @@ static void check_ce_order(struct index_state *istate) int name_compare = strcmp(ce->name, next_ce->name); if (0 < name_compare) - die("unordered stage entries in index"); + die(_("unordered stage entries in index")); if (!name_compare) { if (!ce_stage(ce)) - die("multiple stage entries for merged file '%s'", + die(_("multiple stage entries for merged file '%s'"), ce->name); if (ce_stage(ce) > ce_stage(next_ce)) - die("unordered stage entries for '%s'", + die(_("unordered stage entries for '%s'"), ce->name); } } @@ -2149,19 +2150,19 @@ int do_read_index(struct index_state *istate, const char *path, int must_exist) if (fd < 0) { if (!must_exist && errno == ENOENT) return 0; - die_errno("%s: index file open failed", path); + die_errno(_("%s: index file open failed"), path); } if (fstat(fd, &st)) - die_errno("cannot stat the open index"); + die_errno(_("%s: cannot stat the open index"), path); mmap_size = xsize_t(st.st_size); if (mmap_size < sizeof(struct cache_header) + the_hash_algo->rawsz) - die("index file smaller than expected"); + die(_("%s: index file smaller than expected"), path); mmap = xmmap(NULL, mmap_size, PROT_READ, MAP_PRIVATE, fd, 0); if (mmap == MAP_FAILED) - die_errno("unable to map index file"); + die_errno(_("%s: unable to map index file"), path); close(fd); hdr = (const struct cache_header *)mmap; @@ -2243,7 +2244,7 @@ int do_read_index(struct index_state *istate, const char *path, int must_exist) unmap: munmap((void *)mmap, mmap_size); - die("index file corrupt"); + die(_("index file corrupt")); } /* @@ -2255,7 +2256,7 @@ int do_read_index(struct index_state *istate, const char *path, int must_exist) static void freshen_shared_index(const char *shared_index, int warn) { if (!check_and_freshen_file(shared_index, 1) && warn) - warning("could not freshen shared index '%s'", shared_index); + warning(_("could not freshen shared index '%s'"), shared_index); } int read_index_from(struct index_state *istate, const char *path, @@ -2290,7 +2291,7 @@ int read_index_from(struct index_state *istate, const char *path, base_path = xstrfmt("%s/sharedindex.%s", gitdir, base_oid_hex); ret = do_read_index(split_index->base, base_path, 1); if (!oideq(&split_index->base_oid, &split_index->base->oid)) - die("broken index, expect %s in %s, got %s", + die(_("broken index, expect %s in %s, got %s"), base_oid_hex, base_path, oid_to_hex(&split_index->base->oid)); @@ -3076,7 +3077,7 @@ static int write_shared_index(struct index_state *istate, return ret; ret = adjust_shared_perm(get_tempfile_path(*temp)); if (ret) { - error("cannot fix permission bits on %s", get_tempfile_path(*temp)); + error(_("cannot fix permission bits on '%s'"), get_tempfile_path(*temp)); return ret; } ret = rename_tempfile(temp, @@ -3222,7 +3223,7 @@ int read_index_unmerged(struct index_state *istate) new_ce->ce_namelen = len; new_ce->ce_mode = ce->ce_mode; if (add_index_entry(istate, new_ce, ADD_CACHE_SKIP_DFCHECK)) - return error("%s: cannot drop to stage #0", + return error(_("%s: cannot drop to stage #0"), new_ce->name); } return unmerged; diff --git a/t/t1450-fsck.sh b/t/t1450-fsck.sh index 0f2dd26f74b4b2..90c765df3a0f05 100755 --- a/t/t1450-fsck.sh +++ b/t/t1450-fsck.sh @@ -754,7 +754,7 @@ test_expect_success 'detect corrupt index file in fsck' ' test_when_finished "mv .git/index.backup .git/index" && corrupt_index_checksum && test_must_fail git fsck --cache 2>errors && - grep "bad index file" errors + test_i18ngrep "bad index file" errors ' test_done From a71806a74cc142c41b0c079721d68c6d8b6c0ba1 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, 10 Nov 2018 06:16:06 +0100 Subject: [PATCH 0022/1736] read-cache.c: add missing colon separators MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit typechange_fmt and added_fmt should have a colon before "needs update". Align the statements to make it easier to read and see. Also drop the unnecessary (). Signed-off-by: Nguyễn Thái Ngọc Duy Signed-off-by: Junio C Hamano --- read-cache.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/read-cache.c b/read-cache.c index 858befe73857f1..8d99ae376c0663 100644 --- a/read-cache.c +++ b/read-cache.c @@ -1492,11 +1492,11 @@ int refresh_index(struct index_state *istate, unsigned int flags, istate->cache_nr); trace_performance_enter(); - modified_fmt = (in_porcelain ? "M\t%s\n" : "%s: needs update\n"); - deleted_fmt = (in_porcelain ? "D\t%s\n" : "%s: needs update\n"); - typechange_fmt = (in_porcelain ? "T\t%s\n" : "%s needs update\n"); - added_fmt = (in_porcelain ? "A\t%s\n" : "%s needs update\n"); - unmerged_fmt = (in_porcelain ? "U\t%s\n" : "%s: needs merge\n"); + modified_fmt = in_porcelain ? "M\t%s\n" : "%s: needs update\n"; + deleted_fmt = in_porcelain ? "D\t%s\n" : "%s: needs update\n"; + typechange_fmt = in_porcelain ? "T\t%s\n" : "%s: needs update\n"; + added_fmt = in_porcelain ? "A\t%s\n" : "%s: needs update\n"; + unmerged_fmt = in_porcelain ? "U\t%s\n" : "%s: needs merge\n"; for (i = 0; i < istate->cache_nr; i++) { struct cache_entry *ce, *new_entry; int cache_errno = 0; From dd509db342b8656aa9e137005639e3492f17353c 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, 10 Nov 2018 06:16:07 +0100 Subject: [PATCH 0023/1736] reflog: mark strings for translation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit One string "nothing to delete?" is rephrased to be more helpful. Signed-off-by: Nguyễn Thái Ngọc Duy Signed-off-by: Junio C Hamano --- builtin/reflog.c | 34 +++++++++++++++++++--------------- 1 file changed, 19 insertions(+), 15 deletions(-) diff --git a/builtin/reflog.c b/builtin/reflog.c index b5941c1ff325b9..5a74ccf7ab610e 100644 --- a/builtin/reflog.c +++ b/builtin/reflog.c @@ -13,11 +13,15 @@ /* NEEDSWORK: switch to using parse_options */ static const char reflog_expire_usage[] = -"git reflog expire [--expire=