#286 DeleteTree: Retry on set intervals

    - DeleteTree.cs: Added two new properties:

             public int Retries { get; set; } = 0;
             public int RetryDelayMilliseconds { get; set; }

      The implementation has been enriched to switch over
       to try-catch-retry-mode when/if Retries > 0.

.gitignore

@@ -16,3 +16,4 @@ Test
 *.chm
 /Tools
 /build.txt
+.vs

Source/MSBuild.Community.Tasks.Tests/DeleteTreeTest.cs

@@ -1,8 +1,11 @@
 using System;
 using System.IO;
+using System.Linq;
+using System.Threading;
 using Microsoft.Build.Framework;
 using Microsoft.Build.Utilities;
 using NUnit.Framework;
+using Task = System.Threading.Tasks.Task;
 
 namespace MSBuild.Community.Tasks.Tests
 {
@@ -14,7 +17,7 @@ public void GetDirectories()
         {
             var root = Path.GetFullPath(@"..\..\..\");
 
-            var directories = Directory.GetDirectories(root, "*", SearchOption.AllDirectories);
+            var _ = Directory.GetDirectories(root, "*", SearchOption.AllDirectories);
         }
 
         [Test]
@@ -151,5 +154,77 @@ public void MatchDirectoriesRelativeWildCard()
             path = DeleteTree.MatchDirectories(@"..\..\..\MSBuild.*.Tests\**\obj\**");
             Assert.Greater(path.Count, 0);
         }
+
+        [Test]
+        [Explicit]
+        public void DeleteWithRetriesDirectoryWhichAppearsWithDelay_ShouldSucceed()
+        {
+            //Arrange
+            const string fullTargetDirectoryPath = @"C:\Temp\xn_TestDirectoryWhichDoesntInitiallyExist";
+
+            var task = new DeleteTree
+            {
+                BuildEngine = new MockBuild(),
+
+                Retries = 10,
+                RetryDelayMilliseconds = 300,
+
+                Directories = new ITaskItem[]
+                {
+                    new TaskItem(fullTargetDirectoryPath)
+                }
+            };
+
+            //Act
+            new Task(() =>
+            {
+                Thread.Sleep(2000);
+
+                Directory.CreateDirectory(fullTargetDirectoryPath);
+            }).Start();
+            var result = task.Execute();
+
+            //Assert
+            Assert.IsTrue(result);
+            Assert.AreEqual(1, task.DeletedDirectories.Length);
+            Assert.AreEqual(fullTargetDirectoryPath, task.DeletedDirectories.FirstOrDefault().ItemSpec);
+        }
+
+        [Test]
+        [Explicit]
+        public void DeleteWithRetriesDirectoryWhichDoesntAppearsAtAll_ShouldFailWithException()
+        {
+            //Arrange
+            const string fullTargetDirectoryPath = @"C:\Temp\xn_TestDirectoryWhichDoesntInitiallyExist";
+
+            var task = new DeleteTree
+            {
+                BuildEngine = new MockBuild(),
+
+                Retries = 10,
+                RetryDelayMilliseconds = 300,
+
+                Directories = new ITaskItem[]
+                {
+                    new TaskItem(fullTargetDirectoryPath)
+                }
+            };
+
+            var exception = (Exception) null;
+
+            //Act
+            try
+            {
+                task.Execute();
+            }
+            catch (Exception ex)
+            {
+                exception = ex;
+            }
+
+            //Assert
+            Assert.IsInstanceOf<DirectoryNotFoundException>(exception);
+        }
     }
-}
+}
+

Source/MSBuild.Community.Tasks/DeleteTree.cs

@@ -33,6 +33,7 @@ OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED.
 using System.Linq;
 using System.Text;
 using System.Text.RegularExpressions;
+using System.Threading;
 using Microsoft.Build.Framework;
 using Microsoft.Build.Utilities;
 
@@ -59,11 +60,16 @@ public class DeleteTree : Task
         private const string recursiveDirectoryMatch = "**";
         private static readonly char[] wildcardCharacters = new[] { '*', '?' };
         private static readonly char[] directorySeparatorCharacters = new[] { Path.DirectorySeparatorChar, Path.AltDirectorySeparatorChar };
-        
+
         private const string recursiveRegex = @"(?:\\.*?)*";
         private const string anyCharRegex = @"[^\\]*?";
         private const string singleCharRegex = @"[^\\]";
 
+        /// <summary>
+        /// Default milliseconds to wait between necessary retries
+        /// </summary>
+        private const int RetryDelayMillisecondsDefault = 1000;
+
         /// <summary>
         /// Initializes a new instance of the <see cref="DeleteTree"/> class.
         /// </summary>
@@ -82,6 +88,21 @@ public DeleteTree()
         [Required]
         public ITaskItem[] Directories { get; set; }
 
+        /// <summary>
+        /// The maximum amount of times to attempt to delete the given directories, if all previous
+        /// attempts fail. Defaults to zero.
+        /// </summary>
+        /// <remarks>
+        /// Warning: using Retries may mask a synchronization problem in your build process.
+        /// </remarks>
+        public int Retries { get; set; } = 0;
+
+        /// <summary>
+        /// Delay between any necessary retries.
+        /// Defaults to <see cref="RetryDelayMillisecondsDefault">RetryDelayMillisecondsDefault</see>
+        /// </summary>
+        public int RetryDelayMilliseconds { get; set; } = RetryDelayMillisecondsDefault;
+
         /// <summary>
         /// Gets or sets a value indicating whether this <see cref="DeleteTree"/> is recursive.
         /// </summary>
@@ -108,31 +129,117 @@ public ITaskItem[] DeletedDirectories
         /// </returns>
         public override bool Execute()
         {
-            foreach (var directory in Directories)
-            {
-                var matched = MatchDirectories(directory.ItemSpec);
+            if (!ValidateInputs())
+                return false;
 
-                foreach (var dir in matched)
+            foreach (var dir in Directories.SelectMany(x => MatchDirectories(x.ItemSpec)))
+            {
+                if (Retries > 0)
                 {
-                    _deletedDirectories.Add(new TaskItem(dir));
+                    DeleteTreeImplPersistant(dir);
+                    continue;
+                }
 
-                    if (!Directory.Exists(dir))
-                        continue;
+                DeleteTreeImplOldSchool(dir); //oldschool approach   we keep it around out of respect for backwards compatibility
+            }
 
-                    Log.LogMessage("  Deleting Directory '{0}'", dir);
-                    try
-                    {
-                        Directory.Delete(dir, Recursive);
-                    }
-                    catch (IOException ex) // continue to delete on the following exceptions
+            return true;
+        }
+
+        internal void DeleteTreeImplOldSchool(string dir) //old
+        {
+            _deletedDirectories.Add(new TaskItem(dir));
+
+            Log.LogMessage("  Deleting Directory '{0}'", dir);
+
+            try
+            {
+                Directory.Delete(dir, Recursive);
+            }
+            catch (DirectoryNotFoundException) //no worries
+            {
+            }
+            catch (IOException ex) // resume deletion on such exceptions
+            {
+                Log.LogErrorFromException(ex, false);
+            }
+            catch (UnauthorizedAccessException ex)
+            {
+                Log.LogErrorFromException(ex, false);
+            }
+        }
+
+        internal void DeleteTreeImplPersistant(string dir) //modern
+        {
+            Log.LogMessage("  Deleting Directory '{0}'", dir);
+
+            //if (!Directory.Exists(dir)) return; //dont
+
+            var retries = 0;
+            while (true)
+            {
+                try
+                {
+                    Directory.Delete(dir, Recursive);
+                    _deletedDirectories.Add(new TaskItem(dir));
+                }
+                catch (PathTooLongException) //no point to retry on this one
+                {
+                    throw;
+                }
+                catch (ArgumentNullException) //shouldnt happen but just in case
+                {
+                    throw;
+                }
+                catch (ArgumentException) //path is only whitespace or has invalid chars in it
+                {
+                    throw;
+                }
+                catch (Exception ex) //directorynotfoundexception unauthorizedaccess or ioexceptions fall through here
+                {
+                    if (retries < Retries)
                     {
-                        Log.LogErrorFromException(ex, false);
+                        retries++;
+                        Log.LogWarning(Properties.Resources.DeleteTreeRetrying, dir, retries, RetryDelayMilliseconds, ex.Message);
+                        Thread.Sleep(RetryDelayMilliseconds);
+                        continue;
                     }
-                    catch (UnauthorizedAccessException ex)
+
+                    if (Retries > 0) //retries >= Retries
                     {
-                        Log.LogErrorFromException(ex, false);
+                        Log.LogError(Properties.Resources.DeleteTreeExceededRetries, dir, RetryDelayMilliseconds, ex.Message);
+                        throw;
                     }
+
+                    throw;
                 }
+
+                break;
+            }
+        }
+
+        /// <summary>
+        /// Verify that the inputs are correct.
+        /// </summary>
+        /// <returns>False on an error, implying that the overall copy operation should be aborted.</returns>
+        internal bool ValidateInputs()
+        {
+            if (Retries < 0)
+            {
+                Log.LogError(Properties.Resources.DeleteTreeInvalidRetryCount, Retries);
+                return false;
+            }
+
+            if (RetryDelayMilliseconds < 0)
+            {
+                Log.LogError(Properties.Resources.DeleteTreeInvalidRetryDelay, RetryDelayMilliseconds);
+                return false;
+            }
+
+            if (Directories.Any(x => string.IsNullOrWhiteSpace(x.ItemSpec)))
+            {
+                Log.LogError(Properties.Resources.DeleteTreeInvalidDirectoriesPaths);
+                return false;
             }
 
             return true;
@@ -144,7 +251,7 @@ internal static IList<string> MatchDirectories(string pattern)
 
             var pathIndex = 0; // find root path with no wildcards
             var rootPath = FindRootPath(pathParts, out pathIndex);
-            
+
             var directories = new List<string>(128);
             if (pathIndex >= pathParts.Length) // no wild cards or relative directories because there are no parts left, use root
             {
@@ -184,13 +291,13 @@ internal static IList<string> MatchDirectories(string pattern)
                             pathRegex.Append(singleCharRegex);
                     }
                     partStart = wildIndex + 1;
-                }       
+                }
             }
             pathRegex.Append("$");
 
             var searchRegex = new Regex(pathRegex.ToString(), RegexOptions.IgnoreCase);
             var dirs = Directory.GetDirectories(rootPath, "*", SearchOption.AllDirectories);
-            
+
             directories.AddRange(dirs.Where(dir => searchRegex.IsMatch(dir)));
 
             return directories;
@@ -204,7 +311,7 @@ private static string FindRootPath(string[] parts, out int pathIndex)
                 .ToList();
 
             var rootPath = root.Any()
-                ? Path.Combine(root.ToArray())
+                ? string.Join(Path.DirectorySeparatorChar.ToString(), root.ToArray()) //dont use Path.Combine(root.ToArray()) here   it doesnt work as intended
                 : Environment.CurrentDirectory;
 
             if (!Path.IsPathRooted(rootPath))