git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Matthew DeVore <matvore@google.com>
To: Junio C Hamano <gitster@pobox.com>
Cc: git@vger.kernel.org, Jonathan Tan <jonathantanmy@google.com>,
	jeffhost@microsoft.com, ramsay@ramsayjones.plus.com
Subject: Re: [PATCH] revision.c: drop missing objects from cmdline
Date: Thu, 25 Oct 2018 16:13:40 -0700	[thread overview]
Message-ID: <CAMfpvhJ4_5EtcTiaU6T1T9qPd=kBBvcVpaUFC_AAy4VBo7hv5w@mail.gmail.com> (raw)
In-Reply-To: <xmqqa7n4osgi.fsf@gitster-ct.c.googlers.com>

On Tue, Oct 23, 2018 at 9:54 PM Junio C Hamano <gitster@pobox.com> wrote:
>
> Matthew DeVore <matvore@google.com> writes:
>
> > No code which reads cmdline in struct rev_info can handle NULL objects
> > in cmdline.rev[i].item, so stop adding them to the cmdline.rev array.
>
> "The code is not prepared to have cmdline.rev[].item that is NULL"
> is something everybody would understand and agree with, but that
> does not automatically lead to "so ignoring or rejecting and dying
> is OK", though.  The cmdline thing is used for the commands to learn
> the end-user intent that cannot be learned by the resulting objects
> in the object array (e.g. the user may have said 'master' but the
> pending[] (and later revs.commits) would only have the object names,
> and some callers would want to know if it was a branch name, a
> refname refs/heads/master, or the hexadecimal object name), so
> unless absolutely needed, I'm hesitant to take a change that loses
> information (e.g. the user named this object that is not locally
> available, we cannot afford to add it to the pending[] and add it to
> revs.commits to traverse from there, but we still want to know what
> object was given by the user).
Hmm, when you explain the purpose of cmdline, it's obvious now that it
doesn't make sense to mechanically drop items from it. I'm sending
another version of this patch which uses a more focused approach and
is a bit simpler.

>
> > Objects in cmdline are NULL when the given object is promisor and
> > --exclude-promisor-objects is enabled.
>
> A "promisor" is a remote repository.  It promises certain objects
> that you do not have are later retrievable from it.  The way you can
> see if the promisor promised to later give you an object is to see
> if that missing object is reachable from an object in a packfile the
> promisor gave you earlier.
>
> "The given object" is never a "promisor", so I am not sure what the
> above wants to say.  Is
>
>     When an object is given on the command line and if it is missing
>     from the local repository, add_rev_cmdline() receives NULL in
>     its "item" parameter.
>
> what you meant?  Is that the _only_ case in which "item" could be
> NULL, or is it also true for any missing object due to repository
> corruption?

Yes, that is what I meant. I believe for corruption there is an actual
error shown with die() or the like, though I am not certain.

  reply	other threads:[~2018-10-25 23:13 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-10-23 21:57 [PATCH] revision.c: drop missing objects from cmdline Matthew DeVore
2018-10-24  4:54 ` Junio C Hamano
2018-10-25 23:13   ` Matthew DeVore [this message]
2018-10-25 23:53 ` [PATCH v2] list-objects.c: don't segfault for missing cmdline objects Matthew DeVore
2018-10-29  0:06   ` Junio C Hamano
2018-12-05 21:43 ` [PATCH v3] " Matthew DeVore
2018-12-06  1:12   ` 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

  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='CAMfpvhJ4_5EtcTiaU6T1T9qPd=kBBvcVpaUFC_AAy4VBo7hv5w@mail.gmail.com' \
    --to=matvore@google.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=jeffhost@microsoft.com \
    --cc=jonathantanmy@google.com \
    --cc=ramsay@ramsayjones.plus.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).