Fix error when removing file from GameRoot #3196
Merged
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Problem
If you install a mod that installs a file to GameRoot (e.g. AdvancedFlyByWire), and then remove it, the removal fails:
Cause
In #3180 we fixed
KSPPathUtils.NormalizePath
to no longer incorrectly convert"/"
to""
, because those are two different directories, and normalization is supposed to generate a standardized format of the same directory.That change indirectly confused this loop:
CKAN/Core/ModuleInstaller.cs
Lines 900 to 908 in a494e1d
For a file in GameRoot, the absolute path to the game folder is included in the
directories
parameter ofModuleInstaller.AddParentDirectories
, which results inrelativeHead
containing the empty string. Then in the first iteration of the loop,buildPath
is set to/
, andToAbsolute
callsNormalizePath
on it, after which it is still/
, soToAbsolute
throws because that path is already absolute.Considered and not done
We could change
ToAbsolute
to use some method other thanNormalizePath
to massage its inputs. I think it would be better to continue to uncover places in Core where paths are treated sloppily and fix them.ToAbsolute("/", "/myroot/")
should throw an exception ifToAbsolute("/afile", "/myroot/")
would.Changes
Now if
relativeHead
is empty, that means there are no parent folders to check, so we skip that loop.Fixes #3195.