Re: git reflog delete HEAD@{1} HEAD@{2} caught me by surprise...

["George Spelvin" <linux@horizon.com>]

> I would actually call that behaviour a bug.

Well, yes, that was my inclination, too.  But writing documentation was
easier than writing a code patch.  :-)

Even when it is fixed, a comment about when it was fixed and what the
buggy version did should live in the BUGS section for a while, to warn
people writing portable scripts.

> Perhaps it should grab
> all the command line arguments first, group them per the ref the
> reflog entries are based on, and expire _all_ reflog entries from
> the same reflog in one go?

Two other options are to sort them in decreasing entry order (which you
could do either per-reflog, or simply globally), or to remember previous
deletions so you can adjust the numbers of later ones.

One tricky point is whether it's possible for a reflog to have two names,
via a symlink or something.  That definitely complicates collision
detection.

> Until that happens, it may make sense to error it out when more than
> one entries are given from the command line, at least for the same
> ref.

Detecting this seems like half the implementation work of fixing it,
so I'm not sure it's worth bothering.


Looking at the code (builtin/reflog.c), I notice that expire_reflog()
takes a lock on the ref, but the previous count_reflog_ent code doesn't,
so things aren't necessarily race-proof.  I haven't figured out if the
race is a problem (i.e. does expire_reflog do something nasty if the
struct cmd_reflog_expire_cb holds stale data?), but I noticed...