Re: [PATCH] Add a testcase for the safety of pull-policy='pull'.

From: Catalin Marinas <catalin.marinas@gmail.com>
Date: Mar 6, 2007, 22:37 UTC
Message-ID: <b0943d9e0703061437t67f3be3s3358b4fe3ecb2b0a@mail.gmail.com>
In-Reply-To: <20070301201047.GD4149@nan92-1-81-57-214-146.fbx.proxad.net>

On 01/03/07, Yann Dirson <ydirson@altern.org> wrote:

Show 10 quoted lines

> On Wed, Feb 28, 2007 at 10:48:51PM +0100, Yann Dirson wrote:
> > On Tue, Feb 27, 2007 at 11:38:45PM +0000, Catalin Marinas wrote:
> > > I added it so that one cannot remove the local changes by doing a
> > > "push --undo" in error. You would have to explicitly ask for local
> > > changes removing with status --reset.
> >
> > At least, the message in that case should probably be made better,
> > when undoing to avoid having to resolve a conflict: the message says I
> > cannot undo because I have a conflict, whereas it is the exact reason
> > why I want to undo.

That's because of the order of the safety checks.

Show 6 quoted lines

> > Especially, since the conflict markers are now auto-recorded, we could
> > simply allow undo in that case.
>
> Actually, I find the behaviour to be much worse than before: forcing
> the user to run "status --reset" before "push --undo" indeed brings
> the patch that conflicted in a state where it is partly committed.

I don't have a strong opinion on forcing status --reset before push --undo anyway, so I can revert that commit (it bothers me a bit as well :-)).

Show 5 quoted lines

> Indeed, the problem may well be that we should not commit the
> unresolved conflict.  Why it has some value, recording it as if the
> user had refreshed the patch is probably not a good idea at all,
> precisely because we're mid-way in the push, and that the operation
> can be aborted without stgit knowing.

The idea of commit 0f4eba6a37c1a5454560b097873e5a22bfcde908 was to only show the conflict files with 'stg diff' and also allow me to later see how I fixed a conflict via the 'stg log (-p|-g)' command. But this interim refresh shouldn't affect the backup information (backup = False in Series.refresh_patch). Do you have a simple test showing this problem?

Regards.

-- 
Catalin