Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Find portable installs when enforcing cache limits #2856

Merged
merged 1 commit into from
Aug 31, 2019

Conversation

HebaruSan
Copy link
Member

Problem

A user mentioned this exception with a "portable" install:

System.InvalidOperationException: Sequence contains no elements
   at System.Linq.Enumerable.Aggregate[TSource](IEnumerable`1 source, Func`3 func)
   at CKAN.NetFileCache.EnforceSizeLimit(Int64 bytes, Registry registry)
   at CKAN.ModuleInstaller.EnforceCacheSizeLimit()
   at CKAN.ModuleInstaller.InstallList(ICollection`1 modules, RelationshipResolverOptions options, IDownloader downloader, Boolean ConfirmPrompt)
   at CKAN.Main.InstallMods(Object sender, DoWorkEventArgs e)
   at System.ComponentModel.BackgroundWorker.OnDoWork(DoWorkEventArgs e)
   at System.ComponentModel.BackgroundWorker.WorkerThreadStart(Object argument)

https://forum.kerbalspaceprogram.com/index.php?/topic/154922-ckan-the-comprehensive-kerbal-archive-network-v1264-orion/&do=findComment&comment=3660841

Cause

NetFileCache.EnforceSizeLimit assumes that the current instance will be in manager.Instances.

// This object will let us determine whether a module is compatible with any of our instances
KspVersionCriteria aggregateCriteria = manager?.Instances.Values
.Where(ksp => ksp.Valid)
.Select(ksp => ksp.VersionCriteria())
.Aggregate((a, b) => a.Union(b));

"Portable" instances are an exception to this:

/// This *will not* touch the registry if we find a portable install.

If all you have is a portable install, manager.Instances will be empty, and Aggregate will throw that exception.

Changes

Now we use Aggregate's optional first parameter to set a default element based on the current instance's version criteria, which should cover portables, falling back to an empty range if not found.

@HebaruSan HebaruSan added Bug Core (ckan.dll) Issues affecting the core part of CKAN Pull request labels Aug 27, 2019
@DasSkelett
Copy link
Member

Sooo...
What are portable installs? 🤔

@HebaruSan
Copy link
Member Author

A portable install is when you put ckan.exe in your KSP folder and run it from there. We check to see whether the current folder is a KSP folder, and if so, then we make an ad hoc instance that doesn't get saved to the registry (or the new JSON config file). I think the idea was that you could set this up on a USB drive and then be able to use CKAN on multiple computers conveniently.

CKAN/Core/KSP.cs

Lines 175 to 190 in 9da096b

public static string PortableDir()
{
// Find the directory our executable is stored in.
// In Perl, this is just `use FindBin qw($Bin);` Verbose enough, C#?
string exe_dir = Path.GetDirectoryName(Assembly.GetExecutingAssembly().Location);
log.DebugFormat("Checking if KSP is in my exe dir: {0}", exe_dir);
if (IsKspDir(exe_dir))
{
log.InfoFormat("KSP found at {0}", exe_dir);
return exe_dir;
}
return null;
}

CKAN/Core/KSPManager.cs

Lines 83 to 94 in 9da096b

// First check if we're part of a portable install
// Note that this *does not* register in the registry.
string path = KSP.PortableDir();
if (path != null)
{
KSP portableInst = new KSP(path, "portable", User);
if (portableInst.Valid)
{
return portableInst;
}
}

Copy link
Member

@DasSkelett DasSkelett left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@HebaruSan HebaruSan merged commit 47a7fd0 into KSP-CKAN:master Aug 31, 2019
@HebaruSan HebaruSan deleted the fix/portable-size-limit branch August 31, 2019 19:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Core (ckan.dll) Issues affecting the core part of CKAN Pull request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants