KhronosGroup/Vulkan-ExtensionLayer

Fix WIN32 `.lib` installs

juan-lunarg opened this issue · 4 comments

The Windows install target currently installs various .lib files. It should not, as these are not shipped by the SDK.

The SDK is also planning on not shipping 32-bit artifacts for the VK_LAYER_KHRONOS_shader_object (though it will continue to ship 32-bit VK_LAYER_KHRONOS_synchronization2 artifacts). So the 32-bit install target should not ship Bin\VkLayer_khronos_shader_object.dll, Bin\VkLayer_khronos_shader_object.json, or ,Bin\VkLayer_khronos_shader_object.pdb.

The latest SDK build still shows issues: http://erusea:8080/view/Manual/job/manual-sdk/529/console

WARNING: generated BOM C:\j\msdk\vulkantest-results\bom\qt-repo-BOM.txt added entries not in reference BOM C:\j\msdk\build\Vulkan-SDK-Packaging\repo\SDK_creation\Windows\qt-repo-BOM.txt:
+ CoreSDK,Bin\VkLayer_khronos_shader_object.dll
+ CoreSDK,Bin\VkLayer_khronos_shader_object.json
+ CoreSDK,Bin\VkLayer_khronos_shader_object.pdb
+ CoreSDK,Include\vulkan\vulkan_extension_inspection.hpp
+ CoreSDK,Lib\VkLayer_khronos_shader_object.lib
+ CoreSDK32,Bin32\VkLayer_khronos_shader_object.dll
+ CoreSDK32,Bin32\VkLayer_khronos_shader_object.json
+ CoreSDK32,Bin32\VkLayer_khronos_shader_object.pdb

I observe that VkLayer_khronos_shader_object.lib is still being shipped (I suspect it was overlooked in the earlier fix.

I also note that the 32-bit install target still ships VkLayer_khronos_shader_object, though it's not supposed to; see the SDK design document at https://docs.google.com/document/d/14HULgqYy4k3-UDRKbUr_jWQcRGX4RGPN05iLoz0jSB0 . In the interest of keeping install control at the repository level, that means this repository must not ship the VkLayer_khronos_shader_object files in a 32-bit build.

I observe that VkLayer_khronos_shader_object.lib is still being shipped (I suspect it was overlooked in the earlier fix.

I just checked the build logs from github actions:

https://github.com/KhronosGroup/Vulkan-ExtensionLayer/actions/runs/4690667744/jobs/8314079421
https://github.com/KhronosGroup/Vulkan-ExtensionLayer/actions/runs/4690667744/jobs/8314079208

No .lib files are being installed. I cannot reproduce that.

I also note that the 32-bit install target still ships VkLayer_khronos_shader_object, though it's not supposed to; see the SDK design document at https://docs.google.com/document/d/14HULgqYy4k3-UDRKbUr_jWQcRGX4RGPN05iLoz0jSB0 . In the interest of keeping install control at the repository level, that means this repository must not ship the VkLayer_khronos_shader_object files in a 32-bit build.

Sure but why?

Just had Karen explain to me the rational. I have a new PR up that will remove 32 bit support.

Sure but why?

We debated in an SDK planning video meeting the need for 32-bit. Recalling my notes, the general thoughts went (:heavy_plus_sign: marks arguments for 32-bit support, :heavy_minus_sign: marks arguments against):

  • ➕ we ship 32-bit for everything else
  • ➕ we believe there are 32-bit applications still under active development who cannot or will not make the jump to 64-bit
  • ➖ it's unlikely a 32-bit application still in active development would want to completely revise their pipeline and shader structure to use this new object
  • ➖ if we ship 32-bit once, we have to ship 32-bit forever

IIRC, we decided to ship 64-bit only for now, and reconsider if and when someone complains.