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

Fix tracking of paths with trailing spaces on Windows #3586

Merged

Conversation

HebaruSan
Copy link
Member

@HebaruSan HebaruSan commented Jun 2, 2022

Problem

If you install MrScienceMeistersNuclearWarfare (latest version is for KSP 1.0.5) on Windows and then try to uninstall it, you get this error:

About to remove:

 * MrScienceMeister's Nuclear Warfare 0.5.1
Could not find a part of the path 'D:\SteamLibrary\SteamApps\common\Kerbal Space Program\GameData\NuclearWarfare\Parts\freeFallNukeBrake'.
Error during installation!
If the above message indicates a download error, please try again. Otherwise, please open an issue for us to investigate.
If you suspect a metadata problem: https://github.com/KSP-CKAN/NetKAN/issues/new/choose
If you suspect a bug in the client: https://github.com/KSP-CKAN/CKAN/issues/new/choose

Cause

The path can't be found because the installed_files entry in registry.json doesn't match the path on disk.

ModuleInstaller assumes that Directory.CreateDirectory and File.Create will either create the paths they're given or fail. Astoudingly, this assumption is false on Windows, a purportedly real professional operating system that is sold to users across the world! Instead, both of these functions silently trim trailing spaces from paths. This code:

Directory.CreateDirectory("test path      ");

... creates a directory called test path with no trailing spaces. This happens silently; there's no exception or other explicit indication that something unexpected has happened. If the application code needs to know the path, it has to know about this potential problem (despite it not being particularly clearly documented) and implement a workaround for it.

Two directories in MrScienceMeistersNuclearWarfare end with trailing spaces. So when we uninstall this module, we try to remove the path with the trailing space, but Windows doesn't find it because it's not there, having been replaced by a different path during installation. 🤦

Changes

  • Now ModuleInstaller.CopyZipEntry returns the path it created
    • For directories, the TxFileManager.CreateDirectory function we use has no return value, but Path.GetDirectoryName apparently performs the trimming logic, so we use Path.GetDirectoryName(Path.Combine(fullPath, "DUMMY"))
    • For files, File.Create returns a FileStream object with a Name property that has it
  • Now ModuleInstaller.InstallModule collects the created paths from CopyZipEntry and returns them instead of simply assuming that every InstallableFile.destination will be created as requested

This will ensure that the entries in registry.json always match the real paths on disk, so uninstallation will no longer be tripped up by this Windows quirk.

Fixes #3585.

@HebaruSan HebaruSan added Bug Core (ckan.dll) Issues affecting the core part of CKAN Pull request Windows Issues specific for Windows Registry Issues affecting the registry labels Jun 2, 2022
@HebaruSan HebaruSan requested a review from DasSkelett June 2, 2022 17:36
@HebaruSan HebaruSan force-pushed the fix/windows-paths-trailing-spaces branch from 0e38efd to ed2f93b Compare June 2, 2022 17:42
@HebaruSan HebaruSan force-pushed the fix/windows-paths-trailing-spaces branch from ed2f93b to 67d91ae Compare June 2, 2022 17:44
@HebaruSan

This comment was marked as resolved.

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.

Oh Windows, always good for surprises.
Didn't test on Windows, but it doesn't break anything on Linux and the code looks sensible.

@HebaruSan HebaruSan merged commit 4f3da57 into KSP-CKAN:master Jun 28, 2022
@HebaruSan HebaruSan deleted the fix/windows-paths-trailing-spaces branch June 28, 2022 16:31
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 Registry Issues affecting the registry Windows Issues specific for Windows
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug] Uninstallation fails on Windows if a mod folder name ends with a space
2 participants