Windows Support #246
Open
Windows Support #246
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
`free` should not be used to release the memory allocation. The allocation was performed with `UnsafeMutablePointer.allocate(capacity:)` which does not provide a guarantee on the allocation being performed with `malloc`. In fact, on Windows, this is actively not the case and is is performed with `_aligned_alloc`. Therefore the release must be balanced with the MSVC specific `_aligned_free`. Instead, prefer to use the portable `UnsafeMutablePointer.deallocate` to correctly balance the memory allocation.
Rather than explicitly trying to handle the memory management lifetime, extend the lifetime to the scope and use a `defer` statement to ensure that the memory is always deallocated when exiting the scope.
The `unzGetCurrentFileInfo64` signature does not use a C type for the length but rather a local type definition. The type definition underlying it is a `long` which is differently sized on LP64 and LLP64 systems. Adjust this by casting the parameter to the proper type (`uLong`) as vended by the C library.
When using vim as the editor, some configurations may emit the swap file next to the source. Ignore swap files from accidentally being committed.
`#import` is an Objective-C construct and is not portable. `#include` is the standard mechanism for including a source file. Use of the Objective-C mechanism works in many cases but not globally. On Windows, this is used as a MSVC extension for COM type library inclusion. Switch to the standard spelling to allow building on Windows.
The linking is already handled at the SPM layer. There is no reason to force a secondary link directive. This primarily impacts Darwin platforms where the link directive would form a `LC_LINKER_OPTION` load command to ensure that zlib is linked against. However, we have the platform specific linkage and this is already performed at the SPM layer. Remove the directive and allow SPM to handle it.
The library name for zlib is different on Windows than on Unix platforms (`zlib` and `zlibstatic`). In order to support properly linking against the library the user must specify the linkage type. Since SwiftPM does not have the ability to properly control build settings, use an environment variable to select between the two. If `ZIP_USE_DYNAMIC_ZLIB` is set, we will attempt to perform dynamic linkage against zlib, otherwise, we will use static linking. Take the opportunity to silence some warnings due to deprecated symbol usage on Windows. A build of zlib must be available for use and must be explicitly passed to the build on Windows via extended flags (i.e. `-Xcc` and `-Xlinker`).
Windows does not have full POSIX permissions and instead has a far richer permission model through proper ACLing. Adjust the expectations accordingly.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
There was a Swift forums thread that indicated that people wanted to use this package on Windows but it did not build due to the missing header. In order to verify if it built on Windows, I built it locally. This led to a series of changes which ultimately enabled the Windows support in the package. I am opening the PR in case it is interesting for the project to enable Windows support.