KSP-CKAN · Apr 8, 2018 · Apr 11, 2018
Showing with 73 additions and 29 deletions.

+1 −0 CHANGELOG.md

+3 −6 Core/Net/Repo.cs

+26 −7 Core/Platform.cs

+24 −5 Core/Registry/RegistryManager.cs

+3 −3 GUI/Main.cs

+15 −7 GUI/MainModList.cs

+1 −1 debian/control.in
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -31,6 +31,7 @@ All notable changes to this project will be documented in this file.
 - [Multiple] Deal with threading and download errors (#2374 by: HebaruSan; reviewed: politas)
 - [GUI] More verbose and accurate version displays (#2382 by: HebaruSan; reviewed: politas)
 - [Core] Encode spaces in URL output (#2386 by: HebaruSan; reviewed: politas)
+- [Multiple] Clean-up and debuggability (#2399 by: HebaruSan; reviewed: politas)
 
 ### Internal
 

diff --git a/Core/Net/Repo.cs b/Core/Net/Repo.cs
@@ -23,9 +23,6 @@ public static class Repo
         private static readonly ILog log = LogManager.GetLogger(typeof (Repo));
         private static TxFileManager file_transaction = new TxFileManager();
 
-        // Forward to keep existing code compiling, will be removed soon.
-        public static readonly Uri default_ckan_repo = CKAN.Repository.default_ckan_repo_uri;
-
         /// <summary>
         /// Download and update the local CKAN meta-info.
         /// Optionally takes a URL to the zipfile repo to download.
@@ -92,9 +89,9 @@ private static List<CkanModule> UpdateRegistry(Uri repo, KSP ksp, IUser user)
             {
                 repo_file = Net.Download(repo);
             }
-            catch (System.Net.WebException)
+            catch (System.Net.WebException ex)
             {
-                user.RaiseMessage("Connection to {0} could not be established.", repo);
+                user.RaiseMessage("Failed to download {0}: {1}", repo, ex.ToString());
                 return null;
             }
 
@@ -386,7 +383,7 @@ public static int Update(RegistryManager registry_manager, KSP ksp, IUser user,
             // Use our default repo, unless we've been told otherwise.
             if (repo == null)
             {
-                repo = default_ckan_repo;
+                repo = CKAN.Repository.default_ckan_repo_uri;
             }
 
             List<CkanModule> newAvail = UpdateRegistry(repo, ksp, user);

diff --git a/Core/Platform.cs b/Core/Platform.cs
@@ -20,7 +20,8 @@ static Platform()
         {
             IsMac = IsRunningOnMac();
             IsMono = IsOnMono();
-            IsMonoFour = IsOnMonoFour();//Depends on IsMono
+            // Depends on IsMono
+            IsMonoFourOrLater = IsOnMonoFourOrLater();
         }
 
         // From https://github.com/mono/monodevelop/blob/master/main/src/core/Mono.Texteditor/Mono.TextEditor/Platform.cs
@@ -93,17 +94,35 @@ private static bool IsOnMono()
         }
 
         /// <summary>
-        /// Are we running on a mono with major version 4?
+        /// Are we running on a Mono with major version 4 or later?
         /// </summary>
-        public static bool IsMonoFour { get; private set; }
+        public static bool IsMonoFourOrLater { get; private set; }
 
-        private static bool IsOnMonoFour()
+        private static bool IsOnMonoFourOrLater()
         {
-            if (!IsMono) return false;
+            if (!IsMono)
+                return false;
+
+            // Get Mono's display name and parse the version
             Type type = Type.GetType("Mono.Runtime");
             string display_name =
                 (string) type.GetMethod("GetDisplayName", BindingFlags.NonPublic | BindingFlags.Static).Invoke(null, null);
-            return Regex.IsMatch(display_name, "^\\s*4\\.\\d+\\.\\d+\\s*\\(");
+
+            var match = versionMatcher.Match(display_name);
+            if (match.Success)
+            {
+                int majorVersion = Int32.Parse(match.Groups["majorVersion"].Value);
+                return majorVersion >= 4;
+            }
+            else
+            {
+                return false;
+            }
         }
+
+        private static readonly Regex versionMatcher = new Regex(
+            "^\\s*(?<majorVersion>\\d+)\\.\\d+\\.\\d+\\s*\\(",
+            RegexOptions.Compiled
+        );
     }
-}
+}
diff --git a/Core/Registry/RegistryManager.cs b/Core/Registry/RegistryManager.cs
@@ -40,12 +40,13 @@ private RegistryManager(string path, KSP ksp)
         {
             this.ksp = ksp;
 
-            this.path = Path.Combine(path, "registry.json");
+            this.path    = Path.Combine(path, "registry.json");
             lockfilePath = Path.Combine(path, "registry.locked");
 
             // Create a lock for this registry, so we cannot touch it again.
             if (!GetLock())
             {
+                log.DebugFormat("Unable to acquire registry lock: {0}", lockfilePath);
                 throw new RegistryInUseKraken(lockfilePath);
             }
 
@@ -101,6 +102,7 @@ protected void Dispose(bool safeToAlsoFreeManagedObjects)
             var directory = ksp.CkanDir();
             if (!registryCache.ContainsKey(directory))
             {
+                log.DebugFormat("Registry not in cache at {0}", directory);
                 return;
             }
 
@@ -137,8 +139,10 @@ protected void Dispose(bool safeToAlsoFreeManagedObjects)
         /// </summary>
         private void CheckStaleLock()
         {
+            log.DebugFormat("Checking for stale lock file at {0}", lockfilePath);
             if (File.Exists(lockfilePath))
             {
+                log.DebugFormat("Lock file found at {0}", lockfilePath);
                 string contents;
                 try
                 {
@@ -147,15 +151,18 @@ private void CheckStaleLock()
                 catch
                 {
                     // If we can't read the file, we can't check whether it's stale.
+                    log.DebugFormat("Lock file unreadable at {0}", lockfilePath);
                     return;
                 }
+                log.DebugFormat("Lock file contents: {0}", contents);
                 Int32 pid;
                 if (Int32.TryParse(contents, out pid))
                 {
                     // File contains a valid integer.
                     try
                     {
                         // Try to find the corresponding process.
+                        log.DebugFormat("Looking for process with ID: {0}", pid);
                         Process.GetProcessById(pid);
                         // If no exception is thrown, then a process with this id
                         // is running, and it's not safe to delete the lock file.
@@ -167,6 +174,7 @@ private void CheckStaleLock()
                         // so the lock file is stale and we can delete it.
                         try
                         {
+                            log.DebugFormat("Deleting stale lock file at {0}", lockfilePath);
                             File.Delete(lockfilePath);
                         }
                         catch
@@ -189,16 +197,20 @@ public bool GetLock()
             {
                 CheckStaleLock();
 
+                log.DebugFormat("Trying to create lock file: {0}", lockfilePath);
+
                 lockfileStream = new FileStream(lockfilePath, FileMode.CreateNew, FileAccess.Write, FileShare.None, 512, FileOptions.DeleteOnClose);
 
                 // Write the current process ID to the file.
                 lockfileWriter = new StreamWriter(lockfileStream);
                 lockfileWriter.Write(Process.GetCurrentProcess().Id);
                 lockfileWriter.Flush();
                 // The lock file is now locked and open.
+                log.DebugFormat("Lock file created: {0}", lockfilePath);
             }
             catch (IOException)
             {
+                log.DebugFormat("Failed to create lock file: {0}", lockfilePath);
                 return false;
             }
 
@@ -214,6 +226,7 @@ public void ReleaseLock()
             // it finds the stream is already disposed.
             if (lockfileWriter != null)
             {
+                log.DebugFormat("Disposing of lock file writer at {0}", lockfilePath);
                 lockfileWriter.Dispose();
                 lockfileWriter = null;
             }
@@ -222,11 +235,11 @@ public void ReleaseLock()
             // but we're extra tidy just in case.
             if (lockfileStream != null)
             {
+                log.DebugFormat("Disposing of lock file stream at {0}", lockfilePath);
                 lockfileStream.Dispose();
                 lockfileStream = null;
             }
 
-
         }
 
         /// <summary>
@@ -283,10 +296,11 @@ private void Load()
                     )
             };
 
-
-
+            log.DebugFormat("Trying to load registry from {0}", path);
             string json = File.ReadAllText(path);
+            log.Debug("Registry JSON loaded; parsing...");
             registry = JsonConvert.DeserializeObject<Registry>(json, settings);
+            log.Debug("Registry loaded and parsed");
             ScanDlc();
             log.InfoFormat("Loaded CKAN registry at {0}", path);
         }
@@ -307,14 +321,19 @@ private void LoadOrCreate()
                 Create();
                 Load();
             }
+            catch (Exception ex)
+            {
+                log.ErrorFormat("Uncaught exception loading registry: {0}", ex.ToString());
+                throw;
+            }
 
             AscertainDefaultRepo();
         }
 
         private void Create()
         {
-            registry = Registry.Empty();
             log.InfoFormat("Creating new CKAN registry at {0}", path);
+            registry = Registry.Empty();
             Save();
         }
 

diff --git a/GUI/Main.cs b/GUI/Main.cs
@@ -198,7 +198,7 @@ public Main(string[] cmdlineArgs, GUIUser user, bool showConsole)
             configuration = Configuration.LoadOrCreateConfiguration
                 (
                     Path.Combine(CurrentInstance.CkanDir(), "GUIConfig.xml"),
-                    Repo.default_ckan_repo.ToString()
+                    CKAN.Repository.default_ckan_repo_uri.ToString()
                 );
 
             // Check if there is any other instances already running.
@@ -428,7 +428,7 @@ private void CurrentInstanceUpdated()
             configuration = Configuration.LoadOrCreateConfiguration
             (
                 Path.Combine(CurrentInstance.CkanDir(), "GUIConfig.xml"),
-                Repo.default_ckan_repo.ToString()
+                CKAN.Repository.default_ckan_repo_uri.ToString()
             );
 
             if (CurrentInstance.CompatibleVersionsAreFromDifferentKsp)
@@ -742,7 +742,7 @@ private void pluginsToolStripMenuItem_Click(object sender, EventArgs e)
             Enabled = true;
         }
 
-        private async void installFromckanToolStripMenuItem_Click(object sender, EventArgs e)
+        private void installFromckanToolStripMenuItem_Click(object sender, EventArgs e)
         {
             OpenFileDialog open_file_dialog = new OpenFileDialog { Filter = Resources.CKANFileFilter };
 

diff --git a/GUI/MainModList.cs b/GUI/MainModList.cs
@@ -428,17 +428,25 @@ public class MainModListGUI : DataGridView
     {
         protected override void OnPaint(PaintEventArgs e)
         {
-            //Hacky workaround for https://bugzilla.xamarin.com/show_bug.cgi?id=24372
-            if (Platform.IsMono && !Platform.IsMonoFour)
+            try
             {
-                var first_row_index = typeof (MainModListGUI).BaseType
-                    .GetField("first_row_index", BindingFlags.NonPublic | BindingFlags.Instance);
-                var value = (int) first_row_index.GetValue(this);
-                if (value < 0 || value >= Rows.Count)
+                // Hacky workaround for https://bugzilla.xamarin.com/show_bug.cgi?id=24372
+                if (Platform.IsMono && !Platform.IsMonoFourOrLater)
                 {
-                    first_row_index.SetValue(this, 0);
+                    var first_row_index = typeof (MainModListGUI).BaseType
+                        .GetField("first_row_index", BindingFlags.NonPublic | BindingFlags.Instance);
+                    var value = (int) first_row_index.GetValue(this);
+                    if (value < 0 || value >= Rows.Count)
+                    {
+                        first_row_index.SetValue(this, 0);
+                    }
                 }
             }
+            catch
+            {
+                // Never throw exceptions in OnPaint, or WinForms might decide to replace our control with a big red X
+                // https://blogs.msdn.microsoft.com/shawnhar/2010/11/22/winforms-and-the-big-red-x-of-doom/
+            }
             base.OnPaint(e);
         }
 

diff --git a/debian/control.in b/debian/control.in
@@ -3,7 +3,7 @@ Version: @VERSION@
 Architecture: all
 Section: utils
 Priority: optional
-Depends: mono-runtime (>=5.0), ca-certificates-mono, libmono-microsoft-csharp4.0-cil, liblog4net1.2-cil, libnewtonsoft-json5.0-cil
+Depends: mono-runtime (>=5.0), ca-certificates-mono, libmono-microsoft-csharp4.0-cil, liblog4net1.2-cil, libnewtonsoft-json5.0-cil, libmono-system-net-http-webrequest4.0-cil
 Maintainer: The CKAN authors <debian@ksp-ckan.org>
 Description: KSP-CKAN official client.
  Official client for the Comprehensive Kerbal Archive Network (CKAN).