git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH 2/5] merge-tree: remove redundant code

From
PWPhillip Wood <phillip.wood123@gmail.com>
Date
Feb 18, 2025, 10:01 UTC
Message-ID
<06582f75-c14c-482f-a9d8-b2c2e77ce2c8@gmail.com>
In-Reply-To
<CABPp-BGTSciJMRWBGe9qOFh5wGuLppB6L+v9J5-KVdbNc6H3Hw@mail.gmail.com>
Hi Elijah
On 17/02/2025 20:15, Elijah Newren wrote:
Show 22 quoted lines
> On Sun, Feb 16, 2025 at 8:37 AM Phillip Wood via GitGitGadget
> <gitgitgadget@gmail.com> wrote:
>>
>> From: Phillip Wood <phillip.wood@dunelm.org.uk>
>>
>> real_merge() only ever returns "0" or "1" as it dies if the merge status
>> is less than zero. Therefore the check for "result < 0" is redundant and
>> the result variable is not needed.
> 
> Indeed, the only return statement in real_merge(), occurring on the
> last line of the function, is even:
>      return !result.clean; /* result.clean < 0 handled above */
> 
> However, it might be worth adding to the commit message some comments
> about o->use_stdin here.  When o->use_stdin is true, that the program
> exit status is 0 for both successful merges and conflicts but the
> conflict status for each individual commit is printed as part of the
> output.  As such, the return status isn't used in those cases and
> real_merge() might as well be a void function.  However, when
> o->use_stdin is false, the exit status from real_merge is used, which
> is why that callsite (not visibile in this patch since it is
> unmodified) still pays attention to real_merge()'s return status.
That's a good suggestion - I'll re-roll
Thanks
Phillip
Show 41 quoted lines
>> Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
>> ---
>>   builtin/merge-tree.c | 7 ++-----
>>   1 file changed, 2 insertions(+), 5 deletions(-)
>>
>> diff --git a/builtin/merge-tree.c b/builtin/merge-tree.c
>> index 57f4340faba..3c73482f2b0 100644
>> --- a/builtin/merge-tree.c
>> +++ b/builtin/merge-tree.c
>> @@ -601,7 +601,6 @@ int cmd_merge_tree(int argc,
>>                  line_termination = '\0';
>>                  while (strbuf_getline_lf(&buf, stdin) != EOF) {
>>                          struct strbuf **split;
>> -                       int result;
>>                          const char *input_merge_base = NULL;
>>
>>                          split = strbuf_split(&buf, ' ');
>> @@ -618,16 +617,14 @@ int cmd_merge_tree(int argc,
>>                          if (input_merge_base && split[2] && split[3] && !split[4]) {
>>                                  strbuf_rtrim(split[2]);
>>                                  strbuf_rtrim(split[3]);
>> -                               result = real_merge(&o, input_merge_base, split[2]->buf, split[3]->buf, prefix);
>> +                               real_merge(&o, input_merge_base, split[2]->buf, split[3]->buf, prefix);
>>                          } else if (!input_merge_base && !split[2]) {
>> -                               result = real_merge(&o, NULL, split[0]->buf, split[1]->buf, prefix);
>> +                               real_merge(&o, NULL, split[0]->buf, split[1]->buf, prefix);
>>                          } else {
>>                                  die(_("malformed input line: '%s'."), buf.buf);
>>                          }
>>                          maybe_flush_or_die(stdout, "stdout");
>>
>> -                       if (result < 0)
>> -                               die(_("merging cannot continue; got unclean result of %d"), result);
>>                          strbuf_list_free(split);
>>                  }
>>                  strbuf_release(&buf);
>> --
>> gitgitgadget
> 
> Looks good.
> 
Previous: Elijah NewrenNext: Phillip Wood via GitGitGadget
Message 4 of 25 in “merge-tree --stdin: flush stdout”
  1. 0/5 merge-tree --stdin: flush stdoutPhillip Wood via GitGitGadget, Feb 16, 2025
  2. 2/5 merge-tree: remove redundant codePhillip Wood via GitGitGadget, Feb 16, 2025
  3. Elijah NewrenFeb 17, 2025
  4. Phillip WoodFeb 18, 2025
  5. 1/5 merge-tree --stdin: flush stdout to avoid deadlockPhillip Wood via GitGitGadget, Feb 16, 2025
  6. Elijah NewrenFeb 17, 2025
  7. 3/5 merge-tree: only use basic merge configPhillip Wood via GitGitGadget, Feb 16, 2025
  8. 4/5 merge-tree: improve docs for --stdinPhillip Wood via GitGitGadget, Feb 16, 2025
  9. Elijah NewrenFeb 17, 2025
  10. Phillip WoodFeb 18, 2025
  11. Elijah NewrenFeb 18, 2025
  12. 5/5 merge-tree: fix link formatting in html docsPhillip Wood via GitGitGadget, Feb 16, 2025
  13. Elijah NewrenFeb 17, 2025
  14. 0/5 merge-tree --stdin: flush stdoutPhillip Wood via GitGitGadget, Feb 18, 2025
  15. 1/5 merge-tree --stdin: flush stdout to avoid deadlockPhillip Wood via GitGitGadget, Feb 18, 2025
  16. Patrick SteinhardtFeb 19, 2025
  17. Phillip WoodFeb 19, 2025
  18. Junio C HamanoFeb 19, 2025
  19. 2/5 merge-tree: remove redundant codePhillip Wood via GitGitGadget, Feb 18, 2025
  20. 3/5 merge-tree: only use basic merge configPhillip Wood via GitGitGadget, Feb 18, 2025
  21. 4/5 merge-tree: improve docs for --stdinPhillip Wood via GitGitGadget, Feb 18, 2025
  22. 5/5 merge-tree: fix link formatting in html docsPhillip Wood via GitGitGadget, Feb 18, 2025
  23. Elijah NewrenFeb 18, 2025
  24. Phillip WoodFeb 18, 2025
  25. Junio C HamanoFeb 18, 2025

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.