Loader shared lib: back to default cmake prefix #606
Conversation
|
Author xantares not on autobuild list. Waiting for curator authorization before starting CI build. |
|
Oh! I didn't know that. Apology for that 🙇 |
|
So the way this has been going back and forth makes we think we need to figure out something a little better. Our policy has been that we don't officially support MinGW, but we'll take PRs if people want to make them. As a result, I've mostly just been making sure the changes don't break MSVC/gcc/clang and agreeing to them otherwise. But since this has been going back and forth, it makes we think this approach might not be the best. I could ping the two of you when we have something mingw-related, but that does somewhat obligate the two of you to reply. Alternatively, we could add add a job to our github actions CI to see if things are likely to break. If we did that I'd probably want to mark is as a job that's allowed to fail and we just ping you guys if we know we're going to break things. Does anything like that sound reasonably? I just don't like this back and forth that's been going on. |
|
IMO, adding a little comment in that cmake file would help in future. BTW, as it turns out file naming is important here, it may be possible to check the file names after build and install in CI. I am not familiar with CI, just little thought. |
|
That's good point. Maybe we just need to properly document why it ended up this way and what to watch out for. |
|
CI Vulkan-Loader build queued with queue ID 4068. |
|
CI Vulkan-Loader build # 468 running. |
|
CI Vulkan-Loader build # 468 passed. |
|
Author xantares not on autobuild list. Waiting for curator authorization before starting CI build. |
|
ok I added some comments, feel free to ping me for mingw stuff |
but it changed the import lib name (libvulkan.dll.a instead of libvulkan-1.dll.a) which in turns breaks cmake detection in FindVulkan now we go back to using the default cmake prefix which is equivalent to what KhronosGroup#595 wants, (prior to KhronosGroup#523), but without the broken behavior cc @Biswa96
|
Author xantares not on autobuild list. Waiting for curator authorization before starting CI build. |
|
everything should be ready @lenny-lunarg |
| else() | ||
|
|
||
| # when adding the suffix the import and runtime library names must be consistent | ||
| # mingw: libvulkan-1.dll.a / libvulkan-1.dll |
There was a problem hiding this comment.
#595 restores the default lib prefix for mingw (libvulkan-1.dll instead of vulkan-1.dl) and basically reverts #523
but it changes the import lib name (libvulkan.dll.a instead of libvulkan-1.dll.a) and breaks cmake detection in FindVulkan
(find_library looks for libvulkan-1.dll.a under mingw)
now we go back to using the default cmake prefix which is equivalent to the #595 prefix prior to #523
without the broken import lib name
cc @Biswa96