treating some directories atomically, and ignoring socket files

Julian Squires <[email protected]> Thu, 1 Jan 2015 10:42:22 -0500

Newsgroups	gmane.network.unison.devel
Message-ID	<CAEbLoBL65L2MonUtekcjzxjLZ8rRYvpAh4-ysycsJ+rxsfj9Ew@mail.gmail.com>

Hi,

I was contacted by a client to add two features to Unison for them,
and although I implemented something that worked for them, I'd like to
ask here about how to approach their implementation in a way that
might be accepted upstream.

The first is to ignore Unix domain socket files, which I did by
returning `ABSENT in Fileinfo.get when such a file is encountered.
I've attached some patches for this, but I realized later that my test
could be cleaner, as I didn't fully understand the test scaffold when
I first approached it.

The second is to treat VCS directories (such as .git or .svn)
atomically; that is, all the changes under such a directory should be
reconciled in the same direction, not merged.

For this feature, I provided a workaround for the client that adds a
predicate preference "atomic" which specifies directories to treat in
this manner, and then in Update.buildUpdateRec I treat anything within
those directories as new if there's been any change.  This seems to
work for them, but is clearly pretty inelegant and far from the
cleanest solution.

I had the suspicion that the best-integrated solution would be to
create a new file type like `ATOMIC_DIRECTORY so that the UI could
present it as a single entry, but it would be otherwise processed like
a directory.

I suspect that this would be too invasive a change for such a patch to
ever get merged upstream, though, which is why I'm hoping the members
of this list will have some suggestions.

I've attached patches for ignoring Unix socket files, which may need
some changes, but is probably otherwise uncontroversial.

Thanks,

-- 
Julian Squires

_______________________________________________
Unison-hackers mailing list
[email protected]
http://lists.seas.upenn.edu/mailman/listinfo/unison-hackers

0001-Add-Unix-domain-sockets-test-scaffold.patch (text/x-diff, 1.9 KB)

From 9e8e2830cf0103111c40fa5d5f252481514880ea Mon Sep 17 00:00:00 2001
From: Julian Squires <[email protected]>
Date: Wed, 17 Dec 2014 14:49:54 -0500
Subject: [PATCH 1/6] Add Unix domain sockets test scaffold

---
 src/test.ml | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/src/test.ml b/src/test.ml
index 2f02f5b..37ca1de 100644
--- a/src/test.ml
+++ b/src/test.ml
@@ -102,12 +102,14 @@ let extend p file = Fspath.concat p (Path.fromString file)
 type fs =
   | File of string
   | Link of string
+  | UnixSocket
   | Dir of (string * fs) list
 
 let rec equal fs1 fs2 =
   match fs1,fs2 with
     | File s1, File s2 -> s1=s2
     | Link s1, Link s2 -> s1=s2
+    | UnixSocket, UnixSocket -> true
     | Dir d1, Dir d2 ->
         let dom d = Safelist.sort String.compare (Safelist.map fst d) in
            (dom d1 = dom d2)
@@ -120,6 +122,7 @@ let rec equal fs1 fs2 =
 let rec fs2string = function
   | File s -> "File \"" ^ s ^ "\""
   | Link s -> "Link \"" ^ s ^ "\""
+  | UnixSocket -> "UnixSocket"
   | Dir s -> "Dir [" ^ (String.concat "; "
                           (Safelist.map (fun (n,fs') -> "(\""^n^"\", "^(fs2string fs')^")") s)) ^ "]"
 
@@ -133,6 +136,7 @@ let readfs p =
     match s.Unix.LargeFile.st_kind with
       | Unix.S_REG -> File (read p)
       | Unix.S_LNK -> Link (Fs.readlink p)
+      | Unix.S_SOCK -> UnixSocket
       | Unix.S_DIR -> Dir (Safelist.map (fun x -> (x, loop (extend p x))) (read_dir p))
       | _ -> assert false
   in try Some(loop p) with
@@ -148,6 +152,10 @@ let writefs p fs =
                    (Fspath.toDebugString p) s (Fingerprint.toString (Fingerprint.string s)));
         write p s
     | Link s -> Fs.symlink s p
+    | UnixSocket ->
+        let fd = Unix.socket Unix.PF_UNIX Unix.SOCK_STREAM 0 in
+        Unix.bind fd (Unix.ADDR_UNIX (Fspath.toString p));
+        Unix.close fd
     | Dir files ->
         Fs.mkdir p default_perm;
         Safelist.iter (fun (x,cont) -> loop (extend p x) cont) files
-- 
2.1.3

0002-Add-trivial-test-for-Unix-sockets.patch (text/x-diff, 992 B)

From 300e8e7e037d91936c80017ac8f600d6de9526d9 Mon Sep 17 00:00:00 2001
From: Julian Squires <[email protected]>
Date: Wed, 17 Dec 2014 17:22:55 -0500
Subject: [PATCH 2/6] Add trivial test for Unix sockets

We want to ensure they are ignored during synchronization.
---
 src/test.ml | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/src/test.ml b/src/test.ml
index 37ca1de..a0288b3 100644
--- a/src/test.ml
+++ b/src/test.ml
@@ -330,6 +330,13 @@ let test() =
      running fast enough that the whole thing happens within a second, then the
      update will be missed! *)
 
+  if not (Prefs.read Globals.someHostIsRunningWindows) then
+    (* Test that Unix domain sockets are ignored. *)
+    runtest "Unix domain socket files" [] (fun() ->
+      put R1 (Dir ["a", UnixSocket]); sync();
+      checkmissing "1" R2
+    );
+
   (* Check for the bug reported by Ralf Lehmann *)
   if not bothRootsLocal then 
     runtest "backups 1 (remote)" ["backup = Name *"] (fun() -> 
-- 
2.1.3

0003-Ignore-socket-files.patch (text/x-diff, 716 B)

From 9c099d61c4b4c753026a3a82a56769f26ec0491b Mon Sep 17 00:00:00 2001
From: Julian Squires <[email protected]>
Date: Wed, 17 Dec 2014 17:41:19 -0500
Subject: [PATCH 3/6] Ignore socket files

---
 src/fileinfo.ml | 1 +
 1 file changed, 1 insertion(+)

diff --git a/src/fileinfo.ml b/src/fileinfo.ml
index fe55917..ada91d0 100644
--- a/src/fileinfo.ml
+++ b/src/fileinfo.ml
@@ -79,6 +79,7 @@ let get fromRoot fspath path =
            match stats.Unix.LargeFile.st_kind with
              Unix.S_REG -> `FILE
            | Unix.S_DIR -> `DIRECTORY
+           | Unix.S_SOCK -> `ABSENT
            | Unix.S_LNK ->
                if not fromRoot || Prefs.read symlinksAllowed then
                  `SYMLINK
-- 
2.1.3