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

Python: If headeronly is set, links is not needed #5403

Open
wants to merge 4 commits into
base: dev
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 23 additions & 1 deletion packages/p/pybind11/xmake.lua
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,29 @@ package("pybind11")
add_versions("v2.12.0", "411f77380c43798506b39ec594fc7f2b532a13c4db674fcf2b1ca344efaefb68")
add_versions("v2.13.1", "a3c9ea1225cb731b257f2759a0c12164db8409c207ea5cf851d4b95679dda072")

add_deps("cmake", "python 3.x")
-- On Windows, pybind11 must be linked with python library.
-- But on Linux, Python Packaging Authority recommends using headeronly mode for C extensions,
-- and linking libpythonX.Y.so is not allowed on manylinux, which is the base image for distributing C extensions.

-- see https://gitlab.kitware.com/cmake/cmake/-/issues/20425 and
-- https://peps.python.org/pep-0513/#libpythonx-y-so-1
myuanz marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Member

Choose a reason for hiding this comment

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

还是太长,全删了,只需要保留一个 issue link


Copy link
Member

Choose a reason for hiding this comment

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

空行去了

add_configs("use_python_headeronly", {description = "Use python headeronly", default = false, type = "boolean"})
myuanz marked this conversation as resolved.
Show resolved Hide resolved

add_deps("cmake")
myuanz marked this conversation as resolved.
Show resolved Hide resolved
on_load(function (package)
local python_headeronly = package:config("use_python_headeronly")

myuanz marked this conversation as resolved.
Show resolved Hide resolved
if os.host() == "windows" then
myuanz marked this conversation as resolved.
Show resolved Hide resolved
print("Python headeronly is not supported on Windows")
myuanz marked this conversation as resolved.
Show resolved Hide resolved
python_headeronly = false
Copy link
Member

Choose a reason for hiding this comment

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

如果 windows 不支持,应该 add_configs 时候,直接设置为 readonly = true, default = false 锁定住,原本就会自动给 warning 。。不用额外检测

end

package:add("deps", "python 3.x", {configs = {
headeronly = python_headeronly,
}})
Copy link
Member

Choose a reason for hiding this comment

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

改成一行

end)

on_install("windows|native", "macosx", "linux", function (package)
import("package.tools.cmake").install(package, {"-DPYBIND11_TEST=OFF"})
end)
Expand Down
25 changes: 16 additions & 9 deletions packages/p/python/fetch.lua
Original file line number Diff line number Diff line change
Expand Up @@ -66,17 +66,24 @@ function _find_library(package, opt)
includepath = find_path("Python.h", { path.directory(exepath), out }, { suffixes = { "include/python" .. pyver } })
end

if libpath and includepath then
local result = {
version = version,
includedirs = includepath
}
if not package:config("headeronly") then
result.links = libpath.link
result.linkdirs = libpath.linkdir
end
if not includepath then
return false
Copy link
Member

Choose a reason for hiding this comment

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

不支持返回 false,直接返回 return 就行了

end
local result = {
version = version,
includedirs = includepath
}
-- If headeronly is set, links is not needed
myuanz marked this conversation as resolved.
Show resolved Hide resolved

if package:config("headeronly") then
return result
end
if libpath then
result.links = libpath.link
result.linkdirs = libpath.linkdir
return result
end
return false
myuanz marked this conversation as resolved.
Show resolved Hide resolved
end

function main(package, opt)
Expand Down
Loading