git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: "René Scharfe" <l.s.r@web.de>
To: Jeff King <peff@peff.net>
Cc: Keegan Carruthers-Smith <keegan.csmith@gmail.com>, git@vger.kernel.org
Subject: Re: git archive generates tar with malformed pax extended attribute
Date: Wed, 29 May 2019 19:54:44 +0200	[thread overview]
Message-ID: <24574d74-b6ca-6d36-759f-32cf26b36c3f@web.de> (raw)
In-Reply-To: <20190529011744.GA22340@sigill.intra.peff.net>

Am 29.05.19 um 03:17 schrieb Jeff King:
> On Wed, May 29, 2019 at 01:34:32AM +0200, René Scharfe wrote:
>> Parsing trees with symlinks twice is not ideal, but keeps the set
>> structure simple -- a standard oidset suffices.
>
> If blobs comes after trees (and they usually do in a pack), you can do
> it in a single pass by marking the blob as a symlink target, and then
> when we actually see that blob's contents, marking it as either OK or
> problematic. And then the finish() step just correlates those with the
> tree.

Good idea.  Is that ordering guaranteed?  (Stumbling about the "usually"
in your first sentence.)

An ordering where dependent objects (like trees) follow the objects they
reference would be better suited for these kinds of checks..

> It does require O(n) storage in the number of symlinked blobs, but also
> O(n) in the number of symlinked tree entries (number of trees with
> symlinks times the number of entries in each such tree, _even if they're
> the same entry/blob as another tree).
>
> That makes it a lot worse than the existing gitmodules check. There we
> only care about finding the .gitmodules blobs. So even though you have a
> ton of trees that mention .gitmodules (basically every root tree), the
> the .gitmodules file itself doesn't change much. So we only end up with
> a small oidset (and a small worst case for looking at objects twice).
>
> But here the problem is in the tree, not the blob. So we're not finding
> suspect blobs, but rather re-checking each tree. And no matter what we
> do (whether it's visiting the object again, or creating a set or mapping
> with the object names) is going to be linear there. And a repository
> with a symlink in the root tree is going to revisit or put in our
> mapping every single root tree.

That's true, potentially it needs remember and/or reprocess all trees,
meaning this check may double the run time of fsck in the worst case.
Example from the wild: The kernel repo currently has 36 symlinks and
6+ million objects are checked in total, and the symlink check processes
18943 trees_with_symlinks entries there.

> TBH, I'm not sure this fsck check was worth it even without the
> implementation complexity.

Hmm.  git status reports such truncated symlinks as changed, so the
issue *is* already detectable.

René

  reply	other threads:[~2019-05-29 17:55 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-05-24  6:45 git archive generates tar with malformed pax extended attribute Keegan Carruthers-Smith
2019-05-24  7:06 ` Jeff King
2019-05-24  7:35   ` Keegan Carruthers-Smith
2019-05-24  8:13     ` Jeff King
2019-05-25 13:26       ` René Scharfe
2019-05-25 13:46         ` Andreas Schwab
2019-05-25 21:07         ` Ævar Arnfjörð Bjarmason
2019-05-26 21:33           ` René Scharfe
2019-05-28  5:44             ` Jeff King
2019-05-28  5:58         ` Jeff King
2019-05-28 18:01           ` René Scharfe
2019-05-28 19:08             ` Jeff King
2019-05-28 23:34               ` René Scharfe
2019-05-29  1:17                 ` Jeff King
2019-05-29 17:54                   ` René Scharfe [this message]
2019-05-30 11:55                     ` Jeff King
2019-06-02 16:58                       ` René Scharfe
2019-06-04 20:53                         ` Jeff King
2019-05-27  5:11       ` Keegan Carruthers-Smith
2019-05-25 20:46   ` Ævar Arnfjörð Bjarmason
2019-05-25 21:19     ` brian m. carlson

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

  List information: http://vger.kernel.org/majordomo-info.html

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

  git send-email \
    --in-reply-to=24574d74-b6ca-6d36-759f-32cf26b36c3f@web.de \
    --to=l.s.r@web.de \
    --cc=git@vger.kernel.org \
    --cc=keegan.csmith@gmail.com \
    --cc=peff@peff.net \
    /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.
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).