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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Stop trying to check free space on Mono #3850

Merged
merged 2 commits into from Jun 12, 2023

Conversation

HebaruSan
Copy link
Member

@HebaruSan HebaruSan commented Jun 12, 2023

Problem

Mac and Linux users have reported spurious errors about free disk space when installing:

Not enough space in game folder to install modules!
Need to store 2.3 MiB to /Applications/ksp_osx, but only 0 B is available!     
Free up space on that device or change your settings to use another location.

macspace

Not enough space in game folder to install modules!
Need to store 5.6 GiB to /home/storm/GOG Games/1.12.4-oldmods, but only 537.8 MiB is available!
Free up space on that device or change your settings to use another location.

[storm@defiant 1.12.4-oldmods]$ df -h .
Filesystem            Size  Used Avail Use% Mounted on
shuttlepod/GOG Games  3.5T   12G  3.5T   1% /home/storm/GOG Games

Cause

Mono's DriveInfo.GetDrives() doesn't include mounted filesystems other than /, which makes it useless and unusable:

I tested #3631 on a single partition system, which made it appear to work fine because / is hard-coded to be part of this list, but if you have more partitions mounted than that, only / will be checked, which can be wrong, and we have no way of detecting that it's wrong.

This can cause both false positives (user is told they don't have enough free space when they do) and false negatives (user is allowed to proceed with a nearly full disk and the install fails). So the free space check isn't reliable on Mono.

Changes

Now our DirectoryInfo.GetDrive() extension method always returns null on Mono 馃槥, because finding a directory's drive can't be done on Mono (but should be possible if Mono's code for this wasn't 馃挬). This will bypass the free space checks. Windows users will still be alerted before they fill up their disks, but Mac and Linux users will be back to installing without this guard rail, potentially filling their disks in the middle of downloading or installing.

I'm not going to bother trying to send a patch to Mono because they're not merging fixes anymore and because this code is probably so old that changing it would spook them. 馃懟

Fixes #3768.
Fixes #3848.

Side fix

The localization DLL for Korean GUI strings has the same name as the one for Core, which causes this compile warning and probably breaks something in the UI somewhere:

WARN: Ignoring duplicate resource CKAN.Properties.Resources.ko-KR.resources

Now it's changed to CKAN.GUI.Properties.Resources.ko-KR.resources, consistent with all the other languages.

@HebaruSan HebaruSan added Bug Core (ckan.dll) Issues affecting the core part of CKAN Linux Issues specific for Linux macOS Issues specific for macOS Mono Issues specific for Mono labels Jun 12, 2023
@HebaruSan HebaruSan requested a review from techman83 June 12, 2023 02:11
Copy link
Member

@techman83 techman83 left a comment

Choose a reason for hiding this comment

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

The history of .net/mono is a right mess, and full of decisions that went against the best interests of the language.

Bit of a shame, but is what it is! LGTM, and is better than the false positive on the unsupported systems.

@HebaruSan HebaruSan merged commit 6a377ce into KSP-CKAN:master Jun 12, 2023
10 checks passed
@HebaruSan HebaruSan deleted the fix/mono-check-space branch June 12, 2023 03:38
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 Linux Issues specific for Linux macOS Issues specific for macOS Mono Issues specific for Mono
Projects
None yet
2 participants