All of lore.kernel.org
 help / color / mirror / Atom feed
From: Stefan Beller <sbeller@google.com>
To: Junio C Hamano <gitster@pobox.com>
Cc: git <git@vger.kernel.org>, Jonathan Tan <jonathantanmy@google.com>
Subject: Re: [PATCHv5 7/7] builtin/describe.c: describe a blob
Date: Thu, 16 Nov 2017 11:34:12 -0800	[thread overview]
Message-ID: <CAGZ79kaGGUJSGG6OdfaTepDrvGBGFd17paBNNYuQt7t8XnDfHw@mail.gmail.com> (raw)
In-Reply-To: <xmqqwp2qx5w6.fsf@gitster.mtv.corp.google.com>

On Wed, Nov 15, 2017 at 7:24 PM, Junio C Hamano <gitster@pobox.com> wrote:

> I am not sure if "And if there is ..." is adding much value here (I
> do not think it is even technically correct for that matter).  If
> there are more than one tag that point at the commit the user is
> interested in, we use one of the tags, as tags conceptually sit at a
> higher level.  And we use a heuristic to use one or the other tag to
> make up a name for the commit, so the same commit can have two valid
> names. ---So what?  Neither of these two valid names is "ambigous";
> the commit object the user wanted to name _is_ correctly identified
> (I would assume that we are not discussing a hash collision).
>
> Lucikly, if we remove "And if...precisely", the logic still flows
> nicely, if not more, to the next paragraph.

fixed.

>> When describing a blob, we want to describe the blob from a higher layer
>> as well, which is a tuple of (commit, deep/path) as the tree objects
>> involved are rather uninteresting.  The same blob can be referenced by
>> multiple commits, so how we decide which commit to use?  This patch
>> implements a rather naive approach on this: As there are no back pointers
>> from blobs to commits in which the blob occurs, we'll start walking from
>> any tips available, listing the blobs in-order of the commit and once we
>
> Is "any tips" still the case?  I was wondering why you start your
> traversal at HEAD and nothing else in this iteration.  There seems
> to be no mention of this design decision in the documentation and no
> justification in the log.

fixed the text. The design decision to reverse walk HEAD is tied to
your observation below:

> The reversing may improve the chance of an older commit to be chosen
> rather than the newer one, but it does not even guarantee to show the
> "introduction".

This is what I realized when we started walking all refs including reflog.
The introduction can only be found when we take the commit-parents
into account and look at the diffs. I noticed you started
origin/jc/diff-blobfind
which may be helpful to find the introduction correctly, which I'll play around
with that and see if I can get that working.

> What this guarantees is that a long history will be traversed fully
> before we start considering which commit has the blob of interest, I
> am afraid.  Is this a sensible trade-off?

I am not fully convinced all descriptions are in recent history, but I
tend to agree that most are, so probably the trade off is a wash.

  reply	other threads:[~2017-11-16 19:34 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-11-16  2:00 [PATCHv5 0/7] describe blob Stefan Beller
2017-11-16  2:00 ` [PATCHv5 1/7] t6120: fix typo in test name Stefan Beller
2017-11-16  2:00 ` [PATCHv5 2/7] list-objects.c: factor out traverse_trees_and_blobs Stefan Beller
2017-11-16  2:00 ` [PATCHv5 3/7] revision.h: introduce blob/tree walking in order of the commits Stefan Beller
2017-11-16  2:00 ` [PATCHv5 4/7] builtin/describe.c: rename `oid` to avoid variable shadowing Stefan Beller
2017-11-16  2:00 ` [PATCHv5 5/7] builtin/describe.c: print debug statements earlier Stefan Beller
2017-11-16  2:00 ` [PATCHv5 6/7] builtin/describe.c: factor out describe_commit Stefan Beller
2017-11-16  2:00 ` [PATCHv5 7/7] builtin/describe.c: describe a blob Stefan Beller
2017-11-16  3:24   ` Junio C Hamano
2017-11-16 19:34     ` Stefan Beller [this message]
2017-11-22  7:55       ` Junio C Hamano
2017-11-22 17:00         ` Stefan Beller
2017-11-24  7:18           ` Junio C Hamano
2017-11-27 19:38             ` Stefan Beller
2017-11-27 23:37               ` Junio C Hamano
2017-11-20 15:10   ` Philip Oakley
2017-12-19 19:22   ` Junio C Hamano
2017-12-19 19:39     ` Stefan Beller
2017-12-22 23:10       ` 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=CAGZ79kaGGUJSGG6OdfaTepDrvGBGFd17paBNNYuQt7t8XnDfHw@mail.gmail.com \
    --to=sbeller@google.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --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.