Re: [StGit PATCH 0/6] Survive capital punishment

Previous thread: Re: [PATCH] Make git-clean a builtin by Johannes Schindelin on Sunday, October 7, 2007 - 8:57 pm. (2 messages)

Next thread: Merge problems with git-mingw by Peter Karlsson on Monday, October 8, 2007 - 4:06 am. (17 messages)
From: Karl
Date: Monday, October 8, 2007 - 1:55 am

This series allows StGit to deal with a detached HEAD: in most cases
it will just exit with a nice error message, but in some cases it will
actually handle it.

Preceded by some more or less related cleanups. (The "explicit
crt_series" patch was needed so that I could grep for the files that
didn't contain crt_series.)

---

Karl Hasselström (6):
      Let some commands work with detached HEAD
      Don't have a global crt_series in stgit.commans.common
      Refactor crt_series creation
      Properly detect that HEAD is detached
      Use our nice message printout wrapping system
      Allow caller to customize title of error/warning message


 stgit/commands/add.py      |    2 +-
 stgit/commands/branch.py   |    8 ++++----
 stgit/commands/clean.py    |    4 ++--
 stgit/commands/clone.py    |    2 +-
 stgit/commands/commit.py   |    2 +-
 stgit/commands/common.py   |   24 ++++++++++++------------
 stgit/commands/delete.py   |    4 ++--
 stgit/commands/diff.py     |    9 +++++----
 stgit/commands/edit.py     |    6 +++---
 stgit/commands/files.py    |    4 ++--
 stgit/commands/float.py    |    6 +++---
 stgit/commands/fold.py     |    5 +++--
 stgit/commands/goto.py     |    8 ++++----
 stgit/commands/id.py       |    2 +-
 stgit/commands/imprt.py    |    7 ++++---
 stgit/commands/mail.py     |   16 +++++++++-------
 stgit/commands/new.py      |    2 +-
 stgit/commands/pick.py     |   12 ++++++------
 stgit/commands/pop.py      |    8 ++++----
 stgit/commands/pull.py     |   14 +++++++-------
 stgit/commands/push.py     |    8 ++++----
 stgit/commands/rebase.py   |   12 ++++++------
 stgit/commands/refresh.py  |   10 +++++-----
 stgit/commands/resolved.py |    2 +-
 stgit/commands/series.py   |    5 +++--
 stgit/commands/show.py     |    2 +-
 stgit/commands/sink.py     |    6 +++---
 stgit/commands/status.py   |    2 +-
 stgit/commands/sync.py     |    8 ++++----
 stgit/commands/uncommit.py |    2 +-
 stgit/git.py               |   23 ...
From: Karl
Date: Monday, October 8, 2007 - 1:55 am

Signed-off-by: Karl Hasselström <kha@treskal.com>

---

 stgit/main.py |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)


diff --git a/stgit/main.py b/stgit/main.py
index 15582dd..8e00217 100644
--- a/stgit/main.py
+++ b/stgit/main.py
@@ -282,7 +282,7 @@ def main():
 
         command.func(parser, options, args)
     except (StgException, IOError, ParsingError, NoSectionError), err:
-        print >> sys.stderr, '%s %s: %s' % (prog, cmd, err)
+        out.error(str(err), title = '%s %s' % (prog, cmd))
         if debug_level > 0:
             raise
         else:

-

From: Karl
Date: Monday, October 8, 2007 - 1:55 am

Signed-off-by: Karl Hasselström <kha@treskal.com>

---

 stgit/out.py |   12 ++++++------
 1 files changed, 6 insertions(+), 6 deletions(-)


diff --git a/stgit/out.py b/stgit/out.py
index 3464175..d3c86b4 100644
--- a/stgit/out.py
+++ b/stgit/out.py
@@ -85,12 +85,12 @@ class MessagePrinter(object):
     def info(self, *msgs):
         for msg in msgs:
             self.__out.single_line(msg)
-    def note(self, *msgs):
-        self.__out.tagged_lines('Notice', msgs)
-    def warn(self, *msgs):
-        self.__err.tagged_lines('Warning', msgs)
-    def error(self, *msgs):
-        self.__err.tagged_lines('Error', msgs)
+    def note(self, *msgs, **kw):
+        self.__out.tagged_lines(kw.get('title', 'Notice'), msgs)
+    def warn(self, *msgs, **kw):
+        self.__err.tagged_lines(kw.get('title', 'Warning'), msgs)
+    def error(self, *msgs, **kw):
+        self.__err.tagged_lines(kw.get('title', 'Error'), msgs)
     def start(self, msg):
         """Start a long-running operation."""
         self.__out.single_line('%s ... ' % msg, print_newline = False)

-

From: Karl
Date: Monday, October 8, 2007 - 1:55 am

We still error out on a lot of places we shouldn't, e.g. "stg branch"
when on a detached HEAD, but at least now we give a sane error
message.

Signed-off-by: Karl Hasselström <kha@treskal.com>

---

 stgit/git.py |   23 +++++++++++++++++------
 1 files changed, 17 insertions(+), 6 deletions(-)


diff --git a/stgit/git.py b/stgit/git.py
index 812b77a..cc6acb1 100644
--- a/stgit/git.py
+++ b/stgit/git.py
@@ -246,11 +246,19 @@ def get_head():
         __head = rev_parse('HEAD')
     return __head
 
+class DetachedHeadException(GitException):
+    def __init__(self):
+        GitException.__init__(self, 'Not on any branch')
+
 def get_head_file():
-    """Returns the name of the file pointed to by the HEAD link
-    """
-    return strip_prefix('refs/heads/',
-                        GRun('git-symbolic-ref', 'HEAD').output_one_line())
+    """Return the name of the file pointed to by the HEAD symref.
+    Throw an exception if HEAD is detached."""
+    try:
+        return strip_prefix(
+            'refs/heads/', GRun('git-symbolic-ref', '-q', 'HEAD'
+                                ).output_one_line())
+    except GitRunException:
+        raise DetachedHeadException()
 
 def set_head_file(ref):
     """Resets HEAD to point to a new ref
@@ -385,8 +393,11 @@ def rename_ref(from_ref, to_ref):
 def rename_branch(from_name, to_name):
     """Rename a git branch."""
     rename_ref('refs/heads/%s' % from_name, 'refs/heads/%s' % to_name)
-    if get_head_file() == from_name:
-        set_head_file(to_name)
+    try:
+        if get_head_file() == from_name:
+            set_head_file(to_name)
+    except DetachedHeadException:
+        pass # detached HEAD, so the renamee can't be the current branch
     reflog_dir = os.path.join(basedir.get(), 'logs', 'refs', 'heads')
     if os.path.exists(reflog_dir) \
            and os.path.exists(os.path.join(reflog_dir, from_name)):

-

From: Karl
Date: Monday, October 8, 2007 - 1:55 am

Instead of hard-coding in main.py which commands do and don't need a
current series, let them speak for themselves.

Signed-off-by: Karl Hasselström <kha@treskal.com>

---

 stgit/commands/clone.py  |    2 +-
 stgit/commands/common.py |    2 ++
 stgit/main.py            |    5 ++---
 3 files changed, 5 insertions(+), 4 deletions(-)


diff --git a/stgit/commands/clone.py b/stgit/commands/clone.py
index a150010..c3b0bbe 100644
--- a/stgit/commands/clone.py
+++ b/stgit/commands/clone.py
@@ -29,7 +29,7 @@ usage = """%prog [options] <repository> <dir>
 Clone a GIT <repository> into the local <dir> and initialise the
 patch stack."""
 
-directory = DirectoryAnywhere()
+directory = DirectoryAnywhere(needs_current_series = False)
 options = []
 
 
diff --git a/stgit/commands/common.py b/stgit/commands/common.py
index 27ef465..652039f 100644
--- a/stgit/commands/common.py
+++ b/stgit/commands/common.py
@@ -497,6 +497,8 @@ class DirectoryException(StgException):
     pass
 
 class _Directory(object):
+    def __init__(self, needs_current_series = True):
+        self.needs_current_series =  needs_current_series
     @readonly_constant_property
     def git_dir(self):
         try:
diff --git a/stgit/main.py b/stgit/main.py
index 8e00217..db327f1 100644
--- a/stgit/main.py
+++ b/stgit/main.py
@@ -271,9 +271,8 @@ def main():
         directory.setup()
         config_setup()
 
-        # 'clone' doesn't expect an already initialised GIT tree. A Series
-        # object will be created after the GIT tree is cloned
-        if cmd != 'clone':
+        # Some commands don't (always) need an initialized series.
+        if directory.needs_current_series:
             if hasattr(options, 'branch') and options.branch:
                 command.crt_series = Series(options.branch)
             else:

-

From: Karl
Date: Monday, October 8, 2007 - 1:55 am

Global variables baaad. Instead, pass it as a parameter to the
functions that need it.

Signed-off-by: Karl Hasselström <kha@treskal.com>

---

 stgit/commands/branch.py   |    8 ++++----
 stgit/commands/clean.py    |    4 ++--
 stgit/commands/commit.py   |    2 +-
 stgit/commands/common.py   |   22 ++++++++++------------
 stgit/commands/delete.py   |    4 ++--
 stgit/commands/diff.py     |    7 ++++---
 stgit/commands/edit.py     |    6 +++---
 stgit/commands/files.py    |    4 ++--
 stgit/commands/float.py    |    6 +++---
 stgit/commands/fold.py     |    5 +++--
 stgit/commands/goto.py     |    8 ++++----
 stgit/commands/id.py       |    2 +-
 stgit/commands/imprt.py    |    7 ++++---
 stgit/commands/mail.py     |   16 +++++++++-------
 stgit/commands/new.py      |    2 +-
 stgit/commands/pick.py     |   12 ++++++------
 stgit/commands/pop.py      |    8 ++++----
 stgit/commands/pull.py     |   14 +++++++-------
 stgit/commands/push.py     |    8 ++++----
 stgit/commands/rebase.py   |   12 ++++++------
 stgit/commands/refresh.py  |   10 +++++-----
 stgit/commands/series.py   |    5 +++--
 stgit/commands/show.py     |    2 +-
 stgit/commands/sink.py     |    6 +++---
 stgit/commands/sync.py     |    8 ++++----
 stgit/commands/uncommit.py |    2 +-
 stgit/main.py              |    1 -
 27 files changed, 97 insertions(+), 94 deletions(-)


diff --git a/stgit/commands/branch.py b/stgit/commands/branch.py
index 6e0a6d8..cbb97f6 100644
--- a/stgit/commands/branch.py
+++ b/stgit/commands/branch.py
@@ -112,7 +112,7 @@ def func(parser, options, args):
 
         check_local_changes()
         check_conflicts()
-        check_head_top_equal()
+        check_head_top_equal(crt_series)
 
         tree_id = None
         if len(args) >= 2:
@@ -141,7 +141,7 @@ def func(parser, options, args):
                 # exception in branch = rev_parse() leaves branchpoint unbound
                 branchpoint = None
 
-            tree_id = branchpoint or git_id(args[1])
+    ...
From: Karl
Date: Monday, October 8, 2007 - 1:55 am

add, diff, resolved, and status didn't use the crt_series that was
initialized for them. So don't initialize it, since that means (1)
less work and (2) they won't fail when HEAD is detached.

Note that this doesn't completely fix the problem with detached HEAD:
a number of other commands (e.g. branch) don't always need to refer to
a current series, but currently fails on a detached HEAD even in those
situations.

Signed-off-by: Karl Hasselström <kha@treskal.com>

---

 stgit/commands/add.py      |    2 +-
 stgit/commands/diff.py     |    2 +-
 stgit/commands/resolved.py |    2 +-
 stgit/commands/status.py   |    2 +-
 4 files changed, 4 insertions(+), 4 deletions(-)


diff --git a/stgit/commands/add.py b/stgit/commands/add.py
index 264ab9f..ceea188 100644
--- a/stgit/commands/add.py
+++ b/stgit/commands/add.py
@@ -31,7 +31,7 @@ Add the files or directories passed as arguments to the
 repository. When a directory name is given, all the files and
 subdirectories are recursively added."""
 
-directory = DirectoryHasRepository()
+directory = DirectoryHasRepository(needs_current_series = False)
 options = []
 
 
diff --git a/stgit/commands/diff.py b/stgit/commands/diff.py
index 42e8367..9ffbb4a 100644
--- a/stgit/commands/diff.py
+++ b/stgit/commands/diff.py
@@ -42,7 +42,7 @@ rev = '([patch][//[bottom | top]]) | <tree-ish> | base'
 If neither bottom nor top are given but a '//' is present, the command
 shows the specified patch (defaulting to the current one)."""
 
-directory = DirectoryHasRepository()
+directory = DirectoryHasRepository(needs_current_series = False)
 options = [make_option('-r', '--range',
                        metavar = 'rev1[..[rev2]]', dest = 'revs',
                        help = 'show the diff between revisions'),
diff --git a/stgit/commands/resolved.py b/stgit/commands/resolved.py
index c2ef678..8b2aba2 100644
--- a/stgit/commands/resolved.py
+++ b/stgit/commands/resolved.py
@@ -34,7 +34,7 @@ Mark a merge conflict as resolved. The ...
From: Karl
Date: Monday, October 8, 2007 - 8:34 am

This hunk shouldn't be here, since diff does use crt_series. (The
commit message also has to be modified accordingly.)

So, we don't have a single test that tries to run "stg diff". Duh.

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

From: Karl
Date: Monday, October 8, 2007 - 9:42 pm

A simple test to make sure that we can run "stg diff" without
arguments, just to list local changes.

Note that two subtests currently fail; these are due to plain "stg
diff" failing on a branch where "stg init" hasn't been run, which is
plainly a bug.

Signed-off-by: Karl Hasselström <kha@treskal.com>

---



 t/t2400-diff.sh |   37 +++++++++++++++++++++++++++++++++++++
 1 files changed, 37 insertions(+), 0 deletions(-)
 create mode 100755 t/t2400-diff.sh


diff --git a/t/t2400-diff.sh b/t/t2400-diff.sh
new file mode 100755
index 0000000..6d9ed98
--- /dev/null
+++ b/t/t2400-diff.sh
@@ -0,0 +1,37 @@
+#!/bin/sh
+
+test_description='Run "stg diff"'
+
+. ./test-lib.sh
+
+test_expect_failure 'Diff with no StGit data' '
+    stg diff
+'
+
+test_expect_success 'Make some local changes' '
+    echo foo >> foo.txt &&
+    git add foo.txt
+'
+
+test_expect_failure 'Diff with some local changes' '
+    stg diff
+'
+
+test_expect_success 'Initialize StGit stuff' '
+    stg init &&
+    stg new foo -m foo
+'
+
+test_expect_success 'Diff with some local changes' '
+    stg diff
+'
+
+test_expect_success 'Refresh patch' '
+    stg refresh
+'
+
+test_expect_success 'Diff with no local changes' '
+    stg diff
+'
+
+test_done

-

From: Karl
Date: Monday, October 8, 2007 - 10:12 pm

I've fixed the patch and pushed it out, along with the two new ones.
For good measure, I also demoted it from safe to experimental.

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

From: Karl
Date: Monday, October 8, 2007 - 1:58 am

Will be available from

  git://repo.or.cz/stgit/kha.git safe

momentarily (I just need to rebase the experimental branch on top of
it).

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

From: Karl
Date: Monday, October 8, 2007 - 2:27 am

There; both safe and experimental are updated. (It takes a while to
rebase when you run the test suite for every patch ...)

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

Previous thread: Re: [PATCH] Make git-clean a builtin by Johannes Schindelin on Sunday, October 7, 2007 - 8:57 pm. (2 messages)

Next thread: Merge problems with git-mingw by Peter Karlsson on Monday, October 8, 2007 - 4:06 am. (17 messages)