AssertionError in "stg uncommit" when going too far back

Hello!

"stg uncommit" causes AssertionError if the number commits to uncommit it too large:

$ stg uncommit -n 10000 
Uncommitting 10000 patches ... Traceback (most recent call last):
  File "/home/proski/bin/stg", line 43, in <module>
    main()
  File "home/proski/lib/python2.5/site-packages/stgit/main.py", line 278, in main
  File "home/proski/lib/python2.5/site-packages/stgit/commands/uncommit.py", line 94, in func
  File "home/proski/lib/python2.5/site-packages/stgit/lib/git.py", line 171, in parent
AssertionError

Perhaps parent() should raise an exception that uncommit would
intercept?  Sorry, my Python is not so good to make a correct fix.
Here's a prototype patch that still doesn't terminate gracefully:

AttributeError: 'module' object has no attribute 'NoParentException'

diff --git a/stgit/commands/uncommit.py b/stgit/commands/uncommit.py
index 933ec60..87dab77 100644
--- a/stgit/commands/uncommit.py
+++ b/stgit/commands/uncommit.py

@@ -21,7 +21,7 @@ from optparse import make_option
 from stgit.commands import common
 from stgit.lib import transaction
 from stgit.out import *
-from stgit import utils
+from stgit import utils, git
 
 help = 'turn regular GIT commits into StGIT patches'
 usage = """%prog [<patchnames>] | -n NUM [<prefix>]] | -t <committish> [-x]

@@ -89,9 +89,12 @@ def func(parser, options, args):
     next_commit = stack.base
     if patch_nr:
         out.start('Uncommitting %d patches' % patch_nr)
-        for i in xrange(patch_nr):
-            commits.append(next_commit)
-            next_commit = next_commit.data.parent
+        try:
+            for i in xrange(patch_nr):
+                commits.append(next_commit)
+                next_commit = next_commit.data.parent
+        except git.NoParentException:
+            raise common.CmdException('Cannot go beyond initial commit')
     else:
         if options.exclusive:
             out.start('Uncommitting to %s (exclusive)' % to_commit)

diff --git a/stgit/lib/git.py b/stgit/lib/git.py
index 50dc4f1..d8d339c 100644
--- a/stgit/lib/git.py
+++ b/stgit/lib/git.py

@@ -16,6 +16,9 @@ class DetachedHeadException(RepositoryException):
     def __init__(self):
         RepositoryException.__init__(self, 'Not on any branch')
 
+class NoParentException(exception.StgException):
+    pass
+
 class Repr(object):
     def __repr__(self):
         return str(self)

@@ -168,7 +171,8 @@ class Commitdata(Repr):
     parents = property(lambda self: self.__parents)
     @property
     def parent(self):
-        assert len(self.__parents) == 1
+        if len(self.__parents) != 1:
+            raise NoParentException
         return self.__parents[0]
     author = property(lambda self: self.__author)
     committer = property(lambda self: self.__committer)

-- 
Regards,
Pavel Roskin

On 05/03/2008, Pavel Roskin [off-list ref] wrote:

"stg uncommit" causes AssertionError if the number commits to uncommit it too large:

$ stg uncommit -n 10000
Uncommitting 10000 patches ... Traceback (most recent call last):
 File "/home/proski/bin/stg", line 43, in <module>
   main()
 File "home/proski/lib/python2.5/site-packages/stgit/main.py", line 278, in main
 File "home/proski/lib/python2.5/site-packages/stgit/commands/uncommit.py", line 94, in func
 File "home/proski/lib/python2.5/site-packages/stgit/lib/git.py", line 171, in parent
AssertionError

That's on the master branch. I think on the stable one, it used to
report a message like "Commit doesn't have exactly one parent".

In the latest version, we should indeed raise an exception rather than
just assert. I wouldn't declare a new exception class but rather use
an existing one with a meaningful error message. We can't recover from
this anyway and we would simply cancel the operation. I'll let Karl
comment on this as well.

Thanks for reporting it.

-- 
Catalin