[sfml] Declare Windows library export

[JackBoosY]

When using sfml, the custom project should also link to winmm.lib and gdi32.lib, so change the linking method of these two libraries to public.

Related: #8864.

[JackBoosY]

/azp run

[dan-shaw]

/azp run

[JackBoosY]

/azp run

[cenit]

Are you sure this patches the problem? For static libraries, CMake handles the PRIVATE dependencies and exports them downstream......... This patch should be unnecessary?

[JackBoosY]

@cenit According to the cmake documentation, if we declare a dependency as PRIVATE, it will not be exported downstream. But in this case, if we use sfml, sfml needs these.

[cenit]

@JackBoosY cmake documentation might be deceiving
Please look at this issue if you have any doubt
https://gitlab.kitware.com/cmake/cmake/issues/19611

In fact, just an inspection of the configs would tell you the truth: in case you build a static library, private dependencies are exported downstream no matter what

[eXpl0it3r]

This patch is technically wrong, but practically makes it easier for the user.

The issue is that vcpkg links all the libraries in the vcpkg lib paths, but since winmm and gdi32 arr system libraries and thus not in the vcpkg lib paths, they aren't linked automatically and need to be added manually. Or rather if you use CMake you just need to call find_package and link SFML as part of CMake.

See conversation on Slack: https://cpplang.slack.com/archives/C7BFF7RCJ/p1574294173361500

[cenit]

what's the relationship between vs autolinking using vcpkg and cmake?

also: is this patch really fixing anything? This is what I doubt! Can you give me a test case failing before and having success now?

[eXpl0it3r]

For the original bug and the followed discussion I always assumed that the reporter was using CMake to generate the VS project, but they really just installed SFML and used vcpkg autolinking integration, thus system libs aren't linked, but are required when statically linking.

is this patch really fixing anything?

That's a good question. If the user isn't using CMake to generate a VS project, I don't think that this changes anything and if they do, then it was already working correctly. As such, I'd also say that this doesn't actually "fix" anything.

[eXpl0it3r]

According to the cmake documentation, if we declare a dependency as PRIVATE, it will not be exported downstream. But in this case, if we use sfml, sfml needs these.

The documentation is not very explicit when it comes to shared vs static linking, but I've tested this myself and for static linking the PRIVATE libraries are linked in the final version. Additionally, this has been confirmed and explained independently by others (e.g. here).
Again the underlying issue is, that vcpkg will not auto-link system libraries, i.e. vcpkg doesn't use the CMake targets, but just gobbles up all the libs that it finds in its lib-path.

[JackBoosY]

@cenit Windows system libraries are special. Because the paths of the libraries installed with Visual Studio are not consistent, the compiler will not add the addresses of these libraries to the static libraries that depend on it, which leads us to manually declare and link these system libraries. The usual approach is to force the system libraries to be declared in the link sequence when building a static library that depends on it, but I think this PR approach is better (more concise).

[cenit]

Windows system libraries are special. Because the paths of the libraries installed with Visual Studio are not consistent, the compiler will not add the addresses of these libraries to the static libraries that depend on it, which leads us to manually declare and link these system libraries. The usual approach is to force the system libraries to be declared in the link sequence when building a static library that depends on it, but I think this PR approach is better (more concise).

@JackBoosY i think you are misunderstanding the problem (and maybe a little bit more knowledge would be appropriate, because i think there is nothing correct in your message at least regarding the problem here @ras0219-msft ).

Please try to build the sfml project before and after your patch.

[bindreams]

Hello, sorry for a late response. I'm the original reporter of #8864. I can confirm the issue is not fixed. I'm getting exactly the same linking problems. I think we should re-open #8864.
Also, as @eXpl0it3r mentioned, I am in fact not using CMake - just standard integration between vcpkg and msbuild.

[Tastaturtaste]

I have the same problem, trying to statically link results in unresolved external symbol errors.
I have a log-file with normal verbosity from building a simple hello-world program with sfml attached: sfml_test.log
This is my source I try to build: source.txt

[eXpl0it3r]

As stated you have to link the system dependencies yourself.

[cenit]

@JackBoosY @strega-nil please also remove the unnecessary patch introduced by this merged PR

[Tastaturtaste]

As stated you have to link the system dependencies yourself.

Ok, my previous understanding was that all system dependencies were added to the additional dependencies by default in VS or vcpkg would/should cover me on that front.
In my example I had to manually declare the additional dependencies 'winmm.lib' and 'opengl32.lib' while 'gdi32.lib' was in the default additional dependencies.
Is there some possiblility to notify the user of vcpkg of the necessary system dependencies if they cannot be handled automatically? Maybe after building the downloaded library with 'vcpkg install xxx?

Thanks for your time and effort.
Aside from this issue I am really glad vcpkg exists!

[eXpl0it3r]

gdi32.lib is by default part of the Additiona Libraries that Visual Studio links.
The current script just globs all the *.lib files in the package directory, which of course doesn't contain the system libraries.

I'm not familiar enough with vcpkg to know whether there are options to add system libraries to the recipe.