>
>> } + if (referent) + refs_read_symbolic_ref(refs, ref,
>> referent);
So I've been working on detecting a detached remote/HEAD, and it seems that
"refs_read_symbolic_ref" behaves differently for the files and the reftables
backend. These are the exit codes in the various states:
reftables files
detached -1 1
doesn't exist -1 -1
I would assume this is a bug in reftables? At least the behaviour of files is
more useful for this case ...
This now works fine with the files backend:
if (referent && refs_read_symbolic_ref(refs, ref, referent) == 1) {
struct object_id oid;
refs_read_ref(refs, ref, &oid);
strbuf_addstr(referent, oid_to_hex(&oid));
ret = -1;
}
And 4/8 can now also detect being detached, by checking the return value using
the test you suggested, but this fails for reftables. Just in case it might be
something about the test not being correct:
test_expect_success 'set-head --auto to update a non symbolic ref' '
(
cd test &&
git update-ref --no-deref -d refs/remotes/origin/HEAD &&
git update-ref refs/remotes/origin/HEAD HEAD &&
HEAD=$(git log --pretty="%H") &&
git remote set-head --auto origin >output &&
echo "${SQ}origin/HEAD${SQ} was detached at ${SQ}${HEAD}${SQ} and now points to ${SQ}main${SQ}" >expect &&
test_cmp expect output
)
'
Should I fix the reftables problem or should I mark the above test as a known
breakage? Can that be done for just reftables somehow? The only problem
reftables suffers this way is to incorrectly report "creation" instead of
correctly saying it was a detached head.
> We could write the whole thing as a single "do these and leave as
> soon as we see any failure" ||-cascade,
>
> if (!(transaction = ref_store_transaction_begin(refs, &err)) ||
>
> ref_transaction_update(transaction, ref, NULL, NULL,
> target, NULL, REF_NO_DEREF,
> logmsg, &err) ||
>
> ref_transaction_prepare(transaction, &err)) ||
>
> (referent
> ? refs_read_symbolic_ref(refs, ref, referent)
> : 0) ||
>
> ref_transaction_commit(transaction, &err)) {
> if (!err.len)
> ... stuff default error message to err ...;
> ret = error("%s", err.buf);
> }
>
> which may not necessarily easier to follow (and in fact it is rather
> ugly), but at least, the resulting code does not have to special
> case the "optionally peek into the symref" step.
I realised I misread the above snippet the last time I looked at it, but at
a first glance that would make 6/8 painful.
Thanks,
Bence
--
bence.ferdinandy.com
