Merge #3890 Improve handling of unregistered files at uninstall

CHANGELOG.md

@@ -8,6 +8,7 @@ All notable changes to this project will be documented in this file.
 
 - [Multiple] Support multiple download URLs per module (#3877 by: HebaruSan; reviewed: techman83)
 - [Multiple] French translation updates from Crowdin (#3879 by: vinix38; reviewed: HebaruSan)
+- [Multiple] Improve handling of unregistered files at uninstall (#3890 by: HebaruSan; reviewed: techman83)
 
 ### Bugfixes
 

Core/Extensions/EnumerableExtensions.cs

@@ -87,6 +87,19 @@ public static IEnumerable<V> ZipMany<T, U, V>(this IEnumerable<T> seq1, IEnumera
         public static IEnumerable<T> DistinctBy<T, U>(this IEnumerable<T> seq, Func<T, U> func)
             => seq.GroupBy(func).Select(grp => grp.First());
 
+        /// <summary>
+        /// Generate a sequence from a linked list
+        /// </summary>
+        /// <param name="start">The first node</param>
+        /// <param name="getNext">Function to go from one node to the next</param>
+        /// <returns>All the nodes in the list as a sequence</returns>
+        public static IEnumerable<T> TraverseNodes<T>(this T start, Func<T, T> getNext)
+        {
+            for (T t = start; t != null; t = getNext(t))
+            {
+                yield return t;
+            }
+        }
     }
 
     /// <summary>

Core/Games/IGame.cs

@@ -20,9 +20,10 @@ public interface IGame
         string   PrimaryModDirectoryRelative     { get; }
         string[] AlternateModDirectoriesRelative { get; }
         string   PrimaryModDirectory(GameInstance inst);
-        string[] StockFolders   { get; }
-        string[] ReservedPaths  { get; }
-        string[] CreateableDirs { get; }
+        string[] StockFolders      { get; }
+        string[] ReservedPaths     { get; }
+        string[] CreateableDirs    { get; }
+        string[] AutoRemovableDirs { get; }
         bool     IsReservedDirectory(GameInstance inst, string path);
         bool     AllowInstallationIn(string name, out string path);
         void     RebuildSubdirectories(string absGameRoot);
@@ -40,5 +41,4 @@ public interface IGame
         Uri DefaultRepositoryURL { get; }
         Uri RepositoryListURL    { get; }
     }
-
 }

Core/Games/KerbalSpaceProgram.cs

@@ -120,6 +120,11 @@ public string PrimaryModDirectory(GameInstance inst)
             "GameData", "Tutorial", "Scenarios", "Missions", "Ships/Script"
         };
 
+        public string[] AutoRemovableDirs => new string[]
+        {
+            "@thumbs"
+        };
+
         /// <summary>
         /// Checks the path against a list of reserved game directories
         /// </summary>

Core/Games/KerbalSpaceProgram2.cs

@@ -122,6 +122,8 @@ public string PrimaryModDirectory(GameInstance inst)
             "BepInEx/plugins",
         };
 
+        public string[] AutoRemovableDirs => new string[] { };
+
         /// <summary>
         /// Checks the path against a list of reserved game directories
         /// </summary>

Core/ModuleInstaller.cs

@@ -14,6 +14,7 @@
 using CKAN.Extensions;
 using CKAN.Versioning;
 using CKAN.Configuration;
+using CKAN.Games;
 
 namespace CKAN
 {
@@ -762,7 +763,7 @@ private void Uninstall(string modName, ref HashSet<string> possibleConfigOnlyDir
                 }
 
                 // Walk our registry to find all files for this mod.
-                IEnumerable<string> files = mod.Files;
+                var files = mod.Files.ToArray();
 
                 // We need case insensitive path matching on Windows
                 var directoriesToDelete = Platform.IsWindows
@@ -826,7 +827,7 @@ private void Uninstall(string modName, ref HashSet<string> possibleConfigOnlyDir
 
                 // Sort our directories from longest to shortest, to make sure we remove child directories
                 // before parents. GH #78.
-                foreach (string directory in directoriesToDelete.OrderBy(dir => dir.Length).Reverse())
+                foreach (string directory in directoriesToDelete.OrderByDescending(dir => dir.Length))
                 {
                     log.DebugFormat("Checking {0}...", directory);
                     // It is bad if any of this directories gets removed
@@ -838,16 +839,42 @@ private void Uninstall(string modName, ref HashSet<string> possibleConfigOnlyDir
                         continue;
                     }
 
-                    var contents = Directory
-                        .EnumerateFileSystemEntries(directory, "*", SearchOption.AllDirectories)
-                        .Select(f => ksp.ToRelativeGameDir(f))
-                        .Memoize();
-                    log.DebugFormat("Got contents: {0}", string.Join(", ", contents));
-                    var owners = contents.Select(f => registry.FileOwner(f));
-                    log.DebugFormat("Got owners: {0}", string.Join(", ", owners));
-                    if (!contents.Any())
+                    // See what's left in this folder and what we can do about it
+                    GroupFilesByRemovable(ksp.ToRelativeGameDir(directory),
+                                          registry, files, ksp.game,
+                                          (Directory.Exists(directory)
+                                              ? Directory.EnumerateFileSystemEntries(directory, "*", SearchOption.AllDirectories)
+                                              : Enumerable.Empty<string>())
+                                           .Select(f => ksp.ToRelativeGameDir(f)),
+                                          out string[] removable,
+                                          out string[] notRemovable);
+
+                    // Delete the auto-removable files and dirs
+                    foreach (var relPath in removable)
                     {
+                        var absPath = ksp.ToAbsoluteGameDir(relPath);
+                        if (File.Exists(absPath))
+                        {
+                            log.DebugFormat("Attempting transaction deletion of file {0}", absPath);
+                            file_transaction.Delete(absPath);
+                        }
+                        else if (Directory.Exists(absPath))
+                        {
+                            log.DebugFormat("Attempting deletion of directory {0}", absPath);
+                            try
+                            {
+                                Directory.Delete(absPath);
+                            }
+                            catch
+                            {
+                                // There might be files owned by other mods, oh well
+                                log.DebugFormat("Failed to delete {0}", absPath);
+                            }
+                        }
+                    }
 
+                    if (!notRemovable.Any())
+                    {
                         // We *don't* use our file_transaction to delete files here, because
                         // it fails if the system's temp directory is on a different device
                         // to KSP. However we *can* safely delete it now we know it's empty,
@@ -860,9 +887,10 @@ private void Uninstall(string modName, ref HashSet<string> possibleConfigOnlyDir
                         log.DebugFormat("Removing {0}", directory);
                         Directory.Delete(directory);
                     }
-                    else if (contents.All(f => registry.FileOwner(f) == null))
+                    else if (notRemovable.All(f => registry.FileOwner(f) == null && !files.Contains(f)))
                     {
-                        log.DebugFormat("Directory {0} contains only non-registered files, ask user about it later", directory);
+                        log.DebugFormat("Directory {0} contains only non-registered files, ask user about it later: {1}",
+                                        directory, string.Join(", ", notRemovable));
                         if (possibleConfigOnlyDirs == null)
                         {
                             possibleConfigOnlyDirs = new HashSet<string>();
@@ -879,6 +907,35 @@ private void Uninstall(string modName, ref HashSet<string> possibleConfigOnlyDir
             }
         }
 
+        internal static void GroupFilesByRemovable(string relRoot,
+                                                   Registry registry,
+                                                   string[] alreadyRemoving,
+                                                   IGame game,
+                                                   IEnumerable<string> relPaths,
+                                                   out string[] removable,
+                                                   out string[] notRemovable)
+        {
+            log.DebugFormat("Getting contents of {0}", relRoot);
+            var contents = relPaths
+                // Split into auto-removable and not-removable
+                // Removable must not be owned by other mods
+                .GroupBy(f => registry.FileOwner(f) == null
+                              // Also skip owned by this module since it's already deregistered
+                              && !alreadyRemoving.Contains(f)
+                              // Must have a removable dir name somewhere in path AFTER main dir
+                              && f.Substring(relRoot.Length)
+                                  .Split('/')
+                                  .Where(piece => !string.IsNullOrEmpty(piece))
+                                  .Any(piece => game.AutoRemovableDirs.Contains(piece)))
+                .ToDictionary(grp => grp.Key,
+                              grp => grp.OrderByDescending(f => f.Length)
+                                        .ToArray());
+            removable    = contents.TryGetValue(true,  out string[] val1) ? val1 : new string[] {};
+            notRemovable = contents.TryGetValue(false, out string[] val2) ? val2 : new string[] {};
+            log.DebugFormat("Got removable: {0}",    string.Join(", ", removable));
+            log.DebugFormat("Got notRemovable: {0}", string.Join(", ", notRemovable));
+        }
+
         /// <summary>
         /// Takes a collection of directories and adds all parent directories within the GameData structure.
         /// </summary>

GUI/Controls/DeleteDirectories.cs

@@ -34,6 +34,7 @@ public void LoadDirs(GameInstance ksp, HashSet<string> possibleConfigOnlyDirs)
                 .ToArray();
             Util.Invoke(this, () =>
             {
+                DeleteButton.Focus();
                 DirectoriesListView.Items.Clear();
                 DirectoriesListView.Items.AddRange(items);
                 DirectoriesListView.AutoResizeColumns(ColumnHeaderAutoResizeStyle.ColumnContent);
@@ -79,6 +80,12 @@ public bool Wait(out HashSet<string> toDelete)
             }
         }
 
+        protected override void OnResize(EventArgs e)
+        {
+            base.OnResize(e);
+            ExplanationLabel.Height = Util.LabelStringHeight(CreateGraphics(), ExplanationLabel);
+        }
+
         /// <summary>
         /// Open the user guide when the user presses F1
         /// </summary>

GUI/Controls/DeleteDirectories.resx

@@ -117,8 +117,7 @@
   <resheader name="writer">
     <value>System.Resources.ResXResourceWriter, System.Windows.Forms, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089</value>
   </resheader>
-  <data name="ExplanationLabel.Text" xml:space="preserve"><value>The below directories are leftover after removing some mods. They contain files that were not installed by CKAN (probably either generated by a mod or manually installed). CKAN does not automatically delete files it did not install, but you can choose to remove them if it looks safe to do so (recommended).
-Note that if you decide not to remove a directory, ModuleManager may incorrectly think that mod is still installed.</value></data>
+  <data name="ExplanationLabel.Text" xml:space="preserve"><value>Warning, some folders have been left behind because CKAN does not know whether it is safe to delete their remaining files. Keeping these folders may break other mods! If you do not need these files, deleting them is recommended.</value></data>
   <data name="DirectoryColumn.Text" xml:space="preserve"><value>Directories</value></data>
   <data name="FileColumn.Text" xml:space="preserve"><value>Directory Contents</value></data>
   <data name="SelectDirPrompt.Text" xml:space="preserve"><value>Click a directory at the left to see its contents</value></data>

GUI/Controls/EditModSearch.cs

@@ -143,7 +143,6 @@ private bool SkipDelayIf(object sender, EventArgs e)
                         default:
                             return false;
                     }
-                    break;
             }
             // Always refresh immediately on clear
             return string.IsNullOrEmpty(FilterCombinedTextBox.Text)

GUI/Controls/ManageMods.cs

@@ -502,7 +502,7 @@ private void MarkAllUpdatesToolButton_Click(object sender, EventArgs e)
 
         private void ApplyToolButton_Click(object sender, EventArgs e)
         {
-            Main.Instance.tabController.ShowTab("ChangesetTabPage", 1);
+            StartChangeSet?.Invoke(currentChangeSet);
         }
 
         public void MarkModForUpdate(string identifier, bool value)
@@ -759,7 +759,7 @@ private void ModGrid_CellMouseDoubleClick(object sender, DataGridViewCellMouseEv
             ModGrid.CommitEdit(DataGridViewDataErrorContexts.Commit);
         }
 
-        private async void ModGrid_CellValueChanged(object sender, DataGridViewCellEventArgs e)
+        private void ModGrid_CellValueChanged(object sender, DataGridViewCellEventArgs e)
         {
             int row_index    = e.RowIndex;
             int column_index = e.ColumnIndex;
@@ -804,10 +804,9 @@ private async void ModGrid_CellValueChanged(object sender, DataGridViewCellEvent
                             gui_mod.SetReplaceChecked(row, ReplaceCol);
                             break;
                     }
-                    await UpdateChangeSetAndConflicts(
+                    UpdateChangeSetAndConflicts(
                         Main.Instance.CurrentInstance,
-                        RegistryManager.Instance(Main.Instance.CurrentInstance).registry
-                    );
+                        RegistryManager.Instance(Main.Instance.CurrentInstance).registry);
                 }
             }
         }
@@ -1227,8 +1226,7 @@ private void _UpdateModsList(Dictionary<string, bool> old_modules = null)
             // Update our mod listing
             mainModList.ConstructModList(gui_mods, Main.Instance.CurrentInstance.Name, Main.Instance.CurrentInstance.game, ChangeSet);
 
-            // C# 7.0: Executes the task and discards it
-            _ = UpdateChangeSetAndConflicts(Main.Instance.CurrentInstance, registry);
+            UpdateChangeSetAndConflicts(Main.Instance.CurrentInstance, registry);
 
             Main.Instance.Wait.AddLogMessage(Properties.Resources.MainModListUpdatingFilters);
 
@@ -1651,7 +1649,7 @@ public void InstanceUpdated(GameInstance inst)
             Conflicts = null;
         }
 
-        public async Task UpdateChangeSetAndConflicts(GameInstance inst, IRegistryQuerier registry)
+        public void UpdateChangeSetAndConflicts(GameInstance inst, IRegistryQuerier registry)
         {
             if (freezeChangeSet)
             {
@@ -1662,7 +1660,6 @@ public async Task UpdateChangeSetAndConflicts(GameInstance inst, IRegistryQuerie
             List<ModChange> full_change_set = null;
             Dictionary<GUIMod, string> new_conflicts = null;
 
-            bool too_many_provides_thrown = false;
             var user_change_set = mainModList.ComputeUserChangeSet(registry, inst.VersionCriteria());
             try
             {

GUI/Controls/ModInfo.cs

@@ -44,7 +44,7 @@ public GUIMod SelectedModule
 
         public void RefreshModContentsTree()
         {
-            Contents.Refresh();
+            Contents.RefreshModContentsTree();
         }
 
         public event Action<GUIMod>            OnDownloadClick;
@@ -96,13 +96,10 @@ private void ModInfoTabControl_SelectedIndexChanged(object sender, EventArgs e)
 
         private GameInstanceManager manager => Main.Instance.Manager;
 
-        private int StringHeight(string text, Font font, int maxWidth)
-            => (int)CreateGraphics().MeasureString(text, font, maxWidth).Height;
-
         private int TextBoxStringHeight(TextBox tb)
             => tb.Padding.Vertical + tb.Margin.Vertical
-                + StringHeight(tb.Text, tb.Font,
-                               tb.Width - tb.Padding.Horizontal - tb.Margin.Horizontal);
+                + Util.StringHeight(CreateGraphics(), tb.Text, tb.Font,
+                                    tb.Width - tb.Padding.Horizontal - tb.Margin.Horizontal);
 
         private int DescriptionHeight => TextBoxStringHeight(MetadataModuleDescriptionTextBox);
 

GUI/Controls/ModInfoTabs/Contents.cs

@@ -29,7 +29,7 @@ public GUIMod SelectedModule
             get => selectedModule;
         }
 
-        public void Refresh()
+        public void RefreshModContentsTree()
         {
             if (currentModContentsModule != null)
             {

GUI/Controls/ModInfoTabs/Metadata.cs

@@ -139,16 +139,9 @@ private void LinkLabel_KeyDown(object sender, KeyEventArgs e)
             }
         }
 
-        private int StringHeight(string text, Font font, int maxWidth)
-            => (int)CreateGraphics().MeasureString(text, font, maxWidth).Height;
-
         private int LinkLabelStringHeight(LinkLabel lb, int fitWidth)
             => lb.Padding.Vertical + lb.Margin.Vertical + 10
-                + StringHeight(lb.Text, lb.Font, fitWidth);
-
-        private int LabelStringHeight(Label lb)
-            => lb.Padding.Vertical + lb.Margin.Vertical + 10
-                + StringHeight(lb.Text, lb.Font, lb.Width);
+                + Util.StringHeight(CreateGraphics(), lb.Text, lb.Font, fitWidth);
 
         protected override void OnResize(EventArgs e)
         {
@@ -212,7 +205,7 @@ private void AddResourceLink(string label, Uri link)
                 MetadataTable.RowStyles.Add(
                     new RowStyle(SizeType.Absolute, Math.Max(
                         // "Remote version file" wraps
-                        LabelStringHeight(lbl),
+                        Util.LabelStringHeight(CreateGraphics(), lbl),
                         LinkLabelStringHeight(llbl, RightColumnWidth))));
             }
         }
@@ -229,7 +222,7 @@ private void ResizeResourceRows()
                     {
                         MetadataTable.RowStyles[row].Height = Math.Max(
                             // "Remote version file" wraps
-                            LabelStringHeight(lab),
+                            Util.LabelStringHeight(CreateGraphics(), lab),
                             LinkLabelStringHeight(link, rWidth));
                     }
                 }