Johan Herland <johan@xxxxxxxxxxx> writes: > I see that Rene Scharfe has also worked on the same issue, while I was > preparing these patches... > > On Mon, Oct 15, 2012 at 11:29 AM, Junio C Hamano <jch2355@xxxxxxxxx> wrote: >> Even though update-ref deferences a symref when it updates one to point at a >> new object, I personally don't think update-ref -d that derefs makes any >> sense. I'd rather see it error out when given a symref, with or without >> --no-deref option. > > I'm not sure. We have multiple testcases that directly test deleting a ref > through a symref (e.g. t1400), so supporting this seems like a concious > decision. Erroring out when given a symref will break the following > testcases: > - t1400 (git update-ref -d) > - t3310 & t3311 (git notes merge --abort is broken) > - t5505 (git remote set-head --delete and renaming a remote is broken) "Concious" does not necessarily mean "Sane", though. But this is water under the bridge. Too many people must have started relying on this crazy "feature" since mid 2008, and removing it would break them. - "update-ref -d --no-deref SYMREF", even though it is a synonym for "symbolic-ref -d SYMREF", makes sense. Removing it would only buy us breakage. - "update-ref -d --no-deref SYMREF OLDSHA1" does not make *ANY* sense as a request, but again it would not hurt to keep it. - "update-ref -d --deref SYMREF [OLDSHA1]" is questionable. What is the use case? What is the next step after doing such an operation, now you have SYMREF that is dangling? >> But even if it did, removing a ref pointed by a symref should really remove >> it, and forgetting to remove a leftover entry in packed-ref has no excuse >> bug. >> >> I'd say what you observed is a double bug. > > Patch #1 - #2 fixes the 2nd bug (removing through a symref should remove > both loose and packed versions of the ref). OK. That is surely needed. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html