git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Elijah Newren <newren@gmail.com>
Cc: Johannes Schindelin via GitGitGadget <gitgitgadget@gmail.com>,
	Git Mailing List <git@vger.kernel.org>,
	Taylor Blau <me@ttaylorr.com>,
	Johannes Schindelin <johannes.schindelin@gmx.de>
Subject: Re: [PATCH v4 2/2] merge-ort: return early when failing to write a blob
Date: Tue, 27 Sep 2022 09:45:18 -0700	[thread overview]
Message-ID: <xmqqill87om9.fsf@gitster.g> (raw)
In-Reply-To: <CABPp-BHau2qwoxj121H-cpjTPPCpfxjMpmhya0mV79qyvkpQ+g@mail.gmail.com> (Elijah Newren's message of "Tue, 27 Sep 2022 01:05:32 -0700")

Elijah Newren <newren@gmail.com> writes:

>> Since we will always write out a new tree object in addition to the blob
>> (and if the blob did not exist in the database yet, we can be certain
>> that the tree object did not exist yet), the merge will _still_ fail at
>> that point, but it does unnecessary work by continuing after the blob
>> could not be written.
>
> I don't think this is quite true.  I've had a number of users come to
> me with "messed up git repositories" where I eventually discover that
> users just randomly add "sudo" to some of their commands because when
> things don't work, sometimes "sudo" fixes it.  That means they've
> created stuff in their .git directory which may be owned by root,
> perhaps even some of the .git/objects/XX directories.  However, they
> may have other .git/objects/XX directories owned by their normal user.
>
> If just some .git/objects/XX directories are owned by root and not
> others, then it may be when users run git commands as themselves that
> some things can be written to the object database but not others.  In
> particular, it could be that writing blob objects fail, but writing a
> tree object which references those blobs succeeds.

It is a good example scenario that argues even more strongly for
this change.  We may know the correct object contents and name for
the top-level tree and even manage to write it out, after computing
all the blobs and the trees contained within, some of which we may
know the object name but have failed to write out.  Letting such a
breakage unnoticed would be, eh, bad.

> Patch looks good to me, though.

Yup, thanks for a review.

  reply	other threads:[~2022-09-27 16:45 UTC|newest]

Thread overview: 29+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-09-21 15:30 [PATCH] merge-tree: fix segmentation fault in read-only repositories Johannes Schindelin via GitGitGadget
2022-09-21 15:42 ` Taylor Blau
2022-09-21 20:12   ` Johannes Schindelin
2022-09-21 18:12 ` Junio C Hamano
2022-09-21 20:13   ` Johannes Schindelin
2022-09-21 22:08 ` [PATCH v2] " Johannes Schindelin via GitGitGadget
2022-09-21 22:24   ` Junio C Hamano
2022-09-22 19:52     ` Johannes Schindelin
2022-09-21 22:56   ` Elijah Newren
2022-09-21 23:11     ` Junio C Hamano
2022-09-22 17:24     ` Johannes Schindelin
2022-09-22 19:50     ` Johannes Schindelin
2022-09-23  1:47       ` Elijah Newren
2022-09-22 19:46   ` [PATCH v3] merge-ort: " Johannes Schindelin via GitGitGadget
2022-09-23  1:38     ` Elijah Newren
2022-09-23  9:55       ` Johannes Schindelin
2022-09-26 21:55     ` [PATCH v4 0/2] merge-tree: " Johannes Schindelin via GitGitGadget
2022-09-26 21:55       ` [PATCH v4 1/2] merge-ort: " Johannes Schindelin via GitGitGadget
2022-09-26 21:55       ` [PATCH v4 2/2] merge-ort: return early when failing to write a blob Johannes Schindelin via GitGitGadget
2022-09-26 22:07         ` Junio C Hamano
2022-09-27  8:05         ` Elijah Newren
2022-09-27 16:45           ` Junio C Hamano [this message]
2022-09-27  8:11       ` [PATCH v4 0/2] merge-tree: fix segmentation fault in read-only repositories Elijah Newren
2022-09-28  7:29       ` [PATCH v5 " Johannes Schindelin via GitGitGadget
2022-09-28  7:29         ` [PATCH v5 1/2] merge-ort: " Johannes Schindelin via GitGitGadget
2022-09-28  7:29         ` [PATCH v5 2/2] merge-ort: return early when failing to write a blob Johannes Schindelin via GitGitGadget
2022-09-28 15:53           ` Junio C Hamano
2022-09-29  1:36             ` Elijah Newren
2022-09-29  1:49               ` Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqqill87om9.fsf@gitster.g \
    --to=gitster@pobox.com \
    --cc=git@vger.kernel.org \
    --cc=gitgitgadget@gmail.com \
    --cc=johannes.schindelin@gmx.de \
    --cc=me@ttaylorr.com \
    --cc=newren@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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 NNTP newsgroup(s).