[Checkins] SVN: z3c.vcsync/trunk/ * fixed bug where two new files would not generate a conflict

Martijn Faassen faassen at infrae.com
Fri Jul 4 13:56:51 EDT 2008


Log message for revision 88006:
  * fixed bug where two new files would not generate a conflict
  
  * better file changes tracking.
  

Changed:
  U   z3c.vcsync/trunk/CHANGES.txt
  U   z3c.vcsync/trunk/src/z3c/vcsync/conflict.txt
  U   z3c.vcsync/trunk/src/z3c/vcsync/svn.py

-=-
Modified: z3c.vcsync/trunk/CHANGES.txt
===================================================================
--- z3c.vcsync/trunk/CHANGES.txt	2008-07-04 14:46:56 UTC (rev 88005)
+++ z3c.vcsync/trunk/CHANGES.txt	2008-07-04 17:56:50 UTC (rev 88006)
@@ -4,8 +4,16 @@
 0.14 (unreleased)
 -----------------
 
-* ...
+* Fixed a bug where too many path fragments were returned in case of
+  conflicts. Now only paths that have in fact changed should be
+  returned. Added some tests for this.
 
+* There was a case where two users would add a file with the same name
+  to their own states independently. This used to result in an SVN
+  error, but now also generates a conflict.  This conflict is a bit
+  different in its behavior unfortunately, as it prefers the version
+  already in SVN as opposed to the one last added.
+
 0.13 (2008-06-02)
 -----------------
 

Modified: z3c.vcsync/trunk/src/z3c/vcsync/conflict.txt
===================================================================
--- z3c.vcsync/trunk/src/z3c/vcsync/conflict.txt	2008-07-04 14:46:56 UTC (rev 88005)
+++ z3c.vcsync/trunk/src/z3c/vcsync/conflict.txt	2008-07-04 17:56:50 UTC (rev 88006)
@@ -103,6 +103,17 @@
 File conflicts
 --------------
 
+We define a function first that can get the output of the
+``files_changed`` method on ``info`` and normalize it into testable
+paths::
+
+  >>> def files_changed(synchronizer, info):
+  ...   result = []
+  ...   checkout_path = synchronizer.checkout.path
+  ...   for path in info.files_changed():
+  ...       result.append(path.relto(checkout_path))
+  ...   return sorted(result, key=lambda path: len(path))
+
 Let's now generate a conflict: we change the same object in both trees
 at the same time::
 
@@ -115,13 +126,24 @@
 conflict yet by itself, but sets up for it::
 
   >>> info = s2.sync("synchronize")
+  >>> files_changed(s2, info)
+  []
+  >>> info.objects_changed()
+  ['/data/bar']
 
 Now we'll synchronize the first tree. This will generate a conflict, as
 we saved a different value from the second tree::
 
   >>> current_synchronizer = s1
   >>> info = s1.sync("synchronize")
+  >>> files_changed(s1, info)
+  ['found', 'found/data', 'data/bar.test', 'found/data/bar.test']
+  >>> sorted(info.objects_changed())
+  ['/data/bar']
 
+Note that since ``found`` and ``found/data`` were also newly added to
+the tree, they show up in ``files_changed``.
+
 The conflict will have been resolved in favor of the first tree, as
 this synchronized last::
 
@@ -141,9 +163,14 @@
 
   >>> current_synchronizer = s2
   >>> info = s2.sync("synchronize")
+  >>> files_changed(s2, info)
+  ['found', 'found/data', 'data/bar.test', 'found/data/bar.test']
   >>> data2['bar'].payload
   200
 
+Since ``found`` and ``found/data`` are new to ``s2``, they show up as
+changed files again.
+
 The other version of the conflicting object is also available to the
 second hierarchy::
 
@@ -158,12 +185,29 @@
   >>> current_synchronizer = s2
   >>> data2['sub']['qux'].payload = 36
   >>> info = s2.sync("Synchronize")
+
+No conflict occured yet, so no files changed::
+
+  >>> files_changed(s2, info)
+  []
+
+Now we generate the conflict::
+
   >>> current_synchronizer = s1
   >>> info = s1.sync("Synchronize")
+  >>> files_changed(s1, info)
+  ['found/data/sub', 'data/sub/qux.test', 'found/data/sub/qux.test']
+
+Note that only those files actually changed as a result of this will
+show up, not ``found`` or ``found/data``.
+
   >>> data1['sub']['qux'].payload
   35
   >>> current_synchronizer = s2
   >>> info = s2.sync("Synchronize")
+  >>> files_changed(s2, info)
+  ['found/data/sub', 'data/sub/qux.test', 'found/data/sub/qux.test']
+
   >>> data2['sub']['qux'].payload
   35
 
@@ -173,6 +217,45 @@
   >>> found2['data']['sub']['qux'].payload
   36
 
+Conflict of new files
+---------------------
+
+If both trees have a file with the same name added, these files are in
+conflict before either of them enters svn. This can lead to an SVN
+error when an ``svn up`` is issued - the file is newly updated but has
+also been locally added.
+
+  >>> current_synchronizer = s1
+  >>> data1['simulnew'] = Item(200)
+  >>> current_synchronizer = s2
+  >>> data2['simulnew'] = Item(250)
+
+Let's synchronize the second tree first. This won't be a problem by
+itself::
+
+  >>> current_synchronizer = s2
+  >>> info = s2.sync("synchronize")
+
+The file ``simulnew`` is now in SVN, but at the same time, it will get
+added by ``s1`` as we synchronize it::
+
+  >>> current_synchronizer = s1
+  >>> info = s1.sync("synchronize")
+
+Normally the conflict will have resolved in favor of the one that resolves
+last, but this is currently technically rather hard to do, so we'll favor
+the one that first got the file into SVN::
+
+  >>> data1['simulnew'].payload
+  250
+
+The other version of the conflicting object is stored under the
+``found`` directory::
+
+  >>> found1 = root1['found']
+  >>> found1['data']['simulnew'].payload
+  200
+
 Re-occurrence of a conflict
 ---------------------------
 
@@ -253,12 +336,16 @@
   >>> data1['folder'] = Container()
   >>> data1['folder']['content'] = Item(14)
   >>> info = s1.sync("Synchronize")
+  >>> files_changed(s1, info)
+  []
 
-We'll synchronize this into ``data2`` so that the second user (``user2``) has
-access to it::
+We'll synchronize this into ``data2`` so that the second user
+(``user2``) has access to it::
 
   >>> current_synchronizer = s2
   >>> info = s2.sync("Synchronize")
+  >>> files_changed(s2, info)
+  ['data/folder', 'data/bar.test', 'found/data/bar.test', 'data/folder/content.test']
 
 ``user1`` now throws away ``folder`` in ``data`` and synchronizes this,
 causing ``folder`` to be gone in SVN::
@@ -267,6 +354,8 @@
   >>> state1._removed.append(get_object_path(root1, root1['data']['folder']))
   >>> del root1['data']['folder']
   >>> info = s1.sync("Synchronize")
+  >>> files_changed(s1, info)
+  []
 
 It's really gone now::
 
@@ -287,6 +376,8 @@
 Now ``user2`` does a synchronization too::
 
   >>> info = s2.sync("Synchronize")
+  >>> files_changed(s2, info)
+  ['found/data/folder', 'found/data/folder/content.test']
 
 Since the folder was previously removed, all changes ``user2`` made
 are now gone, as ``folder`` is gone::

Modified: z3c.vcsync/trunk/src/z3c/vcsync/svn.py
===================================================================
--- z3c.vcsync/trunk/src/z3c/vcsync/svn.py	2008-07-04 14:46:56 UTC (rev 88005)
+++ z3c.vcsync/trunk/src/z3c/vcsync/svn.py	2008-07-04 17:56:50 UTC (rev 88006)
@@ -5,6 +5,11 @@
 
 from z3c.vcsync.interfaces import ICheckout
 
+SVN_ERRORS = [("svn: Failed to add file '",
+               "': object of the same name already exists\n"),
+              ("svn: Failed to add file '",
+               "': object of the same name is already scheduled for addition")]
+
 class SvnCheckout(object):
     """A checkout for SVN.
 
@@ -34,12 +39,33 @@
         repos_url = self._repository_url()
         return checkout_url[len(repos_url):]
     
-    def up(self):        
-        self.path.update()
+    def up(self):
+        # these need to be initialized here and will be used
+        # here and in resolve
+        self._found_files = set()
+        while True:
+            try:
+                self.path.update()
+            except py.process.cmdexec.Error, e:
+                # if an added file is in the way of doing an update...
+                err = e.err
+                for start, end in SVN_ERRORS:
+                    if err.startswith(start) and err.endswith(end):
+                        err_path = err[len(start):-len(end)]
+                        path = self._svn_path(py.path.local(err_path))
+                        self._found(path, path.read())
+                        path.remove()
+                        break
+                    # loop again, try another SVN update
+                else:
+                    raise
+            else:
+                # we're done
+                break
+
         self._updated_revision_nr = None
         
     def resolve(self):
-        self._found_files = set()
         self._resolve_helper(self.path)
 
     def commit(self, message):
@@ -66,31 +92,41 @@
                a found path with the same structure.
         content - the file content
         """
-        found = self.path.ensure('found', dir=True)
-        rel_path = path.relto(self.path)
-        save_path = found.join(*rel_path.split(path.sep))
+        save_path = self._found_path(path)
         save_path.ensure()
         save_path.write(content)
-        for part in save_path.parts():
-            self._found_files.add(part)
-        
+        self._add_parts(save_path)
+
     def _found_container(self, path):
         """Store conflicting/lost container in found directory.
 
         path - the path in the original tree. This is translated to
                a found path with the same structure.
         """
-        found = self.path.ensure('found', dir=True)
-        rel_path = path.relto(self.path)
-        save_path = found.join(*rel_path.split(path.sep))
+        save_path = self._found_path(path)
         py.path.local(path.strpath).copy(save_path)
         save_path.add()
-        for part in save_path.parts():
-            self._found_files.add(part)
+        self._add_parts(save_path)
         for new_path in save_path.visit():
             new_path.add()
             self._found_files.add(new_path)
 
+    def _add_parts(self, path):
+        """Given a path, add containing folders if necessary to found files.
+        """
+        self._found_files.add(path)
+        for part in path.parts()[:-1]:
+            if self._is_new(part):
+                self._found_files.add(part)
+        
+    def _is_new(self, path):
+        """Determine whether path is new to SVN.
+        """
+        # if we're above the path in the tree, we're not new
+        if path <= self.path:
+            return False
+        return path in path.status().added
+    
     def _update_files(self, revision_nr):
         """Go through svn log and update self._files and self._removed.
         """
@@ -155,6 +191,26 @@
         for p in path.listdir():
             self._resolve_helper(p)
 
+    def _svn_path(self, path):
+        """Turn a (possibly local) path into an SVN path.
+        """
+        rel_path = path.relto(self.path)
+        return self.path.join(*rel_path.split(path.sep))
+
+    def _found_path(self, path):
+        """Turn a path into a found path.
+        """
+        found = self.path.ensure('found', dir=True)
+        rel_path = path.relto(self.path)
+        return found.join(*rel_path.split(path.sep))
+
+def is_descendant(path, path2):
+    """Determine whether path2 is a true descendant of path.
+
+    a path is not a descendant of itself.
+    """
+    return path2 > path
+
 def conflict_info(conflict):
     path = conflict.dirpath()
     name = conflict.basename



More information about the Checkins mailing list