Merge #3938 Improve file deletion error while the game is running

CHANGELOG.md

@@ -15,6 +15,7 @@ All notable changes to this project will be documented in this file.
 - [Multiple] Refactor repository and available module handling (#3904, #3907, #3908, #3935, #3937 by: HebaruSan; reviewed: techman83)
 - [Multiple] Parallelize for performance, relationship resolver improvements (#3917 by: HebaruSan; reviewed: techman83)
 - [Multiple] Modernize administrator and Mono version checks (#3933 by: HebaruSan; reviewed: techman83)
+- [Multiple] Improve file deletion error while the game is running (#3938 by: HebaruSan; reviewed: techman83)
 
 ### Bugfixes
 

ConsoleUI/Toolkit/ConsoleButton.cs

@@ -72,7 +72,7 @@ public override void OnKeyPress(ConsoleKeyInfo k)
         {
             switch (k.Key) {
                 case ConsoleKey.Tab:
-                    Blur((k.Modifiers & ConsoleModifiers.Shift) == 0);
+                    Blur(!k.Modifiers.HasFlag(ConsoleModifiers.Shift));
                     break;
                 case ConsoleKey.Spacebar:
                 case ConsoleKey.Enter:

ConsoleUI/Toolkit/ConsoleField.cs

@@ -107,7 +107,7 @@ public override void OnKeyPress(ConsoleKeyInfo k)
                     }
                     break;
                 case ConsoleKey.Backspace:
-                    if ((k.Modifiers & ConsoleModifiers.Control) == 0) {
+                    if (!k.Modifiers.HasFlag(ConsoleModifiers.Control)) {
                         if (Position > 0) {
                             --Position;
                             Value = Value.Substring(0, Position) + Value.Substring(Position + 1);
@@ -121,9 +121,9 @@ public override void OnKeyPress(ConsoleKeyInfo k)
                     break;
                 case ConsoleKey.Delete:
                     if (Position < Value.Length) {
-                        Value = (k.Modifiers & ConsoleModifiers.Control) == 0
-                            ? Value.Substring(0, Position) + Value.Substring(Position + 1)
-                            : Value.Substring(0, Position);
+                        Value = k.Modifiers.HasFlag(ConsoleModifiers.Control)
+                            ? Value.Substring(0, Position)
+                            : Value.Substring(0, Position) + Value.Substring(Position + 1);
                         Changed();
                     }
                     break;
@@ -150,7 +150,7 @@ public override void OnKeyPress(ConsoleKeyInfo k)
                     Position = Value.Length;
                     break;
                 case ConsoleKey.Tab:
-                    Blur((k.Modifiers & ConsoleModifiers.Shift) == 0);
+                    Blur(!k.Modifiers.HasFlag(ConsoleModifiers.Shift));
                     break;
                 default:
                     if (!char.IsControl(k.KeyChar)) {

ConsoleUI/Toolkit/ConsoleFileMultiSelectDialog.cs

@@ -196,9 +196,7 @@ private IList<FileSystemInfo> getFileList()
         }
 
         private static bool isDir(FileSystemInfo fi)
-        {
-            return (fi.Attributes & FileAttributes.Directory) == FileAttributes.Directory;
-        }
+            => fi.Attributes.HasFlag(FileAttributes.Directory);
 
         /// <summary>
         /// Check whether two file system references are equal.

ConsoleUI/Toolkit/ConsoleListBox.cs

@@ -242,14 +242,14 @@ public override void OnKeyPress(ConsoleKeyInfo k)
                     selectedRow = sortedFilteredData.Count - 1;
                     break;
                 case ConsoleKey.Tab:
-                    Blur((k.Modifiers & ConsoleModifiers.Shift) == 0);
+                    Blur(!k.Modifiers.HasFlag(ConsoleModifiers.Shift));
                     break;
                 default:
-                    // Go backwards if (k.Modifiers & ConsoleModifiers.Shift)
+                    // Go backwards if k.Modifiers.HasFlag(ConsoleModifiers.Shift)
                     if (!char.IsControl(k.KeyChar)
                             && (k.Modifiers | ConsoleModifiers.Shift) == ConsoleModifiers.Shift) {
 
-                        bool forward = (k.Modifiers & ConsoleModifiers.Shift) == 0;
+                        bool forward = !k.Modifiers.HasFlag(ConsoleModifiers.Shift);
                         Func<RowT, string> dfltRend = columns[defaultSortColumn].Renderer;
                         // Find first row after current, wrap
                         int startRow = forward

Core/Extensions/EnumExtensions.cs

@@ -0,0 +1,13 @@
+using System;
+using System.Linq;
+
+namespace CKAN.Extensions
+{
+    public static class EnumExtensions
+    {
+
+        public static bool HasAnyFlag(this Enum val, params Enum[] flags)
+            => flags.Any(f => val.HasFlag(f));
+
+    }
+}

Core/ModuleInstaller.cs

@@ -721,20 +721,20 @@ public void UninstallList(
         /// Use UninstallList for user queries, it also does dependency handling.
         /// This does *NOT* save the registry.
         /// </summary>
-        /// <param name="modName">Identifier of module to uninstall</param>
+        /// <param name="identifier">Identifier of module to uninstall</param>
         /// <param name="possibleConfigOnlyDirs">Directories that the user might want to remove after uninstall</param>
-        private void Uninstall(string modName, ref HashSet<string> possibleConfigOnlyDirs, Registry registry)
+        private void Uninstall(string identifier, ref HashSet<string> possibleConfigOnlyDirs, Registry registry)
         {
             TxFileManager file_transaction = new TxFileManager();
 
             using (var transaction = CkanTransaction.CreateTransactionScope())
             {
-                InstalledModule mod = registry.InstalledModule(modName);
+                InstalledModule mod = registry.InstalledModule(identifier);
 
                 if (mod == null)
                 {
-                    log.ErrorFormat("Trying to uninstall {0} but it's not installed", modName);
-                    throw new ModNotInstalledKraken(modName);
+                    log.ErrorFormat("Trying to uninstall {0} but it's not installed", identifier);
+                    throw new ModNotInstalledKraken(identifier);
                 }
 
                 // Walk our registry to find all files for this mod.
@@ -745,6 +745,9 @@ private void Uninstall(string modName, ref HashSet<string> possibleConfigOnlyDir
                     ? new HashSet<string>(StringComparer.OrdinalIgnoreCase)
                     : new HashSet<string>();
 
+                // Files that Windows refused to delete due to locking (probably)
+                var undeletableFiles = new List<string>();
+
                 foreach (string file in files)
                 {
                     string path = ksp.ToAbsoluteGameDir(file);
@@ -762,7 +765,7 @@ private void Uninstall(string modName, ref HashSet<string> possibleConfigOnlyDir
                         // explanation. – Kyle Trauberman Aug 30 '12 at 21:28
                         // (https://stackoverflow.com/questions/1395205/better-way-to-check-if-path-is-a-file-or-a-directory)
                         // This is the fastest way to do this test.
-                        if ((attr & FileAttributes.Directory) == FileAttributes.Directory)
+                        if (attr.HasFlag(FileAttributes.Directory))
                         {
                             if (!directoriesToDelete.Contains(path))
                             {
@@ -785,17 +788,32 @@ private void Uninstall(string modName, ref HashSet<string> possibleConfigOnlyDir
                             file_transaction.Delete(path);
                         }
                     }
-                    catch (Exception ex)
+                    catch (IOException)
+                    {
+                        // "The specified file is in use."
+                        undeletableFiles.Add(file);
+                    }
+                    catch (UnauthorizedAccessException)
+                    {
+                        // "The caller does not have the required permission."
+                        // "The file is an executable file that is in use."
+                        undeletableFiles.Add(file);
+                    }
+                    catch (Exception exc)
                     {
-                        // XXX: This is terrible, we're catching all exceptions.
                         // We don't consider this problem serious enough to abort and revert,
                         // so treat it as a "--verbose" level log message.
-                        log.InfoFormat("Failure in locating file {0} : {1}", path, ex.Message);
+                        log.InfoFormat("Failure in locating file {0}: {1}", path, exc.Message);
                     }
                 }
 
+                if (undeletableFiles.Count > 0)
+                {
+                    throw new FailedToDeleteFilesKraken(identifier, undeletableFiles);
+                }
+
                 // Remove from registry.
-                registry.DeregisterModule(ksp, modName);
+                registry.DeregisterModule(ksp, identifier);
 
                 // Our collection of directories may leave empty parent directories.
                 directoriesToDelete = AddParentDirectories(directoriesToDelete);
@@ -877,7 +895,7 @@ private void Uninstall(string modName, ref HashSet<string> possibleConfigOnlyDir
                         log.InfoFormat("Not removing directory {0}, it's not empty", directory);
                     }
                 }
-                log.InfoFormat("Removed {0}", modName);
+                log.InfoFormat("Removed {0}", identifier);
                 transaction.Complete();
             }
         }

Core/Properties/Resources.resx

@@ -153,7 +153,7 @@ Install the `mono-complete` package or equivalent for your operating system.</va
   <data name="NetRepoLoadedDownloadCounts" xml:space="preserve"><value>Loaded download counts from {0} repository</value></data>
   <data name="JsonRelationshipConverterAnyOfCombined" xml:space="preserve"><value>`any_of` should not be combined with `{0}`</value></data>
   <data name="RegistryFileConflict" xml:space="preserve"><value>{0} wishes to install {1}, but this file is registered to {2}</value></data>
-  <data name="RegistryFileNotRemoved" xml:space="preserve"><value>{0} is registered to {1} but has not been removed!</value></data>
+  <data name="RegistryFileNotRemoved" xml:space="preserve"><value>Error unregistering {1}, file not removed: {0}</value></data>
   <data name="RegistryDefaultDLCAbstract" xml:space="preserve"><value>An official expansion pack</value></data>
   <data name="RegistryManagerDirectoryNotFound" xml:space="preserve"><value>Can't find a directory in {0}</value></data>
   <data name="RegistryManagerExportFilenamePrefix" xml:space="preserve"><value>installed</value></data>
@@ -263,6 +263,11 @@ Please remove manually before trying to install it.</value></data>
 
 Which {0} provider would you like to install?</value></data>
   <data name="KrakenInconsistenciesHeader" xml:space="preserve"><value>Inconsistencies were found:</value></data>
+  <data name="KrakenFailedToDeleteFiles" xml:space="preserve"><value>Unable to overwrite or delete files for {0}:
+
+{1}
+
+If the game is still running, close it and try again. Otherwise check the permissions.</value></data>
   <data name="KrakenMissingDependency" xml:space="preserve"><value>{0} missing dependency {1}</value></data>
   <data name="KrakenConflictsWith" xml:space="preserve"><value>{0} conflicts with {1}</value></data>
   <data name="KrakenDownloadErrorsHeader" xml:space="preserve"><value>Uh oh, the following things went wrong when downloading...</value></data>

Core/Registry/Registry.cs

@@ -892,24 +892,20 @@ public void RegisterModule(CkanModule          mod,
         ///
         /// Throws an InconsistentKraken if not all files have been removed.
         /// </summary>
-        public void DeregisterModule(GameInstance inst, string module)
+        public void DeregisterModule(GameInstance inst, string identifier)
         {
-            log.DebugFormat("Deregistering module {0}", module);
+            log.DebugFormat("Deregistering module {0}", identifier);
             EnlistWithTransaction();
 
-            var inconsistencies = new List<string>();
-
-            var absolute_files = installed_modules[module].Files.Select(inst.ToAbsoluteGameDir);
             // Note, this checks to see if a *file* exists; it doesn't
             // trigger on directories, which we allow to still be present
             // (they may be shared by multiple mods.
-
-            foreach (var absolute_file in absolute_files.Where(File.Exists))
-            {
-                inconsistencies.Add(string.Format(
-                    Properties.Resources.RegistryFileNotRemoved,
-                    absolute_file, module));
-            }
+            var inconsistencies = installed_modules[identifier].Files
+                .Where(f => File.Exists(inst.ToAbsoluteGameDir(f)))
+                .Select(relPath => string.Format(
+                                       Properties.Resources.RegistryFileNotRemoved,
+                                       relPath, identifier))
+                .ToList();
 
             if (inconsistencies.Count > 0)
             {
@@ -918,13 +914,13 @@ public void DeregisterModule(GameInstance inst, string module)
             }
 
             // Okay, all the files are gone. Let's clear our metadata.
-            foreach (string rel_file in installed_modules[module].Files)
+            foreach (string rel_file in installed_modules[identifier].Files)
             {
                 installed_files.Remove(rel_file);
             }
 
             // Bye bye, module, it's been nice having you visit.
-            installed_modules.Remove(module);
+            installed_modules.Remove(identifier);
 
             // Installing and uninstalling mods can change compatibility due to conflicts,
             // so we'll need to reset the compatibility sorter

Core/Types/Kraken.cs

@@ -4,6 +4,8 @@
 using System.Text;
 using System.Collections.Generic;
 
+using CKAN.Versioning;
+
 namespace CKAN
 {
     using modRelList = List<Tuple<CkanModule, RelationshipDescriptor, CkanModule>>;
@@ -225,6 +227,20 @@ public override string ToString()
         private readonly ICollection<string> inconsistencies;
     }
 
+    public class FailedToDeleteFilesKraken : Kraken
+    {
+        public FailedToDeleteFilesKraken(string identifier, List<string> undeletableFiles)
+            : base(string.Format(Properties.Resources.KrakenFailedToDeleteFiles,
+                                 identifier,
+                                 string.Join(Environment.NewLine,
+                                             undeletableFiles.Select(f => f.Replace('/', Path.DirectorySeparatorChar)))))
+        {
+            this.undeletableFiles = undeletableFiles;
+        }
+
+        public readonly List<string> undeletableFiles;
+    }
+
     /// <summary>
     /// A mutation of InconsistentKraken that allows catching code to extract the causes of the errors.
     /// </summary>
@@ -512,9 +528,9 @@ public BadGameVersionKraken(string reason = null, Exception inner_exception = nu
     /// </summary>
     public class WrongGameVersionKraken : Kraken
     {
-        public readonly Versioning.GameVersion version;
+        public readonly GameVersion version;
 
-        public WrongGameVersionKraken(Versioning.GameVersion version, string reason = null, Exception inner_exception = null)
+        public WrongGameVersionKraken(GameVersion version, string reason = null, Exception inner_exception = null)
             : base(reason, inner_exception)
         {
             this.version = version;

GUI/Controls/Changeset.cs

@@ -30,8 +30,17 @@ public void LoadChangeset(
             alertLabels    = AlertLabels;
             this.conflicts = conflicts;
             ConfirmChangesButton.Enabled = conflicts == null || !conflicts.Any();
+            CloseTheGameLabel.Visible = changeset != null
+                && changeset.Any(ch => DeletingChanges.Contains(ch.ChangeType));
         }
 
+        private static readonly HashSet<GUIModChangeType> DeletingChanges = new HashSet<GUIModChangeType>
+        {
+            GUIModChangeType.Remove,
+            GUIModChangeType.Update,
+            GUIModChangeType.Replace,
+        };
+
         protected override void OnVisibleChanged(EventArgs e)
         {
             base.OnVisibleChanged(e);

GUI/Controls/Changeset.resx

@@ -120,6 +120,7 @@
   <data name="Mod.Text" xml:space="preserve"><value>Mod</value></data>
   <data name="ChangeType.Text" xml:space="preserve"><value>Change</value></data>
   <data name="Reason.Text" xml:space="preserve"><value>Reasons for action</value></data>
+  <data name="CloseTheGameLabel.Text" xml:space="preserve"><value>Note: To apply these changes, CKAN must overwrite or delete files. If the game is running, close it before continuing!</value></data>
   <data name="BackButton.Text" xml:space="preserve"><value>Back</value></data>
   <data name="CancelChangesButton.Text" xml:space="preserve"><value>Clear</value></data>
   <data name="ConfirmChangesButton.Text" xml:space="preserve"><value>Apply</value></data>