-
Notifications
You must be signed in to change notification settings - Fork 214
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
Shared lib instead of static ? #24
Comments
Hi @MatejGomboc, Have you tried calling CMake with the additional option "-DBUILD_SHARED_LIBS:BOOL=ON"? Best regards, |
I believe there is an issue here https://github.com/ampl/gsl/blob/master/gsl_types.h#L25 it should be "_WIN32" this is preventing GSL_VARS from being set when BUILD_SHARED_LIBS=ON I have updated it here https://github.com/Dekken/gsl/tree/windows and it can be viewed building/testing here https://ci.appveyor.com/project/Dekken/gnu-gsl/ |
Looking at the CMAKE build script, I can see that it (as well as the GSL code itself) requires WIN32 to be defined for a DLL build. It hence seems likely that CMAKE needs to be called with an additional option to define WIN32 during the build process when a DLL is being built. |
_WIN32 is defined when you use the MSVC compiler automatically. So I would guess they meant to do that Reference: https://gcc.gnu.org/wiki/Visibility |
Hi Dekken, I am not sure who the 'they' is in your comment. But GSL itself has always required the WIN32 define and, as far as I can see, this is also true of the CMAKE script. I don't use CMAKE but I would guess it would not be hard to detect the compiler's _WIN32 define and set WIN32 during a Windows DLL build if you wanted to avoid the need for an extra define when calling CMAKE.. |
"They" being the GSL authors.
"WIN32" is fine for CMake, but it is incorrect for MSVC, again, it's supposed to be "#ifdef _WIN32" The simplest solution is updateing gsl_types.h to use the actual macros provided by the compilers or even
|
GSL's use of WIN32 has nothing to do with what proprietary compilers do (or do not) define. It is the define that *** GSL *** itself uses to identify any parts of the GSL source code that need to know that the target OS is Windows. |
Seems pretty pointless when you can just rely on the compiler. If you look at the GCC Visibility doc it's what they use, and perhaps consistency across products should be striven for, if it's good enough for GCC it should be good enough for GSL also considering at no point is "WIN32" defined any build method, changing it to use the builtin macros seems like a no brainer |
It may seem pointless now but it would be wrong to assume that it was pointless when it was first used By the way, your last point is incorrect since my Visual Studio based GSL build defines WIN32 (as far as I can see, this is necessary in order to build the DLL). |
If you're afraid of making changes it's fine to say so, this is a poor justification for not updating it to use the actual macros.
Yes I see that, the DLL is hiding |
Also I should tell you that even if you build the DLL, it's somewhat useless as the functions are missing the prefix of "GSL_VARS" |
Unless of course said functions are not meant to be public which is possible. although I'd be quite surprised if that was so |
My DLL build exports all the GSL functions correctly and passes all the tests. |
Well that may be so, but I think you'll find it's because you're not even linking against the DLLs Which would mean you're linking statically, which is the whole point of this thread. |
You are wrong once again. I link to the DLL using the standard MIcrosoft stub library, which would not work if the DLL did not export the functions properly. |
I am wondering if there is any point in this exchange. I was willing to help with the issues you are facing but I don't think you want my help. |
You're really making it tough for other people to use this library, you know this should come across as a usability problem if you had the slightest thought to it. You are not other people, congratulations that you can make things work. |
It's quite likely because of "/MT" linking https://docs.microsoft.com/en-us/cpp/build/reference/md-mt-ld-use-run-time-library?view=vs-2019 which is considered static, and this is why GSL_VARS is not needed |
My build of the GSL DLL works 'out of the box'. If you use it and run into problems, I am ready to offer help with it. I can't offer help with other build methods. |
Why are you so desperate too try to show my DLL build doesn't work? It needs (and uses) GSL_VAR definitions to build the DLL. |
It's inconsistent. I have built this with /MD and it fails to find the exports. The choice of /MD or /MT is not arbitrary, and supporting both may just help people in future to not run into problems. |
Although I really do wish the dllexport business didn't exist at all |
Before the actual GSL build I scan all the GSL include files and generate the necessary header files with the appropriate GSL_VAR definitions added. Or, if the user chooses, I leave the headers unmodified and export a DEF file instead. |
Its not inconsistent when I build it - I can link to the static or dynamic run-time libraries although I choose to use the DLL version as the default. |
By default I link the GSL libraries with the Microsoft run-time libraries of the same type (static -> static; DLL -> DLL). I do not now test the other 'inconsistent' combinations as a matter of course but I have in the recent past tested them and found that they worked. But I don't really trust these combinations because I have had problems with these configurations on other projects in the past |
Hi,
I'm trying to build the library for Windows 64-bit, using Microsoft VS2013. I have successfully built static version of library. How can I build shared version (.dll) instead ?
Kind regards !
The text was updated successfully, but these errors were encountered: