[Unison-hackers] path option 'root testing' bug

[Benjamin Pierce]

Hi Matthew,

I agree that Unison's current behavior (just failing) is probably less  
good than simply creating the directory it needs.  If you (or someone)  
feel like having a go at fixing it, I'd be happy to accept a patch.

The function Os.createDir is most of what's needed, but it should  
probably be called for every component of the path that's missing, not  
just the final one.  Also, there's the question of what permissions  
should be used for the created directories.  The "right thing" to do  
-- asking the other host what the permissions are for each one -- is  
probably to complicated to be worth the trouble, but any other choice  
is a bit arbitrary.

Best,

     - Benjamin


On Aug 7, 2008, at 5:21 AM, Matthew Richardson wrote:

> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> I've encountered what looks like a well-reported 'bug' in unison,  
> which I'm
> interested in having resolved, but I don't have any ocaml expertise  
> under my
> belt, and thus am appealing to the list to try and get this looked at.
>
> The problem is simple - if you specify a path of format:
>
> path = a/b/c
>
> Then folders a and b must exist on both source and target prior to  
> unison
> running - if this isn't true, then unison fails with the error:
>
> "path a/b is not valid because a/b is not a directory in one of the  
> replicas"
>
> I've had a look at the code, and I believe I can see why this is  
> occurring.  As
> far as I can tell, the unison code treats path options as temporary  
> moves of the
> root to a new 'base'.  In this case it moves to a/b and then tries  
> to sync from
> there.  As neither of these exist, it fails.
>
> The relevant code block is:
>
> | Some(name, path') ->
>    if not (isDir fspath here) then
>      let error =
>        if Path.isEmpty here then
> (* error 1 *)
>          Printf.sprintf
>            "path %s is not valid because the root of one of the  
> replicas \
>             is not a directory"
>            (Path.toString fullpath)
>        else
> (* error 2 *)
>          Printf.sprintf
>            "path %s is not valid because %s is not a directory in  
> one of \
>             the replicas"
>            (Path.toString fullpath) (Path.toString here)
>
> The test runs as follows:
>
> Test if both paths are directories.
>
> If path is 'empty' - error 1 reported.
> Otherwise - error 2 reported.
>
> I would suggest that the following might be a better test:
>
> Test if both paths are directories.
>
> If path is 'empty' - error 1 reported.
> If target isn't a directory, try recursively making the path and  
> repeating the
> test - succeed or fail as appropriate.
> If this 'mkdir -p' fails, report error 2.
>
> This should allow the problem of the directories not existing to be  
> resolved,
> but should still fail if one replica has files, symlinks etc where a  
> directory
> is expected.
>
> Of course, I might have completely misunderstood what unison is  
> doing here (in
> which case fell free to shoot me down), and I don't speak any ocaml  
> to implement
> this change (although if there's agreement this change would be  
> useful I could
> make the effort to learn if there isn't any available coding effort).
>
> Comments?
>
> Matthew
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v1.4.6 (GNU/Linux)
> Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org
>
> iD8DBQFImr6mQwGHl7Fegc8RAnjBAJ9XPNF+Zurxwj5bCgNCmLHpTaV/nwCeMuit
> 39EofwHcsvHTs0IPNzPuMgw=
> =EZs3
> -----END PGP SIGNATURE-----
> -- 
> The University of Edinburgh is a charitable body, registered in
> Scotland, with registration number SC005336.
> _______________________________________________
> Unison-hackers mailing list
> Unison-hackers at lists.seas.upenn.edu
> http://lists.seas.upenn.edu/mailman/listinfo/unison-hackers