* Git makes a merge commit but as a normal (non-merge) commit @ 2017-08-18 22:35 hIpPy 2017-08-21 10:03 ` Michael J Gruber 0 siblings, 1 reply; 22+ messages in thread From: hIpPy @ 2017-08-18 22:35 UTC (permalink / raw) To: Git Mailing List While merging if I do certain actions then the merge commit is made with the merge message but as a normal (non-merge) commit. Repro steps: - Set GIT_MERGE_AUTOEDIT=yes (set other than "no") in .bashrc - Make a merge commit with no conflicts. (external text editor shows the generated merge message) - Focus on Git Bash and Ctrl-C. - Commit (git commit). Actual behavior: Git makes a normal (non-merge) commit (squash merge) but with the merge commit message. It looks like a bug to me. This is very confusing later on as the repo topology would show that the branch is not merged in and there is not an easy way to find out when the merge was made. Expected behavior: Git should stay in a MERGING state. The user can choose to either abort the merge or continue the merge (git merge --continue OR git commit). This does not happen in case of conflicts (at least I'm not able to repro). I get a (master|MERGING) prompt till I resolve the conflicts and commit, which goes through correctly as a merge commit. Environment: $ git version git version 2.14.0.windows.2 $ bash --version GNU bash, version 4.4.12(1)-release (x86_64-pc-msys) Thanks, RM ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: Git makes a merge commit but as a normal (non-merge) commit 2017-08-18 22:35 Git makes a merge commit but as a normal (non-merge) commit hIpPy @ 2017-08-21 10:03 ` Michael J Gruber 2017-08-21 10:06 ` [PATCH] merge: save merge state earlier Michael J Gruber 0 siblings, 1 reply; 22+ messages in thread From: Michael J Gruber @ 2017-08-21 10:03 UTC (permalink / raw) To: hIpPy, Git Mailing List hIpPy venit, vidit, dixit 19.08.2017 00:35: > While merging if I do certain actions then the merge commit is made > with the merge message but as a normal (non-merge) commit. > > Repro steps: > - Set GIT_MERGE_AUTOEDIT=yes (set other than "no") in .bashrc > - Make a merge commit with no conflicts. > (external text editor shows the generated merge message) > - Focus on Git Bash and Ctrl-C. > - Commit (git commit). > > Actual behavior: > Git makes a normal (non-merge) commit (squash merge) but with the > merge commit message. > > It looks like a bug to me. This is very confusing later on as the repo > topology would show that the branch is not merged in and there is not > an easy way to find out when the merge was made. > > Expected behavior: > Git should stay in a MERGING state. The user can choose to either > abort the merge or continue the merge (git merge --continue OR git > commit). > > This does not happen in case of conflicts (at least I'm not able to > repro). I get a (master|MERGING) prompt till I resolve the conflicts > and commit, which goes through correctly as a merge commit. > > Environment: > $ git version > git version 2.14.0.windows.2 > $ bash --version > GNU bash, version 4.4.12(1)-release (x86_64-pc-msys) Thanks for the detailed info! It turns out that his is not Windows specific. The recipe is: - make "git merge" call and wait for an editor - kill "git merge" while it waits for the editor The effect is that prepare_to_commit() has no chance to call abort_commit() any more (which would call write_merge_state()).. Now, I'm not sure why we only do half of write_merge_state() (write out the merge_msg, but not merge_head) before launching the editor. In any case, the following patch solves the problem and passes the existing tests. But I'll wait for more comments (and think about a test meanwhile). Michael ^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH] merge: save merge state earlier 2017-08-21 10:03 ` Michael J Gruber @ 2017-08-21 10:06 ` Michael J Gruber 2017-08-21 12:53 ` [PATCH v2 0/3] Keep merge during kills Michael J Gruber 0 siblings, 1 reply; 22+ messages in thread From: Michael J Gruber @ 2017-08-21 10:06 UTC (permalink / raw) To: git; +Cc: hIpPy If the `git merge` process is killed while waiting for the editor to finish, the merge state is lost but the prepared merge msg and tree is kept. So, a subsequent `git commit` creates a squashed merge even when the user asked for proper merge commit originally. Save the merge state earlier (in the non-squash case) so that it does not get lost. Reported-by: hIpPy <hippy2981@gmail.com> Signed-off-by: Michael J Gruber <git@grubix.eu> --- builtin/merge.c | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/builtin/merge.c b/builtin/merge.c index cc57052993..5379b08824 100644 --- a/builtin/merge.c +++ b/builtin/merge.c @@ -758,6 +758,7 @@ N_("Please enter a commit message to explain why this merge is necessary,\n" "Lines starting with '%c' will be ignored, and an empty message aborts\n" "the commit.\n"); +static void write_merge_heads(struct commit_list *); static void prepare_to_commit(struct commit_list *remoteheads) { struct strbuf msg = STRBUF_INIT; @@ -767,6 +768,8 @@ static void prepare_to_commit(struct commit_list *remoteheads) strbuf_commented_addf(&msg, _(merge_editor_comment), comment_line_char); if (signoff) append_signoff(&msg, ignore_non_trailer(msg.buf, msg.len), 0); + if (!squash) + write_merge_heads(remoteheads); write_file_buf(git_path_merge_msg(), msg.buf, msg.len); if (run_commit_hook(0 < option_edit, get_index_file(), "prepare-commit-msg", git_path_merge_msg(), "merge", NULL)) @@ -908,7 +911,7 @@ static int setup_with_upstream(const char ***argv) return i; } -static void write_merge_state(struct commit_list *remoteheads) +static void write_merge_heads(struct commit_list *remoteheads) { struct commit_list *j; struct strbuf buf = STRBUF_INIT; @@ -924,8 +927,6 @@ static void write_merge_state(struct commit_list *remoteheads) strbuf_addf(&buf, "%s\n", oid_to_hex(oid)); } write_file_buf(git_path_merge_head(), buf.buf, buf.len); - strbuf_addch(&merge_msg, '\n'); - write_file_buf(git_path_merge_msg(), merge_msg.buf, merge_msg.len); strbuf_reset(&buf); if (fast_forward == FF_NO) @@ -933,6 +934,13 @@ static void write_merge_state(struct commit_list *remoteheads) write_file_buf(git_path_merge_mode(), buf.buf, buf.len); } +static void write_merge_state(struct commit_list *remoteheads) +{ + write_merge_heads(remoteheads); + strbuf_addch(&merge_msg, '\n'); + write_file_buf(git_path_merge_msg(), merge_msg.buf, merge_msg.len); +} + static int default_edit_option(void) { static const char name[] = "GIT_MERGE_AUTOEDIT"; -- 2.14.1.364.ge466dba5f7 ^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH v2 0/3] Keep merge during kills 2017-08-21 10:06 ` [PATCH] merge: save merge state earlier Michael J Gruber @ 2017-08-21 12:53 ` Michael J Gruber 2017-08-21 12:53 ` [PATCH v2 1/3] Documentation/git-merge: explain --continue Michael J Gruber ` (2 more replies) 0 siblings, 3 replies; 22+ messages in thread From: Michael J Gruber @ 2017-08-21 12:53 UTC (permalink / raw) To: git; +Cc: hIpPy So here's a little series. 1/3 I just noted along the way 2/3 splits the merge state writing into parts as a preparatory step. This is something we may want to carry further and get rid of some other places which write merge_msg unneccessarily so that merge performance does not suffer because of the additional write. 3/3 comes with a test now. It's crafted after t7502 and does not work under MINGW, unfortunately. But the fix does :) I'm still asking for comments whether this has other side effects. I've been told there are people who do a lot of merges regularly, and they're not the kind of people that I'd like to make angry - at least not without a good reason. Michael J Gruber (3): Documentation/git-merge: explain --continue merge: split write_merge_state in two merge: save merge state earlier Documentation/git-merge.txt | 5 ++++- builtin/merge.c | 14 +++++++++++--- t/t7600-merge.sh | 15 +++++++++++++++ 3 files changed, 30 insertions(+), 4 deletions(-) -- 2.14.1.364.ge466dba5f7 ^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH v2 1/3] Documentation/git-merge: explain --continue 2017-08-21 12:53 ` [PATCH v2 0/3] Keep merge during kills Michael J Gruber @ 2017-08-21 12:53 ` Michael J Gruber 2017-08-21 16:43 ` Martin Ågren 2017-08-22 0:20 ` [PATCH v2 1/3] Documentation/git-merge: explain --continue Junio C Hamano 2017-08-21 12:53 ` [PATCH v2 2/3] merge: split write_merge_state in two Michael J Gruber 2017-08-21 12:53 ` [PATCH v2 3/3] merge: save merge state earlier Michael J Gruber 2 siblings, 2 replies; 22+ messages in thread From: Michael J Gruber @ 2017-08-21 12:53 UTC (permalink / raw) To: git; +Cc: hIpPy Currently, 'git merge --continue' is mentioned but not explained. Explain it. Signed-off-by: Michael J Gruber <git@grubix.eu> --- Documentation/git-merge.txt | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/Documentation/git-merge.txt b/Documentation/git-merge.txt index 6b308ab6d0..615e6bacde 100644 --- a/Documentation/git-merge.txt +++ b/Documentation/git-merge.txt @@ -288,7 +288,10 @@ After seeing a conflict, you can do two things: * Resolve the conflicts. Git will mark the conflicts in the working tree. Edit the files into shape and - 'git add' them to the index. Use 'git commit' to seal the deal. + 'git add' them to the index. Use 'git commit' or + 'git merge --continue' to seal the deal. The latter command + checks whether there is a (interrupted) merge in progress + before calling 'git commit'. You can work through the conflict with a number of tools: -- 2.14.1.364.ge466dba5f7 ^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [PATCH v2 1/3] Documentation/git-merge: explain --continue 2017-08-21 12:53 ` [PATCH v2 1/3] Documentation/git-merge: explain --continue Michael J Gruber @ 2017-08-21 16:43 ` Martin Ågren 2017-08-22 9:26 ` Michael J Gruber 2017-08-22 0:20 ` [PATCH v2 1/3] Documentation/git-merge: explain --continue Junio C Hamano 1 sibling, 1 reply; 22+ messages in thread From: Martin Ågren @ 2017-08-21 16:43 UTC (permalink / raw) To: Michael J Gruber; +Cc: Git Mailing List, hIpPy On 21 August 2017 at 14:53, Michael J Gruber <git@grubix.eu> wrote: > Currently, 'git merge --continue' is mentioned but not explained. > > Explain it. > > Signed-off-by: Michael J Gruber <git@grubix.eu> > --- > Documentation/git-merge.txt | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/Documentation/git-merge.txt b/Documentation/git-merge.txt > index 6b308ab6d0..615e6bacde 100644 > --- a/Documentation/git-merge.txt > +++ b/Documentation/git-merge.txt > @@ -288,7 +288,10 @@ After seeing a conflict, you can do two things: > > * Resolve the conflicts. Git will mark the conflicts in > the working tree. Edit the files into shape and > - 'git add' them to the index. Use 'git commit' to seal the deal. > + 'git add' them to the index. Use 'git commit' or > + 'git merge --continue' to seal the deal. The latter command > + checks whether there is a (interrupted) merge in progress > + before calling 'git commit'. > > You can work through the conflict with a number of tools: There are actually two things going on here. First, this mentions git merge --continue. Second, it explains what that command does. But the latter is done earlier (not exactly like here, but still). When git merge --continue originally appeared, this part of the docs was discussed briefly. Maybe interesting: https://public-inbox.org/git/xmqq60mn671x.fsf@gitster.mtv.corp.google.com/ Martin ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v2 1/3] Documentation/git-merge: explain --continue 2017-08-21 16:43 ` Martin Ågren @ 2017-08-22 9:26 ` Michael J Gruber 2017-08-22 10:06 ` Martin Ågren 0 siblings, 1 reply; 22+ messages in thread From: Michael J Gruber @ 2017-08-22 9:26 UTC (permalink / raw) To: Martin Ågren; +Cc: Git Mailing List, hIpPy Martin Ågren venit, vidit, dixit 21.08.2017 18:43: > On 21 August 2017 at 14:53, Michael J Gruber <git@grubix.eu> wrote: >> Currently, 'git merge --continue' is mentioned but not explained. >> >> Explain it. >> >> Signed-off-by: Michael J Gruber <git@grubix.eu> >> --- >> Documentation/git-merge.txt | 5 ++++- >> 1 file changed, 4 insertions(+), 1 deletion(-) >> >> diff --git a/Documentation/git-merge.txt b/Documentation/git-merge.txt >> index 6b308ab6d0..615e6bacde 100644 >> --- a/Documentation/git-merge.txt >> +++ b/Documentation/git-merge.txt >> @@ -288,7 +288,10 @@ After seeing a conflict, you can do two things: >> >> * Resolve the conflicts. Git will mark the conflicts in >> the working tree. Edit the files into shape and >> - 'git add' them to the index. Use 'git commit' to seal the deal. >> + 'git add' them to the index. Use 'git commit' or >> + 'git merge --continue' to seal the deal. The latter command >> + checks whether there is a (interrupted) merge in progress >> + before calling 'git commit'. >> >> You can work through the conflict with a number of tools: > > There are actually two things going on here. First, this mentions git > merge --continue. Second, it explains what that command does. But the > latter is done earlier (not exactly like here, but still). I didn't see that explained in the man page at all - on the contrary, I only saw a forward reference (see section...), but then only an explanation of what "resolving" means (including the "git commit"-step). It is unclear to me from the man page which steps of "resolving" the command "git merge --continue" does - you could think it does "git commit -a", for example. > When git merge --continue originally appeared, this part of the docs was > discussed briefly. Maybe interesting: > > https://public-inbox.org/git/xmqq60mn671x.fsf@gitster.mtv.corp.google.com/ > > Martin > ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v2 1/3] Documentation/git-merge: explain --continue 2017-08-22 9:26 ` Michael J Gruber @ 2017-08-22 10:06 ` Martin Ågren 2017-08-22 15:24 ` hIpPy 0 siblings, 1 reply; 22+ messages in thread From: Martin Ågren @ 2017-08-22 10:06 UTC (permalink / raw) To: Michael J Gruber; +Cc: Git Mailing List, hIpPy On 22 August 2017 at 11:26, Michael J Gruber <git@grubix.eu> wrote: > Martin Ågren venit, vidit, dixit 21.08.2017 18:43: >> On 21 August 2017 at 14:53, Michael J Gruber <git@grubix.eu> wrote: >>> Currently, 'git merge --continue' is mentioned but not explained. >>> >>> Explain it. >>> >>> Signed-off-by: Michael J Gruber <git@grubix.eu> >>> --- >>> Documentation/git-merge.txt | 5 ++++- >>> 1 file changed, 4 insertions(+), 1 deletion(-) >>> >>> diff --git a/Documentation/git-merge.txt b/Documentation/git-merge.txt >>> index 6b308ab6d0..615e6bacde 100644 >>> --- a/Documentation/git-merge.txt >>> +++ b/Documentation/git-merge.txt >>> @@ -288,7 +288,10 @@ After seeing a conflict, you can do two things: >>> >>> * Resolve the conflicts. Git will mark the conflicts in >>> the working tree. Edit the files into shape and >>> - 'git add' them to the index. Use 'git commit' to seal the deal. >>> + 'git add' them to the index. Use 'git commit' or >>> + 'git merge --continue' to seal the deal. The latter command >>> + checks whether there is a (interrupted) merge in progress >>> + before calling 'git commit'. >>> >>> You can work through the conflict with a number of tools: >> >> There are actually two things going on here. First, this mentions git >> merge --continue. Second, it explains what that command does. But the >> latter is done earlier (not exactly like here, but still). > > I didn't see that explained in the man page at all - on the contrary, I > only saw a forward reference (see section...), but then only an > explanation of what "resolving" means (including the "git commit"-step). > It is unclear to me from the man page which steps of "resolving" the > command "git merge --continue" does - you could think it does "git > commit -a", for example. That's very true, and your change helps immensely. I thought that once git merge --continue was mentioned, e.g., Use 'git commit' or 'git merge --continue' to seal the deal. or Use 'git commit' to conclude (you can also say 'git merge --continue'). then things are in some sense "complete". But you might be right that further stressing that the latter is basically an alias helps avoid some confusion. "Oh, great, so now I have two commands to choose from -- which one should I be using?" :-) Martin ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v2 1/3] Documentation/git-merge: explain --continue 2017-08-22 10:06 ` Martin Ågren @ 2017-08-22 15:24 ` hIpPy 2017-08-22 16:11 ` Junio C Hamano 0 siblings, 1 reply; 22+ messages in thread From: hIpPy @ 2017-08-22 15:24 UTC (permalink / raw) To: Martin Ågren; +Cc: Michael J Gruber, Git Mailing List I think 'git merge --continue' should be advertised more that 'git commit' as typically one is familiar with 'git rebase --continue' and 'git cherry-pick --continue'. I for a long time did not know I could also use 'git commit' to continue a merge but that's just me. Now, 'git commit' is easier to remember if it works in all cases (merge, rebase, cherry-pick). RM On Tue, Aug 22, 2017 at 3:06 AM, Martin Ågren <martin.agren@gmail.com> wrote: > On 22 August 2017 at 11:26, Michael J Gruber <git@grubix.eu> wrote: >> Martin Ågren venit, vidit, dixit 21.08.2017 18:43: >>> On 21 August 2017 at 14:53, Michael J Gruber <git@grubix.eu> wrote: >>>> Currently, 'git merge --continue' is mentioned but not explained. >>>> >>>> Explain it. >>>> >>>> Signed-off-by: Michael J Gruber <git@grubix.eu> >>>> --- >>>> Documentation/git-merge.txt | 5 ++++- >>>> 1 file changed, 4 insertions(+), 1 deletion(-) >>>> >>>> diff --git a/Documentation/git-merge.txt b/Documentation/git-merge.txt >>>> index 6b308ab6d0..615e6bacde 100644 >>>> --- a/Documentation/git-merge.txt >>>> +++ b/Documentation/git-merge.txt >>>> @@ -288,7 +288,10 @@ After seeing a conflict, you can do two things: >>>> >>>> * Resolve the conflicts. Git will mark the conflicts in >>>> the working tree. Edit the files into shape and >>>> - 'git add' them to the index. Use 'git commit' to seal the deal. >>>> + 'git add' them to the index. Use 'git commit' or >>>> + 'git merge --continue' to seal the deal. The latter command >>>> + checks whether there is a (interrupted) merge in progress >>>> + before calling 'git commit'. >>>> >>>> You can work through the conflict with a number of tools: >>> >>> There are actually two things going on here. First, this mentions git >>> merge --continue. Second, it explains what that command does. But the >>> latter is done earlier (not exactly like here, but still). >> >> I didn't see that explained in the man page at all - on the contrary, I >> only saw a forward reference (see section...), but then only an >> explanation of what "resolving" means (including the "git commit"-step). >> It is unclear to me from the man page which steps of "resolving" the >> command "git merge --continue" does - you could think it does "git >> commit -a", for example. > > That's very true, and your change helps immensely. I thought that once > git merge --continue was mentioned, e.g., > > Use 'git commit' or 'git merge --continue' to seal the deal. > > or > > Use 'git commit' to conclude (you can also say 'git merge > --continue'). > > then things are in some sense "complete". But you might be right that > further stressing that the latter is basically an alias helps avoid some > confusion. "Oh, great, so now I have two commands to choose from -- which > one should I be using?" :-) > > Martin ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v2 1/3] Documentation/git-merge: explain --continue 2017-08-22 15:24 ` hIpPy @ 2017-08-22 16:11 ` Junio C Hamano 2017-08-23 12:10 ` [PATCH v3 0/4] Keep merge during kills Michael J Gruber 0 siblings, 1 reply; 22+ messages in thread From: Junio C Hamano @ 2017-08-22 16:11 UTC (permalink / raw) To: hIpPy; +Cc: Martin Ågren, Michael J Gruber, Git Mailing List hIpPy <hippy2981@gmail.com> writes: > I think 'git merge --continue' should be advertised more that 'git > commit' as typically one is familiar with 'git rebase --continue' and > 'git cherry-pick --continue'. I for a long time did not know I could > also use 'git commit' to continue a merge but that's just me. Perhaps. "rebase" and "am" (and range pick "cherry-pick A..B") are operations that works on more than one change, so it makes perfect sense to have a way to say "I am done with this step. Please go on and do the rest". There is no "go on and do the rest" after resolving a conflicted merge, as "merge" is, unlike the other ones to which "--continue" legitimately has a reasonable meaning, an operation that does just one thing. From that point of view, "git merge --continue" is a mistaken UI that shouldn't have happened. ^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH v3 0/4] Keep merge during kills 2017-08-22 16:11 ` Junio C Hamano @ 2017-08-23 12:10 ` Michael J Gruber 2017-08-23 12:10 ` [PATCH v3 1/4] Documentation/git-merge: explain --continue Michael J Gruber ` (3 more replies) 0 siblings, 4 replies; 22+ messages in thread From: Michael J Gruber @ 2017-08-23 12:10 UTC (permalink / raw) To: git; +Cc: hIpPy Compared to the 3-item v2: 1/4 == 1/3 2/4 is new as per Junio's suggestion: clarify the call-chain leading up to prepare_to_commit() 3/4 == 2/3 with amended subject 4/4 is 3/3 rebased, squash-if removed Michael J Gruber (4): Documentation/git-merge: explain --continue merge: clarify call chain merge: split write_merge_state in two merge: save merge state earlier Documentation/git-merge.txt | 5 ++++- builtin/merge.c | 15 ++++++++++++--- t/t7600-merge.sh | 15 +++++++++++++++ 3 files changed, 31 insertions(+), 4 deletions(-) -- 2.14.1.426.g4352aa77a5 ^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH v3 1/4] Documentation/git-merge: explain --continue 2017-08-23 12:10 ` [PATCH v3 0/4] Keep merge during kills Michael J Gruber @ 2017-08-23 12:10 ` Michael J Gruber 2017-08-23 12:10 ` [PATCH v3 2/4] merge: clarify call chain Michael J Gruber ` (2 subsequent siblings) 3 siblings, 0 replies; 22+ messages in thread From: Michael J Gruber @ 2017-08-23 12:10 UTC (permalink / raw) To: git; +Cc: hIpPy Currently, 'git merge --continue' is mentioned but not explained. Explain it. Signed-off-by: Michael J Gruber <git@grubix.eu> --- Documentation/git-merge.txt | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/Documentation/git-merge.txt b/Documentation/git-merge.txt index 6b308ab6d0..615e6bacde 100644 --- a/Documentation/git-merge.txt +++ b/Documentation/git-merge.txt @@ -288,7 +288,10 @@ After seeing a conflict, you can do two things: * Resolve the conflicts. Git will mark the conflicts in the working tree. Edit the files into shape and - 'git add' them to the index. Use 'git commit' to seal the deal. + 'git add' them to the index. Use 'git commit' or + 'git merge --continue' to seal the deal. The latter command + checks whether there is a (interrupted) merge in progress + before calling 'git commit'. You can work through the conflict with a number of tools: -- 2.14.1.426.g4352aa77a5 ^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH v3 2/4] merge: clarify call chain 2017-08-23 12:10 ` [PATCH v3 0/4] Keep merge during kills Michael J Gruber 2017-08-23 12:10 ` [PATCH v3 1/4] Documentation/git-merge: explain --continue Michael J Gruber @ 2017-08-23 12:10 ` Michael J Gruber 2017-08-23 12:10 ` [PATCH v3 3/4] merge: split write_merge_state in two Michael J Gruber 2017-08-23 12:10 ` [PATCH v3 4/4] merge: save merge state earlier Michael J Gruber 3 siblings, 0 replies; 22+ messages in thread From: Michael J Gruber @ 2017-08-23 12:10 UTC (permalink / raw) To: git; +Cc: hIpPy prepare_to_commit() cannot be reached in the non-squash case: It is called by merge_trivial() and finish_automerge() only, but the calls to the latter are somewhat hard to track: If option_commit is not set, the code in cmd_merge() uses a fake conflict return code (ret=1) to avoid writing the tree, which also avoids setting automerge_was_ok (just as in the proper ret==1 case), so that finish_automerge() is not called. To ensure that no code change breaks that assumption, safe-guard prepare_to_commit() by a BUG() statement. Suggested-by: junio Signed-off-by: Michael J Gruber <git@grubix.eu> --- builtin/merge.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/builtin/merge.c b/builtin/merge.c index cc57052993..dafec80fa9 100644 --- a/builtin/merge.c +++ b/builtin/merge.c @@ -763,6 +763,8 @@ static void prepare_to_commit(struct commit_list *remoteheads) struct strbuf msg = STRBUF_INIT; strbuf_addbuf(&msg, &merge_msg); strbuf_addch(&msg, '\n'); + if (squash) + BUG("the control must not reach here under --squash"); if (0 < option_edit) strbuf_commented_addf(&msg, _(merge_editor_comment), comment_line_char); if (signoff) -- 2.14.1.426.g4352aa77a5 ^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH v3 3/4] merge: split write_merge_state in two 2017-08-23 12:10 ` [PATCH v3 0/4] Keep merge during kills Michael J Gruber 2017-08-23 12:10 ` [PATCH v3 1/4] Documentation/git-merge: explain --continue Michael J Gruber 2017-08-23 12:10 ` [PATCH v3 2/4] merge: clarify call chain Michael J Gruber @ 2017-08-23 12:10 ` Michael J Gruber 2017-08-23 12:10 ` [PATCH v3 4/4] merge: save merge state earlier Michael J Gruber 3 siblings, 0 replies; 22+ messages in thread From: Michael J Gruber @ 2017-08-23 12:10 UTC (permalink / raw) To: git; +Cc: hIpPy write_merge_state() writes out the merge heads, mode, and msg. But we may want to write out heads, mode without the msg. So, split out heads (+mode) into a separate function write_merge_heads() that is called by write_merge_state(). No funtional change so far, except when these non-atomic writes are interrupted: we write heads-mode-msg now when we used to write heads-msg-mode. Signed-off-by: Michael J Gruber <git@grubix.eu> --- builtin/merge.c | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/builtin/merge.c b/builtin/merge.c index dafec80fa9..db3335b3bf 100644 --- a/builtin/merge.c +++ b/builtin/merge.c @@ -910,7 +910,7 @@ static int setup_with_upstream(const char ***argv) return i; } -static void write_merge_state(struct commit_list *remoteheads) +static void write_merge_heads(struct commit_list *remoteheads) { struct commit_list *j; struct strbuf buf = STRBUF_INIT; @@ -926,8 +926,6 @@ static void write_merge_state(struct commit_list *remoteheads) strbuf_addf(&buf, "%s\n", oid_to_hex(oid)); } write_file_buf(git_path_merge_head(), buf.buf, buf.len); - strbuf_addch(&merge_msg, '\n'); - write_file_buf(git_path_merge_msg(), merge_msg.buf, merge_msg.len); strbuf_reset(&buf); if (fast_forward == FF_NO) @@ -935,6 +933,13 @@ static void write_merge_state(struct commit_list *remoteheads) write_file_buf(git_path_merge_mode(), buf.buf, buf.len); } +static void write_merge_state(struct commit_list *remoteheads) +{ + write_merge_heads(remoteheads); + strbuf_addch(&merge_msg, '\n'); + write_file_buf(git_path_merge_msg(), merge_msg.buf, merge_msg.len); +} + static int default_edit_option(void) { static const char name[] = "GIT_MERGE_AUTOEDIT"; -- 2.14.1.426.g4352aa77a5 ^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH v3 4/4] merge: save merge state earlier 2017-08-23 12:10 ` [PATCH v3 0/4] Keep merge during kills Michael J Gruber ` (2 preceding siblings ...) 2017-08-23 12:10 ` [PATCH v3 3/4] merge: split write_merge_state in two Michael J Gruber @ 2017-08-23 12:10 ` Michael J Gruber 3 siblings, 0 replies; 22+ messages in thread From: Michael J Gruber @ 2017-08-23 12:10 UTC (permalink / raw) To: git; +Cc: hIpPy If the `git merge` process is killed while waiting for the editor to finish, the merge state is lost but the prepared merge msg and tree is kept. So, a subsequent `git commit` creates a squashed merge even when the user asked for proper merge commit originally. Demonstrate the problem with a test crafted after the in t7502. The test requires EXECKEEPSPID (thus does not run under MINGW). Save the merge state earlier (in the non-squash case) so that it does not get lost. This makes the test pass. Reported-by: hIpPy <hippy2981@gmail.com> Signed-off-by: Michael J Gruber <git@grubix.eu> --- builtin/merge.c | 2 ++ t/t7600-merge.sh | 15 +++++++++++++++ 2 files changed, 17 insertions(+) diff --git a/builtin/merge.c b/builtin/merge.c index db3335b3bf..2d5c45a904 100644 --- a/builtin/merge.c +++ b/builtin/merge.c @@ -758,6 +758,7 @@ N_("Please enter a commit message to explain why this merge is necessary,\n" "Lines starting with '%c' will be ignored, and an empty message aborts\n" "the commit.\n"); +static void write_merge_heads(struct commit_list *); static void prepare_to_commit(struct commit_list *remoteheads) { struct strbuf msg = STRBUF_INIT; @@ -769,6 +770,7 @@ static void prepare_to_commit(struct commit_list *remoteheads) strbuf_commented_addf(&msg, _(merge_editor_comment), comment_line_char); if (signoff) append_signoff(&msg, ignore_non_trailer(msg.buf, msg.len), 0); + write_merge_heads(remoteheads); write_file_buf(git_path_merge_msg(), msg.buf, msg.len); if (run_commit_hook(0 < option_edit, get_index_file(), "prepare-commit-msg", git_path_merge_msg(), "merge", NULL)) diff --git a/t/t7600-merge.sh b/t/t7600-merge.sh index 2ebda509ac..80194b79f9 100755 --- a/t/t7600-merge.sh +++ b/t/t7600-merge.sh @@ -774,4 +774,19 @@ test_expect_success 'merge can be completed with --continue' ' verify_parents $c0 $c1 ' +write_script .git/FAKE_EDITOR <<EOF +# kill -TERM command added below. +EOF + +test_expect_success EXECKEEPSPID 'killed merge can be completed with --continue' ' + git reset --hard c0 && + ! "$SHELL_PATH" -c '\'' + echo kill -TERM $$ >> .git/FAKE_EDITOR + GIT_EDITOR=.git/FAKE_EDITOR + export GIT_EDITOR + exec git merge --no-ff --edit c1'\'' && + git merge --continue && + verify_parents $c0 $c1 +' + test_done -- 2.14.1.426.g4352aa77a5 ^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [PATCH v2 1/3] Documentation/git-merge: explain --continue 2017-08-21 12:53 ` [PATCH v2 1/3] Documentation/git-merge: explain --continue Michael J Gruber 2017-08-21 16:43 ` Martin Ågren @ 2017-08-22 0:20 ` Junio C Hamano 1 sibling, 0 replies; 22+ messages in thread From: Junio C Hamano @ 2017-08-22 0:20 UTC (permalink / raw) To: Michael J Gruber; +Cc: git, hIpPy Michael J Gruber <git@grubix.eu> writes: > Currently, 'git merge --continue' is mentioned but not explained. > > Explain it. > > Signed-off-by: Michael J Gruber <git@grubix.eu> > --- > Documentation/git-merge.txt | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/Documentation/git-merge.txt b/Documentation/git-merge.txt > index 6b308ab6d0..615e6bacde 100644 > --- a/Documentation/git-merge.txt > +++ b/Documentation/git-merge.txt > @@ -288,7 +288,10 @@ After seeing a conflict, you can do two things: > > * Resolve the conflicts. Git will mark the conflicts in > the working tree. Edit the files into shape and > - 'git add' them to the index. Use 'git commit' to seal the deal. > + 'git add' them to the index. Use 'git commit' or > + 'git merge --continue' to seal the deal. The latter command > + checks whether there is a (interrupted) merge in progress > + before calling 'git commit'. > > You can work through the conflict with a number of tools: Thanks, looks good. Will queue. ^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH v2 2/3] merge: split write_merge_state in two 2017-08-21 12:53 ` [PATCH v2 0/3] Keep merge during kills Michael J Gruber 2017-08-21 12:53 ` [PATCH v2 1/3] Documentation/git-merge: explain --continue Michael J Gruber @ 2017-08-21 12:53 ` Michael J Gruber 2017-08-22 0:20 ` Junio C Hamano 2017-08-21 12:53 ` [PATCH v2 3/3] merge: save merge state earlier Michael J Gruber 2 siblings, 1 reply; 22+ messages in thread From: Michael J Gruber @ 2017-08-21 12:53 UTC (permalink / raw) To: git; +Cc: hIpPy write_merge_state() writes out the merge heads, mode, and msg. But we may want to write out heads, mode without the msg. So, split out heads (+mode) into a separate function write_merge_heads() that is called by write_merge_state(). No funtional change so far. Signed-off-by: Michael J Gruber <git@grubix.eu> --- builtin/merge.c | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/builtin/merge.c b/builtin/merge.c index cc57052993..86f0adde3b 100644 --- a/builtin/merge.c +++ b/builtin/merge.c @@ -908,7 +908,7 @@ static int setup_with_upstream(const char ***argv) return i; } -static void write_merge_state(struct commit_list *remoteheads) +static void write_merge_heads(struct commit_list *remoteheads) { struct commit_list *j; struct strbuf buf = STRBUF_INIT; @@ -924,8 +924,6 @@ static void write_merge_state(struct commit_list *remoteheads) strbuf_addf(&buf, "%s\n", oid_to_hex(oid)); } write_file_buf(git_path_merge_head(), buf.buf, buf.len); - strbuf_addch(&merge_msg, '\n'); - write_file_buf(git_path_merge_msg(), merge_msg.buf, merge_msg.len); strbuf_reset(&buf); if (fast_forward == FF_NO) @@ -933,6 +931,13 @@ static void write_merge_state(struct commit_list *remoteheads) write_file_buf(git_path_merge_mode(), buf.buf, buf.len); } +static void write_merge_state(struct commit_list *remoteheads) +{ + write_merge_heads(remoteheads); + strbuf_addch(&merge_msg, '\n'); + write_file_buf(git_path_merge_msg(), merge_msg.buf, merge_msg.len); +} + static int default_edit_option(void) { static const char name[] = "GIT_MERGE_AUTOEDIT"; -- 2.14.1.364.ge466dba5f7 ^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [PATCH v2 2/3] merge: split write_merge_state in two 2017-08-21 12:53 ` [PATCH v2 2/3] merge: split write_merge_state in two Michael J Gruber @ 2017-08-22 0:20 ` Junio C Hamano 0 siblings, 0 replies; 22+ messages in thread From: Junio C Hamano @ 2017-08-22 0:20 UTC (permalink / raw) To: Michael J Gruber; +Cc: git, hIpPy Michael J Gruber <git@grubix.eu> writes: > write_merge_state() writes out the merge heads, mode, and msg. But we > may want to write out heads, mode without the msg. So, split out heads > (+mode) into a separate function write_merge_heads() that is called by > write_merge_state(). > > No funtional change so far. "We may want to" leaves readers in suspense. Hopefully 3/3 makes it clear why we may in what situation. One thing this changes is that we used to write merge-heads first, then message and then finally mode, but now we write merge-heads, then mode and finally message. So killing us in the middle may leave different set of files on the filesystem. It is unclear from the above description and the patch text if that is a potential issue. Let's keep reading ;-) Thanks. > > Signed-off-by: Michael J Gruber <git@grubix.eu> > --- > builtin/merge.c | 11 ++++++++--- > 1 file changed, 8 insertions(+), 3 deletions(-) > > diff --git a/builtin/merge.c b/builtin/merge.c > index cc57052993..86f0adde3b 100644 > --- a/builtin/merge.c > +++ b/builtin/merge.c > @@ -908,7 +908,7 @@ static int setup_with_upstream(const char ***argv) > return i; > } > > -static void write_merge_state(struct commit_list *remoteheads) > +static void write_merge_heads(struct commit_list *remoteheads) > { > struct commit_list *j; > struct strbuf buf = STRBUF_INIT; > @@ -924,8 +924,6 @@ static void write_merge_state(struct commit_list *remoteheads) > strbuf_addf(&buf, "%s\n", oid_to_hex(oid)); > } > write_file_buf(git_path_merge_head(), buf.buf, buf.len); > - strbuf_addch(&merge_msg, '\n'); > - write_file_buf(git_path_merge_msg(), merge_msg.buf, merge_msg.len); > > strbuf_reset(&buf); > if (fast_forward == FF_NO) > @@ -933,6 +931,13 @@ static void write_merge_state(struct commit_list *remoteheads) > write_file_buf(git_path_merge_mode(), buf.buf, buf.len); > } > > +static void write_merge_state(struct commit_list *remoteheads) > +{ > + write_merge_heads(remoteheads); > + strbuf_addch(&merge_msg, '\n'); > + write_file_buf(git_path_merge_msg(), merge_msg.buf, merge_msg.len); > +} > + > static int default_edit_option(void) > { > static const char name[] = "GIT_MERGE_AUTOEDIT"; ^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH v2 3/3] merge: save merge state earlier 2017-08-21 12:53 ` [PATCH v2 0/3] Keep merge during kills Michael J Gruber 2017-08-21 12:53 ` [PATCH v2 1/3] Documentation/git-merge: explain --continue Michael J Gruber 2017-08-21 12:53 ` [PATCH v2 2/3] merge: split write_merge_state in two Michael J Gruber @ 2017-08-21 12:53 ` Michael J Gruber 2017-08-22 0:38 ` Junio C Hamano 2 siblings, 1 reply; 22+ messages in thread From: Michael J Gruber @ 2017-08-21 12:53 UTC (permalink / raw) To: git; +Cc: hIpPy If the `git merge` process is killed while waiting for the editor to finish, the merge state is lost but the prepared merge msg and tree is kept. So, a subsequent `git commit` creates a squashed merge even when the user asked for proper merge commit originally. Save the merge state earlier (in the non-squash case) so that it does not get lost. Reported-by: hIpPy <hippy2981@gmail.com> Signed-off-by: Michael J Gruber <git@grubix.eu> --- builtin/merge.c | 3 +++ t/t7600-merge.sh | 15 +++++++++++++++ 2 files changed, 18 insertions(+) diff --git a/builtin/merge.c b/builtin/merge.c index 86f0adde3b..5379b08824 100644 --- a/builtin/merge.c +++ b/builtin/merge.c @@ -758,6 +758,7 @@ N_("Please enter a commit message to explain why this merge is necessary,\n" "Lines starting with '%c' will be ignored, and an empty message aborts\n" "the commit.\n"); +static void write_merge_heads(struct commit_list *); static void prepare_to_commit(struct commit_list *remoteheads) { struct strbuf msg = STRBUF_INIT; @@ -767,6 +768,8 @@ static void prepare_to_commit(struct commit_list *remoteheads) strbuf_commented_addf(&msg, _(merge_editor_comment), comment_line_char); if (signoff) append_signoff(&msg, ignore_non_trailer(msg.buf, msg.len), 0); + if (!squash) + write_merge_heads(remoteheads); write_file_buf(git_path_merge_msg(), msg.buf, msg.len); if (run_commit_hook(0 < option_edit, get_index_file(), "prepare-commit-msg", git_path_merge_msg(), "merge", NULL)) diff --git a/t/t7600-merge.sh b/t/t7600-merge.sh index 2ebda509ac..80194b79f9 100755 --- a/t/t7600-merge.sh +++ b/t/t7600-merge.sh @@ -774,4 +774,19 @@ test_expect_success 'merge can be completed with --continue' ' verify_parents $c0 $c1 ' +write_script .git/FAKE_EDITOR <<EOF +# kill -TERM command added below. +EOF + +test_expect_success EXECKEEPSPID 'killed merge can be completed with --continue' ' + git reset --hard c0 && + ! "$SHELL_PATH" -c '\'' + echo kill -TERM $$ >> .git/FAKE_EDITOR + GIT_EDITOR=.git/FAKE_EDITOR + export GIT_EDITOR + exec git merge --no-ff --edit c1'\'' && + git merge --continue && + verify_parents $c0 $c1 +' + test_done -- 2.14.1.364.ge466dba5f7 ^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [PATCH v2 3/3] merge: save merge state earlier 2017-08-21 12:53 ` [PATCH v2 3/3] merge: save merge state earlier Michael J Gruber @ 2017-08-22 0:38 ` Junio C Hamano 2017-08-22 9:36 ` Michael J Gruber 0 siblings, 1 reply; 22+ messages in thread From: Junio C Hamano @ 2017-08-22 0:38 UTC (permalink / raw) To: Michael J Gruber; +Cc: git, hIpPy Michael J Gruber <git@grubix.eu> writes: > static void prepare_to_commit(struct commit_list *remoteheads) > { > struct strbuf msg = STRBUF_INIT; > @@ -767,6 +768,8 @@ static void prepare_to_commit(struct commit_list *remoteheads) > strbuf_commented_addf(&msg, _(merge_editor_comment), comment_line_char); > if (signoff) > append_signoff(&msg, ignore_non_trailer(msg.buf, msg.len), 0); > + if (!squash) > + write_merge_heads(remoteheads); > write_file_buf(git_path_merge_msg(), msg.buf, msg.len); > if (run_commit_hook(0 < option_edit, get_index_file(), "prepare-commit-msg", > git_path_merge_msg(), "merge", NULL)) I can understand that you would never want to write out MERGE_HEAD while squashing, but I somehow think it would be a bug in the caller to call prepare_to_commit(), whose point is to prepare the merge message to be recorded in the resulting merge commit, when the user gave us the "--squash" option, which is an explicit instruction that the user does not want the merge commit the message is used. Can squash ever be true in this function? This function has two callsites: merge_trivial() and finish_automerge(). I think merge_trivial() will not be called under "--squash", which turns option_commit off and the only callsite of it is inside an else-if clause that requres option_commit to be true. You can do a similar deduction around the "automerge_was_ok" variable to see if finish_automerge() can be called when "--squash" is given; I suspect the answer may be no. > diff --git a/t/t7600-merge.sh b/t/t7600-merge.sh > index 2ebda509ac..80194b79f9 100755 > --- a/t/t7600-merge.sh > +++ b/t/t7600-merge.sh > @@ -774,4 +774,19 @@ test_expect_success 'merge can be completed with --continue' ' > verify_parents $c0 $c1 > ' > > +write_script .git/FAKE_EDITOR <<EOF > +# kill -TERM command added below. > +EOF > + > +test_expect_success EXECKEEPSPID 'killed merge can be completed with --continue' ' > + git reset --hard c0 && > + ! "$SHELL_PATH" -c '\'' > + echo kill -TERM $$ >> .git/FAKE_EDITOR > + GIT_EDITOR=.git/FAKE_EDITOR > + export GIT_EDITOR > + exec git merge --no-ff --edit c1'\'' && This is a tricky construct. You "reserve" a process ID by using a shell, arrange it to be killed and then using "exec" to make it the "git merge" program to be killed. I kind of like the convolutedness. > + git merge --continue && > + verify_parents $c0 $c1 > +' > + > test_done ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v2 3/3] merge: save merge state earlier 2017-08-22 0:38 ` Junio C Hamano @ 2017-08-22 9:36 ` Michael J Gruber 2017-08-22 16:03 ` Junio C Hamano 0 siblings, 1 reply; 22+ messages in thread From: Michael J Gruber @ 2017-08-22 9:36 UTC (permalink / raw) To: Junio C Hamano; +Cc: git, hIpPy Junio C Hamano venit, vidit, dixit 22.08.2017 02:38: > Michael J Gruber <git@grubix.eu> writes: > >> static void prepare_to_commit(struct commit_list *remoteheads) >> { >> struct strbuf msg = STRBUF_INIT; >> @@ -767,6 +768,8 @@ static void prepare_to_commit(struct commit_list *remoteheads) >> strbuf_commented_addf(&msg, _(merge_editor_comment), comment_line_char); >> if (signoff) >> append_signoff(&msg, ignore_non_trailer(msg.buf, msg.len), 0); >> + if (!squash) >> + write_merge_heads(remoteheads); >> write_file_buf(git_path_merge_msg(), msg.buf, msg.len); >> if (run_commit_hook(0 < option_edit, get_index_file(), "prepare-commit-msg", >> git_path_merge_msg(), "merge", NULL)) > > I can understand that you would never want to write out MERGE_HEAD > while squashing, but I somehow think it would be a bug in the caller > to call prepare_to_commit(), whose point is to prepare the merge > message to be recorded in the resulting merge commit, when the user > gave us the "--squash" option, which is an explicit instruction that > the user does not want the merge commit the message is used. That sounds reasonable. I vaguely remember a failing test for an earlier version that I tried, but that was before the "split". > Can squash ever be true in this function? > > This function has two callsites: merge_trivial() and > finish_automerge(). > > I think merge_trivial() will not be called under "--squash", which > turns option_commit off and the only callsite of it is inside an > else-if clause that requres option_commit to be true. You can do a > similar deduction around the "automerge_was_ok" variable to see if > finish_automerge() can be called when "--squash" is given; I suspect > the answer may be no. I'll go without the if, after more testing. >> diff --git a/t/t7600-merge.sh b/t/t7600-merge.sh >> index 2ebda509ac..80194b79f9 100755 >> --- a/t/t7600-merge.sh >> +++ b/t/t7600-merge.sh >> @@ -774,4 +774,19 @@ test_expect_success 'merge can be completed with --continue' ' >> verify_parents $c0 $c1 >> ' >> >> +write_script .git/FAKE_EDITOR <<EOF >> +# kill -TERM command added below. >> +EOF >> + >> +test_expect_success EXECKEEPSPID 'killed merge can be completed with --continue' ' >> + git reset --hard c0 && >> + ! "$SHELL_PATH" -c '\'' >> + echo kill -TERM $$ >> .git/FAKE_EDITOR >> + GIT_EDITOR=.git/FAKE_EDITOR >> + export GIT_EDITOR >> + exec git merge --no-ff --edit c1'\'' && > > This is a tricky construct. You "reserve" a process ID by using a > shell, arrange it to be killed and then using "exec" to make it the > "git merge" program to be killed. I kind of like the convolutedness. That is from t7502. Sorry for hiding that note in the cover letter, I should put it into 3/3's message or a test comment. When testing, I simply used "git merge... &" and "ps" or "jobs" to know which process to kill. Apparantly, the above is the most portable way to script that. t7502 went through a few iterations to ensure this. >> + git merge --continue && >> + verify_parents $c0 $c1 >> +' >> + >> test_done ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v2 3/3] merge: save merge state earlier 2017-08-22 9:36 ` Michael J Gruber @ 2017-08-22 16:03 ` Junio C Hamano 0 siblings, 0 replies; 22+ messages in thread From: Junio C Hamano @ 2017-08-22 16:03 UTC (permalink / raw) To: Michael J Gruber; +Cc: git, hIpPy Michael J Gruber <michael@grubix.eu> writes: >> Can squash ever be true in this function? >> >> This function has two callsites: merge_trivial() and >> finish_automerge(). >> >> I think merge_trivial() will not be called under "--squash", which >> turns option_commit off and the only callsite of it is inside an >> else-if clause that requres option_commit to be true. You can do a >> similar deduction around the "automerge_was_ok" variable to see if >> finish_automerge() can be called when "--squash" is given; I suspect >> the answer may be no. > > I'll go without the if, after more testing. I was sort of expecting that tracing the control flow would give us the definite answer and that would be much better than any amount of testing. In any case, I wasn't even suggesting to remove "if". It might even be worth doing if (squash) BUG("the control must not reach here under --squash"); write_emrge_heads(...); if we know the control does not have to reach with "--squash" in today's code, so that future careless refactoring does not break this fix. ^ permalink raw reply [flat|nested] 22+ messages in thread
end of thread, other threads:[~2017-08-23 12:11 UTC | newest] Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2017-08-18 22:35 Git makes a merge commit but as a normal (non-merge) commit hIpPy 2017-08-21 10:03 ` Michael J Gruber 2017-08-21 10:06 ` [PATCH] merge: save merge state earlier Michael J Gruber 2017-08-21 12:53 ` [PATCH v2 0/3] Keep merge during kills Michael J Gruber 2017-08-21 12:53 ` [PATCH v2 1/3] Documentation/git-merge: explain --continue Michael J Gruber 2017-08-21 16:43 ` Martin Ågren 2017-08-22 9:26 ` Michael J Gruber 2017-08-22 10:06 ` Martin Ågren 2017-08-22 15:24 ` hIpPy 2017-08-22 16:11 ` Junio C Hamano 2017-08-23 12:10 ` [PATCH v3 0/4] Keep merge during kills Michael J Gruber 2017-08-23 12:10 ` [PATCH v3 1/4] Documentation/git-merge: explain --continue Michael J Gruber 2017-08-23 12:10 ` [PATCH v3 2/4] merge: clarify call chain Michael J Gruber 2017-08-23 12:10 ` [PATCH v3 3/4] merge: split write_merge_state in two Michael J Gruber 2017-08-23 12:10 ` [PATCH v3 4/4] merge: save merge state earlier Michael J Gruber 2017-08-22 0:20 ` [PATCH v2 1/3] Documentation/git-merge: explain --continue Junio C Hamano 2017-08-21 12:53 ` [PATCH v2 2/3] merge: split write_merge_state in two Michael J Gruber 2017-08-22 0:20 ` Junio C Hamano 2017-08-21 12:53 ` [PATCH v2 3/3] merge: save merge state earlier Michael J Gruber 2017-08-22 0:38 ` Junio C Hamano 2017-08-22 9:36 ` Michael J Gruber 2017-08-22 16:03 ` Junio C Hamano
Code repositories for project(s) associated with this public inbox https://80x24.org/mirrors/git.git This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).