From: Junio C Hamano <hidden> Date: 2016-06-15 22:46:43
Jeff King [off-list ref] writes:
Switching branches and clobbering some other branch
with --abort is just _one_ thing you can do to screw yourself. You could
also have been doing useful work on the _same_ branch, and that would
get clobbered by --abort. However, I'm not sure if we have a good way
of telling the difference between "work which I did to try to get these
patches to apply, but which should be thrown away when I abort" and
"work which I did because I forgot I had an active git-am".
I think I've said this already, but honestly speaking, I think --abort
should not do --reset at all, but just remove the $dotest directory. Or
perhaps introduce a --clear option to do so.
At least your patch is an improvement.
What I sometimes see to my users happen is to try applying to the oldest
integration branch the patch (the users think) ought to apply, see it fail
to apply, switch to a bit newer branch and run "am" again (trusting that
it will pick up the material from $dotest), repeat the above and then give
up with "git am --abort". I do not think anybody can offhand explain to
which branch and to what state the command takes the user back to in such
a situation without looking at what the code actually does X-<; even
though I think it should take the user back to the original branch, I do
not think that is what the code does.
From: Jeff King <hidden> Date: 2016-06-15 22:46:43
On Fri, May 08, 2009 at 02:01:16AM -0700, Junio C Hamano wrote:
quoted
Switching branches and clobbering some other branch
with --abort is just _one_ thing you can do to screw yourself. You could
also have been doing useful work on the _same_ branch, and that would
get clobbered by --abort. However, I'm not sure if we have a good way
of telling the difference between "work which I did to try to get these
patches to apply, but which should be thrown away when I abort" and
"work which I did because I forgot I had an active git-am".
I think I've said this already, but honestly speaking, I think --abort
should not do --reset at all, but just remove the $dotest directory. Or
perhaps introduce a --clear option to do so.
I assumed that people actually liked the current "reset" behavior, so I
didn't want to propose getting rid of it. Personally, I hate it. So I
would be very happy to see it ripped out entirely, and then that neatly
solves the problem (i.e., it now errs on the side of not throwing away
work).
What I sometimes see to my users happen is to try applying to the oldest
integration branch the patch (the users think) ought to apply, see it fail
to apply, switch to a bit newer branch and run "am" again (trusting that
it will pick up the material from $dotest), repeat the above and then give
up with "git am --abort". I do not think anybody can offhand explain to
which branch and to what state the command takes the user back to in such
a situation without looking at what the code actually does X-<; even
though I think it should take the user back to the original branch, I do
not think that is what the code does.
No, the current code clobbers whatever is in the current HEAD with
ORIG_HEAD. So not only might you set another random branch back to the
originally am'd branch, but if you did a pull in between you can pick up
some random commit.
-Peff
From: Jeff King <hidden> Date: 2016-06-15 22:46:49
We really have no idea what state the tree is in at this
point, and whether the user might have done useful work on
top of it. So let's err on the side of keeping the user's
data intact.
The downside is that if they do have cruft to get rid of, or
want to pretend as if earlier parts of the series that were
applied did not exist, they must manually "git reset --hard"
now.
Signed-off-by: Jeff King <redacted>
---
This is a followup to:
http://thread.gmane.org/gmane.comp.version-control.git/118373
git-am.sh | 4 ----
1 files changed, 0 insertions(+), 4 deletions(-)
From: Johannes Schindelin <hidden> Date: 2016-06-15 22:46:49
Hi,
On Mon, 25 May 2009, Jeff King wrote:
We really have no idea what state the tree is in at this
point, and whether the user might have done useful work on
top of it. So let's err on the side of keeping the user's
data intact.
The downside is that if they do have cruft to get rid of, or
want to pretend as if earlier parts of the series that were
applied did not exist, they must manually "git reset --hard"
now.
Signed-off-by: Jeff King <redacted>
Hmm. I think I would revert that patch after merging git.git right away.
Can you at least check for a dirty tree and reset --hard if it is clean?
In the other case, you could still say "you seem to have modifications,
bla bla bla"...
Ciao,
Dscho
From: Jeff King <hidden> Date: 2016-06-15 22:46:49
On Mon, May 25, 2009 at 01:49:18PM +0200, Johannes Schindelin wrote:
quoted
We really have no idea what state the tree is in at this
point, and whether the user might have done useful work on
top of it. So let's err on the side of keeping the user's
data intact.
The downside is that if they do have cruft to get rid of, or
want to pretend as if earlier parts of the series that were
applied did not exist, they must manually "git reset --hard"
now.
Hmm. I think I would revert that patch after merging git.git right away.
You know, you can just say you don't like it. ;)
Can you at least check for a dirty tree and reset --hard if it is clean?
No, that would defeat the purpose. The problem is that we have no idea
what has happened since the initial "git am". The user may have made
commits they want to keep, and we don't want to reset those away. They
may even have pulled, which means ORIG_HEAD can no longer be trusted for
a reset.
In the other case, you could still say "you seem to have modifications,
bla bla bla"...
I think you raise a good point here. If we do decide to stop doing the
reset automatically, it is probably better not to simply stop doing it
(as my patch does), but to downgrade it to a warning ("You probably want
to reset, etc...").
-Peff
From: Johannes Sixt <hidden> Date: 2016-06-15 22:46:49
Jeff King schrieb:
On Mon, May 25, 2009 at 01:49:18PM +0200, Johannes Schindelin wrote:
quoted
quoted
We really have no idea what state the tree is in at this
point, and whether the user might have done useful work on
top of it. So let's err on the side of keeping the user's
data intact.
The downside is that if they do have cruft to get rid of, or
want to pretend as if earlier parts of the series that were
applied did not exist, they must manually "git reset --hard"
now.
Hmm. I think I would revert that patch after merging git.git right away.
You know, you can just say you don't like it. ;)
quoted
Can you at least check for a dirty tree and reset --hard if it is clean?
No, that would defeat the purpose. The problem is that we have no idea
what has happened since the initial "git am". The user may have made
commits they want to keep, and we don't want to reset those away. They
may even have pulled, which means ORIG_HEAD can no longer be trusted for
a reset.
I wonder why we have this problem (and do something about it) with git-am,
but not with git-rebase. Is it perhaps that the usual case were people
were bitten by the old behavior is:
$ git am mbox
.... stops with conflicts
.... oops, wrong branch
$ git checkout other-branch
$ git am mbox
error: git am is already in progress
$ git am --abort
OUTCH! other-branch was reset!
rebase is not used in this manner, and even though it does reset --hard,
it doesn't hurt (that often).
-- Hannes
On Mon, May 25, 2009 at 8:17 AM, Johannes Sixt [off-list ref] wrote:
$ git am --abort
OUTCH! other-branch was reset!
If *that's* your problem, then presumably you could avoid it just by
checking whether the right branch corresponds to HEAD before doing a
reset.
Have fun,
Avery
From: Jeff King <hidden> Date: 2016-06-15 22:46:49
On Mon, May 25, 2009 at 11:54:36AM -0400, Avery Pennarun wrote:
On Mon, May 25, 2009 at 8:17 AM, Johannes Sixt [off-list ref] wrote:
quoted
$ git am --abort
OUTCH! other-branch was reset!
If *that's* your problem, then presumably you could avoid it just by
checking whether the right branch corresponds to HEAD before doing a
reset.
But that only covers one problem. How about you forgot that you had a
failed am in progress, waited hours or days, made some commits on the
same branch, tried to am a series, got the "in progress" message and
then did an "--abort"?
And yes, I have done that.
In both examples, an alternative method for dealing with this is to try
to alert the user that we are in the middle of an am when doing
potentially suspicious things (like switching branches or making commits
outside of "git am --resolved"). I don't know how well that would work
in practice.
-Peff
From: Jeff King <hidden> Date: 2016-06-15 22:46:49
On Mon, May 25, 2009 at 02:17:48PM +0200, Johannes Sixt wrote:
I wonder why we have this problem (and do something about it) with git-am,
but not with git-rebase. Is it perhaps that the usual case were people
were bitten by the old behavior is:
I don't know. I had assumed a safety valve we put in git-am might need
to be matched in rebase. But I don't recall whether I have screwed
myself in the same way with rebase. Perhaps because rebase happens on a
detached HEAD, I tend to notice sooner that something is not right.
-Peff
On Mon, May 25, 2009 at 12:02 PM, Jeff King [off-list ref] wrote:
On Mon, May 25, 2009 at 02:17:48PM +0200, Johannes Sixt wrote:
quoted
I wonder why we have this problem (and do something about it) with git-am,
but not with git-rebase. Is it perhaps that the usual case were people
were bitten by the old behavior is:
I don't know. I had assumed a safety valve we put in git-am might need
to be matched in rebase. But I don't recall whether I have screwed
myself in the same way with rebase. Perhaps because rebase happens on a
detached HEAD, I tend to notice sooner that something is not right.
Ah, maybe that's the difference. rebase seems to detach the HEAD,
then do a bunch of stuff, then reset the original branch only when
it's done. So aborting doesn't reset any branches at all, it just
checks out the original branch.
Thus one option would be to try to make am more like rebase: detach
the HEAD before it starts, and reattach it only on success. At least
then you only have one set of UI problems to fix.
(Of course, I've gotten myself into trouble anyway by checking new
stuff in on the detached HEAD and later aborting a rebase. I have
quite a love-hate relationship with detached HEADs.)
Maybe the best solution here isn't to prevent people from shooting
themselves in the foot, but instead to help them recover afterwards.
I've noticed most people don't know about 'git reflog'; they often
seem astonished when I tell them about it. reflog + not blowing away
dirty repositories would mean that you're always safe.
Have fun,
Avery
From: Junio C Hamano <hidden> Date: 2016-06-15 22:46:49
Jeff King [off-list ref] writes:
We really have no idea what state the tree is in at this
point, and whether the user might have done useful work on
top of it. So let's err on the side of keeping the user's
data intact.
The downside is that if they do have cruft to get rid of, or
want to pretend as if earlier parts of the series that were
applied did not exist, they must manually "git reset --hard"
now.
I do not see it as a major downside, but not telling them that they may
want to when we avoid doing it ourselves might be.