On 2008-11-23 21:20:59 +0000, Catalin Marinas wrote:
> 2008/11/12 Karl Hasselström <kha@xxxxxxxxxxx>:
>
> > On 2008-11-12 10:02:10 +0000, Catalin Marinas wrote:
> >
> > > I think it's just a matter of updating HEAD on the
> > > "merge_conflict" path but I'm still not fully confident in
> > > modifying the new lib infrastructure.
> >
> > You're probably right.
>
> The simple patch below seems to fix it the goto issue. Could you
> please confirm its correctness (the patch might be wrapped by the web
> interface)?
>
> diff --git a/stgit/lib/transaction.py b/stgit/lib/transaction.py
> index 6623645..0f414d8 100644
> --- a/stgit/lib/transaction.py
> +++ b/stgit/lib/transaction.py
> @@ -321,6 +321,7 @@ class StackTransaction(object):
> if any(getattr(cd, a) != getattr(orig_cd, a) for a in
> ['parent', 'tree', 'author', 'message']):
> comm = self.__stack.repository.commit(cd)
> + self.head = comm
> else:
> comm = None
> s = ' (unmodified)'
OK, so I just took a quick look, and my understanding of the problem
(without having had time to actually run any code to confirm) is that
the following happens:
1. In push_patch(), we delay the final stack update (the update()
function) since we want to record the state just before the
conflict in the stack log.
2. In run(), we update the branch head before running the delayed
stack update (self.__conflicting_push()).
Your patch works around this problem by explicitly specifying what the
branch head should be; this mechanism is used by undo etc. to be able
to set the branch head to something that isn't the stack top.
At first I thought it was something of a hack, and that the run()
function should probably be modified to do something clever when
self.__conflicting_push != None, such as writing the branch head
later. But there's a reason the checkout is done first in run(): it's
the one and only thing that can go wrong, in which case we have to
roll back. So I guess your solution is indeed at least as right and
proper as anything I would have come up with.
But I would recommend a big fat comment just before the line you
insert that explains why we have to set self.head hard in this case
(namely, that the update() function is run _after_ checkout in run()).
With that comment,
Acked-by: Karl Hasselström <kha@xxxxxxxxxxx>
--
Karl Hasselström, kha@xxxxxxxxxxx
www.treskal.com/kalle
--
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
