All of lore.kernel.org
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Jonathan Tan <jonathantanmy@google.com>
Cc: git@vger.kernel.org, git@jeffhostetler.com
Subject: Re: [WIP 1/2] pack-objects: rename want_.* to ignore_.*
Date: Fri, 02 Jun 2017 12:45:54 +0900	[thread overview]
Message-ID: <xmqqwp8vyrv1.fsf@gitster.mtv.corp.google.com> (raw)
In-Reply-To: <5e8aebf2726481ef63838291c32e07439289d922.1496361873.git.jonathantanmy@google.com> (Jonathan Tan's message of "Thu, 1 Jun 2017 17:14:54 -0700")

Jonathan Tan <jonathantanmy@google.com> writes:

> Currently, pack-objects conflates the concepts of "ignoring an object"
> and "including it in to_pack".

Hmph, that statement is a hard to read and agree to.  I thought an
ignored object that is not going to be packed is one that won't hit
to_pack?  

I agree that "including to to_pack" and "actually appearing in the
resulting pack" are three different things, though.  Preferred base
objects that are used when constructing a thin pack are thrown into
to_pack because they need to participate in the delta base selection
computation, but by definition they shouldn't be contained in the
resulting pack (hence, nr_result is not incremented for them).

> This is fine for now, but a subsequent
> commit will introduce the concept of an object that cannot be completely
> ignored, but should not be included in to_pack either. To separate these
> concepts, restrict want_found_object() and want_object_in_pack() to only
> indicate if the object is to be ignored. This is done by renaming these
> methods and swapping the meanings of the return values 0 and 1.

I am a bit confused by your reasoning.  I guess it will become
clearer if I knew exactly what you mean by "ignoring".  It is not
like "pretend as if it didn't exist in the rev-list --objects output
we are working off of".

> We also take the opportunity to use the terminology "preferred_base"
> instead of "excluded" in these methods. It is true that preferred bases
> are not included in the final packfile generation, but at this point in
> the code, there is no exclusion taking place - on the contrary, if
> something is "excluded", it is in fact guaranteed to be in to_pack.

This one I can understand.

> Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
> ---
>  builtin/pack-objects.c | 50 +++++++++++++++++++++++++-------------------------
>  1 file changed, 25 insertions(+), 25 deletions(-)

Without understanding why this change is desirable, I can tell that
overall this does not change the behaviour, which is good ;-).


  reply	other threads:[~2017-06-02  3:46 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-06-02  0:14 [WIP 0/2] Modifying pack-objects to support --blob-size-limit Jonathan Tan
2017-06-02  0:14 ` [WIP 1/2] pack-objects: rename want_.* to ignore_.* Jonathan Tan
2017-06-02  3:45   ` Junio C Hamano [this message]
2017-06-02  0:14 ` [WIP 2/2] pack-objects: support --blob-size-limit Jonathan Tan
2017-06-02  4:48   ` Junio C Hamano
2017-06-02 19:38 ` [WIP v2 0/2] Modifying pack objects to support --blob-max-bytes Jonathan Tan
2017-06-02 19:38   ` [WIP v2 1/2] pack-objects: rename want_.* to ignore_.* Jonathan Tan
2017-06-02 19:38   ` [WIP v2 2/2] pack-objects: support --blob-max-bytes Jonathan Tan
2017-06-02 22:26     ` Jeff King
2017-06-02 23:25       ` Jonathan Nieder
2017-06-07  9:44         ` Jeff King
2017-06-03 23:48       ` Junio C Hamano
2017-06-15 20:28       ` Jeff Hostetler
2017-06-15 21:03         ` Jonathan Tan
2017-06-02 22:16   ` [WIP v2 0/2] Modifying pack objects to " Jeff King
2017-06-05 17:35     ` Jonathan Tan
2017-06-07  9:46       ` Jeff King

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=xmqqwp8vyrv1.fsf@gitster.mtv.corp.google.com \
    --to=gitster@pobox.com \
    --cc=git@jeffhostetler.com \
    --cc=git@vger.kernel.org \
    --cc=jonathantanmy@google.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.