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

2012-10-14 Thread George Spelvin
 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...
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2012-10-13 Thread George Spelvin
Try the following commands in an empty directory:
(I'm using the bash extention for {1..5}.)

git init
for i in {1..5}; do git commit -m Commit $i --allow-empty; done
git reflog
bc93e06 HEAD@{0}: commit: Commit 5
e14f92d HEAD@{1}: commit: Commit 4
ac64d8e HEAD@{2}: commit: Commit 3
287602b HEAD@{3}: commit: Commit 2
183a378 HEAD@{4}: commit (initial): Commit 1
git reflog delete HEAD@{{1,2}}
git reflog
bc93e06 HEAD@{0}: commit: Commit 5
e14f92d HEAD@{1}: commit: Commit 3
287602b HEAD@{2}: commit (initial): Commit 1

Er...I meant to delete Commit 3 from the reflog, not Commit 2.

In hindsight, it's obvious how this could happen, but it definitely
took me by surprise when I was trying to delete the reference to
a large and messy merge (that I didn't want and bloated my repository
size) from the reflog.

If this is, in fact, Working As Designed, could I request a paragraph
in the man page clarifying it?  something like:


To delete single entries from the reflog, use the subcommand delete
and specify the _exact_ entry (e.g. `git reflog delete master@{2}`).

You may delete multiple reflog entries with one delete command,
_however_ they are processed one at a time, so earlier deletes will cause
renumbering that will affect later ones.  To delete reflog entries @{2}
and @{3}, the command would be either `git reflog delete @{3} @{2}`,
or `git reflog delete @{2} @{2}`.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2012-10-13 Thread Junio C Hamano
George Spelvin li...@horizon.com writes:

 Try the following commands in an empty directory:
 (I'm using the bash extention for {1..5}.)

 git init
 for i in {1..5}; do git commit -m Commit $i --allow-empty; done
 git reflog
   bc93e06 HEAD@{0}: commit: Commit 5
   e14f92d HEAD@{1}: commit: Commit 4
   ac64d8e HEAD@{2}: commit: Commit 3
   287602b HEAD@{3}: commit: Commit 2
   183a378 HEAD@{4}: commit (initial): Commit 1
 git reflog delete HEAD@{{1,2}}
 git reflog
   bc93e06 HEAD@{0}: commit: Commit 5
   e14f92d HEAD@{1}: commit: Commit 3
   287602b HEAD@{2}: commit (initial): Commit 1

 Er...I meant to delete Commit 3 from the reflog, not Commit 2.

Yeah, I've never tried to delete more than one with one command,
but I am not surprised a naaïve implementation removes the first,
and then removes the second in the resulting list.

 To delete single entries from the reflog, use the subcommand delete
 and specify the _exact_ entry (e.g. `git reflog delete master@{2}`).

 You may delete multiple reflog entries with one delete command,
 _however_ they are processed one at a time, so earlier deletes will cause
 renumbering that will affect later ones.  To delete reflog entries @{2}
 and @{3}, the command would be either `git reflog delete @{3} @{2}`,
 or `git reflog delete @{2} @{2}`.

I would actually call that behaviour a bug.  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?

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.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html