From: Junio C Hamano <gitster@pobox.com>
To: "brian m. carlson" <sandals@crustytoothpaste.net>
Cc: <git@vger.kernel.org>, Derrick Stolee <dstolee@gmail.com>,
Thomas Gummerer <t.gummerer@gmail.com>
Subject: Re: [PATCH 3/6] object-name: make get_oid quietly return an error
Date: Wed, 16 Mar 2022 09:56:22 -0700 [thread overview]
Message-ID: <xmqqo825n8eh.fsf@gitster.g> (raw)
In-Reply-To: <20220310173236.4165310-4-sandals@crustytoothpaste.net> (brian m. carlson's message of "Thu, 10 Mar 2022 17:32:33 +0000")
"brian m. carlson" <sandals@crustytoothpaste.net> writes:
> A reasonable person looking at the signature and usage of get_oid and
> friends might conclude that in the event of an error, it always returns
> -1. However, this is not the case. Instead, get_oid_basic dies if we
> go too far back into the history of a reflog (or, when quiet, simply
> exits).
>
> This is not especially useful, since in many cases, we might want to
> handle this error differently. Let's add a flag here to make it just
> return -1 like elsewhere in these code paths.
>
> Note that we cannot make this behavior the default, since we have many
> other codepaths that rely on the existing behavior, including in tests.
>
> Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
> ---
> cache.h | 21 +++++++++++----------
> object-name.c | 6 +++++-
> 2 files changed, 16 insertions(+), 11 deletions(-)
>
> diff --git a/cache.h b/cache.h
> index 825ec17198..416a9d9983 100644
> --- a/cache.h
> +++ b/cache.h
> @@ -1366,16 +1366,17 @@ struct object_context {
> char *path;
> };
>
> -#define GET_OID_QUIETLY 01
> -#define GET_OID_COMMIT 02
> -#define GET_OID_COMMITTISH 04
> -#define GET_OID_TREE 010
> -#define GET_OID_TREEISH 020
> -#define GET_OID_BLOB 040
> -#define GET_OID_FOLLOW_SYMLINKS 0100
> -#define GET_OID_RECORD_PATH 0200
> -#define GET_OID_ONLY_TO_DIE 04000
> -#define GET_OID_REQUIRE_PATH 010000
> +#define GET_OID_QUIETLY 01
> +#define GET_OID_COMMIT 02
> +#define GET_OID_COMMITTISH 04
> +#define GET_OID_TREE 010
> +#define GET_OID_TREEISH 020
> +#define GET_OID_BLOB 040
> +#define GET_OID_FOLLOW_SYMLINKS 0100
> +#define GET_OID_RECORD_PATH 0200
> +#define GET_OID_ONLY_TO_DIE 04000
> +#define GET_OID_REQUIRE_PATH 010000
> +#define GET_OID_RETURN_FAILURE 020000
I do not think we want this change. The next time somebody adds an
overly long symbol, we reformat all the lines, making it hard to
spot that the change is only adding a single new symbol?
I think we'd rather go the other way not to tempt people into
right-aligning these constants, either by rewriting them into
#define GET_OID_QUIETLY<TAB>(1U << 1)
#define GET_OID_COMMIT<TAB>(1U << 2)
#define GET_OID_COMMITTISH<TAB>(1U << 3)
...
in a separate preliminary patch without adding a new symbol, or
adding the new symbol unaligned and without touching existing lines.
> diff --git a/object-name.c b/object-name.c
> index 92862eeb1a..daa3ef77ef 100644
> --- a/object-name.c
> +++ b/object-name.c
> @@ -911,13 +911,17 @@ static int get_oid_basic(struct repository *r, const char *str, int len,
> len, str,
> show_date(co_time, co_tz, DATE_MODE(RFC2822)));
> }
> - } else {
> + } else if (!(flags & GET_OID_RETURN_FAILURE)) {
> if (flags & GET_OID_QUIETLY) {
> exit(128);
> }
> die(_("log for '%.*s' only has %d entries"),
> len, str, co_cnt);
> }
> + if (flags & GET_OID_RETURN_FAILURE) {
> + free(real_ref);
> + return -1;
> + }
> }
So, without the new bit, we used to die loudly or quietly. The new
bit allows us to return an error to the caller without dying
ourselves.
You can call the bit _RETURN_ERROR and not to worry about the
right-alignment above ;-), but better yet, how about calling it
_GENTLY, which is how we call such a variant of behaviour?
next prev parent reply other threads:[~2022-03-16 16:56 UTC|newest]
Thread overview: 76+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-03-10 17:32 [PATCH 0/6] Importing and exporting stashes to refs brian m. carlson
2022-03-10 17:32 ` [PATCH 1/6] builtin/stash: factor out generic function to look up stash info brian m. carlson
2022-03-10 17:32 ` [PATCH 2/6] builtin/stash: fill in all commit data brian m. carlson
2022-03-16 16:50 ` Junio C Hamano
2022-03-10 17:32 ` [PATCH 3/6] object-name: make get_oid quietly return an error brian m. carlson
2022-03-16 16:56 ` Junio C Hamano [this message]
2022-03-16 17:01 ` Drew Stolee
2022-03-16 21:40 ` brian m. carlson
2022-03-10 17:32 ` [PATCH 4/6] builtin/stash: provide a way to export stashes to a ref brian m. carlson
2022-03-11 2:08 ` Ævar Arnfjörð Bjarmason
2022-03-14 21:19 ` Phillip Wood
2022-03-15 10:50 ` Phillip Wood
2022-03-16 21:48 ` brian m. carlson
2022-03-18 13:34 ` C99 %zu support (on MSVC) (was: [PATCH 4/6] builtin/stash: provide a way to export stashes to a ref) Ævar Arnfjörð Bjarmason
2022-03-18 16:26 ` Phillip Wood
2022-03-24 14:02 ` [PATCH 4/6] builtin/stash: provide a way to export stashes to a ref Johannes Schindelin
2022-03-18 13:41 ` ssize_t portability (was: [PATCH 4/6] builtin/stash: provide a way to export stashes to a ref) Ævar Arnfjörð Bjarmason
2022-03-16 17:05 ` [PATCH 4/6] builtin/stash: provide a way to export stashes to a ref Junio C Hamano
2022-03-10 17:32 ` [PATCH 5/6] builtin/stash: provide a way to import stashes from " brian m. carlson
2022-03-16 17:26 ` Junio C Hamano
2022-03-16 21:50 ` brian m. carlson
2022-03-10 17:32 ` [PATCH 6/6] doc: add stash export and import to docs brian m. carlson
2022-03-16 17:34 ` Junio C Hamano
2022-03-16 21:44 ` Junio C Hamano
2022-03-10 19:14 ` [PATCH 0/6] Importing and exporting stashes to refs Junio C Hamano
2022-03-10 21:04 ` brian m. carlson
2022-03-10 21:38 ` Junio C Hamano
2022-03-10 22:42 ` brian m. carlson
2022-03-29 21:49 ` [PATCH v2 0/4] " brian m. carlson
2022-03-29 21:49 ` [PATCH v2 1/4] object-name: make get_oid quietly return an error brian m. carlson
2022-03-29 21:49 ` [PATCH v2 2/4] builtin/stash: factor out revision parsing into a function brian m. carlson
2022-03-29 21:49 ` [PATCH v2 3/4] builtin/stash: provide a way to export stashes to a ref brian m. carlson
2022-03-30 23:05 ` Junio C Hamano
2022-03-30 23:44 ` brian m. carlson
2022-03-31 1:56 ` Ævar Arnfjörð Bjarmason
2022-03-31 17:43 ` Junio C Hamano
2022-04-05 10:55 ` brian m. carlson
2022-04-06 9:05 ` Ævar Arnfjörð Bjarmason
2022-04-06 16:38 ` Junio C Hamano
2022-03-31 2:09 ` Ævar Arnfjörð Bjarmason
2022-04-05 10:22 ` brian m. carlson
2022-03-29 21:49 ` [PATCH v2 4/4] builtin/stash: provide a way to import stashes from " brian m. carlson
2022-03-31 1:48 ` [PATCH v2 0/4] Importing and exporting stashes to refs Junio C Hamano
2022-03-31 2:18 ` Ævar Arnfjörð Bjarmason
2022-04-03 18:22 ` [PATCH v3 " brian m. carlson
2022-04-03 18:22 ` [PATCH v3 1/4] object-name: make get_oid quietly return an error brian m. carlson
2022-04-03 18:22 ` [PATCH v3 2/4] builtin/stash: factor out revision parsing into a function brian m. carlson
2022-04-04 15:44 ` Phillip Wood
2022-04-03 18:22 ` [PATCH v3 3/4] builtin/stash: provide a way to export stashes to a ref brian m. carlson
2022-04-04 6:46 ` Ævar Arnfjörð Bjarmason
2022-04-03 18:22 ` [PATCH v3 4/4] builtin/stash: provide a way to import stashes from " brian m. carlson
2022-04-04 10:38 ` Ævar Arnfjörð Bjarmason
2022-04-05 10:03 ` brian m. carlson
2022-04-06 9:00 ` Ævar Arnfjörð Bjarmason
2022-04-04 0:05 ` [PATCH v3 0/4] Importing and exporting stashes to refs Junio C Hamano
2022-04-04 0:29 ` Junio C Hamano
2022-04-04 6:20 ` Ævar Arnfjörð Bjarmason
2022-04-05 9:15 ` brian m. carlson
2022-04-07 21:53 ` [PATCH v4 " brian m. carlson
2022-04-07 21:53 ` [PATCH v4 1/4] object-name: make get_oid quietly return an error brian m. carlson
2022-04-07 21:53 ` [PATCH v4 2/4] builtin/stash: factor out revision parsing into a function brian m. carlson
2022-04-07 21:53 ` [PATCH v4 3/4] builtin/stash: provide a way to export stashes to a ref brian m. carlson
2022-04-13 15:29 ` Ævar Arnfjörð Bjarmason
2022-04-13 15:36 ` Ævar Arnfjörð Bjarmason
2022-04-13 15:55 ` Ævar Arnfjörð Bjarmason
2022-04-07 21:53 ` [PATCH v4 4/4] builtin/stash: provide a way to import stashes from " brian m. carlson
2022-04-12 20:14 ` Jonathan Tan
2022-04-13 1:12 ` brian m. carlson
2022-04-13 17:34 ` Jonathan Tan
2022-04-13 18:25 ` Ævar Arnfjörð Bjarmason
2022-04-13 19:14 ` Jonathan Tan
2022-04-13 20:10 ` Junio C Hamano
2022-04-13 21:33 ` brian m. carlson
2022-04-13 21:43 ` Junio C Hamano
2022-04-13 18:33 ` Ævar Arnfjörð Bjarmason
2022-04-13 15:25 ` [PATCH v4 0/4] Importing and exporting stashes to refs Ævar Arnfjörð Bjarmason
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=xmqqo825n8eh.fsf@gitster.g \
--to=gitster@pobox.com \
--cc=dstolee@gmail.com \
--cc=git@vger.kernel.org \
--cc=sandals@crustytoothpaste.net \
--cc=t.gummerer@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.
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.