Merge #4469 Refactor module installer to use relative paths internally

Author: HebaruSan

CHANGELOG.md

@@ -42,6 +42,7 @@ All notable changes to this project will be documented in this file.
 - [Netkan] Make CachingHttpService more thread-safe (#4402 by: HebaruSan)
 - [Netkan] Lookback for `version_from_asset` (#4407 by: HebaruSan)
 - [Build] Migrate Mac app bundle from Mono to dotnet (#4410, #4418 by: HebaruSan; reviewed: lewisfm)
+- [Core] Refactor module installer to use relative paths internally (#4469 by: HebaruSan)
 
 ## v1.36.0 (Quasar)
 

Core/IO/ModuleInstaller.cs

@@ -361,7 +361,7 @@ private List<string> InstallModule(CkanModule
                 var filters = config.GetGlobalInstallFilters(instance.Game)
                                     .Concat(instance.InstallFilters)
                                     .ToHashSet();
-                var groups = FindInstallableFiles(module, zipfile, instance, instance.Game)
+                var groups = FindInstallableFiles(module, zipfile, instance.Game)
                     // Skip the file if it's a ckan file, these should never be copied to GameData
                     .Where(instF => !IsInternalCkan(instF.source))
                     // Check whether each file matches any installation filter
@@ -377,7 +377,7 @@ private List<string> InstallModule(CkanModule
                         // Find where we're installing identifier.optionalversion.dll
                         // (file name might not be an exact match with manually installed)
                         var dllFolders = files
-                            .Select(f => instance.ToRelativeGameDir(f.destination))
+                            .Select(f => f.destination)
                             .Where(relPath => instance.DllPathToIdentifier(relPath) == module.identifier)
                             .Select(Path.GetDirectoryName)
                             .ToHashSet();
@@ -409,7 +409,7 @@ private List<string> InstallModule(CkanModule
                         {
                             var fileMsg = string.Join(Environment.NewLine,
                                                       conflicting.OrderBy(tuple => tuple.same)
-                                                                 .Select(tuple => $"- {instance.ToRelativeGameDir(tuple.file.destination)}  ({(tuple.same ? Properties.Resources.ModuleInstallerFileSame : Properties.Resources.ModuleInstallerFileDifferent)})"));
+                                                                 .Select(tuple => $"- {tuple.file.destination}  ({(tuple.same ? Properties.Resources.ModuleInstallerFileSame : Properties.Resources.ModuleInstallerFileDifferent)})"));
                             if (User.RaiseYesNoDialog(string.Format(Properties.Resources.ModuleInstallerOverwrite,
                                                                     module.name, fileMsg)))
                             {
@@ -431,8 +431,8 @@ private List<string> InstallModule(CkanModule
                             throw new CancelledActionKraken();
                         }
                         log.DebugFormat("Copying {0}", file.source.Name);
-                        var path = InstallFile(zipfile, file.source, file.destination, file.makedir,
-                                               candidateDuplicates?.GetValueOrDefault((relPath: instance.ToRelativeGameDir(file.destination),
+                        var path = InstallFile(zipfile, file.source, instance.ToAbsoluteGameDir(file.destination), file.makedir,
+                                               candidateDuplicates?.GetValueOrDefault((relPath: file.destination,
                                                                                        size:    file.source.Size))
                                                                   ?? Array.Empty<string>(),
                                                fileProgress);
@@ -479,21 +479,20 @@ public static bool IsInternalCkan(ZipEntry ze)
                                                                                     IEnumerable<InstallableFile> files,
                                                                                     Registry                     registry)
             => files.Where(file => !file.source.IsDirectory
-                                   && File.Exists(file.destination)
-                                   && registry.FileOwner(instance.ToRelativeGameDir(file.destination)) == null)
-                    .Select(file =>
-                    {
-                        log.DebugFormat("Comparing {0}", file.destination);
-                        using (Stream     zipStream = zip.GetInputStream(file.source))
-                        using (FileStream curFile   = new FileStream(file.destination,
-                                                                     FileMode.Open,
-                                                                     FileAccess.Read))
-                        {
-                            return (file,
-                                    same: file.source.Size == curFile.Length
-                                          && StreamsEqual(zipStream, curFile));
-                        }
-                    });
+                                   && registry.FileOwner(file.destination) == null)
+                    .Select(file => (file, absPath: instance.ToAbsoluteGameDir(file.destination)))
+                    .Where(tuple => File.Exists(tuple.absPath))
+                    .Select(tuple =>
+                            {
+                                log.DebugFormat("Comparing {0}", tuple.absPath);
+                                using (Stream     zipStream = zip.GetInputStream(tuple.file.source))
+                                using (FileStream curFile   = File.OpenRead(tuple.absPath))
+                                {
+                                    return (tuple.file,
+                                            same: tuple.file.source.Size == curFile.Length
+                                                  && StreamsEqual(zipStream, curFile));
+                                }
+                            });
 
         /// <summary>
         /// Compare the contents of two streams
@@ -547,10 +546,10 @@ private static bool StreamsEqual(Stream s1, Stream s2)
         private void DeleteConflictingFiles(IEnumerable<InstallableFile> files)
         {
             var txFileMgr = new TxFileManager(instance.CkanDir);
-            foreach (InstallableFile file in files)
+            foreach (var absPath in files.Select(f => instance.ToAbsoluteGameDir(f.destination)))
             {
-                log.DebugFormat("Trying to delete {0}", file.destination);
-                txFileMgr.Delete(file.destination);
+                log.DebugFormat("Trying to delete {0}", absPath);
+                txFileMgr.Delete(absPath);
             }
         }
 
@@ -566,74 +565,31 @@ private void DeleteConflictingFiles(IEnumerable<InstallableFile> files)
         ///
         /// Throws a BadMetadataKraken if the stanza resulted in no files being returned.
         /// </summary>
-
-        public static List<InstallableFile> FindInstallableFiles(CkanModule    module,
-                                                                 ZipFile       zipfile,
-                                                                 GameInstance  inst)
-            => FindInstallableFiles(module, zipfile, inst, inst.Game);
-
-        public static List<InstallableFile> FindInstallableFiles(CkanModule module,
-                                                                 ZipFile    zipfile,
-                                                                 IGame      game)
-            => FindInstallableFiles(module, zipfile, null, game);
-
-        private static List<InstallableFile> FindInstallableFiles(CkanModule    module,
-                                                                  ZipFile       zipfile,
-                                                                  GameInstance? inst,
-                                                                  IGame         game)
-        {
-            try
-            {
-                // Use the provided stanzas, or use the default install stanza if they're absent.
-                return module.install is { Length: > 0 }
-                    ? module.install
-                            .SelectMany(stanza => stanza.FindInstallableFiles(zipfile, inst))
-                            .ToList()
-                    : ModuleInstallDescriptor.DefaultInstallStanza(game,
-                                                                   module.identifier)
-                                             .FindInstallableFiles(zipfile, inst);
-            }
-            catch (BadMetadataKraken kraken)
-            {
-                // Decorate our kraken with the current module, as the lower-level
-                // methods won't know it.
-                kraken.module ??= module;
-                throw;
-            }
-        }
+        public static IEnumerable<InstallableFile> FindInstallableFiles(CkanModule module,
+                                                                        ZipFile    zipfile,
+                                                                        IGame      game)
+            // Use the provided stanzas, or use the default install stanza if they're absent.
+            => module.GetInstallStanzas(game)
+                     .SelectMany(stanza => stanza.FindInstallableFiles(module, zipfile, game))
+                     .ToArray();
 
         /// <summary>
         /// Given a module and a path to a zipfile, returns all the files that would be installed
         /// from that zip for this module.
         ///
-        /// This *will* throw an exception if the file does not exist.
-        ///
         /// Throws a BadMetadataKraken if the stanza resulted in no files being returned.
         ///
         /// If a KSP instance is provided, it will be used to generate output paths, otherwise these will be null.
         /// </summary>
-        // TODO: Document which exception!
-        public static List<InstallableFile> FindInstallableFiles(CkanModule  module,
-                                                                 string      zip_filename,
-                                                                 IGame       game)
+        public static IEnumerable<InstallableFile> FindInstallableFiles(CkanModule module,
+                                                                        string     zip_filename,
+                                                                        IGame      game)
         {
             // `using` makes sure our zipfile gets closed when we exit this block.
             using (ZipFile zipfile = new ZipFile(zip_filename))
             {
                 log.DebugFormat("Searching {0} using {1} as module", zip_filename, module);
-                return FindInstallableFiles(module, zipfile, null, game);
-            }
-        }
-
-        public static List<InstallableFile> FindInstallableFiles(CkanModule   module,
-                                                                 string       zip_filename,
-                                                                 GameInstance inst)
-        {
-            // `using` makes sure our zipfile gets closed when we exit this block.
-            using (ZipFile zipfile = new ZipFile(zip_filename))
-            {
-                log.DebugFormat("Searching {0} using {1} as module", zip_filename, module);
-                return FindInstallableFiles(module, zipfile, inst);
+                return FindInstallableFiles(module, zipfile, game);
             }
         }
 
@@ -653,10 +609,10 @@ public static List<InstallableFile> FindInstallableFiles(CkanModule   module,
                                  filters);
 
         private static IEnumerable<(string path, bool dir, bool exists)> GetModuleContents(
-                GameInstance                instance,
-                IReadOnlyCollection<string> installed,
-                HashSet<string>             parents,
-                HashSet<string>             filters)
+                GameInstance        instance,
+                IEnumerable<string> installed,
+                HashSet<string>     parents,
+                HashSet<string>     filters)
             => installed.Where(f => !filters.Any(filt => f.Contains(filt)))
                         .GroupBy(parents.Contains)
                         .SelectMany(grp =>
@@ -677,20 +633,18 @@ public static List<InstallableFile> FindInstallableFiles(CkanModule   module,
                 CkanModule      module,
                 HashSet<string> filters)
             => (Cache.GetCachedFilename(module) is string filename
-                    ? GetModuleContents(instance,
-                                        Utilities.DefaultIfThrows(
-                                            () => FindInstallableFiles(module, filename, instance)),
+                    ? GetModuleContents(Utilities.DefaultIfThrows(
+                                            () => FindInstallableFiles(module, filename, instance.Game)),
                                         filters)
                     : null)
                ?? Enumerable.Empty<(string path, bool dir, bool exists)>();
 
         private static IEnumerable<(string path, bool dir, bool exists)>? GetModuleContents(
-                GameInstance                  instance,
                 IEnumerable<InstallableFile>? installable,
                 HashSet<string>               filters)
             => installable?.Where(instF => !filters.Any(filt => instF.destination != null
                                                                 && instF.destination.Contains(filt)))
-                           .Select(f => (path:   instance.ToRelativeGameDir(f.destination),
+                           .Select(f => (path:   f.destination,
                                          dir:    f.source.IsDirectory,
                                          exists: true));
 

Core/Net/NetModuleCache.cs

@@ -142,7 +142,7 @@ public string GetFileHashSha256(string filePath, IProgress<int> progress, Cancel
         /// </summary>
         /// <param name="module">The module object corresponding to the download</param>
         /// <param name="path">Path to the file to add</param>
-        /// <param name="progress">Callback to notify as we traverse the input, called with percentages from 0 to 100</param>
+        /// <param name="progress">Callback to notify as we traverse the input, called with byte counts</param>
         /// <param name="description">Description of the file</param>
         /// <param name="move">True to move the file, false to copy</param>
         /// <param name="cancelToken">Cancellation token to cancel the operation</param>
@@ -209,7 +209,7 @@ public string Store(CkanModule         module,
         /// </summary>
         /// <param name="filename">Path to zip file to check</param>
         /// <param name="invalidReason">Description of problem with the file</param>
-        /// <param name="progress">Callback to notify as we traverse the input, called with percentages from 0 to 100</param>
+        /// <param name="progress">Callback to notify as we traverse the input, called with byte counts</param>
         /// <param name="cancelToken">Cancellation token to cancel the operation</param>
         /// <returns>
         /// True if valid, false otherwise. See invalidReason param for explanation.

Core/Types/CkanModule.cs

@@ -140,6 +140,16 @@ public class CkanModule : IEquatable<CkanModule>
         [JsonProperty("install", Order = 24, NullValueHandling = NullValueHandling.Ignore)]
         public ModuleInstallDescriptor[]? install;
 
+        public ModuleInstallDescriptor[] GetInstallStanzas(IGame game)
+            => install switch
+               {
+                   { Length: > 0 } => install,
+                   _ => new ModuleInstallDescriptor[]
+                        {
+                            ModuleInstallDescriptor.DefaultInstallStanza(game, identifier)
+                        }
+               };
+
         [JsonProperty("localizations", Order = 17, NullValueHandling = NullValueHandling.Ignore)]
         public string[]? localizations;
 
@@ -182,23 +192,14 @@ public ModuleVersion spec_version
         [JsonProperty("release_date", Order = 30, NullValueHandling = NullValueHandling.Ignore)]
         public DateTime? release_date;
 
-        // A list of eveything this mod provides.
-        public List<string> ProvidesList
-        {
-            // TODO: Consider caching this, but not in a way that the serialiser will try and
-            // serialise it.
-            get
-            {
-                var provides = new List<string> { identifier };
-
-                if (this.provides != null)
-                {
-                    provides.AddRange(this.provides);
-                }
+        [JsonIgnore]
+        private string[]? providesList = null;
 
-                return provides;
-            }
-        }
+        // A list of eveything this mod provides.
+        public string[] ProvidesList
+            => providesList ??= (provides ?? Enumerable.Empty<string>())
+                                    .Prepend(identifier)
+                                    .ToArray();
 
         // These are used to simplify the search by dropping special chars.
         [JsonIgnore]
@@ -714,10 +715,7 @@ public override string ToString()
             => string.Format("{0} {1}", identifier, version);
 
         public string DescribeInstallStanzas(IGame game)
-            => install == null
-                ? ModuleInstallDescriptor.DefaultInstallStanza(game, identifier)
-                                         .DescribeMatch()
-                : string.Join(", ", install.Select(mid => mid.DescribeMatch()));
+            => string.Join(", ", GetInstallStanzas(game).Select(mid => mid.DescribeMatch()));
 
         /// <summary>
         /// Return an archive.org URL for this download, or null if it's not there.