Re: 1.3.2 bzr 'fixes' - still can't deal with exec bit changes

[Kai <kai willadsen gmail com>]

On 13 July 2010 19:07, Kai <kai willadsen gmail com> wrote:
> On 13 July 2010 10:21, Andrew Beyer <beyer andrew gmail com> wrote:
>> On Mon, Jul 12, 2010 at 4:42 PM, Andrew Beyer <beyer andrew gmail com> wrote:
>>> In any case, couldn't meld use the "--short" form of the stat command?
>>> Which I believe puts all the flags before each filename, and is
>>> probably easier to parse anyway.
>>>
>>
>> a quick glance at _lookup_tree_cache() in vc/bzr.py makes it look like
>> the parsing is broken for other things too. Symlinks,  file renames,
>> and kind changes (dir to file, or vice versa) can all add extra
>> annotation after the filename in stat command output which isn't dealt
>> with at all. The short form at least puts flags before the filename,
>> which would disambiguate the "*" issue, but still shows renames as
>> "old-filename => new-filename" and similar for kind changes. It looks
>> like even in the short form, an "@" is appended on symlinks, which is
>> also a valid if uncommon character in a filename.
>
> Sounds like using --short would be a thoroughly sensible option.
> Obviously the parsing could use some improvement as well, but if we
> can easily move to --short and fix several bugs, then that sounds
> great. Can someone file a bug?

Lacking a bug, I thought I'd have a go at this anyway. I've attached a
patch that switches bzr to using the short version of the status,
which certainly seems more parseable. Unfortunately, bzr doesn't seem
to have any documentation on what most of the status flags actually
mean, so I've just ported the subset that I think are interesting for
(and supported by) Meld.

I'm not a bzr user, and only have a limited array of test cases at my
disposal, so testing and feedback would be great.

Looking at this has also made it obvious that we should at least
figure out how to deal with moved files, being the one common,
obviously interesting case that we don't support properly for any VC.

cheers,
Kai

From dc6f5e3f704cc84c243a2afc59f477a5ed44d01c Mon Sep 17 00:00:00 2001
From: Kai Willadsen <kai willadsen gmail com>
Date: Sun, 29 Aug 2010 08:28:00 +1000
Subject: [PATCH] Use bzr 'short' status for more robust parsing (closes bgo#348722)

bzr's 'short' version of the status command is easier to parse reliably
than the default version, and lets us easily ignore status changes that
we don't yet support.

For a list of possible status flags, see:
http://doc.bazaar.canonical.com/latest/en/user-reference/
status-flags-help.html
---
 meld/vc/bzr.py |   44 ++++++++++++++++++++++++++++----------------
 1 files changed, 28 insertions(+), 16 deletions(-)

diff --git a/meld/vc/bzr.py b/meld/vc/bzr.py
index 3ad7720..777a74f 100644
--- a/meld/vc/bzr.py
+++ b/meld/vc/bzr.py
@@ -33,14 +33,23 @@ class Vc(_vc.CachedVc):
     NAME = "Bazaar-NG"
     VC_DIR = ".bzr"
     PATCH_INDEX_RE = "^=== modified file '(.*)'$"
-    state_map = {
-        "unknown:":   _vc.STATE_NONE,
-        "added:":     _vc.STATE_NEW,
-        "unchanged:": _vc.STATE_NORMAL,
-        "removed:":   _vc.STATE_REMOVED,
-        "ignored:":   _vc.STATE_IGNORED,
-        "modified:":  _vc.STATE_MODIFIED,
-        "conflicts:": _vc.STATE_CONFLICT,
+
+    # We use None here to indicate flags that we don't deal with or care about
+    state_1_map = {
+        "+": None,               # File versioned
+        "-": None,               # File unversioned
+        "R": None,               # File renamed
+        "?": _vc.STATE_NONE,     # File unknown
+        "X": None,               # File nonexistent (and unknown to bzr)
+        "C": _vc.STATE_CONFLICT, # File has conflicts
+        "P": None,               # Entry for a pending merge (not a file)
+    }
+
+    state_2_map = {
+        "N": _vc.STATE_NEW,      # File created
+        "D": _vc.STATE_REMOVED,  # File deleted
+        "K": None,               # File kind changed
+        "M": _vc.STATE_MODIFIED, # File modified
     }
 
     def commit_command(self, message):
@@ -69,7 +78,8 @@ class Vc(_vc.CachedVc):
         branch_root = _vc.popen([self.CMD] + self.CMDARGS + ["root", rootdir]).read().rstrip('\n')
         while 1:
             try:
-                proc = _vc.popen([self.CMD] + self.CMDARGS + ["status", branch_root])
+                proc = _vc.popen([self.CMD] + self.CMDARGS +
+                                 ["status", "-S", "--no-pending", branch_root])
                 entries = proc.read().split("\n")[:-1]
                 break
             except OSError, e:
@@ -77,13 +87,15 @@ class Vc(_vc.CachedVc):
                     raise
         tree_state = {}
         for entry in entries:
-            if entry == "pending merges:":
-                break
-            if entry in self.state_map:
-                cur_state = self.state_map[entry]
-            else:
-                if entry.startswith("  "):
-                    tree_state[os.path.join(rootdir, entry[2:])] = cur_state
+            state_string, name = entry[:3], entry[4:]
+
+            # TODO: We don't do anything with exec bit changes.
+            path = os.path.join(branch_root, name.strip())
+            state = self.state_1_map.get(state_string[0], None)
+            if state is None:
+                state = self.state_2_map.get(state_string[1], _vc.STATE_NORMAL)
+            tree_state[path] = state
+
         return tree_state
 
     def _get_dirsandfiles(self, directory, dirs, files):
-- 
1.7.2.2