Skip to content
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

Windows CI build is broken despite finding Vulkan SDK #470

Closed
azonenberg opened this issue Aug 18, 2022 · 20 comments · Fixed by ngscopeclient/scopehal#686
Closed

Windows CI build is broken despite finding Vulkan SDK #470

azonenberg opened this issue Aug 18, 2022 · 20 comments · Fixed by ngscopeclient/scopehal#686
Labels
bug Something isn't working build Build system and infrastructure

Comments

@azonenberg
Copy link
Collaborator

[code]
-- Could NOT find Vulkan (missing: Vulkan_LIBRARY) (found version "1.3.223")
CMake Error at CMakeLists.txt:61 (message):
No Vulkan SDK found.
[/code]

@azonenberg azonenberg added bug Something isn't working build Build system and infrastructure labels Aug 18, 2022
@stanciuadrian
Copy link
Contributor

stanciuadrian commented Aug 18, 2022

It might not work because of the way environment variables are inherited in the CI.

Let's say process A is the CI runner and during its execution it spawns process B which is the Vulkan SDK installer. B ends by successfully setting global environment variables. Then A spawns the build step as a new process C. C doesn't see the environment variables set by B because it inherits the ones from A.

My suggestion here is to:

  1. force the installation path with the --root command line argument while installing the Vulkan SDK
  2. set the VK_SDK_PATH and VULKAN_SDK to the same path in the MSYS2 build step.

@stanciuadrian
Copy link
Contributor

You may find act useful for quickly iterating the CI changes.

I can't make Docker work on my machine, otherwise I would have helped you more with it.

@Johnsel
Copy link
Contributor

Johnsel commented Aug 19, 2022

I have been able to reproduce this same issue on a local Windows install so I can work on figuring this out tomorrow.

Johnsel added a commit to Johnsel/scopehal-apps that referenced this issue Aug 19, 2022
…ded VULKAN_SDK for the makepkg-mingw command.
@Biswa96
Copy link
Contributor

Biswa96 commented Aug 31, 2022

This workaround may fix the Windows CI build.

--- a/msys2/PKGBUILD
+++ b/msys2/PKGBUILD
@@ -48,6 +48,7 @@ build() {
     -G "Ninja" \
     -DCMAKE_INSTALL_PREFIX="${MINGW_PREFIX}" \
     -DBUILD_TESTING=OFF \
+    -DVulkan_LIBRARY=${MINGW_PREFIX}/lib/libvulkan.dll.a \
     ../
 
   ${MINGW_PREFIX}/bin/cmake.exe --build ./

But this is a workaround till now. The libvulkan.dll.a name may change to libvulkan-1.dll.a in future. There is some debate over what the vulkan DLL and import library should be in mingw environment. Maybe I started that controversy 🙇 Feel free to ping me anytime anywhere.

@azonenberg
Copy link
Collaborator Author

(paging @bvernoux since he's working on this)

@bvernoux
Copy link
Contributor

I have found a fix see https://github.com/bvernoux/scopehal-apps/blob/master/.github/workflows/build.yml
See the build here https://github.com/bvernoux/scopehal-apps/runs/8120159932
I have a last issue is the path to glslc is not found even if I have exported it before
export PATH=/c/VulkanSDK/1.3.224.1/Bin:$PATH
https://github.com/bvernoux/scopehal-apps/runs/8120159932?check_suite_focus=true#step:6:1431
'glslc' is not recognized as an internal or external command,
We shall find a way to set this path in ninja instance env
Note: When I build manually with mingw32-make all work fine on my computer and I can build glscopeclient.exe with success

@Biswa96
Copy link
Contributor

Biswa96 commented Aug 31, 2022

That's OK. The solution is a bit different. The executables may link with the MSVC compiled vulkan libraries. Setting VULKAN_SDK will satisfy this condition in cmake's FindVulkan module.

    set(_Vulkan_hint_library_search_paths
      "$ENV{VULKAN_SDK}/Lib"
      "$ENV{VULKAN_SDK}/Bin"
    )

Whereas, setting Vulkan_LIBRARY will satisfy this condition

find_library(Vulkan_LIBRARY
  NAMES ${_Vulkan_library_name}
  HINTS
    ${_Vulkan_hint_library_search_paths}
  )
mark_as_advanced(Vulkan_LIBRARY)

But both reach to same destination.

@bvernoux
Copy link
Contributor

bvernoux commented Sep 1, 2022

Build fixed see PR #481

@Johnsel
Copy link
Contributor

Johnsel commented Sep 2, 2022

@bvernoux on https://github.com/Johnsel/scopehal-apps/runs/8164338202 glscopeclient --help runs too now. I'm not entirely sure what has fixed it, presumably something related to the errors or the shell. Perhaps you will be able to figure it out with fresh eyes on it.

@bvernoux
Copy link
Contributor

bvernoux commented Sep 4, 2022

@Johnsel Could you create a PR with your modifications ? (that is really better than what I have done and that seems to fix issues with msys2 shell with explicit definition which is always better ...)

@Johnsel
Copy link
Contributor

Johnsel commented Sep 5, 2022

I could, but I am still confused about the build test phase fixing itself with seemingly unrelated changes so I'd like to confirm that we have a fully functional build before we switch over. Is the glslc issue fixed? The missing header I mean.

@bvernoux
Copy link
Contributor

bvernoux commented Sep 7, 2022

glslc is not fixed as the required headers are not present in Windows VulkanSDK but present for Linux (an issue shall be created for VulkanSDK to ask they add those headers and static lib if some are missing)
The fix for Linux is easy and I have it which build fine and run fine on Ubuntu 22.04 LTS (requires to add 2 lines in the Cmake)
I'm searching a way to also fix the windows build but so far it will requires to clone glslc git and rebuild it

@azonenberg
Copy link
Collaborator Author

Is it possible that on Windows we might have to install https://github.com/KhronosGroup/glslang/ separately from the SDK?

@bvernoux
Copy link
Contributor

bvernoux commented Sep 7, 2022

Is it possible that on Windows we might have to install https://github.com/KhronosGroup/glslang/ separately from the SDK?

Yes so far it seems to be the only way to do it until VulkanSDK add it (maybe that also impact OSX to be checked)

@Johnsel
Copy link
Contributor

Johnsel commented Sep 7, 2022

Do we need to find this header in the VulkanSDK (or glslang) though? It seems that the mingw package that supplies glslc should provide this, no? For some reason it does not properly add that path to the -I includes though. I can work on it some more tomorrow,

@Johnsel
Copy link
Contributor

Johnsel commented Sep 7, 2022

@Johnsel
Copy link
Contributor

Johnsel commented Sep 7, 2022

See one of the first 10-ish includes at https://github.com/glscopeclient/scopehal-apps/runs/8210668826?check_suite_focus=true#step:7:257 it is refering to /usr instead of the MinGW prefixed path

edit:
That specific build does not have the mingw-xx-glslang package, but adding it doesn't solve the problem.
Alternatively it might be an optional module that the Vulkan SDK installer is not installing.

@bvernoux
Copy link
Contributor

bvernoux commented Sep 7, 2022

https://packages.msys2.org/package/mingw-w64-x86_64-glslang this one I mean

Yes it is an other easier (potentially better) solution
I have not fully checked that especially to check if all lib are ok for the link

Johnsel added a commit to Johnsel/scopehal-apps that referenced this issue Sep 7, 2022
@Biswa96
Copy link
Contributor

Biswa96 commented Sep 9, 2022

Update. The mingw-w64 vulkan loader package has been fixed in msys2. The workaround is not required anymore.

@bvernoux
Copy link
Contributor

bvernoux commented Sep 10, 2022

Update. The mingw-w64 vulkan loader package has been fixed in msys2. The workaround is not required anymore.

I do not understand what is your workaround as we are using VulkanSDK and so we shall use VulkanSDK lib and not mix it with mingw64 lib which are probably too old not compatible with VulkanSDK we are using.

  • For information VulkanSDK for Windows is built with Visual Studio 2017 so the static libraries are not usable with mingw64 (but only Visual Studio), it is why we can only use DLL (like vulkan dll).

Anyway so far the actual master have different issues to be fixed

# In Mingw64 shell execute following commands
# Windows glslang (as it is not integrated fully in VulkanSDK-1.3.224.1 for Windows)
cd ~
git clone https://github.com/KhronosGroup/glslang.git
cd glslang
git checkout tags/sdk-1.3.224.1
git clone https://github.com/google/googletest.git External/googletest
cd External/googletest
git checkout 0c400f67fcf305869c5fb113dd296eca266c9725
cd ../..
./update_glslang_sources.py

SOURCE_DIR=~/glslang
BUILD_DIR=$SOURCE_DIR/build
mkdir -p $BUILD_DIR
cd $BUILD_DIR
cmake -DCMAKE_BUILD_TYPE=Release -G"MinGW Makefiles" $SOURCE_DIR -DCMAKE_INSTALL_PREFIX="$(pwd)/install"
cmake --build . --config Release --target install

export GLSLANG_BUILD_PATH=~/glslang/build/install
# scopehal-apps\CMakeLists.txt modifications for Linux(tested with Ubuntu 22.04 LTS) / Windows Mingw64 (WIP)
# To be added after line 
# if(NOT Vulkan_glslc_FOUND)
#	message(FATAL_ERROR "No Vulkan glslc found. This is needed to compile shaders.")
# endif()
# Add specific VulkanSDK Lib & Include for glslang
if(NOT WIN32)
	include_directories($ENV{VULKAN_SDK}/include/glslang/Include)
	message("Vulkan SDK include glslang: $ENV{VULKAN_SDK}/include/glslang/Include")
	link_directories($ENV{VULKAN_SDK}/Lib)
	message("Vulkan SDK lib: $ENV{VULKAN_SDK}/Lib")
endif()
if(WIN32)
# Includes are incomplete in VulkanSDK 1.3.224.1 so we use the include path from MINGW64 mingw-w64-x86_64-glslang
	find_path(Glslang_BUILD_INCLUDE_DIR
	  NAMES include/glslang/Include/glslang_c_interface.h
	  HINTS
		$ENV{GLSLANG_BUILD_PATH}
	  )
	mark_as_advanced(Glslang_BUILD_INCLUDE_DIR)

	if(Glslang_BUILD_INCLUDE_DIR)
		include_directories($ENV{GLSLANG_BUILD_PATH}/include/glslang/Include)
		message("GLSLANG_BUILD_PATH found: $ENV{GLSLANG_BUILD_PATH}/include/glslang/Include")
	else()
		message(FATAL_ERROR "Glslang_BUILD_INCLUDE_DIR Not found")
	endif()
	
	link_directories($ENV{GLSLANG_BUILD_PATH}/lib)
	message("GLSLANG_BUILD_PATH lib: $ENV{GLSLANG_BUILD_PATH}/lib/")

	link_directories($ENV{GLSLANG_BUILD_PATH}/lib/glslang)
	message("GLSLANG_BUILD_PATH lib glslang: $ENV{GLSLANG_BUILD_PATH}/lib/glslang")
endif()
  • glfw (lib/include) was added recently so it shall be added on Windows MINGW64 to be checked with pacman -S mingw-w64-x86_64-glfw then add lib/include in scopehal-apps\CMakeLists.txt...
# scopehal-apps\CMakeLists.txt modifications for Linux(tested with Ubuntu 22.04 LTS) / Windows Mingw64 (WIP)
# To be added after line include(FindOpenMP)
find_package(glfw3 3.2 REQUIRED)
  • How to build with Mingw64 shell (do not include all git clone required just final step and export variables)
export GLSLANG_BUILD_PATH=~/glslang/build/install
export VK_SDK_PATH=/c/VulkanSDK/1.3.224.1
export VULKAN_SDK=/c/VulkanSDK/1.3.224.1
export PATH=/c/VulkanSDK/1.3.224.1/Bin:$PATH
cd ~
cd scopehal-apps
mkdir build
cd build
cmake -G"MinGW Makefiles" -DBUILD_TESTING=OFF -DCMAKE_BUILD_TYPE=Release ..
mingw32-make -j8

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working build Build system and infrastructure
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants