threads / patch / 7253

patch, 10 partsVarious fixes and improvements

Subject: [StGIT PATCH 00/10] Various fixes and improvements

## tl;dr

11 messages between Mar 15, 2007 and Mar 15, 2007. Diffs are folded; open one to read it.

replies: 10people: 1as markdown or json

Yann Dirson· Mar 15, 2007, 00:26 UTC · lore

I found a couple of issues involving stack deletion, here are fixes for the most annoying ones. Also includes some small cleanups involving parent information, as well as mostly-unrelated small improvements gathered while hacking on the other patches.

Note: since I have seen the patch entitled 'git-fetch: Support the
local remote "."', I'm much less sure about my patch that stops
recording this type of information (the "Stop recording
branch.*.remote to '.' for local parents." one), but I add it
nevertheless, mostly for discussion.
-- 
Yann Dirson    <ydirson@altern.org> |
Debian-related: <dirson@debian.org> |   Support Debian GNU/Linux:
                                    |  Freedom, Power, Stability, Gratis
     http://ydirson.free.fr/        | Check <http://www.debian.org/>
Yann Dirson· Mar 15, 2007, 00:26 UTC · re: Yann Dirson · lore

[PATCH 01/10] Fix stack deletion when orig-base is present.

The introduction of the orig-base file makes the stack deletion fail to remove the stack's directory, leaving the stack in a zombie state, where attempting to recreate it was refused because of the directory already existing, but attempting to delete it failed since the applied and unapplied files were not found.

Signed-off-by: Yann Dirson <ydirson@altern.org>
---
 stgit/stack.py |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)
Show changes to stgit/stack.py +2 −0
diff --git a/stgit/stack.py b/stgit/stack.py
index feb77e3..8a63102 100644
--- a/stgit/stack.py
+++ b/stgit/stack.py
@@ -676,6 +676,8 @@ class Series(StgitObject):
                 os.remove(self.__current_file)
             if os.path.exists(self.__descr_file):
                 os.remove(self.__descr_file)
+            if os.path.exists(self._dir()+'/orig-base'):
+                os.remove(self._dir()+'/orig-base')
             if not os.listdir(self.__patch_dir):
                 os.rmdir(self.__patch_dir)
             else:
Yann Dirson· Mar 15, 2007, 00:26 UTC · re: Yann Dirson · lore

[PATCH 02/10] Fix diagnostic messages on patch deletion and simplify others.

Also make use standard os.removedirs(), allowing for the same simplification.

Signed-off-by: Yann Dirson <ydirson@altern.org>
---
 stgit/stack.py |   15 ++++++++-------
 1 files changed, 8 insertions(+), 7 deletions(-)
Show changes to stgit/stack.py +8 −7
diff --git a/stgit/stack.py b/stgit/stack.py
index 8a63102..8fdaf2c 100644
--- a/stgit/stack.py
+++ b/stgit/stack.py
@@ -571,7 +571,7 @@ class Series(StgitObject):
                 os.rmdir(self.__patch_dir)
                 print 'done'
             else:
-                print 'Patch directory %s is not empty.' % self.__name
+                print 'Patch directory %s is not empty.' % self.__patch_dir
 
             self.__patch_dir = self._dir()
 
@@ -678,18 +678,19 @@ class Series(StgitObject):
                 os.remove(self.__descr_file)
             if os.path.exists(self._dir()+'/orig-base'):
                 os.remove(self._dir()+'/orig-base')
+
             if not os.listdir(self.__patch_dir):
                 os.rmdir(self.__patch_dir)
             else:
-                print 'Patch directory %s is not empty.' % self.__name
+                print 'Patch directory %s is not empty.' % self.__patch_dir
+
             if not os.listdir(self._dir()):
-                remove_dirs(os.path.join(self.__base_dir, 'patches'),
-                            self.__name)
+                os.removedirs(self._dir())
             else:
-                print 'Series directory %s is not empty.' % self.__name
+                raise StackException, 'Series directory %s is not empty.' % self._dir()
+
             if not os.listdir(self.__refs_dir):
-                remove_dirs(os.path.join(self.__base_dir, 'refs', 'patches'),
-                            self.__name)
+                os.removedirs(self.__refs_dir)
             else:
                 print 'Refs directory %s is not empty.' % self.__refs_dir
Yann Dirson· Mar 15, 2007, 00:26 UTC · re: Yann Dirson · lore

[PATCH 03/10] Make use exception raised by removedirs.

Signed-off-by: Yann Dirson <ydirson@altern.org>
---
 stgit/stack.py |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)
Show changes to stgit/stack.py +4 −4
diff --git a/stgit/stack.py b/stgit/stack.py
index 8fdaf2c..ec75507 100644
--- a/stgit/stack.py
+++ b/stgit/stack.py
@@ -684,14 +684,14 @@ class Series(StgitObject):
             else:
                 print 'Patch directory %s is not empty.' % self.__patch_dir
 
-            if not os.listdir(self._dir()):
+            try:
                 os.removedirs(self._dir())
-            else:
+            except OSError:
                 raise StackException, 'Series directory %s is not empty.' % self._dir()
 
-            if not os.listdir(self.__refs_dir):
+            try:
                 os.removedirs(self.__refs_dir)
-            else:
+            except OSError:
                 print 'Refs directory %s is not empty.' % self.__refs_dir
 
         if os.path.exists(self.__base_file):
Yann Dirson· Mar 15, 2007, 00:26 UTC · re: Yann Dirson · lore

[PATCH 04/10] Drop utils.remove_dirs() in favor of os.removedirs().

Signed-off-by: Yann Dirson <ydirson@altern.org>
---
 stgit/utils.py |   24 ++++++++++--------------
 1 files changed, 10 insertions(+), 14 deletions(-)
Show changes to stgit/utils.py +10 −14
diff --git a/stgit/utils.py b/stgit/utils.py
index bfe7797..d04d077 100644
--- a/stgit/utils.py
+++ b/stgit/utils.py
@@ -116,24 +116,16 @@ def strip_suffix(suffix, string):
     assert string.endswith(suffix)
     return string[:-len(suffix)]
 
-def remove_dirs(basedir, dirs):
-    """Starting at join(basedir, dirs), remove the directory if empty,
-    and try the same with its parent, until we find a nonempty
-    directory or reach basedir."""
-    path = dirs
-    while path:
-        try:
-            os.rmdir(os.path.join(basedir, path))
-        except OSError:
-            return # can't remove nonempty directory
-        path = os.path.dirname(path)
-
 def remove_file_and_dirs(basedir, file):
     """Remove join(basedir, file), and then remove the directory it
     was in if empty, and try the same with its parent, until we find a
     nonempty directory or reach basedir."""
     os.remove(os.path.join(basedir, file))
-    remove_dirs(basedir, os.path.dirname(file))
+    try:
+        os.removedirs(os.path.join(basedir, os.path.dirname(file)))
+    except OSError:
+        # file's parent dir may not be empty after removal
+        pass
 
 def create_dirs(directory):
     """Create the given directory, if the path doesn't already exist."""
@@ -152,7 +144,11 @@ def rename(basedir, file1, file2):
     full_file2 = os.path.join(basedir, file2)
     create_dirs(os.path.dirname(full_file2))
     os.rename(os.path.join(basedir, file1), full_file2)
-    remove_dirs(basedir, os.path.dirname(file1))
+    try:
+        os.removedirs(os.path.join(basedir, os.path.dirname(file1)))
+    except OSError:
+        # file1's parent dir may not be empty after move
+        pass
 
 class EditorException(Exception):
     pass
Yann Dirson· Mar 15, 2007, 00:26 UTC · re: Yann Dirson · lore

[PATCH 05/10] Copy parentbranch setting on 'stg branch --clone'.

Signed-off-by: Yann Dirson <ydirson@altern.org>
---
 stgit/stack.py |    6 +++++-
 1 files changed, 5 insertions(+), 1 deletions(-)
Show changes to stgit/stack.py +5 −1
diff --git a/stgit/stack.py b/stgit/stack.py
index ec75507..d1b04ca 100644
--- a/stgit/stack.py
+++ b/stgit/stack.py
@@ -639,7 +639,7 @@ class Series(StgitObject):
         # fast forward the cloned series to self's top
         new_series.forward_patches(applied)
 
-        # Clone remote and merge settings
+        # Clone parent informations
         value = config.get('branch.%s.remote' % self.__name)
         if value:
             config.set('branch.%s.remote' % target_series, value)
@@ -648,6 +648,10 @@ class Series(StgitObject):
         if value:
             config.set('branch.%s.merge' % target_series, value)
 
+        value = config.get('branch.%s.stgit.parentbranch' % self.__name)
+        if value:
+            config.set('branch.%s.stgit.parentbranch' % target_series, value)
+
     def delete(self, force = False):
         """Deletes an stgit series
         """
Yann Dirson· Mar 15, 2007, 00:27 UTC · re: Yann Dirson · lore

[PATCH 06/10] Remove debugging output from contrib/stg-sink

Signed-off-by: Yann Dirson <ydirson@altern.org>
---
 contrib/stg-sink |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
Show changes to contrib/stg-sink +1 −1
diff --git a/contrib/stg-sink b/contrib/stg-sink
index e63eb93..cb6e25d 100755
--- a/contrib/stg-sink
+++ b/contrib/stg-sink
@@ -3,7 +3,7 @@ use strict;
 use warnings;
 
 sub _run {
-  print(' >> ', @_, "\n");# and 0;
+  #print(' >> ', @_, "\n");# and 0;
   system(@_);
 }
Yann Dirson· Mar 15, 2007, 00:27 UTC · re: Yann Dirson · lore

[PATCH 07/10] Stop recording branch.*.remote to '.' for local parents.

We should be using the remote name of the remote branch in branch.*.merge, instead of the local one as we currently do (I'll ensure this in a later patch).

As a consequence, setting "remote" to '.' and "merge" to the name of a *local* parent would become inconsistent with the remote case. Anyway the local case should make use of pull-policy=rebase instead, and this change revealed that we were fetching useless information in the local case (no need for a remote).

Signed-off-by: Yann Dirson <ydirson@altern.org>
---
 stgit/commands/pull.py |   24 +++++++++++++++++-------
 stgit/stack.py         |    4 +---
 2 files changed, 18 insertions(+), 10 deletions(-)
Show changes to 2 files +18 −10

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

diff --git a/stgit/commands/pull.py b/stgit/commands/pull.py
index 7b3c90c..f551b98 100644
--- a/stgit/commands/pull.py
+++ b/stgit/commands/pull.py
@@ -50,13 +50,25 @@ options = [make_option('-n', '--nopush',
 def func(parser, options, args):
     """Pull the changes from a remote repository
     """
-    if len(args) > 1:
-        parser.error('incorrect number of arguments')
+    policy = config.get('branch.%s.stgit.pull-policy' % crt_series.get_branch()) or \
+             config.get('stgit.pull-policy')
+
+    if policy == 'rebase':
+        # parent is local
+        if len(args) == 1:
+            parser.error('specifying a repository is meaningless for policy="%s"' % policy)
+        if len(args) > 0:
+            parser.error('incorrect number of arguments')
 
-    if len(args) >= 1:
-        repository = args[0]
     else:
-        repository = crt_series.get_parent_remote()
+        # parent is remote
+        if len(args) > 1:
+            parser.error('incorrect number of arguments')
+
+        if len(args) >= 1:
+            repository = args[0]
+        else:
+            repository = crt_series.get_parent_remote()
 
     if crt_series.get_protected():
         raise CmdException, 'This branch is protected. Pulls are not permitted'
@@ -65,8 +77,6 @@ def func(parser, options, args):
     check_conflicts()
     check_head_top_equal()
 
-    policy = config.get('branch.%s.stgit.pull-policy' % crt_series.get_branch()) or \
-             config.get('stgit.pull-policy')
     if policy == 'pull':
         must_rebase = 0
     elif policy == 'fetch-rebase':
diff --git a/stgit/stack.py b/stgit/stack.py
index d1b04ca..c3dab68 100644
--- a/stgit/stack.py
+++ b/stgit/stack.py
@@ -450,10 +450,8 @@ class Series(StgitObject):
         config.set('branch.%s.stgit.parentbranch' % self.__name, name)
 
     def set_parent(self, remote, localbranch):
-        # policy: record local branches as remote='.'
-        recordremote = remote or '.'
         if localbranch:
-            self.__set_parent_remote(recordremote)
+            self.__set_parent_remote(remote)
             self.__set_parent_branch(localbranch)
         # We'll enforce this later
 #         else:
Yann Dirson· Mar 15, 2007, 00:27 UTC · re: Yann Dirson · lore

[PATCH 08/10] Add a script to quickly run an interactive test session.

This uses the capabilities of the test library to provide a throw-away playground to test how the current stgit tree behaves.

To use it, run the following command:
	(cd t && ./interactive.sh)

You'll get a shell in a brand new git tree. Exiting that shell will get rid of this temporary directory.

Signed-off-by: Yann Dirson <ydirson@altern.org>
---
 t/interactive.sh |   12 ++++++++++++
 1 files changed, 12 insertions(+), 0 deletions(-)
Show changes to t/interactive.sh +12 −0
diff --git a/t/interactive.sh b/t/interactive.sh
new file mode 100755
index 0000000..e4eaef9
--- /dev/null
+++ b/t/interactive.sh
@@ -0,0 +1,12 @@
+#!/bin/sh
+#
+# Copyright (c) 2007 Yann Dirson
+#
+
+test_description='Get a test environment for interactive experimentation.'
+
+. ./test-lib.sh
+
+bash
+
+test_done
Yann Dirson· Mar 15, 2007, 00:27 UTC · re: Yann Dirson · lore

[PATCH 09/10] Add support for unsetting config vars.

Signed-off-by: Yann Dirson <ydirson@altern.org>
---
 stgit/config.py |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)
Show changes to stgit/config.py +3 −0
diff --git a/stgit/config.py b/stgit/config.py
index b016fbd..79cd12f 100644
--- a/stgit/config.py
+++ b/stgit/config.py
@@ -103,6 +103,9 @@ class GitConfig:
     def set(self, name, value):
         self.__run('git-repo-config', [name, value])
 
+    def unset(self, name):
+        self.__run('git-repo-config --unset', [name])
+
     def sections_matching(self, regexp):
         """Takes a regexp with a single group, matches it against all
         config variables, and returns a list whose members are the
Yann Dirson· Mar 15, 2007, 00:27 UTC · re: Yann Dirson · lore

[PATCH 10/10] Cleanup parent information on stgit branch deletion.

Signed-off-by: Yann Dirson <ydirson@altern.org>
---
 stgit/stack.py |    7 +++++++
 1 files changed, 7 insertions(+), 0 deletions(-)
Show changes to stgit/stack.py +7 −0
diff --git a/stgit/stack.py b/stgit/stack.py
index c3dab68..c6f338d 100644
--- a/stgit/stack.py
+++ b/stgit/stack.py
@@ -700,6 +700,13 @@ class Series(StgitObject):
             remove_file_and_dirs(
                 os.path.join(self.__base_dir, 'refs', 'bases'), self.__name)
 
+        # Cleanup parent informations
+        # FIXME: should one day make use of git-config --section-remove,
+        # scheduled for 1.5.1
+        config.unset('branch.%s.remote' % self.__name)
+        config.unset('branch.%s.merge' % self.__name)
+        config.unset('branch.%s.stgit.parentbranch' % self.__name)
+
     def refresh_patch(self, files = None, message = None, edit = False,
                       show_patch = False,
                       cache_update = True,

← back to recent threads