git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
* [PATCH 0/3] Misc stgit patches
@ 2006-11-30  0:23 Yann Dirson
  2006-11-30  0:27 ` [PATCH 1/3] Document some current bugs and add to the TODO list Yann Dirson
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Yann Dirson @ 2006-11-30  0:23 UTC (permalink / raw
  To: Catalin Marinas; +Cc: GIT list

One of the following patches contains a pretty substantial change to stack.py to help
adding new stored properties to a Series just like it is done for a Patch.  It was
expected to be the first in a series for storing branch parent information, but as you
may have read already, it's not for today :)

-- 
Yann Dirson    <ydirson@altern.org> |
Debian-related: <dirson@debian.org> |   Support Debian GNU/Linux:
                                    |  Freedom, Power, Stability, Gratis

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH 1/3] Document some current bugs and add to the TODO list.
  2006-11-30  0:23 [PATCH 0/3] Misc stgit patches Yann Dirson
@ 2006-11-30  0:27 ` Yann Dirson
  2006-12-05 17:30   ` Catalin Marinas
  2006-11-30  0:27 ` [PATCH 2/3] More config examples Yann Dirson
  2006-11-30  0:27 ` [PATCH 3/3] Create a StgitObject class to factorise code for property handling Yann Dirson
  2 siblings, 1 reply; 6+ messages in thread
From: Yann Dirson @ 2006-11-30  0:27 UTC (permalink / raw
  To: Catalin Marinas; +Cc: GIT list




Signed-off-by: Yann Dirson <ydirson@altern.org>
---

 TODO |   51 +++++++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 51 insertions(+), 0 deletions(-)

diff --git a/TODO b/TODO
index 6f7a132..ad02a0e 100644
--- a/TODO
+++ b/TODO
@@ -26,3 +26,54 @@ The future, when time allows or if someone else does them:
 - patch synchronisation between between branches (as some people,
   including me have the same patches based on different branches and
   they have scripts for moving the changes in one to the others)
+- numeric shortcuts for naming patches near top (eg. +1, -2)
+- (config?) parameter for number of patches included by "series -s"
+- refuse to "stg init" a branch known as remote (through .git/remotes/,
+  .git/branches/ or any other info)
+
+Bugs:
+
+- cannot use "stg refresh file" after "cg-rm file"
+- patch names with spaces are accepted by "stg new" but break "stg series -d"
+- "stg goto $(stg top)" fails with unhandled exception
+- at least "commit is not robust wrt out-of-diskspace condition:
+|deps$ stg commit
+|error: git-checkout-index: unable to write file MANIFEST
+|error: git-checkout-index: unable to write file META.yml
+|error: git-checkout-index: unable to write file Makefile.PL
+|error: git-checkout-index: unable to write file doc/README.dbk.xml
+|error: git-checkout-index: unable to write file graph-includes
+|error: git-checkout-index: unable to write file lib/graphincludes/params.pm
+|fatal: unable to write new index file
+|stg commit: git-read-tree failed (local changes maybe?)
+|Committing 4 patches...
+(luckily nothing was really committed)
+
+- cannot branch off arbitrary branch when current branch not under
+stgit control:
+|$ stg branch 
+|bar
+|$ stg branch -c foo2 foo
+|stg branch: Branch "bar" not initialised
+
+- patch created with empty description ("stg new" and quit editor
+without saving) confuse "series -d":
+|$ stg series -ds
+|+ p5  | p5
+|Traceback (most recent call last):
+|  File "/usr/bin/stg", line 43, in ?
+|    main()
+|  File "/var/lib/python-support/python2.4/stgit/main.py", line 261, in main
+|    command.func(parser, options, args)
+|  File "/var/lib/python-support/python2.4/stgit/commands/series.py", line 107, in func
+|    __print_patch(applied[-1], '> ', '0>', max_len, options)
+|  File "/var/lib/python-support/python2.4/stgit/commands/series.py", line 63, in __print_patch
+|    print prefix + patch.ljust(length) + '  | ' + __get_description(patch)
+|  File "/var/lib/python-support/python2.4/stgit/commands/series.py", line 55, in __get_description
+|    descr = p.get_description().strip()
+|AttributeError: 'NoneType' object has no attribute 'strip'
+|dwitch@gandelf:/export/work/yann/git/foo/a$ stg series
+|+ p5
+|> y
+|$ cat .git/patches/master/patches/y/description

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH 2/3] More config examples.
  2006-11-30  0:23 [PATCH 0/3] Misc stgit patches Yann Dirson
  2006-11-30  0:27 ` [PATCH 1/3] Document some current bugs and add to the TODO list Yann Dirson
@ 2006-11-30  0:27 ` Yann Dirson
  2006-11-30  0:27 ` [PATCH 3/3] Create a StgitObject class to factorise code for property handling Yann Dirson
  2 siblings, 0 replies; 6+ messages in thread
From: Yann Dirson @ 2006-11-30  0:27 UTC (permalink / raw
  To: Catalin Marinas; +Cc: GIT list




Signed-off-by: Yann Dirson <ydirson@altern.org>
---

 examples/stgitrc |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/examples/stgitrc b/examples/stgitrc
index cc6e910..9ed2988 100644
--- a/examples/stgitrc
+++ b/examples/stgitrc
@@ -32,6 +32,7 @@
 
 # this value overrides the default PAGER environment variable
 #pager: ~/share/stgit/contrib/diffcol.sh
+#pager: filterdiff --annotate | colordiff | less -FRX
 
 # GIT pull command (should take the same arguments as git-pull)

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH 3/3] Create a StgitObject class to factorise code for property handling.
  2006-11-30  0:23 [PATCH 0/3] Misc stgit patches Yann Dirson
  2006-11-30  0:27 ` [PATCH 1/3] Document some current bugs and add to the TODO list Yann Dirson
  2006-11-30  0:27 ` [PATCH 2/3] More config examples Yann Dirson
@ 2006-11-30  0:27 ` Yann Dirson
  2 siblings, 0 replies; 6+ messages in thread
From: Yann Dirson @ 2006-11-30  0:27 UTC (permalink / raw
  To: Catalin Marinas; +Cc: GIT list


This change makes it easier to add new stored fields to the Series object,
without having to duplicate existing code from Patch.

Generic field accessors were taken from the Patch class.  Dir accessors
were added to avoid making the __dir attribute public, and were used
to replace Series::__series_dir (that name was a bit redundant anyway).
Create_empty_field came as a natural addition to factorise more code.

Signed-off-by: Yann Dirson <ydirson@altern.org>
---

 stgit/stack.py |  198 +++++++++++++++++++++++++++++---------------------------
 1 files changed, 103 insertions(+), 95 deletions(-)

diff --git a/stgit/stack.py b/stgit/stack.py
index 8fa3846..8cd632e 100644
--- a/stgit/stack.py
+++ b/stgit/stack.py
@@ -119,27 +119,58 @@ def edit_file(series, line, comment, show_patch = True):
 # Classes
 #
 
-class Patch:
+class StgitObject:
+    """An object with stgit-like properties stored as files in a directory
+    """
+    def _set_dir(self, dir):
+        self.__dir = dir
+    def dir(self):
+        return self.__dir
+
+    def create_empty_field(self, name):
+        print "Creating '%s'" % os.path.join(self.__dir, name)
+        create_empty_file(os.path.join(self.__dir, name))
+
+    def _get_field(self, name, multiline = False):
+        id_file = os.path.join(self.__dir, name)
+        if os.path.isfile(id_file):
+            line = read_string(id_file, multiline)
+            if line == '':
+                return None
+            else:
+                return line
+        else:
+            return None
+
+    def _set_field(self, name, value, multiline = False):
+        fname = os.path.join(self.__dir, name)
+        if value and value != '':
+            write_string(fname, value, multiline)
+        elif os.path.isfile(fname):
+            os.remove(fname)
+
+    
+class Patch(StgitObject):
     """Basic patch implementation
     """
     def __init__(self, name, series_dir, refs_dir):
         self.__series_dir = series_dir
         self.__name = name
-        self.__dir = os.path.join(self.__series_dir, self.__name)
+        self._set_dir(os.path.join(self.__series_dir, self.__name))
         self.__refs_dir = refs_dir
         self.__top_ref_file = os.path.join(self.__refs_dir, self.__name)
         self.__log_ref_file = os.path.join(self.__refs_dir,
                                            self.__name + '.log')
 
     def create(self):
-        os.mkdir(self.__dir)
-        create_empty_file(os.path.join(self.__dir, 'bottom'))
-        create_empty_file(os.path.join(self.__dir, 'top'))
+        os.mkdir(self.dir())
+        self.create_empty_field('bottom')
+        self.create_empty_field('top')
 
     def delete(self):
-        for f in os.listdir(self.__dir):
-            os.remove(os.path.join(self.__dir, f))
-        os.rmdir(self.__dir)
+        for f in os.listdir(self.dir()):
+            os.remove(os.path.join(self.dir(), f))
+        os.rmdir(self.dir())
         os.remove(self.__top_ref_file)
         if os.path.exists(self.__log_ref_file):
             os.remove(self.__log_ref_file)
@@ -148,16 +179,16 @@ class Patch:
         return self.__name
 
     def rename(self, newname):
-        olddir = self.__dir
+        olddir = self.dir()
         old_top_ref_file = self.__top_ref_file
         old_log_ref_file = self.__log_ref_file
         self.__name = newname
-        self.__dir = os.path.join(self.__series_dir, self.__name)
+        self._set_dir(os.path.join(self.__series_dir, self.__name))
         self.__top_ref_file = os.path.join(self.__refs_dir, self.__name)
         self.__log_ref_file = os.path.join(self.__refs_dir,
                                            self.__name + '.log')
 
-        os.rename(olddir, self.__dir)
+        os.rename(olddir, self.dir())
         os.rename(old_top_ref_file, self.__top_ref_file)
         if os.path.exists(old_log_ref_file):
             os.rename(old_log_ref_file, self.__log_ref_file)
@@ -173,69 +204,51 @@ class Patch:
         if top:
             self.__update_top_ref(top)
 
-    def __get_field(self, name, multiline = False):
-        id_file = os.path.join(self.__dir, name)
-        if os.path.isfile(id_file):
-            line = read_string(id_file, multiline)
-            if line == '':
-                return None
-            else:
-                return line
-        else:
-            return None
-
-    def __set_field(self, name, value, multiline = False):
-        fname = os.path.join(self.__dir, name)
-        if value and value != '':
-            write_string(fname, value, multiline)
-        elif os.path.isfile(fname):
-            os.remove(fname)
-
     def get_old_bottom(self):
-        return self.__get_field('bottom.old')
+        return self._get_field('bottom.old')
 
     def get_bottom(self):
-        return self.__get_field('bottom')
+        return self._get_field('bottom')
 
     def set_bottom(self, value, backup = False):
         if backup:
-            curr = self.__get_field('bottom')
-            self.__set_field('bottom.old', curr)
-        self.__set_field('bottom', value)
+            curr = self._get_field('bottom')
+            self._set_field('bottom.old', curr)
+        self._set_field('bottom', value)
 
     def get_old_top(self):
-        return self.__get_field('top.old')
+        return self._get_field('top.old')
 
     def get_top(self):
-        return self.__get_field('top')
+        return self._get_field('top')
 
     def set_top(self, value, backup = False):
         if backup:
-            curr = self.__get_field('top')
-            self.__set_field('top.old', curr)
-        self.__set_field('top', value)
+            curr = self._get_field('top')
+            self._set_field('top.old', curr)
+        self._set_field('top', value)
         self.__update_top_ref(value)
 
     def restore_old_boundaries(self):
-        bottom = self.__get_field('bottom.old')
-        top = self.__get_field('top.old')
+        bottom = self._get_field('bottom.old')
+        top = self._get_field('top.old')
 
         if top and bottom:
-            self.__set_field('bottom', bottom)
-            self.__set_field('top', top)
+            self._set_field('bottom', bottom)
+            self._set_field('top', top)
             self.__update_top_ref(top)
             return True
         else:
             return False
 
     def get_description(self):
-        return self.__get_field('description', True)
+        return self._get_field('description', True)
 
     def set_description(self, line):
-        self.__set_field('description', line, True)
+        self._set_field('description', line, True)
 
     def get_authname(self):
-        return self.__get_field('authname')
+        return self._get_field('authname')
 
     def set_authname(self, name):
         if not name:
@@ -243,10 +256,10 @@ class Patch:
                 name = config.get('stgit', 'authname')
             elif 'GIT_AUTHOR_NAME' in os.environ:
                 name = os.environ['GIT_AUTHOR_NAME']
-        self.__set_field('authname', name)
+        self._set_field('authname', name)
 
     def get_authemail(self):
-        return self.__get_field('authemail')
+        return self._get_field('authemail')
 
     def set_authemail(self, address):
         if not address:
@@ -254,18 +267,18 @@ class Patch:
                 address = config.get('stgit', 'authemail')
             elif 'GIT_AUTHOR_EMAIL' in os.environ:
                 address = os.environ['GIT_AUTHOR_EMAIL']
-        self.__set_field('authemail', address)
+        self._set_field('authemail', address)
 
     def get_authdate(self):
-        return self.__get_field('authdate')
+        return self._get_field('authdate')
 
     def set_authdate(self, date):
         if not date and 'GIT_AUTHOR_DATE' in os.environ:
             date = os.environ['GIT_AUTHOR_DATE']
-        self.__set_field('authdate', date)
+        self._set_field('authdate', date)
 
     def get_commname(self):
-        return self.__get_field('commname')
+        return self._get_field('commname')
 
     def set_commname(self, name):
         if not name:
@@ -273,10 +286,10 @@ class Patch:
                 name = config.get('stgit', 'commname')
             elif 'GIT_COMMITTER_NAME' in os.environ:
                 name = os.environ['GIT_COMMITTER_NAME']
-        self.__set_field('commname', name)
+        self._set_field('commname', name)
 
     def get_commemail(self):
-        return self.__get_field('commemail')
+        return self._get_field('commemail')
 
     def set_commemail(self, address):
         if not address:
@@ -284,17 +297,17 @@ class Patch:
                 address = config.get('stgit', 'commemail')
             elif 'GIT_COMMITTER_EMAIL' in os.environ:
                 address = os.environ['GIT_COMMITTER_EMAIL']
-        self.__set_field('commemail', address)
+        self._set_field('commemail', address)
 
     def get_log(self):
-        return self.__get_field('log')
+        return self._get_field('log')
 
     def set_log(self, value, backup = False):
-        self.__set_field('log', value)
+        self._set_field('log', value)
         self.__update_log_ref(value)
 
 
-class Series:
+class Series(StgitObject):
     """Class including the operations on series
     """
     def __init__(self, name = None):
@@ -309,22 +322,21 @@ class Series:
         except git.GitException, ex:
             raise StackException, 'GIT tree not initialised: %s' % ex
 
-        self.__series_dir = os.path.join(self.__base_dir, 'patches',
-                                         self.__name)
+        self._set_dir(os.path.join(self.__base_dir, 'patches', self.__name))
         self.__refs_dir = os.path.join(self.__base_dir, 'refs', 'patches',
                                        self.__name)
         self.__base_file = os.path.join(self.__base_dir, 'refs', 'bases',
                                         self.__name)
 
-        self.__applied_file = os.path.join(self.__series_dir, 'applied')
-        self.__unapplied_file = os.path.join(self.__series_dir, 'unapplied')
-        self.__current_file = os.path.join(self.__series_dir, 'current')
-        self.__descr_file = os.path.join(self.__series_dir, 'description')
+        self.__applied_file = os.path.join(self.dir(), 'applied')
+        self.__unapplied_file = os.path.join(self.dir(), 'unapplied')
+        self.__current_file = os.path.join(self.dir(), 'current')
+        self.__descr_file = os.path.join(self.dir(), 'description')
 
         # where this series keeps its patches
-        self.__patch_dir = os.path.join(self.__series_dir, 'patches')
+        self.__patch_dir = os.path.join(self.dir(), 'patches')
         if not os.path.isdir(self.__patch_dir):
-            self.__patch_dir = self.__series_dir
+            self.__patch_dir = self.dir()
 
         # if no __refs_dir, create and populate it (upgrade old repositories)
         if self.is_initialised() and not os.path.isdir(self.__refs_dir):
@@ -333,7 +345,7 @@ class Series:
                 self.get_patch(patch).update_top_ref()
 
         # trash directory
-        self.__trash_dir = os.path.join(self.__series_dir, 'trash')
+        self.__trash_dir = os.path.join(self.dir(), 'trash')
         if self.is_initialised() and not os.path.isdir(self.__trash_dir):
             os.makedirs(self.__trash_dir)
 
@@ -345,10 +357,7 @@ class Series:
     def __set_current(self, name):
         """Sets the topmost patch
         """
-        if name:
-            write_string(self.__current_file, name)
-        else:
-            create_empty_file(self.__current_file)
+        self._set_field('current', name)
 
     def get_patch(self, name):
         """Return a Patch object for the given name
@@ -366,10 +375,7 @@ class Series:
     def get_current(self):
         """Return the name of the topmost patch, or None if there is
         no such patch."""
-        if os.path.isfile(self.__current_file):
-            name = read_string(self.__current_file)
-        else:
-            return None
+        name = self._get_field('current')
         if name == '':
             return None
         else:
@@ -396,26 +402,26 @@ class Series:
         return self.__base_file
 
     def get_protected(self):
-        return os.path.isfile(os.path.join(self.__series_dir, 'protected'))
+        return os.path.isfile(os.path.join(self.dir(), 'protected'))
 
     def protect(self):
-        protect_file = os.path.join(self.__series_dir, 'protected')
+        protect_file = os.path.join(self.dir(), 'protected')
         if not os.path.isfile(protect_file):
             create_empty_file(protect_file)
 
     def unprotect(self):
-        protect_file = os.path.join(self.__series_dir, 'protected')
+        protect_file = os.path.join(self.dir(), 'protected')
         if os.path.isfile(protect_file):
             os.remove(protect_file)
 
     def get_description(self):
-        if os.path.isfile(self.__descr_file):
-            return read_string(self.__descr_file)
-        else:
-            return ''
+        return self._get_field('description')
+
+    def set_description(self, line):
+        self._set_field('description', line)
 
     def __patch_is_current(self, patch):
-        return patch.get_name() == read_string(self.__current_file)
+        return patch.get_name() == self.get_current()
 
     def patch_applied(self, name):
         """Return true if the patch exists in the applied list
@@ -481,10 +487,10 @@ class Series:
 
         create_dirs(bases_dir)
 
-        create_empty_file(self.__applied_file)
-        create_empty_file(self.__unapplied_file)
-        create_empty_file(self.__descr_file)
-        os.makedirs(os.path.join(self.__series_dir, 'patches'))
+        self.create_empty_field('applied')
+        self.create_empty_field('unapplied')
+        self.create_empty_field('description')
+        os.makedirs(os.path.join(self.dir(), 'patches'))
         os.makedirs(self.__refs_dir)
         self.__begin_stack_check()
 
@@ -493,15 +499,15 @@ class Series:
         unconvert to place the patches in the same directory with
         series control files
         """
-        if self.__patch_dir == self.__series_dir:
+        if self.__patch_dir == self.dir():
             print 'Converting old-style to new-style...',
             sys.stdout.flush()
 
-            self.__patch_dir = os.path.join(self.__series_dir, 'patches')
+            self.__patch_dir = os.path.join(self.dir(), 'patches')
             os.makedirs(self.__patch_dir)
 
             for p in self.get_applied() + self.get_unapplied():
-                src = os.path.join(self.__series_dir, p)
+                src = os.path.join(self.dir(), p)
                 dest = os.path.join(self.__patch_dir, p)
                 os.rename(src, dest)
 
@@ -513,7 +519,7 @@ class Series:
 
             for p in self.get_applied() + self.get_unapplied():
                 src = os.path.join(self.__patch_dir, p)
-                dest = os.path.join(self.__series_dir, p)
+                dest = os.path.join(self.dir(), p)
                 os.rename(src, dest)
 
             if not os.listdir(self.__patch_dir):
@@ -522,7 +528,7 @@ class Series:
             else:
                 print 'Patch directory %s is not empty.' % self.__name
 
-            self.__patch_dir = self.__series_dir
+            self.__patch_dir = self.dir()
 
     def rename(self, to_name):
         """Renames a series
@@ -536,7 +542,7 @@ class Series:
 
         git.rename_branch(self.__name, to_name)
 
-        if os.path.isdir(self.__series_dir):
+        if os.path.isdir(self.dir()):
             rename(os.path.join(self.__base_dir, 'patches'),
                    self.__name, to_stack.__name)
         if os.path.exists(self.__base_file):
@@ -556,7 +562,7 @@ class Series:
         new_series = Series(target_series)
 
         # generate an artificial description file
-        write_string(new_series.__descr_file, 'clone of "%s"' % self.__name)
+        new_series.set_description('clone of "%s"' % self.__name)
 
         # clone self's entire series as unapplied patches
         patches = self.get_applied() + self.get_unapplied()
@@ -590,6 +596,8 @@ class Series:
                 os.remove(fname)
             os.rmdir(self.__trash_dir)
 
+            # FIXME: find a way to get rid of those manual removals
+            # (move functionnality to StgitObject ?)
             if os.path.exists(self.__applied_file):
                 os.remove(self.__applied_file)
             if os.path.exists(self.__unapplied_file):
@@ -602,7 +610,7 @@ class Series:
                 os.rmdir(self.__patch_dir)
             else:
                 print 'Patch directory %s is not empty.' % self.__name
-            if not os.listdir(self.__series_dir):
+            if not os.listdir(self.dir()):
                 remove_dirs(os.path.join(self.__base_dir, 'patches'),
                             self.__name)

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH 1/3] Document some current bugs and add to the TODO list.
  2006-11-30  0:27 ` [PATCH 1/3] Document some current bugs and add to the TODO list Yann Dirson
@ 2006-12-05 17:30   ` Catalin Marinas
  2006-12-05 22:02     ` Yann Dirson
  0 siblings, 1 reply; 6+ messages in thread
From: Catalin Marinas @ 2006-12-05 17:30 UTC (permalink / raw
  To: Yann Dirson; +Cc: GIT list

Hi Yann,

See some comments below (I removed those which I am OK with or I fixed).

On 30/11/06, Yann Dirson <ydirson@altern.org> wrote:
> +- numeric shortcuts for naming patches near top (eg. +1, -2)

We currently have the -n option for push and pop that accepts number.
Because of python, you can also, for example, push to the last but one
with "push -n -1" (similar for pop). Do you mean shortcuts for the
"goto" command?

> +- refuse to "stg init" a branch known as remote (through .git/remotes/,
> +  .git/branches/ or any other info)

I think it is up to the user not to do this. You would first need to
check out such a branch anyway.

> +- cannot use "stg refresh file" after "cg-rm file"

It seems to work for me. Can you send some log messages?

> +- "stg goto $(stg top)" fails with unhandled exception

It works for me. What StGIT version do you use?

> +- at least "commit is not robust wrt out-of-diskspace condition:
> +|deps$ stg commit
> +|error: git-checkout-index: unable to write file MANIFEST
> +|error: git-checkout-index: unable to write file META.yml
> +|error: git-checkout-index: unable to write file Makefile.PL
> +|error: git-checkout-index: unable to write file doc/README.dbk.xml
> +|error: git-checkout-index: unable to write file graph-includes
> +|error: git-checkout-index: unable to write file lib/graphincludes/params.pm
> +|fatal: unable to write new index file
> +|stg commit: git-read-tree failed (local changes maybe?)
> +|Committing 4 patches...
> +(luckily nothing was really committed)

But that's the correct behaviour, not to commit anything. The only
problem I see (and I fixed) is the "Committing..." message on stdout
without flushing, causing it to appear afterwards. StGIT cannot know
how much space is needed by GIT to check this beforehand. It simply
exits when a GIT command failed.

Thanks.

-- 

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 1/3] Document some current bugs and add to the TODO list.
  2006-12-05 17:30   ` Catalin Marinas
@ 2006-12-05 22:02     ` Yann Dirson
  0 siblings, 0 replies; 6+ messages in thread
From: Yann Dirson @ 2006-12-05 22:02 UTC (permalink / raw
  To: Catalin Marinas; +Cc: GIT list

On Tue, Dec 05, 2006 at 05:30:56PM +0000, Catalin Marinas wrote:
> On 30/11/06, Yann Dirson <ydirson@altern.org> wrote:
> >+- numeric shortcuts for naming patches near top (eg. +1, -2)
> 
> We currently have the -n option for push and pop that accepts number.
> Because of python, you can also, for example, push to the last but one
> with "push -n -1" (similar for pop). Do you mean shortcuts for the
> "goto" command?

I rather meant shortcuts for "show", "fold --pick", and possibly a
handful of others.

While we're talking about shortcuts for goto, one that I regularly miss
would be something like "goto BACK", to allow for short excursions and
quickly going back without having to worry on which exact patch I was
before.  "refresh --patch" will make this less essential, but still
possibly useful.


> >+- refuse to "stg init" a branch known as remote (through .git/remotes/,
> >+  .git/branches/ or any other info)
> 
> I think it is up to the user not to do this.

It may not be obvious to a new user, so I'd think it would be useful to
guard against things we know should not be done.

"git checkout" being probably used as plumbing in several places should
probably not be taught to refuse switching to remote branches, so I'd
think porcelains should take care of this.


> You would first need to check out such a branch anyway.

Sure.  Especially, the following should probably fail :)

|stgit$ stg branch origin
|Switching to branch "origin"... done

Maybe it could be made to accept only to change to stgit-managed
branches ?
After all, if we're switching to a non-stgit branch, we're probably
going to use another set of tools anyway, so we can probably tell the
user to use git-checkout or cg-switch instead.


> >+- cannot use "stg refresh file" after "cg-rm file"
> 
> It seems to work for me. Can you send some log messages?

I should have done that first, I cannot reproduce it any more.


> >+- "stg goto $(stg top)" fails with unhandled exception
> 
> It works for me. What StGIT version do you use?

I got that error on 0.11 - and just checked that
9b63cf56576bf219d26f490f3c011e937a5f7129 fixes exactly this problem
already.  Sorry, I should have checked on master first.


> >+- at least "commit is not robust wrt out-of-diskspace condition:
> >+|deps$ stg commit
> >+|error: git-checkout-index: unable to write file MANIFEST
> >+|error: git-checkout-index: unable to write file META.yml
> >+|error: git-checkout-index: unable to write file Makefile.PL
> >+|error: git-checkout-index: unable to write file doc/README.dbk.xml
> >+|error: git-checkout-index: unable to write file graph-includes
> >+|error: git-checkout-index: unable to write file 
> >lib/graphincludes/params.pm
> >+|fatal: unable to write new index file
> >+|stg commit: git-read-tree failed (local changes maybe?)
> >+|Committing 4 patches...
> >+(luckily nothing was really committed)
> 
> But that's the correct behaviour, not to commit anything.

Right.

> StGIT cannot know
> how much space is needed by GIT to check this beforehand. It simply
> exits when a GIT command failed.

What I had in mind, is that when something fails midway, if we just exit
we may end up in an inconsistent state.  I have not taken the time to
check about that, that's why I wanted it to appear in the TODO file.
Again, it was quite poorly worded, to say the least.

Best regards,
-- 

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2006-12-05 22:03 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2006-11-30  0:23 [PATCH 0/3] Misc stgit patches Yann Dirson
2006-11-30  0:27 ` [PATCH 1/3] Document some current bugs and add to the TODO list Yann Dirson
2006-12-05 17:30   ` Catalin Marinas
2006-12-05 22:02     ` Yann Dirson
2006-11-30  0:27 ` [PATCH 2/3] More config examples Yann Dirson
2006-11-30  0:27 ` [PATCH 3/3] Create a StgitObject class to factorise code for property handling Yann Dirson

Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).