git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Jeff King <peff@peff.net>
To: "Kyle J. McKay" <mackyle@gmail.com>
Cc: Junio C Hamano <gitster@pobox.com>,
	Git mailing list <git@vger.kernel.org>
Subject: Re: Git v2.11.0 breaks max depth nested alternates
Date: Mon, 5 Dec 2016 02:14:31 -0500	[thread overview]
Message-ID: <20161205071431.cf3oy7nceyb7hggw@sigill.intra.peff.net> (raw)
In-Reply-To: <E3C2AF2A-FE07-4C94-B549-3BDAF9B3DB5D@gmail.com>

On Sun, Dec 04, 2016 at 01:37:00AM -0800, Kyle J. McKay wrote:

> On Dec 3, 2016, at 20:55, Jeff King wrote:
> 
> > So I do think this is worth dealing with, but I'm also curious why
> > you're hitting the depth-5 limit. I'm guessing it has to do with hosting
> > a hierarchy of related repos. But is your system then always in danger
> > of busting the 5-limit if people create too deep a repository hierarchy?
> 
> No we check for the limit.  Anything at the limit gets broken by the
> quarantine change though.

OK. So the limit is an issue for your system, but one that you're able
to deal gracefully with (and the quarantine change makes that a lot
harder). I buy that line of reasoning.

> The patch is a step on that road.  It doesn't go that far but all it would
> take is connecting the introduced variable to a config item.  But you still
> need to bump it by 1 during quarantine operations.  Such support would even
> allow alternates to be disallowed (except during quarantine).  I wonder if
> there's an opportunity for further pack operation optimizations in such a
> case (you know there are no alternates because they're not allowed)?

I doubt it. We look at the list of alternates early on, and in most
cases there aren't any. So any optimization there can be done already at
that point.

The only optimization I know if in that area is 56dfeb626 (pack-objects:
compute local/ignore_pack_keep early, 2016-07-29), which works already.

> All true.  And I had similar thoughts.  Perhaps we should add your comments
> to the patch description?  There seems to be a trend towards having longer
> patch descriptions these days... ;)

Feel free to pick out anything that's useful and add it in verbatim or
rephrased, whichever is more convenient.

> You took the words right out of my mouth...   I guess I need to work on
> doing a better job of dumping my stream-of-thoughts that go into a patch
> into the emails to the list.

It's a lot easier when you're the reviewer, because you don't start
reading through the commit-message with a full understanding of the
problem yet. :)

> Most all of your comments could be dumped into the patch description as-is
> to pimp it out some.  I have no objection to that, even adding an
> "Additional-analysis-by:" (or similar) credit line too.  :)

Sure. I don't really need credit, or even just "reviewed-by" is fine.
Talking and generating a shared understanding of the problem is part of
the review process.

-Peff

  reply	other threads:[~2016-12-05  7:14 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-12-04  0:24 Git v2.11.0 breaks max depth nested alternates Kyle J. McKay
2016-12-04  4:55 ` Jeff King
2016-12-04  9:37   ` Kyle J. McKay
2016-12-05  7:14     ` Jeff King [this message]
2016-12-04 11:22 ` Philip Oakley
2016-12-05  7:18   ` Jeff King
2016-12-05 12:07     ` Philip Oakley

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=20161205071431.cf3oy7nceyb7hggw@sigill.intra.peff.net \
    --to=peff@peff.net \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=mackyle@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.
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).