On Thu, May 02, 2024 at 05:50:47AM +0000, Karthik Nayak wrote: > Junio C Hamano <gitster@xxxxxxxxx> writes: > > Karthik Nayak <karthik.188@xxxxxxxxx> writes: > >> From: Karthik Nayak <karthik.188@xxxxxxxxx> > >> We do not add reflog for dangling symref updates, because currently > >> 'git-symbolic-ref' doesn't add reflog for dangling symref updates and it > >> would be best to keep this behavior consistent as we would move it to > >> start using transaction based updates in the following commit. > > > > If we are not changing the behaviour, does it deserve a four-line > > paragraph? It is not like we describe every no changes (i.e. "we > > could break the behaviour by introducing this and that bugs, but we > > did not" is not something we usually say in proposed log messages). > > > > At most, if you want to highlight that behaviour, I would expect a > > brief mention like: > > > > Note that a dangling symref update does not record a new reflog > > entry, which is unchanged before and after this commit. > > > > As a reflog entry records name of the object that is pointed by the > > ref (either directly or indirectly) before and after an operation, > > an operation that involve a dangling reflog that does not point at > > any object cannot be expressed in a reflog, no? It is way too late > > to change this, but it would have been interesting if the design of > > reflog had a room to log the change of symbolic ref target as well > > as object names. It would have allowed us to say "HEAD at time T > > pointed at refs/heads/main (which did not exist)", "HEAD at time T+1 > > directly pointed at commit X (detached)", "HEAD at time T+2 pointed > > at refs/heads/next", etc. and allowed us to much more cleanly > > support "previous branch". > > > > While I agree that four lines may seem excessive, I think it is indeed > an important point to note. Mostly because this shows that when doing > dangling symref updates, there is no record of this update. The best > situation would be like you mentioned, to record the symref target > changes. But even with the current design, it would have been nice to at > least acknowledge that there was some update done to the symref. By > having zero-oid for the new and old value in the reflog. But seems like > we can't do that either. I wouldn't say we can't do that. We already do log when symrefs become dangling when updating references via HEAD by logging a zero OID as new OID. That is, if we have "HEAD -> refs/heads/foo" and you delete the latter, then we create a new reflog message for "HEAD" with zero OID as new OID. I would claim that the current behaviour where we don't create a reflog entry when updating a ref to become dangling is a mere bug. I think it's fair to declare this a #leftoverbit and handle it in a follow-up patch series. But it would be nice to say so in an in-code comment. Patrick
Attachment:
signature.asc
Description: PGP signature
