-
Notifications
You must be signed in to change notification settings - Fork 124
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
Disable more compression methods for minizip #602
Conversation
I see that some compression features are already disabled in this repository (https://github.com/R2Northstar/NorthstarLauncher/blob/aeecd7a69be8de3afdca124ab624677bfd5582cf/cmake/Findminizip.cmake#L5C61-L5C61), maybe we wanna disable more indeed. My guess is that EDIT: my dumbass thought this was an issue, not a pull request... |
ZLIB would be required for DEFLATE compressed zips (which is the most common format) but others are also possible. Considering mod verification is a manual process, should there be some kind of requirement what compression it needs to use? |
Updated to enable ZLIB for DEFLATE |
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.
I confirm disabling those methods does not have any negative impact on mod downloading (tested with #595 branch).
Given we agreed on Discord to disable as many flags as possible, I tried disabling more, while ensuring mod downloading still works; I ended up with the following configuration:
if(NOT minizip_FOUND)
check_init_submodule(${PROJECT_SOURCE_DIR}/thirdparty/minizip)
set(MZ_ZLIB ON CACHE BOOL "Disable ZLIB compression")
set(MZ_BZIP2 OFF CACHE BOOL "Disable BZIP2 compression")
set(MZ_LZMA OFF CACHE BOOL "Disable LZMA & XZ compression")
set(MZ_PKCRYPT OFF CACHE BOOL "Disable PKWARE traditional encryption")
set(MZ_WZAES OFF CACHE BOOL "Disable WinZIP AES encryption")
set(MZ_ZSTD OFF CACHE BOOL "Disable ZSTD compression")
set(MZ_SIGNING OFF CACHE BOOL "Disable zip signing support")
add_subdirectory(${PROJECT_SOURCE_DIR}/thirdparty/minizip minizip)
set(minizip_FOUND 1 PARENT_SCOPE)
endif()
Given that minizip is only used for the not officially release mod-auto-download, I think we can be a lot more lax with review requirements on this PR ^^ |
commited, rebased and merged (because why not) |
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.
Confirmed working as described in #602 (review).
Merging based on reviews |
Doing this because I ran into the following error while compiling with msvc-wine
What is actually needed from minizip?
This was likely not an issue on windows because of
MZ_FETCH_LIBS
. Should we turn that off to ensure no extra dependencies get in without our knowledge?