On 03.01.2022 08:51, Elijah Newren wrote:
On Mon, Jan 3, 2022 at 4:31 AM Fabian Stelzer <fs@xxxxxxxxxxxx> wrote:
On 31.12.2021 05:04, Elijah Newren via GitGitGadget wrote:
>From: Elijah Newren <newren@xxxxxxxxx>
[...]
>
> static int real_merge(struct merge_tree_options *o,
>@@ -442,8 +443,15 @@ static int real_merge(struct merge_tree_options *o,
> */
>
> merge_incore_recursive(&opt, merge_bases, parent1, parent2, &result);
>+
>+ if (o->messages_file) {
>+ FILE *fp = xfopen(o->messages_file, "w");
>+ merge_display_update_messages(&opt, &result, fp);
>+ fclose(fp);
I don't know enough about how merge-ort works internally, but it looks to me
like at this point the merge already happened and we just didn't clean up
(finalize) yet. It feels wrong to die() at this point just because we can't
open messages_file.
Yes, the merge already happened; there now exists a new toplevel tree
(that nothing references). I'm not sure I understand what's wrong
with die'ing here, though. I can't tell if you want to defer the
die-ing until later, or just avoid the die-ing and return some kind of
success despite failing to complete what the user requested.
I think i would prefer the merge operation to abort before actually merging
when not being able to write its logfile. Otherwise we possibly do a whole
lot of work that`s inaccessible afterwards isn't it? (since we don`t print
the hash)
Thanks for your work on this feature. I think this could open a lot of new
possibilities.
