Hey guys - I’d been ignoring this thread as spam but just opened it and see you meant to include Derrick Stolee. I’m Drew Stolee, likely related but who knows how. dstolee@xxxxxxxxx is not the developer you’re looking for.
Best,
Drew
Sent from my iPhone
> On Mar 16, 2022, at 10:56 AM, Junio C Hamano <gitster@xxxxxxxxx> wrote:
>
> "brian m. carlson" <sandals@xxxxxxxxxxxxxxxxxxxx> 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@xxxxxxxxxxxxxxxxxxxx>
>> ---
>> 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?
>
