From e7e99503cc0dea56da857a48b8a647f384755243 Mon Sep 17 00:00:00 2001
From: Tomas Petricek <tomas@tomasp.net>
Date: Sat, 16 May 2015 23:53:07 +0100
Subject: [PATCH] Fix WatchChanges on Mac, fix Dispose, improve Timer usage

---
 src/app/FakeLib/ChangeWatcher.fs     | 46 ++++++++++++++++------------
 src/app/FakeLib/Globbing/Globbing.fs | 30 ++++++++++++------
 2 files changed, 48 insertions(+), 28 deletions(-)

diff --git a/src/app/FakeLib/ChangeWatcher.fs b/src/app/FakeLib/ChangeWatcher.fs
index eb019e4dbb7..875b992f847 100644
--- a/src/app/FakeLib/ChangeWatcher.fs
+++ b/src/app/FakeLib/ChangeWatcher.fs
@@ -47,19 +47,20 @@ let WatchChanges (onChange : FileChange seq -> unit) (fileIncludes : FileInclude
                            dirsToWatch
                            |> Seq.exists (fun p -> p.StartsWith d && p <> d)
                            |> not)
-    
+    tracefn "dirs to watch: %A" dirsToWatch
+ 
+    // we collect changes in a mutable ref cell and wait for a few milliseconds to 
+    // receive all notifications when the system sends them repetedly or sends multiple
+    // updates related to the same file; then we call 'onChange' with all cahnges
     let unNotifiedChanges = ref List.empty<FileChange>
-    
-    let acumChanges (fileChange : FileChange) = 
-        if fileIncludes.IsMatch fileChange.FullPath then 
-            lock unNotifiedChanges (fun () -> unNotifiedChanges := [ fileChange ] @ !unNotifiedChanges)
-    
-    let timer = new System.Timers.Timer(5.0)
+    // when running 'onChange' we ignore all notifications to avoid infinite loops
+    let runningHandlers = ref false
+
+    let timer = new System.Timers.Timer(50.0)
+    timer.AutoReset <- false
     timer.Elapsed.Add(fun _ -> 
         lock unNotifiedChanges (fun () -> 
-            if !unNotifiedChanges
-               |> Seq.length
-               > 0 then 
+            if not (Seq.isEmpty !unNotifiedChanges) then
                 let changes = 
                     !unNotifiedChanges
                     |> Seq.groupBy (fun c -> c.FullPath)
@@ -67,13 +68,25 @@ let WatchChanges (onChange : FileChange seq -> unit) (fileIncludes : FileInclude
                            changes
                            |> Seq.sortBy (fun c -> c.Status)
                            |> Seq.head)
-                unNotifiedChanges := List.empty<FileChange>
-                onChange changes))
+                unNotifiedChanges := []
+                try 
+                    runningHandlers := true
+                    onChange changes
+                finally
+                    runningHandlers := false ))
 
-    tracefn "dirs to watch: %A" dirsToWatch
+    let acumChanges (fileChange : FileChange) = 
+        // only record the changes if we are not currently running 'onChange' handler
+        if not !runningHandlers && fileIncludes.IsMatch fileChange.FullPath then 
+            lock unNotifiedChanges (fun () -> 
+              unNotifiedChanges := fileChange :: !unNotifiedChanges
+              // start the timer (ignores repeated calls) to trigger events in 50ms
+              (timer:System.Timers.Timer).Start() )
+    
     let watchers = 
-        dirsToWatch |> Seq.map (fun dir -> 
+        dirsToWatch |> List.ofSeq |> List.map (fun dir -> 
                            tracefn "watching dir: %s" dir
+
                            let watcher = new FileSystemWatcher(FullName dir, "*.*")
                            watcher.EnableRaisingEvents <- true
                            watcher.IncludeSubdirectories <- true
@@ -88,11 +101,6 @@ let WatchChanges (onChange : FileChange seq -> unit) (fileIncludes : FileInclude
                                              Name = e.Name
                                              Status = Created })
                            watcher)
-    watchers
-    |> Seq.length
-    |> ignore //force iteration
-
-    timer.Start()
 
     { new System.IDisposable with
           member this.Dispose() = 
diff --git a/src/app/FakeLib/Globbing/Globbing.fs b/src/app/FakeLib/Globbing/Globbing.fs
index ffb457c0d25..e65c516518b 100644
--- a/src/app/FakeLib/Globbing/Globbing.fs
+++ b/src/app/FakeLib/Globbing/Globbing.fs
@@ -65,17 +65,29 @@ let inline private normalizeOutputPath (p : string) =
      .TrimEnd(Path.DirectorySeparatorChar)
 
 let internal getRoot (baseDirectory : string) (pattern : string) =
-    let baseDirectory = (normalizePath baseDirectory)
+    let baseDirectory = normalizePath baseDirectory
+    let normPattern = normalizePath pattern
+
+    let patternParts = normPattern.Split([| '/'; '\\' |], StringSplitOptions.RemoveEmptyEntries)
+    let patternPathParts = 
+        patternParts
+        |> Seq.takeWhile(fun p -> not (p.Contains("*")))
+        |> Seq.toArray
+
+    let globRoot = 
+        // If we did not find any "*", then drop the last bit (it is a file name, not a pattern)
+        ( if patternPathParts.Length = patternParts.Length then
+              patternPathParts.[0 .. patternPathParts.Length-2]     
+          else patternPathParts )
+        |> String.concat (Path.DirectorySeparatorChar.ToString())
 
     let globRoot = 
-        (normalizePath pattern).Split([| '/'; '\\' |], StringSplitOptions.RemoveEmptyEntries) |> 
-        Seq.takeWhile(fun p -> not (p.Contains("*"))) |> 
-        String.concat(Path.DirectorySeparatorChar.ToString())
-
-    if Path.IsPathRooted globRoot then
-        globRoot
-    else
-        Path.Combine(baseDirectory, globRoot)
+        // If we dropped "/" from the beginning of the path in the 'Split' call, put it back!
+        if normPattern.StartsWith("/") then "/" + globRoot
+        else globRoot
+
+    if Path.IsPathRooted globRoot then globRoot
+    else Path.Combine(baseDirectory, globRoot)
 
 let internal search (baseDir : string) (input : string) = 
     let baseDir = normalizePath baseDir