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

fix python lib if no vulkan env #5860

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

Baiyuetribe
Copy link
Contributor

@Baiyuetribe Baiyuetribe commented Dec 31, 2024

#5857
修复在mac设备,vulkan静态库未定义时,默认的vulkan开启会导致依赖vk。

@Baiyuetribe Baiyuetribe changed the title fix python lib with vulkan env fix python lib if no vulkan env Jan 6, 2025
@nihui nihui requested a review from Copilot February 8, 2025 03:00

Choose a reason for hiding this comment

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

Copilot reviewed 1 out of 1 changed files in this pull request and generated no comments.

Comments suppressed due to low confidence (1)

setup.py:96

  • The new behavior introduced by the conditional enable_vulkan variable should be covered by tests to ensure it works as expected.
enable_vulkan = "ON"
Copy link
Member

@nihui nihui left a comment

Choose a reason for hiding this comment

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

diff中不应有空行与引号的修改

Copy link
Member

@nihui nihui left a comment

Choose a reason for hiding this comment

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

有关这个Vulkan_LIBRARY环境变量的设置,需要在 python/README.md 文档中提供帮助

@Baiyuetribe
Copy link
Contributor Author

Baiyuetribe commented Feb 8, 2025

  1. 谁会没事去增删空格和引号?背后的逻辑:ncnn的格式化ci没有覆盖这个py文件,另一个根本原因,根目录下的pyproject.toml定义函数,没有约定缩进和单双引号规则。严重失责
  2. Vulkan_LIBRARY此处的定义为环境变量,当环境变量缺失,就可以只编译cpu版的ncnn库。但原代码强制开启GPU编译,最终导出ncnn库依赖找不到的vk. 该bug很明显呀 参见[BUG] 本地编译python库后,无法正常使用 #5857

@Baiyuetribe Baiyuetribe requested a review from nihui February 8, 2025 04:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants