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

[Benjamin Pierce]

Hi Jerome,

[Going back and cleaning emails that accumulated while I was out of  
town a few weeks ago...]

The new behaviors seem reasonable to me.  We should change the  
documentation to say where we're dropping atomicity.  (Not that people  
read it, but still... :-)

> - 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.

I guess this can be handled by adding an optional error annotation to  
directories in the result of update detection?

> - 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.)


Although we've already got a lot of switches, I wonder if we should  
add one more to control this.  But I think the default should be the  
more convenient and slightly less safe way.  Partial deletion seems  
confusing.

Best,

    - Benjamin


On Jun 21, 2009, at 9:07 AM, Jerome Vouillon wrote:

> 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
> <patch.txt>