twogood / unshield

Tool and library to extract CAB files from InstallShield installers
MIT License
348 stars 75 forks source link

Fixes for the Windows build #91

Closed znek closed 1 year ago

znek commented 5 years ago

These commits fix the Windows build which was broken before (tested with VS2017). I also tested on macOS (Xcode 10) to make sure I didn't introduce a regression here.

twogood commented 5 years ago

Thank you @znek ! Do you have any idea what the errors from the AppVeyor build means? https://ci.appveyor.com/project/twogood/unshield/builds/22931639

znek commented 5 years ago

Do you have any idea what the errors from the AppVeyor build means? https://ci.appveyor.com/project/twogood/unshield/builds/22931639

I can only guess, but it looks like the linker cannot resolve the references to libz. It's unclear to determine why that is without having a look at the linker command line. (I'm almost certain that reverting [00f4bed] will fix this, however, it's just a hackish workaround IMO).

twogood commented 5 years ago

Yeah maybe libz is there for a reason? :)

znek commented 5 years ago

Yeah maybe libz is there for a reason? :)

It seems so. But it's just a kludge IMO. AppVeyor's build log states:

157 -- Found ZLIB: C:/Users/appveyor/scoop/apps/zlib-dev/current/lib/libz.dll.a (found version "1.2.3")

I'm pretty new to cmake, but I guess that ZLIB_LIBRARY must have been set also - and it seems to be wrong, as otherwise the kludge of providing a z argument (reversed by [00f4bed]) wouldn't be necessary.

Maybe a bit more context: I built zlib 1.2.11 (current version) with VS2017 and cmake from source. I then provided ZLIB_INCLUDE_DIR and ZLIB_LIBRARY=C:\\Users\\…\\CMakeBuilds\\…\\install\\x64-Release\\lib\\zlibstatic.lib as arguments to the cmake build of unshield. I'd say that seems to be the modern native way to build both projects but I may be wrong. Without [00f4bed] this broke, however, due to a z library nowhere to be found (see above, there's only libzstatic.lib).

twogood commented 5 years ago

When you provide ZLIB* environment variables then you also get "Found ZLIB:" at that location, or?

znek commented 5 years ago

David, sorry for the confusion - I just read up on cmake and now have a proper understanding what's really wrong. Could you just cherry-pick [b3aad49] and drop [00f4bed]? I'm preparing a proper fix for the latter separately.

twogood commented 5 years ago

Using ${ZLIB_LIBRARIES} looks very good. I'm not very good with CMake myself.

wdlkmpx commented 3 years ago

All of these changes are already in the master branch, except the VS .gitignore entries getopt.h was replaced with a more complete version, but I guess it's compatible with VS 2013+, since it contains comments that start with //, I forgot to change them to /* */

twogood commented 2 years ago

Is this still relevant?