[Unison-hackers] [unison-users] Unison aborts all the synchronization after a permission issue

[Jerome Vouillon]

Hi Benjamin,

On Mon, Jun 15, 2009 at 08:11:05AM -0400, Benjamin Pierce wrote:
> This is a longstanding issue.  It's not a bug -- the behavior is not  
> only "correct" but deeply built into the spec's definition of  
> conflict.  Of course, in practice it's often irritating, and by  
> hindsight we should have designed it differently!  But at this point  
> it will take some careful thought to make it work differently, even as  
> an option.  (If anybody is up for giving it careful thought and  
> creating a patch, I'd be glad to consider adding it.)

I believe the following patch (against the developer version) does the
right thing (*).  Then, there are some user interface issues:
- With this patch, errors deep inside a directory are not reported to
  the user.  Unison should report them all while still allowing to
  propagate the directory.
- Overwriting (or deleting) a directory with deep errors will fail
  during propagation.  We should either forbid the user from
  attempting to overwrite such a directory, or relax the checks in
  function Update.checkNoUpdates. (Or implement partial deletion.)

I will not have time to work further on this this week, though.

(*) We have a change of behavior when one replica contains a directory
with a deep error at some location and the other does not contain a
directory there.  Here are the different cases to consider:
 1) Propagating the directory.
   a) The directory is not in the archive.
      If the other side is unchanged, Unison will propagate everything
      in the directory but the locations with an error and will update
      the archives accordingly.  The spec without the atomicity rule
      allows this.  (Note that, in this case, the change in function
      Update.updateArchiveRec does nothing as the archives contain
      NoArchive at the location of the error.)
      If the other side is changed, the user has to force the
      synchronization of the directory.  Unison will propagate the
      directory and update the archives the same way.  We are outside
      the scope of the spec, but I think Unison behaves reasonnably.
  (b) The directory is in the archives.
      In this case, the other side must have changed.  So, the user
      has to force the synchronization of the directory and we are
      outside the scope of the spec.
      The locations with errors are going to be removed from the
      archives (change in Update.checkNoUpdatesRec).  It is not clear
      whether this is the right thing to do.  One could argue both
      ways.  One may want to keep the previous archive contents as the
      synchronization of these locations was not performed.  On the
      other hand, the user has chosen to propagate back the directory.
      Clearing the archive is a way of taking this into account.
      Also, this is easier to implement: this is exactly what we need
      for propagation in case (1.b) and in Update.checkNoUpdatesRec in
      case (2).
 2) Propagating the contents of the other replica.
    Unison will not do anything by default, which is correct.
    Forcing the deletion of the directory will either fail if the
    errors are still there, or succeed if the locations corresponding
    to an error are now empty (thanks to the change in function
    Update.checkNoUpdatesRec).  The spec without the atomicity rule
    allows to remove every unchanged part from the directory.  So,
    what is done here is correct when we are under the scope of the
    spec.

-- Jerome
-------------- next part --------------
Index: update.ml
===================================================================
--- update.ml	(r?vision 360)
+++ update.ml	(copie de travail)
@@ -1646,8 +1646,10 @@
 (* the result of patching [archive] using [ui] *)
 let rec updateArchiveRec ui archive =
   match ui with
-    NoUpdates | Error _ ->
+    NoUpdates ->
       archive
+  | Error _ ->
+      NoArchive
   | Updates (uc, _) ->
       match uc with
         Absent ->
Index: recon.ml
===================================================================
--- recon.ml	(r?vision 360)
+++ recon.ml	(copie de travail)
@@ -214,7 +214,8 @@
           ()
 
 (* lifting errors in individual updates to replica problems                  *)
-let propagateErrors (rplc: Common.replicas): Common.replicas =
+let propagateErrors (rplc: Common.replicas): Common.replicas = rplc
+(*
   match rplc with
     Problem _ ->
       rplc
@@ -228,6 +229,7 @@
           Problem ("[root 2]: " ^ err)
       with UpdateError err ->
         Problem ("[root 1]: " ^ err)
+*)
 
 type singleUpdate = Rep1Updated | Rep2Updated