Stgit and refresh-temp

Stgit and refresh-temp

[Jon Smirl]

I hit a case when refreshing a buried patch that needed a merge
conflict sorted out.
I'm unable to recover out of the state.

jonsmirl@terra:~/fs$ stg status
A drivers/input/ir/ir-configfs.c
A drivers/input/ir/ir-core.c
A drivers/input/ir/ir.h
C drivers/input/ir/Makefile
D drivers/input/ir-configfs.c
D drivers/input/ir-core.c
D drivers/input/ir.h
M drivers/input/Makefile
M drivers/input/input.c
jonsmirl@terra:~/fs$ gedit drivers/input/Makefile drivers/input/input.c
jonsmirl@terra:~/fs$ gedit drivers/input/ir/Makefile
jonsmirl@terra:~/fs$ stg resolve drivers/input/ir/Makefile
jonsmirl@terra:~/fs$ stg help
usage: stg <command> [options]

Generic commands:
  help        print the detailed command usage
  version     display version information
  copyright   display copyright information

Repository commands:
  clone     Make a local clone of a remote repository
  id        Print the git hash value of a StGit reference

Stack (branch) commands:
  branch    Branch operations: switch, list, create, rename, delete, ...
  clean     Delete the empty patches in the series
  coalesce  Coalesce two or more patches into one
  commit    Permanently store the applied patches into the stack base
  float     Push patches to the top, even if applied
  goto      Push or pop patches to the given one
  hide      Hide a patch in the series
  init      Initialise the current branch for use with StGIT
  log       Display the patch changelog
  patches   Show the applied patches modifying a file
  pop       Pop one or more patches from the stack
  pull      Pull changes from a remote repository
  push      Push one or more patches onto the stack
  rebase    Move the stack base to another point in history
  redo      Undo the last undo operation
  repair    Fix StGit metadata if branch was modified with git commands
  reset     Reset the patch stack to an earlier state
  series    Print the patch series
  sink      Send patches deeper down the stack
  top       Print the name of the top patch
  uncommit  Turn regular git commits into StGit patches
  undo      Undo the last operation
  unhide    Unhide a hidden patch

Patch commands:
  delete    Delete patches
  edit      edit a patch description or diff
  export    Export patches to a directory
  files     Show the files modified by a patch (or the current patch)
  fold      Integrate a GNU diff patch into the current patch
  import    Import a GNU diff file as a new patch
  mail      Send a patch or series of patches by e-mail
  new       Create a new, empty patch
  pick      Import a patch from a different branch or a commit object
  refresh   Generate a new commit for the current patch
  rename    Rename a patch
  show      Show the commit corresponding to a patch
  sync      Synchronise patches with a branch or a series

Index/worktree commands:
  diff      Show the tree diff
  resolved  Mark a file conflict as solved
  status    Show the tree status
jonsmirl@terra:~/fs$ stg help coalesce
jonsmirl@terra:~/fs$ stg series
+ Makefile
+ anton_1
+ anton_3
+ add-of_find_i2c_device_by_node
+ jds_platform
+ max9485
+ jds-soc-machine
+ jds-psc-c
+ soc-u32-cleanup
+ jds-audio
+ g_spi_4
+ m_1
+ spi-mmc
+ mpc5200-rtc
+ lirc
+ jds-lirc
> refresh-temp
- jds-lirc-gpt
- jds-lirc-device-tree
- jds-lirc-mce2
jonsmirl@terra:~/fs$ ls
arch     Documentation  init    MAINTAINERS     net
samples     usr             vmlinux.strip.gz
block    drivers        ipc     Makefile        patches-master  scripts     virt
COPYING  firmware       Kbuild  mm              patches-save
security    vmlinux
CREDITS  fs             kernel  modules.order   README          sound
     vmlinux.bin.gz
crypto   include        lib     Module.symvers  REPORTING-BUGS
System.map  vmlinux.o
jonsmirl@terra:~/fs$ mv patches-master patches-foo
jonsmirl@terra:~/fs$ stg export
Checking for changes in the working directory ... done
Warning: Local changes in the tree; you might want to commit them first
jonsmirl@terra:~/fs$ stg status
A drivers/input/ir/ir-configfs.c
A drivers/input/ir/ir-core.c
A drivers/input/ir/ir.h
D drivers/input/ir-configfs.c
D drivers/input/ir-core.c
D drivers/input/ir.h
M drivers/input/Makefile
M drivers/input/input.c
M drivers/input/ir/Makefile
jonsmirl@terra:~/fs$ ls patches-master
add-of_find_i2c_device_by_node  g_spi_4    jds_platform     lirc
max9485       series
anton_1                         jds-audio  jds-psc-c        m_1
mpc5200-rtc   soc-u32-cleanup
anton_3                         jds-lirc   jds-soc-machine  Makefile
refresh-temp  spi-mmc
jonsmirl@terra:~/fs$ vi patches-master/refresh-temp
jonsmirl@terra:~/fs$ stg series
+ Makefile
+ anton_1
+ anton_3
+ add-of_find_i2c_device_by_node
+ jds_platform
+ max9485
+ jds-soc-machine
+ jds-psc-c
+ soc-u32-cleanup
+ jds-audio
+ g_spi_4
+ m_1
+ spi-mmc
+ mpc5200-rtc
+ lirc
+ jds-lirc
> refresh-temp
- jds-lirc-gpt
- jds-lirc-device-tree
- jds-lirc-mce2
jonsmirl@terra:~/fs$ stg refresh -p jds-lirc
Error: HEAD and top are not the same.
       This can happen if you modify a branch with git.
       "stg repair --help" explains more about what to do next.
stg refresh: Command aborted (all changes rolled back)
jonsmirl@terra:~/fs$ stg refresh
Error: HEAD and top are not the same.
       This can happen if you modify a branch with git.
       "stg repair --help" explains more about what to do next.
stg refresh: Command aborted (all changes rolled back)
jonsmirl@terra:~/fs$ stg new foo
Invoking the editor: "vi .stgit-new.txt" ... done
Error: HEAD and top are not the same.
       This can happen if you modify a branch with git.
       "stg repair --help" explains more about what to do next.
stg new: Command aborted (all changes rolled back)
jonsmirl@terra:~/fs$ stg new foo
Invoking the editor: "vi .stgit-new.txt" ... done
Error: HEAD and top are not the same.
       This can happen if you modify a branch with git.
       "stg repair --help" explains more about what to do next.
stg new: Command aborted (all changes rolled back)
jonsmirl@terra:~/fs$ stg repair --help
Usage: stg repair

Fix StGit metadata if branch was modified with git commands

Options:
  -h, --help  show this help message and exit
jonsmirl@terra:~/fs$




-- 
Jon Smirl
jonsmirl@gmail.com

Re: Stgit and refresh-temp

[Jon Smirl]

jonsmirl@terra:~/fs$ stg version
Stacked GIT 0.14.3.270.g0f36
git version 1.6.0.3.523.g304d0
Python version 2.5.2 (r252:60911, Oct  5 2008, 19:29:17)
[GCC 4.3.2]
jonsmirl@terra:~/fs$

-- 
Jon Smirl
jonsmirl@gmail.com

Re: Stgit and refresh-temp

[Jon Smirl]

I think I fixed my tree up. After a stg repair I was able to delete
'refresh-temp' which was empty, then apply the changes to jds-lirc.

It may have been possible to make the merge smarter. The conflicts
were with things in the popped-off patches. Your typical end of file
append merge conflicts.

On Tue, Nov 4, 2008 at 8:38 AM, Jon Smirl <jonsmirl@gmail.com> wrote:
> jonsmirl@terra:~/fs$ stg version
> Stacked GIT 0.14.3.270.g0f36
> git version 1.6.0.3.523.g304d0
> Python version 2.5.2 (r252:60911, Oct  5 2008, 19:29:17)
> [GCC 4.3.2]
> jonsmirl@terra:~/fs$
>
> --
> Jon Smirl
> jonsmirl@gmail.com
>



-- 
Jon Smirl
jonsmirl@gmail.com

Re: Stgit and refresh-temp

[Karl Hasselström]

On 2008-11-04 08:37:24 -0500, Jon Smirl wrote:

> I hit a case when refreshing a buried patch that needed a merge
> conflict sorted out. I'm unable to recover out of the state.

Hmm, so what you're saying is basically that you did something with
"stg refresh -p" that caused a merge conflict, and that messed things
up so that you needed to run "stg repair". Is that right?

Have you been able to reproduce it? (I would like to add the failing
case to the test suite.)

-- 
Karl Hasselström, kha@treskal.com
      www.treskal.com/kalle

Re: Stgit and refresh-temp

[Jon Smirl]

On Fri, Nov 7, 2008 at 12:44 AM, Karl Hasselström <kha@treskal.com> wrote:
> On 2008-11-04 08:37:24 -0500, Jon Smirl wrote:
>
>> I hit a case when refreshing a buried patch that needed a merge
>> conflict sorted out. I'm unable to recover out of the state.
>
> Hmm, so what you're saying is basically that you did something with
> "stg refresh -p" that caused a merge conflict, and that messed things
> up so that you needed to run "stg repair". Is that right?

Missed the reply.

Yes, that is what happed.

I think the problem was this:

File - xxxxxxx
Patch A adds a line
File - xxxxxxxa
Patch B in the middle adds a line
File - xxxxxxxab
I edit it and add a line
File - xxxxxxxabc
Line c needs to be patch A
stg refresh -p A
..messed up tree



>
> Have you been able to reproduce it? (I would like to add the failing
> case to the test suite.)
>
> --
> Karl Hasselström, kha@treskal.com
>      www.treskal.com/kalle
>



-- 
Jon Smirl
jonsmirl@gmail.com

Re: Stgit and refresh-temp

[Jon Smirl]

On Mon, Nov 10, 2008 at 6:08 PM, Jon Smirl <jonsmirl@gmail.com> wrote:
> On Fri, Nov 7, 2008 at 12:44 AM, Karl Hasselström <kha@treskal.com> wrote:
>> On 2008-11-04 08:37:24 -0500, Jon Smirl wrote:
>>
>>> I hit a case when refreshing a buried patch that needed a merge
>>> conflict sorted out. I'm unable to recover out of the state.
>>
>> Hmm, so what you're saying is basically that you did something with
>> "stg refresh -p" that caused a merge conflict, and that messed things
>> up so that you needed to run "stg repair". Is that right?
>
> Missed the reply.
>
> Yes, that is what happed.
>
> I think the problem was this:
>
> File - xxxxxxx
> Patch A adds a line
> File - xxxxxxxa
> Patch B in the middle adds a line
> File - xxxxxxxab
> I edit it and add a line
> File - xxxxxxxabc
> Line c needs to be patch A
> stg refresh -p A
> ..messed up tree
>
>
>
>>
>> Have you been able to reproduce it? (I would like to add the failing
>> case to the test suite.)

Yes, it happens every time the 'stg refresh -p' triggers a merge conflict.
stg repair seem to fix it and then I touch things up manually.

>>
>> --
>> Karl Hasselström, kha@treskal.com
>>      www.treskal.com/kalle
>>
>
>
>
> --
> Jon Smirl
> jonsmirl@gmail.com
>



-- 
Jon Smirl
jonsmirl@gmail.com

Re: Stgit and refresh-temp

[Catalin Marinas]

2008/11/7 Karl Hasselström <kha@treskal.com>:
> On 2008-11-04 08:37:24 -0500, Jon Smirl wrote:
>
>> I hit a case when refreshing a buried patch that needed a merge
>> conflict sorted out. I'm unable to recover out of the state.
>
> Hmm, so what you're saying is basically that you did something with
> "stg refresh -p" that caused a merge conflict, and that messed things
> up so that you needed to run "stg repair". Is that right?
>
> Have you been able to reproduce it? (I would like to add the failing
> case to the test suite.)

Could be related to this - if I run 'stg goto some-patch' and it fails
with a conflict, the HEAD points to the previous patch though the
stack has the conflicting patch as empty (which is normal) and the
conflicts in the index. Anything after that says HEAD and top not
equal and 'stg repair' is needed.

-- 
Catalin

Re: Stgit and refresh-temp

[Karl Hasselström]

On 2008-11-11 17:59:02 +0000, Catalin Marinas wrote:

> 2008/11/7 Karl Hasselström <kha@treskal.com>:
>
> > On 2008-11-04 08:37:24 -0500, Jon Smirl wrote:
> >
> > > I hit a case when refreshing a buried patch that needed a merge
> > > conflict sorted out. I'm unable to recover out of the state.
> >
> > Hmm, so what you're saying is basically that you did something
> > with "stg refresh -p" that caused a merge conflict, and that
> > messed things up so that you needed to run "stg repair". Is that
> > right?
>
> Could be related to this - if I run 'stg goto some-patch' and it
> fails with a conflict, the HEAD points to the previous patch though
> the stack has the conflicting patch as empty (which is normal) and
> the conflicts in the index. Anything after that says HEAD and top
> not equal and 'stg repair' is needed.

Ah, yes, that could definitely be the same problem, since those two
things end up calling the same functions to handle the conflict.

I'll build a test case for that as well.

-- 
Karl Hasselström, kha@treskal.com
      www.treskal.com/kalle

Re: Stgit and refresh-temp

[Catalin Marinas]

2008/11/12 Karl Hasselström <kha@treskal.com>:
> On 2008-11-11 17:59:02 +0000, Catalin Marinas wrote:
>> Could be related to this - if I run 'stg goto some-patch' and it
>> fails with a conflict, the HEAD points to the previous patch though
>> the stack has the conflicting patch as empty (which is normal) and
>> the conflicts in the index. Anything after that says HEAD and top
>> not equal and 'stg repair' is needed.
>
> Ah, yes, that could definitely be the same problem, since those two
> things end up calling the same functions to handle the conflict.

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.

> I'll build a test case for that as well.

We test the conflicts quite a lot in the "push" tests but this command
hasn't been converted to the new infrastructure yet.

-- 
Catalin

Re: Stgit and refresh-temp

[Karl Hasselström]

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.

I'll do it, but you're welcome to beat me to it if you like; I promise
to read your patch extra carefully if you do.

(I'll drop you a mail when I start attacking this, to prevent any
duplicated work.)

> 2008/11/12 Karl Hasselström <kha@treskal.com>:
>
> > I'll build a test case for that as well.
>
> We test the conflicts quite a lot in the "push" tests but this
> command hasn't been converted to the new infrastructure yet.

Yes. As soon as we convert push and/or pop, this will be caught by the
test suite. But that just means the test suite needs extending ...

-- 
Karl Hasselström, kha@treskal.com
      www.treskal.com/kalle

Re: Stgit and refresh-temp

[Karl Hasselström]

On 2008-11-12 09:01:03 +0100, Karl Hasselström wrote:

> On 2008-11-11 17:59:02 +0000, Catalin Marinas wrote:
>
> > 2008/11/7 Karl Hasselström <kha@treskal.com>:
> >
> > > On 2008-11-04 08:37:24 -0500, Jon Smirl wrote:
> > >
> > > > I hit a case when refreshing a buried patch that needed a
> > > > merge conflict sorted out. I'm unable to recover out of the
> > > > state.
> > >
> > > Hmm, so what you're saying is basically that you did something
> > > with "stg refresh -p" that caused a merge conflict, and that
> > > messed things up so that you needed to run "stg repair". Is that
> > > right?
> >
> > Could be related to this - if I run 'stg goto some-patch' and it
> > fails with a conflict, the HEAD points to the previous patch
> > though the stack has the conflicting patch as empty (which is
> > normal) and the conflicts in the index. Anything after that says
> > HEAD and top not equal and 'stg repair' is needed.
>
> Ah, yes, that could definitely be the same problem, since those two
> things end up calling the same functions to handle the conflict.

OK, I just got this error with goto. :-)

FWIW, the convenient way to recover is

  $ git reset --soft $(stg id $(stg top))

This will point your branch head to the correct commit. stg repair
would pop the top patch, which is much less convenient in this case.

-- 
Karl Hasselström, kha@treskal.com
      www.treskal.com/kalle

Re: Stgit and refresh-temp

[Catalin Marinas]

2008/11/12 Karl Hasselström <kha@treskal.com>:
> 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)'

Thanks.

-- 
Catalin

Re: Stgit and refresh-temp

[Karl Hasselström]

On 2008-11-23 21:20:59 +0000, Catalin Marinas wrote:

> 2008/11/12 Karl Hasselström <kha@treskal.com>:
>
> > 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@treskal.com>

-- 
Karl Hasselström, kha@treskal.com
      www.treskal.com/kalle

Re: Stgit and refresh-temp

[Jon Smirl]

A useful future enhancement would be to adjust patches that insert
adjacent areas so that they can pass each other in a pop/pop
operation. The most common case of this being appends to end of file.

This would be a form of automatic conflict resolution. The pending
patch would be automatically corrected to resolve the adjacent insert
conflict.  You might want a prompt asking if this was ok andt then
remember the answer so that question is not asked repeatedly.

It's a mechanism to say that multiple insertions at point X in the
original file don't matter in their order of insertion.

-- 
Jon Smirl
jonsmirl@gmail.com