patch, 7 parts[DRAFT 2] StGIT, remotes, and parent information

stgit/commands/common.py, stgit/commands/mail.py, stgit/commands/pull.py, stgit/commands/refresh.py, stgit/config.py, stgit/git.py, stgit/gitmergeonefile.py, stgit/stack.py

diff --git a/stgit/commands/common.py b/stgit/commands/common.py
index 2e57824..432edce 100644
--- a/stgit/commands/common.py
+++ b/stgit/commands/common.py
@@ -311,9 +311,10 @@ def address_or_alias(addr_str):
         if addr.find('@') >= 0:
             # it's an e-mail address
             return addr
-        if config.has_option('mail "alias"', addr):
+        alias = config.get('mail.alias.'+addr)
+        if alias:
             # it's an alias
-            return config.get('mail "alias"', addr)
+            return alias
 
         raise CmdException, 'unknown e-mail alias: %s' % addr
 
diff --git a/stgit/commands/mail.py b/stgit/commands/mail.py
index 6f61b83..236b519 100644
--- a/stgit/commands/mail.py
+++ b/stgit/commands/mail.py
@@ -129,9 +129,8 @@ def __get_sender():
     """Return the 'authname <authemail>' string as read from the
     configuration file
     """
-    if config.has_option('stgit', 'sender'):
-        sender = config.get('stgit', 'sender')
-    else:
+    sender=config.get('stgit.sender')
+    if not sender:
         try:
             sender = str(git.user())
         except git.GitException:
@@ -207,9 +206,8 @@ def __build_address_headers(msg, options, extra_cc = []):
     cc_addr = ''
     bcc_addr = ''
 
-    if config.has_option('stgit', 'autobcc'):
-        autobcc = config.get('stgit', 'autobcc')
-    else:
+    autobcc = config.get('stgit.autobcc')
+    if not autobcc:
         autobcc = ''
 
     if options.to:
@@ -283,8 +281,9 @@ def __edit_message(msg):
     f.close()
 
     # the editor
-    if config.has_option('stgit', 'editor'):
-        editor = config.get('stgit', 'editor')
+    editor = config.get('stgit.editor')
+    if editor:
+        pass
     elif 'EDITOR' in os.environ:
         editor = os.environ['EDITOR']
     else:
@@ -469,14 +468,10 @@ def func(parser, options, args):
     """Send the patches by e-mail using the patchmail.tmpl file as
     a template
     """
-    smtpserver = config.get('stgit', 'smtpserver')
+    smtpserver = config.get('stgit.smtpserver')
 
-    smtpuser = None
-    smtppassword = None
-    if config.has_option('stgit', 'smtpuser'):
-        smtpuser = config.get('stgit', 'smtpuser')
-    if config.has_option('stgit', 'smtppassword'):
-        smtppassword = config.get('stgit', 'smtppassword')
+    smtpuser = config.get('stgit.smtpuser')
+    smtppassword = config.get('stgit.smtppassword')
 
     applied = crt_series.get_applied()
 
@@ -511,7 +506,7 @@ def func(parser, options, args):
     if options.sleep != None:
         sleep = options.sleep
     else:
-        sleep = config.getint('stgit', 'smtpdelay')
+        sleep = config.getint('stgit.smtpdelay')
 
     # send the cover message (if any)
     if options.cover or options.edit_cover:
diff --git a/stgit/commands/pull.py b/stgit/commands/pull.py
index 7c5db22..15ed636 100644
--- a/stgit/commands/pull.py
+++ b/stgit/commands/pull.py
@@ -53,10 +53,8 @@ def func(parser, options, args):
     if len(args) >= 1:
         repository = args[0]
     else:
-        section = 'branch "%s"' % git.get_head_file()
-        if config.has_option(section, 'remote'):
-            repository = config.get(section, 'remote')
-        else:
+        repository = config.get('branch.%s.remote' % git.get_head_file())
+        if not repository:
             repository = 'origin'
 
     refspec = None
@@ -88,8 +86,7 @@ def func(parser, options, args):
         push_patches(applied, options.merged)
 
     # maybe tidy up
-    repack = config.get('stgit', 'keepoptimized')
-    if repack == 'yes':
+    if config.get('stgit.keepoptimized') == 'yes':
         git.repack()
 
     print_crt_patch()
diff --git a/stgit/commands/refresh.py b/stgit/commands/refresh.py
index 0efe1df..7a9bf7d 100644
--- a/stgit/commands/refresh.py
+++ b/stgit/commands/refresh.py
@@ -77,7 +77,7 @@ options = [make_option('-f', '--force',
 
 
 def func(parser, options, args):
-    autoresolved = config.get('stgit', 'autoresolved')
+    autoresolved = config.get('stgit.autoresolved')
 
     if autoresolved != 'yes':
         check_conflicts()
diff --git a/stgit/config.py b/stgit/config.py
index f5fbdab..c2d413b 100644
--- a/stgit/config.py
+++ b/stgit/config.py
@@ -18,90 +18,86 @@ along with this program; if not, write to the Free Software
 Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
 """
 
-import os, ConfigParser
-from StringIO import StringIO
+import os
 from stgit import basedir
 
-config = ConfigParser.RawConfigParser()
-
-def git_config(filename):
-    """Open a git config file and convert it to be understood by
-    Python."""
-    try:
-        f = file(filename)
-        cont = False
-        lines = []
-        for line in f:
-            line = line.strip()
-
-            if cont:
-                # continued line, add a space at the beginning
-                line = ' ' + line
-
-            if line and line[-1] == '\\':
-                line = line[:-1].rstrip()
-                cont = True
-            else:
-                line = line + '\n'
-                cont = False
-
-            lines.append(line)
-
-        f.close()
-        cfg_str = ''.join(lines)
-    except IOError:
-        cfg_str = ''
-
-    strio = StringIO(cfg_str)
-    strio.name = filename
-
-    return strio
+class GitConfigException(Exception):
+    pass
+
+class GitConfig:
+    __defaults={
+        'stgit.autoresolved':	'no',
+        'stgit.smtpserver':		'localhost:25',
+        'stgit.smtpdelay':		'5',
+        'stgit.pullcmd':		'git-pull',
+        'stgit.merger':		'diff3 -L current -L ancestor -L patched -m -E ' \
+				'"%(branch1)s" "%(ancestor)s" "%(branch2)s" > "%(output)s"',
+        'stgit.autoimerge':		'no',
+        'stgit.keeporig':		'yes',
+        'stgit.keepoptimized':	'no',
+        'stgit.extensions':		'.ancestor .current .patched',
+        }
+
+    def __run(self, cmd, args=None):
+        """__run: runs cmd using spawnvp.
+    
+        Runs cmd using spawnvp.  The shell is avoided so it won't mess up
+        our arguments.  If args is very large, the command is run multiple
+        times; args is split xargs style: cmd is passed on each
+        invocation.  Unlike xargs, returns immediately if any non-zero
+        return code is received.  
+        """
+        
+        args_l=cmd.split()
+        if args is None:
+            args = []
+        for i in range(0, len(args)+1, 100):
+            r=os.spawnvp(os.P_WAIT, args_l[0], args_l + args[i:min(i+100, len(args))])
+        if r:
+            return r
+        return 0
+    
+    def get(self, name):
+        stream = os.popen('git repo-config --get %s' % name, 'r')
+        value = stream.readline().strip()
+        stream.close()
+        if len(value) > 0:
+            return value
+#         elif found in mergeonefile:
+#             XXX
+        elif (self.__defaults.has_key(name)):
+            return self.__defaults[name]
+        else:
+            return None
+
+    def getall(self, name):
+        stream = os.popen('git repo-config --get-all %s' % name, 'r')
+        values = [line.strip() for line in stream]
+        stream.close()
+#        if len(values)==0 and found in mergeonefile:
+#             XXX
+        return values
+
+    def getint(self, name):
+        value = self.get(name)
+        if value.isdigit():
+            return int(value)
+        else:
+            raise GitConfigException, 'Value for "%s" is not an integer: "%s"' % (name, value)
+
+    def set(self, name, value):
+        self.__run('git-repo-config', [name, value])
+
+config=GitConfig()
 
 def config_setup():
     global config
 
-    # Set the defaults
-    config.add_section('stgit')
-    config.set('stgit', 'autoresolved', 'no')
-    config.set('stgit', 'smtpserver', 'localhost:25')
-    config.set('stgit', 'smtpdelay', '5')
-    config.set('stgit', 'pullcmd', 'git-pull')
-    config.set('stgit', 'merger',
-               'diff3 -L current -L ancestor -L patched -m -E ' \
-               '"%(branch1)s" "%(ancestor)s" "%(branch2)s" > "%(output)s"')
-    config.set('stgit', 'autoimerge', 'no')
-    config.set('stgit', 'keeporig', 'yes')
-    config.set('stgit', 'keepoptimized', 'no')
-    config.set('stgit', 'extensions', '.ancestor .current .patched')
-
-    # Read the configuration files (if any) and override the default settings
-    # stgitrc are read for backward compatibility
-    config.read('/etc/stgitrc')
-    config.read(os.path.expanduser('~/.stgitrc'))
-    config.read(os.path.join(basedir.get(), 'stgitrc'))
-
-    # GIT configuration files can have a [stgit] section
-    try:
-        global_config = os.environ['GIT_CONFIG']
-    except KeyError:
-        global_config = os.path.expanduser('~/.gitconfig')
-    try:
-        local_config = os.environ['GIT_CONFIG_LOCAL']
-    except KeyError:
-        local_config = os.path.join(basedir.get(), 'config')
-    config.readfp(git_config(global_config))
-    config.readfp(git_config(local_config))
-
     # Set the PAGER environment to the config value (if any)
-    if config.has_option('stgit', 'pager'):
-        os.environ['PAGER'] = config.get('stgit', 'pager')
-
-    # [gitmergeonefile] section is deprecated. In case it exists copy the
-    # options/values to the [stgit] one
-    if config.has_section('gitmergeonefile'):
-        for option, value in config.items('gitmergeonefile'):
-            config.set('stgit', option, value)
-
+    pager = config.get('stgit.pager')
+    if pager:
+        os.environ['PAGER'] = pager
+    # FIXME: handle EDITOR the same way ?
 
 class ConfigOption:
     """Delayed cached reading of a configuration option.
@@ -113,7 +109,7 @@ class ConfigOption:
 
     def __str__(self):
         if not self.__value:
-            self.__value = config.get(self.__section, self.__option)
+            self.__value = config.get(self.__section + '.' + self.__option)
         return self.__value
 
 
@@ -126,7 +122,7 @@ def file_extensions():
     global __extensions
 
     if not __extensions:
-        cfg_ext = config.get('stgit', 'extensions').split()
+        cfg_ext = config.get('stgit.extensions').split()
         if len(cfg_ext) != 3:
             raise CmdException, '"extensions" configuration error'
 
diff --git a/stgit/git.py b/stgit/git.py
index 4c5685a..c21c074 100644
--- a/stgit/git.py
+++ b/stgit/git.py
@@ -449,10 +449,10 @@ def user():
     """
     global __user
     if not __user:
-        if config.has_option('user', 'name') \
-               and config.has_option('user', 'email'):
-            __user = Person(config.get('user', 'name'),
-                            config.get('user', 'email'))
+        name=config.get('user.name')
+        email=config.get('user.email')
+        if name and email:
+            __user = Person(name, email)
         else:
             raise GitException, 'unknown user details'
     return __user;
@@ -816,7 +816,7 @@ def pull(repository = 'origin', refspec = None):
     if refspec:
         args.append(refspec)
 
-    if __run(config.get('stgit', 'pullcmd'), args) != 0:
+    if __run(config.get('stgit.pullcmd'), args) != 0:
         raise GitException, 'Failed "git-pull %s"' % repository
 
 def repack():
diff --git a/stgit/gitmergeonefile.py b/stgit/gitmergeonefile.py
index 587c8c4..b9d02aa 100644
--- a/stgit/gitmergeonefile.py
+++ b/stgit/gitmergeonefile.py
@@ -102,9 +102,8 @@ def interactive_merge(filename):
     """Run the interactive merger on the given file. Note that the
     index should not have any conflicts.
     """
-    try:
-        imerger = config.get('stgit', 'imerger')
-    except Exception, err:
+    imerger = config.get('stgit.imerger')
+    if not imerger:
         raise GitMergeException, 'Configuration error: %s' % err
 
     extensions = file_extensions()
@@ -185,7 +184,7 @@ def merge(orig_hash, file1_hash, file2_hash,
                     os.system('git-update-index --cacheinfo %s %s %s'
                               % (file1_mode, file1_hash, path))
 
-                    if config.get('stgit', 'autoimerge') == 'yes':
+                    if config.get('stgit.autoimerge') == 'yes':
                         print >> sys.stderr, \
                               'Trying the interactive merge'
                         try:
diff --git a/stgit/stack.py b/stgit/stack.py
index 2ae4dd5..087403f 100644
--- a/stgit/stack.py
+++ b/stgit/stack.py
@@ -92,8 +92,9 @@ def edit_file(series, line, comment, show_patch = True):
     f.close()
 
     # the editor
-    if config.has_option('stgit', 'editor'):
-        editor = config.get('stgit', 'editor')
+    editor = config.get('stgit.editor')
+    if editor:
+        pass
     elif 'EDITOR' in os.environ:
         editor = os.environ['EDITOR']
     else:

stgit/commands/pull.py, stgit/stack.py

diff --git a/stgit/commands/pull.py b/stgit/commands/pull.py
index 15ed636..0608fb6 100644
--- a/stgit/commands/pull.py
+++ b/stgit/commands/pull.py
@@ -53,9 +53,7 @@ def func(parser, options, args):
     if len(args) >= 1:
         repository = args[0]
     else:
-        repository = config.get('branch.%s.remote' % git.get_head_file())
-        if not repository:
-            repository = 'origin'
+        repository = crt_series.get_parent_remote()
 
     refspec = None
     if len(args) == 2:
diff --git a/stgit/stack.py b/stgit/stack.py
index 087403f..026fc36 100644
--- a/stgit/stack.py
+++ b/stgit/stack.py
@@ -398,6 +398,16 @@ class Series(StgitObject):
     def set_description(self, line):
         self._set_field('description', line)
 
+    def get_parent_remote(self):
+        value = config.get('branch.%s.remote' % self.__name)
+        if value:
+            return value
+        else:
+            return 'origin'
+
+    def __set_parent_remote(self, remote):
+        value = config.set('branch.%s.remote' % self.__name, remote)
+
     def __patch_is_current(self, patch):
         return patch.get_name() == self.get_current()
 

diff --git a/stgit/git.py b/stgit/git.py
index c21c074..849a417 100644
--- a/stgit/git.py
+++ b/stgit/git.py
@@ -879,3 +879,78 @@ def modifying_revs(files, base_rev):
     revs = [line.strip() for line in _output_lines(cmd + files)]
 
     return revs
+
+
+def __remotes_from_config():
+    configremotes = []
+    stream = os.popen('git repo-config --get-regexp "^remote\..*\.url$"', 'r')
+    for line in stream:
+        m = re.match('^remote\.(.*)\.url ', line)
+        if m:
+            configremotes.append(m.group(1))
+    stream.close()
+    return configremotes
+
+def __remotes_from_dir(dir):
+    return os.listdir(os.path.join(basedir.get(), dir))
+
+def remotes_list():
+    """Return the list of remotes in the repository
+    """
+
+    return set(__remotes_from_config()) | \
+           set(__remotes_from_dir('remotes')) | \
+           set(__remotes_from_dir('branches'))
+
+def remotes_local_branches(remote):
+    """Returns the list of local branches fetched from given remote
+    """
+
+    branches = []
+    if remote in __remotes_from_config():
+        stream = os.popen('git repo-config --get-all "remote.%s.fetch"' % remote, 'r')
+        for line in stream:
+            # FIXME: should factorize refspec handling
+            m = re.match('^[^:]*:([^:]*)\n$', line)
+            if m:
+                branches.append(m.group(1))
+            else:
+                raise GitException, 'Cannot parse refspec "%s"' % line
+        stream.close()
+    elif remote in __remotes_from_dir('remotes'):
+        stream = open(os.path.join(basedir.get(), 'remotes', remote), 'r')
+        for line in stream:
+            # Only consider Pull lines
+            m = re.match('^Pull: (.*)\n$', line)
+            if m:
+                refspec = m.group(1)
+                m = re.match('^[^:]*:([^:]*)$', refspec)
+                if m:
+                    branches.append(m.group(1))
+                else:
+                    raise GitException, 'Cannot parse refspec "%s"' % refspec
+        stream.close()
+        pass
+    elif remote in __remotes_from_dir('branches'):
+        # old-style branches only declare one branch
+        branches.append('refs/heads/'+remote);
+        pass
+    else:
+        raise GitException, 'Unknown remote "%s"' % remote
+
+    return branches
+
+def identify_remote(branchname):
+    """Return the name for the remote to pull the given branchname
+    from, or None if we believe it is a local branch.
+    """
+
+    for remote in remotes_list():
+        if branchname in remotes_local_branches(remote):
+            return remote
+
+    # FIXME: in the case of local branch we should maybe set remote to
+    # "." but are we even sure it is the only case left ?
+
+    # if we get here we've found nothing
+    return None

TODO, stgit/stack.py

diff --git a/TODO b/TODO
index 549bc9d..884b831 100644
--- a/TODO
+++ b/TODO
@@ -20,8 +20,6 @@ The future, when time allows or if someone else does them:
 - multiple heads in a patch - useful for forking a patch,
   synchronising with other patches (diff format or in other
   repositories)
-- "pull" argument should default to a sane value, "origin" is wrong in
-  many cases
 - commit directly to a patch which is not top
 - patch synchronisation between between branches (as some people,
   including me have the same patches based on different branches and
diff --git a/stgit/stack.py b/stgit/stack.py
index 026fc36..603c539 100644
--- a/stgit/stack.py
+++ b/stgit/stack.py
@@ -408,6 +408,26 @@ class Series(StgitObject):
     def __set_parent_remote(self, remote):
         value = config.set('branch.%s.remote' % self.__name, remote)
 
+    def get_parent_branch(self):
+        value = config.get('branch.%s.merge' % self.__name)
+        if value:
+            return value
+        elif rev_parse('heads/origin'):
+            return 'heads/origin'
+        else:
+            raise StackException, 'Cannot find a parent branch for "%s"' % self.__name
+
+    def __set_parent_branch(self, name):
+        config.set('branch.%s.merge' % self.__name, name)
+
+    def set_parent(self, remote, localbranch):
+        if localbranch:
+            self.__set_parent_branch(localbranch)
+            if remote:
+                self.__set_parent_remote(remote)
+        elif remote:
+            raise StackException, 'Remote "%s" without a branch cannot be used as parent' % remote
+
     def __patch_is_current(self, patch):
         return patch.get_name() == self.get_current()
 
@@ -456,7 +476,7 @@ class Series(StgitObject):
         """
         return os.path.isdir(self.__patch_dir)
 
-    def init(self, create_at=False):
+    def init(self, create_at=False, parent_remote=None, parent_branch=None):
         """Initialises the stgit series
         """
         bases_dir = os.path.join(self.__base_dir, 'refs', 'bases')
@@ -473,6 +493,8 @@ class Series(StgitObject):
 
         os.makedirs(self.__patch_dir)
 
+        self.set_parent(parent_remote, parent_branch)
+        
         create_dirs(bases_dir)
 
         self.create_empty_field('applied')

diff --git a/stgit/commands/branch.py b/stgit/commands/branch.py
index d348409..f074d47 100644
--- a/stgit/commands/branch.py
+++ b/stgit/commands/branch.py
@@ -123,10 +123,41 @@ def func(parser, options, args):
         check_head_top_equal()
 
         tree_id = None
-        if len(args) == 2:
+        if len(args) >= 2:
+            try:
+                if git.rev_parse(args[1]) == git.rev_parse('refs/heads/' + args[1]):
+                    # we are for sure refering to a branch
+                    parentbranch = 'refs/heads/' + args[1]
+                    print 'Recording "%s" as parent branch.' % parentbranch
+                elif git.rev_parse(args[1]) and re.search('/', args[1]):
+                    # FIXME: should the test be more strict ?
+                    parentbranch = args[1]
+                else:
+                    # Note: this includes refs to StGIT patches
+                    print 'Don\'t know how to determine parent branch from "%s".' % args[1]
+                    parentbranch = None
+            except git.GitException:
+                # should use a more specific exception to catch only non-git refs ?
+                print 'Don\'t know how to determine parent branch from "%s".' % args[1]
+                parentbranch = None
+
             tree_id = git_id(args[1])
-        
-        stack.Series(args[0]).init(create_at = tree_id)
+        else:
+            # branch stack off current branch
+            parentbranch = git.get_head_file()
+
+        if parentbranch:
+            parentremote = git.identify_remote(parentbranch)
+            if parentremote:
+                print 'Using "%s" remote to pull parent from.' % parentremote
+            else:
+                print 'Not identified a remote to pull parent from.'
+        else:
+            parentremote = None
+
+        stack.Series(args[0]).init(create_at = tree_id,
+                                   parent_remote = parentremote,
+                                   parent_branch = parentbranch)
 
         print 'Branch "%s" created.' % args[0]
         return

examples/gitconfig, stgit/config.py, stgit/git.py

diff --git a/examples/gitconfig b/examples/gitconfig
index 5e7b240..e3d9889 100644
--- a/examples/gitconfig
+++ b/examples/gitconfig
@@ -33,8 +33,14 @@
 	#pager = ~/share/stgit/contrib/diffcol.sh
 	#pager = filterdiff --annotate | colordiff | less -FRX
 
-	# GIT pull command (should take the same arguments as git-pull)
+	# GIT pull command (should take the same arguments as
+	# git-fetch or git-pull).  By default:
+	#pullcmd = git-fetch
+	#pull-does-rebase = yes
+	# Alternative (old behaviour), less intuitive but maybe useful
+	# for some workflows:
 	#pullcmd = git-pull
+	#pull-does-rebase = no
 
 	# The three-way merge tool. Note that the 'output' file contains the
 	# same data as 'branch1'. This is useful for tools that do not take an
diff --git a/stgit/config.py b/stgit/config.py
index c2d413b..9b66686 100644
--- a/stgit/config.py
+++ b/stgit/config.py
@@ -29,7 +29,8 @@ class GitConfig:
         'stgit.autoresolved':	'no',
         'stgit.smtpserver':		'localhost:25',
         'stgit.smtpdelay':		'5',
-        'stgit.pullcmd':		'git-pull',
+        'stgit.pullcmd':		'git-fetch',
+        'stgit.pull-does-rebase':	'yes',
         'stgit.merger':		'diff3 -L current -L ancestor -L patched -m -E ' \
 				'"%(branch1)s" "%(ancestor)s" "%(branch2)s" > "%(output)s"',
         'stgit.autoimerge':		'no',
diff --git a/stgit/git.py b/stgit/git.py
index 849a417..708dd56 100644
--- a/stgit/git.py
+++ b/stgit/git.py
@@ -806,18 +806,23 @@ def reset(files = None, tree_id = None, check_out = True):
         __set_head(tree_id)
 
 def pull(repository = 'origin', refspec = None):
-    """Pull changes from the remote repository. At the moment, just
-    use the 'git-pull' command
+    """Pull changes from the remote repository. Uses 'git-fetch'
+    and moves the stack base.
     """
-    # 'git-pull' updates the HEAD
+    # we update the HEAD
     __clear_head_cache()
 
     args = [repository]
     if refspec:
         args.append(refspec)
 
-    if __run(config.get('stgit.pullcmd'), args) != 0:
-        raise GitException, 'Failed "git-pull %s"' % repository
+    command = config.get('stgit.pullcmd')
+    if __run(command, args) != 0:
+        raise GitException, 'Failed "%s %s"' % (command, repository)
+
+    if (config.get('stgit.pull-does-rebase')):
+        # FIXME!
+        reset(tree_id = rev_parse(repository))
 
 def repack():
     """Repack all objects into a single pack