archive mirror
 help / color / mirror / Atom feed
From: Junio C Hamano <>
To: Johannes Schindelin <>
Cc: Christian Couder <>,
	git <>,
	Christian Couder <>,
	Miriam Rubio <>
Subject: Re: [PATCH] bisect: don't use invalid oid as rev when starting
Date: Fri, 25 Sep 2020 09:54:19 -0700	[thread overview]
Message-ID: <> (raw)
In-Reply-To: <> (Johannes Schindelin's message of "Fri, 25 Sep 2020 09:13:44 +0200 (CEST)")

Johannes Schindelin <> writes:

> A slightly bigger question is whether `git_oid_committish()` would be okay
> enough, or whether we do need `get_oidf(&oid, "%s^{commit}", arg)` (as
> your `SQUASH???` does).
> I'm not quite sure: aren't those two calls idempotent, with the latter
> going through some unnecessary string processing dances?

I tend to agree that get_oidf() is not quite satisfactory but it is
a handy short-hand that is readily available to us to parse a string
into the name of the object of type commit the string peels into.

While get_oid_X_ish() makes sure the result can be peeled to type X,
it gives the outer object back to the caller to allow it to behave
differently and it is up to the caller to peel the tag down to

I audited all uses of get_oid_X_ish(), in the hope that perhaps we
have enough callers that want to get peeled object back, and more
importantly, no caller that wants the original.  Then we could
change these to peel for the callers, and we may be able to lose a
few lines from the callers.

But unfortunately that was not the case.  A new helper

	int oid_of_type(struct object_id *result_oid,
			const char *name_text,
			enum object_type peel_to);

is a possibility, but I have a suspicion that the API in question
encourages to manipulate and swap objects at their hash level for
too long with little upside.  If we were considering to clean the
callers up, we would want to encourage them to work with in-core
objects longer.

IOW, repo_peel_to_type() might be a better fit for some callers that
use get_oid_X_ish() and then peel the result to obtain an object of
desired type.


  parent reply	other threads:[~2020-09-25 16:54 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-09-23 17:09 Christian Couder
2020-09-23 17:27 ` Junio C Hamano
2020-09-23 20:37 ` Johannes Schindelin
2020-09-23 21:05   ` Johannes Schindelin
2020-09-23 21:39     ` Junio C Hamano
2020-09-24  6:10       ` Christian Couder
2020-09-24  6:48         ` Junio C Hamano
2020-09-24  7:51         ` Johannes Schindelin
2020-09-24 16:39           ` Junio C Hamano
2020-09-24 18:38             ` Junio C Hamano
2020-09-25  7:13               ` Johannes Schindelin
2020-09-25  7:14                 ` Johannes Schindelin
2020-09-25 16:54                 ` Junio C Hamano [this message]
2020-09-24  6:03 ` [PATCH v2] " Christian Couder
2020-09-24  7:49   ` Johannes Schindelin
2020-09-24 11:08     ` Christian Couder
2020-09-24 16:44       ` Junio C Hamano
2020-09-24 18:55   ` Junio C Hamano
2020-09-24 19:25     ` Junio C Hamano
2020-09-24 19:56     ` Junio C Hamano
2020-09-24 20:53       ` Junio C Hamano
2020-09-25 13:09     ` Christian Couder
2020-09-25 13:01   ` [PATCH v3] " Christian Couder

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:

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

  git send-email \ \ \ \ \ \ \ \
    --subject='Re: [PATCH] bisect: don'\''t use invalid oid as rev when starting' \

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

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 NNTP newsgroup(s).