Skip to content

ci: support Vulkan build on Windows #1726

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 6 commits into from
Mar 26, 2024
Merged

ci: support Vulkan build on Windows #1726

merged 6 commits into from
Mar 26, 2024

Conversation

vgf89
Copy link
Contributor

@vgf89 vgf89 commented Mar 26, 2024 •
edited
Loading

I tried compiling the Vulkan feature on Windows, but was running into the vulkan library not being found by the build script. This is the fix. Now I can run tabby on my AMD 5700XT on Windows. It's seems quite fast too!

Changes:

  • Remove =native in vulkan feature in build.rs so that all library kinds are part of the search. This shouldn't break the Linux builds, but if it does CI should catch it.
  • Add a CI step to create a Windows Vulkan build automatically. If I'm lucky it'll work first try? 🤞
  • User-facing documentation changes to reflect Vulkan support.

@vgf89 vgf89 marked this pull request as draft March 26, 2024 13:47
@vgf89
Copy link
Contributor Author

vgf89 commented Mar 26, 2024 •
edited
Loading

Ah, CI failed, yeah I had to copy the vulkan-1.lib to vulkan.lib on my machine. I need to see if I can fix issue build-side instead of manually

@vgf89
Copy link
Contributor Author

vgf89 commented Mar 26, 2024 •
edited
Loading

It looks like the vulkan build is currently broken on Linux CI due to an upstream issue with the manylinux docker container. It's trying to pull the completely wrong (outdated?) URL format for the vulkan sdk.

@wsxiaoys
Copy link
Member

cc @icycodes

@icycodes
Copy link
Member

It looks like the vulkan build is currently broken on Linux CI due to an upstream issue with the manylinux docker container. It's trying to pull the completely wrong (outdated?) URL format for the vulkan sdk.

@vgf89 Thanks, I confirmed that the artifact built by the windows pipeline is working well on my windows machine.
Looking into the issue on the Linux CI pipeline.

@icycodes
Copy link
Member

It looks like the vulkan build is currently broken on Linux CI due to an upstream issue with the manylinux docker container. It's trying to pull the completely wrong (outdated?) URL format for the vulkan sdk.

@vgf89 Thanks, I confirmed that the artifact built by the windows pipeline is working well on my windows machine. Looking into the issue on the Linux CI pipeline.

@vgf89 @wsxiaoys
It appears that the SDK version used in the Linux pipeline has recently been removed from https://vulkan.lunarg.com/sdk/home. The cached download file was being used before this PR.
I believe this PR can be safely merged without considering the failed Linux CI pipeline. I will address this in the next PR.

wsxiaoys
wsxiaoys approved these changes Mar 26, 2024
View reviewed changes
Copy link
Member

@wsxiaoys wsxiaoys left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Otherwise LGTM

@wsxiaoys wsxiaoys marked this pull request as ready for review March 26, 2024 15:19
@wsxiaoys wsxiaoys changed the title fix(tabby): Fix Vulkan build on Windows, Add CI and documentation ci: support Vulkan build on Windows, Add CI and documentation Mar 26, 2024
@wsxiaoys wsxiaoys changed the title ci: support Vulkan build on Windows, Add CI and documentation ci: support Vulkan build on Windows Mar 26, 2024
@wsxiaoys wsxiaoys merged commit d4c9908 into TabbyML:main Mar 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@wsxiaoys wsxiaoys wsxiaoys approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@vgf89 @wsxiaoys @icycodes