Ticket #295: patch_for_295-1107.diff

htdocs/css/diff.css

@@ -20 +20 @@
 /* Colors for change types */
 #overview .mod, .diff #legend .mod { background: #fd8 }
 #overview .rem, .diff #legend .rem { background: #f88 }
-#overview .add, .diff #legend .add { background: #dfd }
+#overview .add, .diff #legend .add { background: #bfb }
+#overview .cp, .diff #legend .cp { background: #88f }
+#overview .mv, .diff #legend .mv { background: #ccc }
 
 /* Legend for diff colors */
 .diff #legend {
@@ -46 +48 @@
  margin: 0;
  margin-right: .5em;
 }
-
+.diff #legend dt.unmod div.mod {
+ border: 0;
+ margin: 0;
+ float: right;
+ width: .4em; height: .8em;
+}
 /* Styles for the list of diffs */
 .diff ul.entries { clear: both; margin: 0; padding: 0 }
 .diff li.entry {

htdocs/css/changeset.css

@@ -12 +12 @@
  overflow: hidden;
  width: .8em; height: .8em;
 }
+#overview div.add div, #overview div.cp div, #overview div.mv div {
+ border: 0;
+ margin: 0;
+ float: right;
+ width: .35em; 
+}
+
 #overview .message { padding: 1em 0 1px }
 #overview dd.message p, #overview dd.message ul, #overview dd.message ol {
  margin-bottom: 1em;

trac/sync.py

@@ -21 +21 @@
 
 from svn import fs, util, delta, repos, core
 
+import posixpath
+
 def sync(db, repos, fs_ptr, pool):
     """
-    updates the revision and node_change tables to be in sync with
+    Update the revision and node_change tables to be in sync with
     the repository.
     """
 
@@ -59 +61 @@
     core.svn_pool_destroy(subpool)
     db.commit()
 
-def insert_change (pool, fs_ptr, rev, cursor):
 
+def insert_change(pool, fs_ptr, rev, cursor):
+    """
+    Save node changes for revision 'rev'.
+
+    Analyse the difference tree at revision 'rev', as given by
+    'repos.svn_repos_replay' (which offers usable 'copyfrom_' information).
+
+    The results are cached in the 'node_change' table, as follows:
+
+    || rev || action || name                                          ||
+    || --- || ------ || --------------------------------------------- ||
+    ||     ||  'A'   || added path                                    ||
+    ||     ||  'D'   || removed path                                  ||
+    ||     ||  'M'   || modified path                                 ||
+    ||     ||  'C'   || original rev, original path // copied path    ||
+    ||     ||  'R'   || original rev, original path // renamed path   ||
+    ||     ||  'd'   || original rev, original path // deleted path   ||
+
+    The 'ADM' operations are direct operations.
+    The 'CR' operation can be direct operations.
+    The 'CRdm' may happen after a 'CR' operation on a parent path.
+    """
+
     class ChangeEditor(delta.Editor):
-        def __init__(self, rev, old_root, new_root, cursor):
+        def __init__(self, rev, new_root, cursor):
             self.rev = rev
             self.cursor = cursor
-            self.old_root = old_root
             self.new_root = new_root
-            self.dir_has_prop_change = 0
+            self.additions = [] # List of tuples
+                                # (file/dir,new_path,old_path,old_rev,action)
+            self.deletions = {} # Used to detect rename and copy operations.
+            self.skip_dir_prop_change = 0
+            
+        def _norm(self, path):
+            if path and path[0] == '/':
+                path = path[1:]
+            return path
 
-        def delete_entry(self, path, revision, parent_baton, pool):
-            self.cursor.execute('INSERT INTO node_change (rev, name, change) '
-                                'VALUES (%s, %s, \'D\')', self.rev, path)
+        # -- svn.delta.Editor callbacks
+        
+        # A directory_baton is a tuple (old_path,old_rev,new_path).
+        # This information is used to keep track of the original path
+        # after a copy or a rename operation on the parent.
 
-        def add_directory(self, path, parent_baton,
-                          copyfrom_path, copyfrom_revision, dir_pool):
-            self.cursor.execute('INSERT INTO node_change (rev, name, change) '
-                                'VALUES (%s, %s, \'A\')', self.rev, path)
+        # -- -- directory
+        
+        def open_root(self, base_revision, dir_pool):
+            return (None, None, '/')
+        # Note: '/' is needed for proper handling of prop changes at root
+        #       (like SVK does for the svm:mirrors property).
 
-        def open_directory(self, path, parent_baton, base_revision, dir_pool):
-            self.dir_has_prop_change = 0
-            return [path, path, dir_pool]
+        def add_directory(self, path, dir_baton, copyfrom_path, copyfrom_rev,
+                          dir_pool):
+            old_path, old_rev = dir_baton[:2]
+            if copyfrom_path: # copied or renamed directory
+                old_path = self._norm(copyfrom_path)
+                old_rev = copyfrom_rev
+                action = 'C'
+            elif old_path:    # already on a branch, expand the original path
+                old_path = posixpath.join(old_path, posixpath.split(path)[1])
+                action = 'A'
+            else:
+                self._save_change(core.svn_node_file, 'A', path) 
+                action = None
 
+            if action:
+                self.additions.append( (core.svn_node_dir, self._norm(path),
+                                        old_path, old_rev, action) )
+
+            # don't create an additional 'M' entry for this directory
+            # in case there's also a dir property change
+            self.skip_dir_prop_change = 1 
+
+            return (old_path, old_rev, path)
+
+        def open_directory(self, path, dir_baton, base_revision, dir_pool):
+            old_path, old_rev = dir_baton[:2]
+            if old_path: # already on a branch, expand the original path
+                old_path = posixpath.join(old_path, posixpath.split(path)[1])
+            self.skip_dir_prop_change = 0
+            return (old_path, old_rev, path)
+
         def change_dir_prop(self, dir_baton, name, value, pool):
-            if not dir_baton or self.dir_has_prop_change:
+            if self.skip_dir_prop_change:
                 return
-            self.cursor.execute('INSERT INTO node_change (rev, name, change) '
-                                'VALUES (%s, %s, \'M\')', self.rev, dir_baton[1])
-            self.dir_has_prop_change = 1
+            old_path, old_rev, path = dir_baton
+            if old_path: # already on a branch
+                self._save_change(core.svn_node_dir, 'm', path, old_path, old_rev)
+            else:
+                self._save_change(core.svn_node_dir, 'M', path)
 
+            self.skip_dir_prop_change = 1
+
         def close_directory(self, dir_baton):
-            if not dir_baton:
-                return
-            self.dir_has_prop_change = 0
+            self.skip_dir_prop_change = 0
 
-        def add_file(self, path, parent_baton,
-                     copyfrom_path, copyfrom_revision, file_pool):
-            self.cursor.execute('INSERT INTO node_change (rev, name, change) '
-                                'VALUES (%s, %s, \'A\')',self.rev, path)
+        def delete_entry(self, path, revision, dir_baton, pool):
+            """
+            This is a removed path. It corresponds to one of the
+            following actions: 'R'ename, 'D'elete, or 'd'elete on a branch.
+            """
+            old_path, old_rev = dir_baton[:2]
+            if old_path: # already on a branch, expand the original path
+                old_path = posixpath.join(old_path, posixpath.split(path)[1])
+                path_info = (old_path, old_rev)
+            else:
+                path_info = core.svn_node_unknown
+            self.deletions[self._norm(path)] = path_info
 
-        def open_file(self, path, parent_baton, base_revision, file_pool):
+        # -- -- file
+
+        def add_file(self, path, dir_baton, copyfrom_path, copyfrom_revision,
+                     dir_pool):
+            old_path, old_rev = dir_baton[:2]
+            if copyfrom_path: # copied or renamed file
+                old_path = self._norm(copyfrom_path)
+                old_rev = copyfrom_revision
+                action = 'C'
+            elif old_path: # already on a branch, resolve old_rev later
+                old_path = posixpath.join(old_path, posixpath.split(path)[1])
+                old_rev = -1
+                action = 'A'
+            else:
+                return self._save_change(core.svn_node_file, 'A', self._norm(path))
+            
+            self.additions.append( (core.svn_node_file, self._norm(path),
+                                    old_path, old_rev, action) )
+
+        def open_file(self, path, dir_baton, dummy_rev, file_pool):
+            old_path, old_rev = dir_baton[:2]
+            if old_path: # already on a branch (at b_rev)
+                old_path = posixpath.join(old_path, posixpath.split(path)[1])
+                # then this is a copy from a file for which f_rev > b_rev
+                self.additions.append( (core.svn_node_file, self._norm(path),
+                                        old_path, -1, 'C') )
+            else: # no branch, it must be a modification
+                self._save_change(core.svn_node_file, 'M', self._norm(path))
+
+        def _save_change(self, node_type, action, path, old_path=None, old_rev=None):
+            # Note: node_type is ignored for now
+            if old_path and old_rev:
+                path = "%s // %d, %s" % ( path, old_rev, old_path )
             self.cursor.execute('INSERT INTO node_change (rev, name, change) '
-                                'VALUES (%s, %s, \'M\')',self.rev, path)
+                                'VALUES (%s, %s, %s)', self.rev, path, action)
 
+        def finalize(self):
+            """
+            The rename detection is deferred until the end of edition,
+            as 'delete' and 'add' notifications can happen in any order.
+            """
+            for node_type, path, old_path, old_rev, action in self.additions:
+                if self.deletions.has_key(old_path): # normal rename
+                    self.deletions.pop(old_path)
+                    action = 'R'
+                elif self.deletions.has_key(path):   # copy+modification in a branch
+                    action = 'C'
+                    # FIXME: actually, this could be a 'R' if the parent branch
+                    #        is a 'R'. This can be fixed if the parent branch
+                    #        is recorded in addition to the old_path.
+                    self.deletions.pop(path)
+                if action == 'A':                    # add on a branch
+                    old_path = old_rev = None
+                self._save_change(node_type, action, path, old_path, old_rev)
+            for path, path_info in self.deletions.items(): 
+                if path_info == core.svn_node_unknown: # simple deletion
+                    self._save_change(core.svn_node_unknown, 'D', path)
+                else:                   # delete on a branch
+                    self._save_change(core.svn_node_unknown, 'd', path, *path_info)
 
-    old_root = fs.revision_root(fs_ptr, rev - 1, pool)
+
     new_root = fs.revision_root(fs_ptr, rev, pool)
-    
-    editor = ChangeEditor(rev, old_root, new_root, cursor)
+
+    editor = ChangeEditor(rev, new_root, cursor)
     e_ptr, e_baton = delta.make_editor(editor, pool)
 
-    def authz_cb(root, path, pool): return 1
-    repos.svn_repos_dir_delta(old_root, '', '',
-                              new_root, '', e_ptr, e_baton, authz_cb,
-                              0, 1, 0, 1, pool)
+    repos.svn_repos_replay(new_root, e_ptr, e_baton, pool)
+
+    editor.finalize() # Editor's close_edit not called...
+
+

trac/Changeset.py

@@ -23 +23 @@
 import sys
 import time
 import util
+import re
+import posixpath
 from StringIO import StringIO
 from zipfile import ZipFile, ZipInfo, ZIP_DEFLATED
 
 import svn
 import svn.delta
 import svn.fs
+import svn.core
 
 import Diff
 import perm
@@ -41 +44 @@
     Base class for diff renderers.
     """
 
-    def __init__(self, old_root, new_root, rev, req, args, env):
+    def __init__(self, old_root, new_root, rev, req, args, env, path_info):
+        self.path_info = path_info
         self.old_root = old_root
         self.new_root = new_root
         self.rev = rev
@@ -49 +53 @@
         self.args = args
         self.env = env
 
-    def open_directory(self, path, parent_baton, base_revision, dir_pool):
-        return [path, path, dir_pool]
+    # svn.delta.Editor callbacks:
+    #   This editor will be driven by a 'repos.svn_repos_dir_delta' call.
+    #   With this driver, The 'copyfrom_path' will always be 'None'.
+    #   We can't use it.
+    #   This is why we merge the path_info data (obtained during a
+    #   'repos.svn_repos_replay' call) back into this process.
+    
+    def _retrieve_old_path(self, parent_baton, path, pool):
+        old_path = parent_baton[0]
+        self.prefix = None
+        if self.path_info.has_key(path): # retrieve 'copyfrom_path' info
+            seq, old_path = self.path_info[path][:2]
+            self.prefix = 'changeset.changes.%d' % seq
+        elif old_path:    # already on a branch, expand the original path
+            old_path = posixpath.join(old_path, posixpath.split(path)[1])
+        else:
+            old_path = path
+        return (old_path, path, pool)
 
+    def open_root(self, base_revision, dir_pool):
+        return self._retrieve_old_path((None, None, None), '/', dir_pool)
+    
+    def open_directory(self, path, dir_baton, base_revision, dir_pool):
+        return self._retrieve_old_path(dir_baton, path, dir_pool)
+
     def open_file(self, path, parent_baton, base_revision, file_pool):
-        return [path, path, file_pool]
+        return self._retrieve_old_path(parent_baton, path, file_pool)
 
 
 class HtmlDiffEditor(BaseDiffEditor):
@@ -62 +88 @@
     the output is written to stdout.
     """
 
-    def __init__(self, old_root, new_root, rev, req, args, env):
-        BaseDiffEditor.__init__(self, old_root, new_root, rev, req, args, env)
-        self.prev_path = None
-        self.fileno = -1
+    def __init__(self, old_root, new_root, rev, req, args, env, change_info):
+        BaseDiffEditor.__init__(self, old_root, new_root, rev, req, args,
+                                env, change_info)
         self.prefix = None
 
-    def _check_next(self, old_path, new_path, pool):
-        if self.prev_path == (old_path or new_path):
-            return
-
-        self.fileno += 1
-        self.prev_path = old_path or new_path
-
-        self.prefix = 'changeset.changes.%d' % (self.fileno)
-        if old_path:
-            old_rev = svn.fs.node_created_rev(self.old_root, old_path, pool)
-            self.req.hdf.setValue('%s.rev.old' % self.prefix, str(old_rev))
-            self.req.hdf.setValue('%s.browser_href.old' % self.prefix,
-                                  self.env.href.browser(old_path, old_rev))
-        if new_path:
-            new_rev = svn.fs.node_created_rev(self.new_root, new_path, pool)
-            self.req.hdf.setValue('%s.rev.new' % self.prefix, str(new_rev))
-            self.req.hdf.setValue('%s.browser_href.new' % self.prefix,
-                                  self.env.href.browser(new_path, new_rev))
-
     def add_directory(self, path, parent_baton, copyfrom_path,
                       copyfrom_revision, dir_pool):
-        self._check_next(None, path, dir_pool)
+        return self._retrieve_old_path(parent_baton, path, dir_pool)
 
+    def add_file(self, path, parent_baton, copyfrom_path, copyfrom_revision,
+                 file_pool):
+        return self._retrieve_old_path(parent_baton, path, file_pool)
+
     def delete_entry(self, path, revision, parent_baton, pool):
-        self._check_next(path, None, pool)
+        old_path = self._retrieve_old_path(parent_baton, path, pool)[0]
+        return old_path, None, pool
 
-    def change_dir_prop(self, dir_baton, name, value, dir_pool):
-        if not dir_baton:
-            return
-        (old_path, new_path, pool) = dir_baton
-        self._check_next(old_path, new_path, dir_pool)
 
-        prefix = '%s.props.%s' % (self.prefix, name)
-        if old_path:
-            old_value = svn.fs.node_prop(self.old_root, old_path, name, dir_pool)
-            if old_value:
-                self.req.hdf.setValue(prefix + '.old', old_value)
-        if value:
-            self.req.hdf.setValue(prefix + '.new', value)
+    # -- changes:
 
-    def add_file(self, path, parent_baton, copyfrom_path, copyfrom_revision,
-                 file_pool):
-        self._check_next(None, path, file_pool)
+    def _old_root(self, new_path, pool):
+        if not new_path:
+            return 
+        old_rev = self.path_info[new_path][2]
+        if not old_rev:
+            return 
+        elif old_rev == self.rev - 1:
+            return self.old_root
+        else:
+            return svn.fs.revision_root(svn.fs.root_fs(self.old_root),
+                                        old_rev, pool)
+        
+    # -- -- textual changes:
 
     def apply_textdelta(self, file_baton, base_checksum):
-        if not file_baton:
+        old_path, new_path, pool = file_baton
+        if not self.prefix or not (old_path and new_path):
             return
-        (old_path, new_path, pool) = file_baton
-        self._check_next(old_path, new_path, pool)
-
+        old_root = self._old_root(new_path, pool)
+        if not old_root:
+            return
+        
         # Try to figure out the charset used. We assume that both the old
         # and the new version uses the same charset, not always the case
         # but that's all we can do...
@@ -128 +140 @@
         ctpos = mime_type and mime_type.find('charset=') or -1
         if ctpos >= 0:
             charset = mime_type[ctpos + 8:]
-            self.log.debug("Charset %s selected" % charset)
         else:
             charset = self.env.get_config('trac', 'default_charset',
                                           'iso-8859-15')
 
         # Start up the diff process
         options = Diff.get_options(self.env, self.req, self.args, 1)
-        differ = svn.fs.FileDiff(self.old_root, old_path, self.new_root,
-                                 new_path, pool, options)
+        differ = svn.fs.FileDiff(old_root, old_path,
+                                 self.new_root, new_path, pool, options)
         differ.get_files()
         pobj = differ.get_pipe()
 
@@ -157 +168 @@
                 os.waitpid(-1, 0)
             except OSError: pass
 
+    # -- -- property changes:
+    
+    def change_dir_prop(self, dir_baton, name, value, dir_pool):
+        self._change_prop(dir_baton, name, value, dir_pool)
+
     def change_file_prop(self, file_baton, name, value, file_pool):
-        if not file_baton:
+        self._change_prop(file_baton, name, value, file_pool)
+
+    def _change_prop(self, baton, name, value, pool):
+        if not self.prefix:
             return
-        (old_path, new_path, pool) = file_baton
-        self._check_next(old_path, new_path, file_pool)
+        old_path, new_path, pool = baton
 
         prefix = '%s.props.%s' % (self.prefix, name)
         if old_path:
-            old_value = svn.fs.node_prop(self.old_root, old_path, name, file_pool)
-            if old_value:
-                self.req.hdf.setValue(prefix + '.old', old_value)
+            old_root = self._old_root(new_path, pool)
+            if old_root:
+                old_value = svn.fs.node_prop(old_root, old_path, name, pool)
+                if old_value:
+                    if value == old_value:
+                        return # spurious change prop after a copy
+                    self.req.hdf.setValue(prefix + '.old', util.escape(old_value))
         if value:
-            self.req.hdf.setValue(prefix + '.new', value)
+            self.req.hdf.setValue(prefix + '.new', util.escape(value))
 
 
 class UnifiedDiffEditor(BaseDiffEditor):
@@ -178 +200 @@
     the output is written to stdout.
     """
 
+    def add_file(self, path, parent_baton, copyfrom_path, copyfrom_revision,
+                 file_pool):
+        return (None, path, file_pool)
+
+    def delete_entry(self, path, revision, parent_baton, pool):
+        if svn.fs.check_path(self.old_root, path, pool) == svn.core.svn_node_file:
+            self.apply_textdelta((path, None, pool),None)
+
     def apply_textdelta(self, file_baton, base_checksum):
         if not file_baton:
             return
@@ -193 +223 @@
         differ.get_files()
         pobj = differ.get_pipe()
         line = pobj.readline()
+        # rewrite 'None' as appropriate ('A' and 'D' support)
+        fix_second_line = 0
+        if line[:6] != 'Files ' and line[:7] != 'Binary ':
+            if old_path == None:            # 'A'
+                line = '--- %s %s' % (new_path, line[9:])
+            elif new_path == None:          # 'D'        
+                fix_second_line = 1
         while line:
             self.req.write(line)
             line = pobj.readline()
+            if fix_second_line:         # 'D'
+                line = '--- %s %s' % (old_path, line[9:])
+                fix_second_line = 0
         pobj.close()
         if sys.platform[:3] != "win" and sys.platform != "os2emx":
             try:
@@ -208 +248 @@
     Generates a ZIP archive containing the modified and added files.
     """
 
-    def __init__(self, old_root, new_root, rev, req, args, env):
-        BaseDiffEditor.__init__(self, old_root, new_root, rev, req, args, env)
+    def __init__(self, old_root, new_root, rev, req, args, env, path_info):
+        BaseDiffEditor.__init__(self, old_root, new_root, rev, req, args,
+                                env, path_info)
         self.buffer = StringIO()
         self.zip = ZipFile(self.buffer, 'w', ZIP_DEFLATED)
 
@@ -256 +297 @@
         row = cursor.fetchone()
         if not row:
             raise util.TracError('Changeset %d does not exist.' % rev,
-                            'Invalid Changset')
+                                 'Invalid Changset')
         return row
 
-    def get_change_info (self, rev):
+    def get_change_info(self, rev):
         cursor = self.db.cursor ()
         cursor.execute ('SELECT name, change FROM node_change ' +
                         'WHERE rev=%d', rev)
@@ -268 +309 @@
             row = cursor.fetchone()
             if not row:
                 break
-            info.append({'name': row['name'],
-                         'change': row['change'],
-                         'log_href': self.env.href.log(row['name'])})
-        return info
+            change = row['change']
+            name = row['name']
+            if change in 'CRdm': # 'C'opy, 'R'eplace or 'd'elete on a branch
+                # the name column contains the encoded ''path_info''
+                # (see _save_change method in sync.py).
+                m = re.match('(.*) // (-?\d+), (.*)', name)
+                if change == 'd':
+                    new_path = None
+                else:
+                    new_path = m.group(1)
+                old_rev = int(m.group(2))
+                if old_rev < 0:
+                    old_rev = None
+                old_path = m.group(3)
+            elif change == 'D':         # 'D'elete
+                new_path = None
+                old_path = name
+                old_rev = None
+            elif change == 'A':         # 'A'dd
+                new_path = name
+                old_path = old_rev = None
+            else:                       # 'M'odify
+                new_path = old_path = name
+                old_rev = None
+            if old_path and not old_rev: # 'D' and 'M'
+                history = svn.fs.node_history(self.old_root, old_path, self.pool)
+                history = svn.fs.history_prev(history, 0, self.pool) # what an API...
+                old_rev = svn.fs.history_location(history, self.pool)[1]
+                # Note: 'node_created_rev' doesn't work reliably
+            key = (new_path or old_path)
+            info.append((key, change, new_path, old_path, old_rev))
 
+        info.sort(lambda x,y: cmp(x[0],y[0]))
+        self.path_info = {}
+        #  path_info is a mapping of paths to sequence number and additional info
+        #   'new_path' to '(seq, copyfrom_path, copyfrom_rev)',
+        #   'old_path' to '(seq)'
+        sinfo = []
+        seq = 0
+        for _, change, new_path, old_path, old_rev in info:
+            cinfo = { 'name.new': new_path,
+                      'name.old': old_path,
+                      'log_href': new_path or old_path }
+            if new_path:
+                self.path_info[new_path] = (seq, old_path, old_rev)
+                cinfo['rev.new'] = str(rev)
+                cinfo['browser_href.new'] = self.env.href.browser(new_path, rev)
+            if old_path:
+                cinfo['rev.old'] = str(old_rev)
+                cinfo['browser_href.old'] = self.env.href.browser(old_path, old_rev)
+            if change in 'CRm':
+                cinfo['copyfrom_path'] = old_path
+            cinfo['change'] = change.upper()
+            cinfo['seq'] = seq
+            sinfo.append(cinfo)
+            seq += 1
+        return sinfo
+
     def render(self):
         self.perm.assert_permission (perm.CHANGESET_VIEW)
 
@@ -291 +385 @@
         if self.args.has_key('update'):
             self.req.redirect(self.env.href.changeset(self.rev))
 
-        change_info = self.get_change_info (self.rev)
-        changeset_info = self.get_changeset_info (self.rev)
+        try:
+            self.old_root = svn.fs.revision_root(self.fs_ptr, int(self.rev) - 1, self.pool)
+            self.new_root = svn.fs.revision_root(self.fs_ptr, int(self.rev), self.pool)
+        except svn.core.SubversionException:
+            raise util.TracError('Invalid revision number: %d' % int(self.rev))
 
+        cinfo = self.get_change_info(self.rev)
+        changeset_info = self.get_changeset_info(self.rev)
+
         self.req.hdf.setValue('title', '[%d] (changeset)' % self.rev)
         self.req.hdf.setValue('changeset.time',
                               time.asctime(time.localtime(int(changeset_info['time']))))