Rev 5425: Add ``bzrlib.transform.orphan_policy`` and allows ``never`` to restore the previous behaviour. in file:///home/vila/src/bzr/bugs/323111-orphans/

Vincent Ladeuil v.ladeuil+lp at free.fr
Thu Sep 16 17:13:18 BST 2010


At file:///home/vila/src/bzr/bugs/323111-orphans/

------------------------------------------------------------
revno: 5425
revision-id: v.ladeuil+lp at free.fr-20100916161318-c9k41sne1m12rxl9
parent: v.ladeuil+lp at free.fr-20100916095233-ik3jqalkoig53nbm
committer: Vincent Ladeuil <v.ladeuil+lp at free.fr>
branch nick: orphan-config-option
timestamp: Thu 2010-09-16 18:13:18 +0200
message:
  Add ``bzrlib.transform.orphan_policy`` and allows ``never`` to restore the previous behaviour.
  
  * bzrlib/transform.py:
  (TreeTransformBase._get_potential_orphans): Simplified.
  (DiskTreeTransform.new_orphan): Use the orphaning_registry.
  (refuse_orphan): Implement the previous behaviour (never orphan).
  (conflict_pass): Clarify orphan creation and fallback on error.
  
  * bzrlib/tests/test_transform.py:
  (TestOrphan.test_never_orphan): Add more tests to cover no
  orphaning and orphaning errors.
-------------- next part --------------
=== modified file 'NEWS'
--- a/NEWS	2010-09-10 14:33:20 +0000
+++ b/NEWS	2010-09-16 16:13:18 +0000
@@ -81,6 +81,12 @@
   @needs_write_lock (set_option() for example).
   (Vincent Ladeuil,  #525571)
 
+* Orphan creation (when a directory is deleted) is now controlled by the
+  ``bzrlib.transform.orphan_policy`` configuration variable. Acceptable
+  values are ``never`` (the previous behaviour) or ``always`` to create
+  orphans when a versioned directory is deleted and contains unversioned
+  files (the orphans). (Vincent Ladeuil, #323111)
+
 * The ``lp:`` prefix will now use your known username (from
   ``bzr launchpad-login``) to expand ``~`` to your username.  For example:
   ``bzr launchpad-login user && bzr push lp:~/project/branch`` will now

=== modified file 'bzrlib/tests/test_transform.py'
--- a/bzrlib/tests/test_transform.py	2010-09-16 09:52:33 +0000
+++ b/bzrlib/tests/test_transform.py	2010-09-16 16:13:18 +0000
@@ -3256,35 +3256,61 @@
 
 class TestOrphan(tests.TestCaseWithTransport):
 
-    # Alternative implementations may want to test:
-    # - can't create orphan dir
-    # - orphaning forbidden
-    # - can't create orphan
-
     def test_no_orphan_for_transform_preview(self):
         tree = self.make_branch_and_tree('tree')
         tt = transform.TransformPreview(tree)
         self.addCleanup(tt.finalize)
         self.assertRaises(NotImplementedError, tt.new_orphan, 'foo', 'bar')
 
-    def test_new_orphan_created(self):
-        wt = self.make_branch_and_tree('.')
+    def _set_orphan_policy(self, wt, policy):
+        wt.branch.get_config().set_user_option('bzrlib.transform.orphan_policy',
+                                               policy)
+
+    def _prepare_orphan(self, wt):
         self.build_tree(['dir/', 'dir/foo'])
         wt.add(['dir'], ['dir-id'])
         wt.commit('add dir')
         tt = transform.TreeTransform(wt)
         self.addCleanup(tt.finalize)
         dir_tid = tt.trans_id_tree_path('dir')
-        foo_tid = tt.trans_id_tree_path('dir/foo')
+        orphan_tid = tt.trans_id_tree_path('dir/foo')
         tt.delete_contents(dir_tid)
         tt.unversion_file(dir_tid)
         raw_conflicts = tt.find_conflicts()
         self.assertLength(1, raw_conflicts)
         self.assertEqual(('missing parent', 'new-1'), raw_conflicts[0])
+        return tt, orphan_tid
+
+    def test_new_orphan_created(self):
+        wt = self.make_branch_and_tree('.')
+        tt, orphan_tid = self._prepare_orphan(wt)
         remaining_conflicts = resolve_conflicts(tt)
         # Yeah for resolved conflicts !
         self.assertLength(0, remaining_conflicts)
         # We have a new orphan
-        self.assertEquals('foo.~1~', tt.final_name(foo_tid))
+        self.assertEquals('foo.~1~', tt.final_name(orphan_tid))
         self.assertEquals('bzr-orphans',
-                          tt.final_name(tt.final_parent(foo_tid)))
+                          tt.final_name(tt.final_parent(orphan_tid)))
+
+    def test_never_orphan(self):
+        wt = self.make_branch_and_tree('.')
+        self._set_orphan_policy(wt, 'never')
+        tt, orphan_tid = self._prepare_orphan(wt)
+        remaining_conflicts = resolve_conflicts(tt)
+        self.assertLength(1, remaining_conflicts)
+        self.assertEqual(('deleting parent', 'Not deleting', 'new-1'),
+                         remaining_conflicts.pop())
+
+    def test_orphan_error(self):
+        def bogus_orphan(tt, orphan_id, parent_id):
+            raise transform.OrphaningError(tt.final_name(orphan_id),
+                                           tt.final_name(parent_id))
+        transform.orphaning_registry.register('bogus', bogus_orphan,
+                                              'Raise an error when orphaning')
+        wt = self.make_branch_and_tree('.')
+        self._set_orphan_policy(wt, 'bogus')
+        tt, orphan_tid = self._prepare_orphan(wt)
+        remaining_conflicts = resolve_conflicts(tt)
+        self.assertLength(1, remaining_conflicts)
+        self.assertEqual(('deleting parent', 'Not deleting', 'new-1'),
+                         remaining_conflicts.pop())

=== modified file 'bzrlib/transform.py'
--- a/bzrlib/transform.py	2010-09-16 09:06:56 +0000
+++ b/bzrlib/transform.py	2010-09-16 16:13:18 +0000
@@ -76,6 +76,7 @@
     map[key] = value
 
 
+
 class _TransformResults(object):
     def __init__(self, modified_paths, rename_count):
         object.__init__(self)
@@ -813,19 +814,16 @@
              versioned file is present.
         """
         orphans = []
-        dont_delete = False
         # Find the potential orphans, stop if one item should be kept
         for c in self.by_parent()[dir_id]:
             if self.final_file_id(c) is None:
                 orphans.append(c)
             else:
-                dont_delete = True
+                # We have a versioned file here, searching for orphans is
+                # meaningless.
+                orphans = None
                 break
-        if dont_delete:
-            return None
-        else:
-            return orphans
-
+        return orphans
 
     def _affected_ids(self):
         """Return the set of transform ids affected by the transform"""
@@ -1340,7 +1338,37 @@
         delete_any(self._limbo_name(trans_id))
 
     def new_orphan(self, trans_id, parent_id):
-        move_orphan(self, trans_id, parent_id)
+        # FIXME: There is no tree config, so we use the branch one (it's weird
+        # to define it this way as orphaning can only occur in a working tree,
+        # but that's all we have (for now). It will find the option in
+        # locations,conf or bazaar.conf though) -- vila 20100916
+        conf = self._tree.branch.get_config()
+        orphan_policy = conf.get_user_option('bzrlib.transform.orphan_policy')
+        if orphan_policy is None:
+            orphan_policy = orphaning_registry.default_key
+        handle_orphan = orphaning_registry.get(orphan_policy)
+        handle_orphan(self, trans_id, parent_id)
+
+
+class OrphaningError(errors.BzrError):
+
+    # Only bugs could lead to such exception being seen by the user
+    internal_error = True
+    _fmt = "Error while orphaning %s in %s directory"
+
+    def __init__(self, orphan, parent):
+        errors.BzrError.__init__(self)
+        self.orphan = orphan
+        self.parent = parent
+
+
+class OrphaningForbidden(OrphaningError):
+
+    _fmt = "Policy: %s doesn't allow creating orphans."
+
+    def __init__(self, policy):
+        errors.BzrError.__init__(self)
+        self.policy = policy
 
 
 def move_orphan(tt, orphan_id, parent_id):
@@ -1356,8 +1384,8 @@
     :param parent_id: The orphan parent trans id.
     """
     # Add the orphan dir if it doesn't exist
-    orphan_dir = 'bzr-orphans'
-    od_id = tt.trans_id_tree_path(orphan_dir)
+    orphan_dir_basename = 'bzr-orphans'
+    od_id = tt.trans_id_tree_path(orphan_dir_basename)
     if tt.final_kind(od_id) is None:
         tt.create_directory(od_id)
     parent_path = tt._tree_id_paths[parent_id]
@@ -1366,13 +1394,23 @@
     new_name = tt._available_backup_name(actual_name, od_id)
     tt.adjust_path(new_name, od_id, orphan_id)
     trace.warning('%s has been orphaned in %s'
-                  % (joinpath(parent_path, actual_name), orphan_dir))
+                  % (joinpath(parent_path, actual_name), orphan_dir_basename))
+
+
+def refuse_orphan(tt, orphan_id, parent_id):
+    """See TreeTransformBase.new_orphan.
+
+    This refuses to create orphan, letting caller handle the conflict.
+    """
+    raise OrphaningForbidden('never')
 
 
 orphaning_registry = registry.Registry()
-orphaning_registry.register('bzr-orphans', move_orphan,
+orphaning_registry.register('always', move_orphan,
                             'Move orphans into the bzr-orphans directory.')
-orphaning_registry._set_default_key('bzr-orphans')
+orphaning_registry._set_default_key('always')
+orphaning_registry.register('never', refuse_orphan,
+                            'Never create orphans.')
 
 
 class TreeTransform(DiskTreeTransform):
@@ -2909,16 +2947,24 @@
         elif c_type == 'missing parent':
             trans_id = conflict[1]
             if trans_id in tt._removed_contents:
+                cancel_deletion = True
                 orphans = tt._get_potential_orphans(trans_id)
                 if orphans:
+                    cancel_deletion = False
                     # All children are orphans
                     for o in orphans:
                         try:
                             tt.new_orphan(o, trans_id)
-                        except OrphaningForbidden:
-                            orphans = None
+                        except OrphaningError:
+                            # Something bad happened so we cancel the directory
+                            # deletion which will leave it in place with a
+                            # conflict. The user can deal with it from there.
+                            # Note that this also catch the case where we don't
+                            # want to create orphans and leave the directory in
+                            # place.
+                            cancel_deletion = True
                             break
-                if orphans is None:
+                if cancel_deletion:
                     # Cancel the directory deletion
                     tt.cancel_deletion(trans_id)
                     new_conflicts.add(('deleting parent', 'Not deleting',

=== modified file 'doc/en/whats-new/whats-new-in-2.3.txt'
--- a/doc/en/whats-new/whats-new-in-2.3.txt	2010-09-10 17:36:46 +0000
+++ b/doc/en/whats-new/whats-new-in-2.3.txt	2010-09-16 16:13:18 +0000
@@ -58,8 +58,10 @@
 
 * Deleting a versioned directory containing unversioned files will no
   longer create a conflict. Instead, the unversioned files will be moved
-  into a 'bzr-orphans' directory at the root of the working tree.
-  (Vincent Ladeuil, #323111)
+  into a 'bzr-orphans' directory at the root of the working tree.  This is
+  controlled by the ``bzrlib.transform.orphan_policy`` configuration
+  variable with a value of ``always`` (the default). The previous behaviour
+  can be restored by using the ``never`` value.  (Vincent Ladeuil, #323111)
 
 Documentation
 *************



More information about the bazaar-commits mailing list