-
-
Notifications
You must be signed in to change notification settings - Fork 50
added -liphlpapi compilation flag to win32 config #697
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
Conversation
@@ -9,7 +9,7 @@ class Win32ConfigInfo(ConfigInfo): | |||
build = "win32" | |||
compatible_hosts = ["fedora", "debian"] | |||
arch_full = "i686-w64-mingw32" | |||
extra_libs = ["-lwinmm", "-lshlwapi", "-lws2_32", "-lssp"] | |||
extra_libs = ["-lwinmm", "-lshlwapi", "-lws2_32", "-lssp", "-liphlpapi"] |
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.
This extra_libs is for crosscompilation.
It tell mingw to always link with those libraries when crosscompiling.
They are not needed when compiling natively (see https://github.com/kiwix/kiwix-build/blob/main/appveyor/install_libzim.cmd) and they are use for all projects.
If iphlpapi
don't fall in this category, we should add it to libkiwix's meson.build
instead.
Have you some pointer to a documentation explaining why we need it?
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.
This flag is only needed when libkiwix is compiled for windows as I have changed the api for getting networking adapters of the system.
https://learn.microsoft.com/en-us/windows/win32/api/iphlpapi/nf-iphlpapi-getadaptersaddresses
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.
So adding the flag to meson.build will link the library in both the cases: cross-compilation and native build(windows)?
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.
Yes please. We probably need the library "on Windows" (whatever we build native or crosscompile)
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.
One more question:
networkTools.cpp in libkiwix requires winsock2.h and ws2tcpip.h. These two headers were also used in the previous way of obtaining network adapter on windows. But the compilation flag -lws2_32
required to link these two headers is not in the meson.build of libkiwix but libzim.
So if libzim is using -lws2_32
and libkiwix is linked to libzim, so that means there is no need to again specify -lws2_32
this flag in meson.build of libkiwix?
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.
But the compilation flag -lws2_32 required to link these two headers is not in the meson.build of libkiwix but libzim.
Sound like a bug. If you can fix it while you are working on IPV6 it would be nice.
So if libzim is using -lws2_32 and libkiwix is linked to libzim, so that means there is no need to again specify -lws2_32 this flag in meson.build of libkiwix?
With dynamic linking, it will work yes (although, semantically wrong).
With static linking, it will not. As at libzim compilation linker may remove unused symbol from ws2_32 library.
ws2_32 is probably always dynamic so it is why it works.
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.
Yes please. We probably need the library "on Windows" (whatever we build native or crosscompile)
I don't think so because in meson.build of libkiwix:
if build_machine.system() == 'windows'
extra_link_args = ['-lshlwapi', '-lwinmm', '-lWs2_32', '-liphlpapi']
build_machine.system()
determines the system it getting built at not the system it is targeted for.
So I think adding the linker flag in kiwix-build makes sense too.
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.
Hum, there is definitvly something wrong here.
Meson is adding extra libs if building on Windows but miss to add them when building for Windows. And so we add it in kiwix-build.
It would be better that libkiwix's meson all the time add needed libraries and we don't add them on kiwix-build.
@aryanA101a What is the status here? |
Concluding from the above conversation, I think this pr should be closed and the flags should be added to the meson.build of libkiwix. |
resolves #696