Patrick Steinhardt <ps@xxxxxx> writes: > 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. > I think _can't_ wasn't the best terminology. My previous series actually added a reflog, but I noticed a bunch of tests were failing and I think it made sense to keep the existing behaviour. But addressing it as a bug would definitely be a good way to go and fix this, I'll add a comment in the code for now.
Attachment:
signature.asc
Description: PGP signature
