-
-
Notifications
You must be signed in to change notification settings - Fork 344
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
Handle backslashes in ZIP paths #3893
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great! Thanks @HebaruSan
Did some more research into the PowerShell tool today. This bug was fixed several years ago, see PowerShell/Microsoft.PowerShell.Archive#48. For any mod authors landing here after having problems with backslashes in your ZIPs, run this in PowerShell to install the fix: Install-Module Microsoft.PowerShell.Archive -MinimumVersion 1.2.3.0 -Repository PSGallery -Force
Import-Module Microsoft.PowerShell.Archive |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's annoying, but so is dealing with broken mods that "work" via other tools that allow the spec not to be adhered to. Thanks @HebaruSan !
A quick note: This doesn't need to be reviewed again. I'm holding off on merging it until shortly before the next release, to minimize the amount of time mods with backslash ZIPs will be indexed before a client that can install them is available. |
3dda0fb
to
46cc108
Compare
Problem
KSP-CKAN/KSP2-NetKAN#80 has some errors in the validation checks:
https://github.com/KSP-CKAN/KSP2-NetKAN/pull/80/checks
An already indexed mod is having the same problem with a recent update:
Cause
These mods use backslashes (
\
) instead of forward slashes (/
) in their filenames. The ZIP spec says:https://pkware.cachefly.net/webdocs/casestudies/APPNOTE.TXT
There is a PowerShell tool that some modders use that doesn't obey this, and CKAN doesn't currently handle that well.
Changes
There are already many places where backslashes are replaced by forward slashes when CKAN interprets ZIP files, so it looks like the intent all along was to allow for this, but it wasn't complete. Now one more substitution is made, and it actually works.
Note that the bot will be able to handle these mods immediately after merge, but the old client that people are using now will fail to install them. We will need to release a new client pretty quickly after this is merged, so it should wait until the other pending pull requests are done.
In addition, a .netkan with
find: ''
, which is sometimes generated by the SpaceDock mod analyzer code, will now raise an inflation error: