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

check_links, check_cincludes, check_cxxincludes export as private #1720

Closed
idealvin opened this issue Oct 2, 2021 · 11 comments
Closed

check_links, check_cincludes, check_cxxincludes export as private #1720

idealvin opened this issue Oct 2, 2021 · 11 comments

Comments

@idealvin
Copy link

idealvin commented Oct 2, 2021

Is your feature request related to a problem? Please describe.

-- libco
check_cincludes("HAS_BACKTRACE_H", "backtrace.h")
check_cxxincludes("HAS_CXXABI_H", "cxxabi.h")
check_links("HAS_LIBBACKTRACE", "backtrace")

By default, macros and links exported from check_xxx will be public and can be seen in projects depends on that target. At present, we are not able to set them private.

Describe the solution you'd like

check_cincludes("HAS_BACKTRACE_H", "backtrace.h", { public = false })
check_cxxincludes("HAS_CXXABI_H", "cxxabi.h", { public = false })
check_links("HAS_LIBBACKTRACE", "backtrace", { public = false })

or

check_cincludes("HAS_BACKTRACE_H", "backtrace.h", { private = true })
check_cxxincludes("HAS_CXXABI_H", "cxxabi.h", { private = true })
check_links("HAS_LIBBACKTRACE", "backtrace", { private = true })

So the macros and links can only be seen in the current target.

@idealvin
Copy link
Author

idealvin commented Oct 2, 2021

Here is the cmake-way:

check_include_files(backtrace.h HAS_BACKTRACE)
check_include_file_cxx(cxxabi.h HAS_CXXABI)
if(HAS_BACKTRACE)
    target_compile_definitions(co PRIVATE HAS_BACKTRACE_H)
    target_link_libraries(co PRIVATE backtrace)
endif()
if(HAS_CXXABI)
    target_compile_definitions(co PRIVATE HAS_CXXABI_H)
endif()

@xq114
Copy link
Contributor

xq114 commented Oct 2, 2021

option("HAS_BACKTRACE") add_cincludes("backtrace.h")
option("HAS_BACKTRACE_LIB") add_clinks("backtrace")
option("HAS_CXXABI") add_cxxincludes("cxxabi.h")

target("co")
if has_config("HAS_BACKTRACE") and has_config("HAS_BACKTRACE_LIB") then
    add_defines("HAS_BACKTRACE_H")
    add_syslinks("backtrace")
end
if has_config("HAS_CXXABI") then
    add_defines("HAS_CXXABI_H", {public = true})
end

I'm not sure if the following simpler code works

option("HAS_BACKTRACE") add_cincludes("backtrace.h")
option("HAS_BACKTRACE_LIB") add_clinks("backtrace")
option("HAS_CXXABI")
    add_cxxincludes("cxxabi.h")
    add_defines("HAS_CXXABI_H", {public = true})

target("co")
	if has_config("HAS_BACKTRACE") and has_config("HAS_BACKTRACE_LIB") then
	    add_defines("HAS_BACKTRACE_H")
	    add_syslinks("backtrace")
	end
	add_options("HAS_CXXABI")

@idealvin
Copy link
Author

idealvin commented Oct 2, 2021

@xq114

I have tried to modify the check_xxx.lua. It seems that config in options does not work.

function check_cincludes(definition, includes, opt)
    opt = opt or {}
    local optname = "__" .. (opt.name or definition)
    local is_public = opt.public or false
    option(optname)
        add_cincludes(includes)
        add_defines(definition, { public = is_public })
    option_end()
    add_options(optname)
end

@xq114
Copy link
Contributor

xq114 commented Oct 2, 2021

Maybe currently option.add_defines() does not support extra configs. @waruqi can extra configs be added to option.add_defines(), and a set of other apis?

As for now, you can try the former solution, which does not need the config-in-option feature

@waruqi
Copy link
Member

waruqi commented Oct 2, 2021

都是国人,就没必要 en 文了,我回复好累。

这个主要原因是 check_xxx 辅助接口,内部用了 option() 域定义

而所有 target/option 等域定义都会自动关闭上个域,进入当前域。

也就是说进入 option() 后,option_end 退出就自动进入 root 域了,所以 check_xxx 在 target 里面设置好,之后的 add_options 其实是被设置在 root 域下,没对当前 target 生效

另外也会干扰后面用户自己的 add_defines 等对 target 的设置。。

所以,也算是个 bug ,目前只能全局生效,对特定 target 设置 check_xxx 就会有问题。。

我刚弄了个 patch ,通过新开放的 save_scope/restore_scope 接口,也保存之前的域访问,这对于一些封装性的自定义接口也会有用

可以自己先测试下 scope 分支,应该可以了的。

@waruqi
Copy link
Member

waruqi commented Oct 2, 2021

option.add_defines 等接口目前不考虑加 public ,因为只有 add_options 到特定 target 才会生效。我记得 add_options 是支持 public 设置的,目前只能到这个粒度

另外,现在这个 issue 的本质问题就是之前说的 scope 重入中断问题,跟是否 public 没什么关系

当然,你可以不走 check_xxx,自己定义 option 去配置 check 设置到特定 target 也就解决了。

@waruqi waruqi added the bug label Oct 2, 2021
@waruqi waruqi added this to the v2.5.8 milestone Oct 2, 2021
@idealvin
Copy link
Author

idealvin commented Oct 2, 2021 via email

@idealvin
Copy link
Author

idealvin commented Oct 2, 2021 via email

@waruqi
Copy link
Member

waruqi commented Oct 2, 2021

option("HAS_BACKTRACE") add_cincludes("backtrace.h")
option("HAS_BACKTRACE_LIB") add_clinks("backtrace")
option("HAS_CXXABI") add_cxxincludes("cxxabi.h")

target("co")
if has_config("HAS_BACKTRACE") and has_config("HAS_BACKTRACE_LIB") then
    add_defines("HAS_BACKTRACE_H")
    add_syslinks("backtrace")
end
if has_config("HAS_CXXABI") then
    add_defines("HAS_CXXABI_H", {public = true})
end

I'm not sure if the following simpler code works

option("HAS_BACKTRACE") add_cincludes("backtrace.h")
option("HAS_BACKTRACE_LIB") add_clinks("backtrace")
option("HAS_CXXABI")
    add_cxxincludes("cxxabi.h")
    add_defines("HAS_CXXABI_H", {public = true})

target("co")
	if has_config("HAS_BACKTRACE") and has_config("HAS_BACKTRACE_LIB") then
	    add_defines("HAS_BACKTRACE_H")
	    add_syslinks("backtrace")
	end
	add_options("HAS_CXXABI")

@idealvin 你可以按照这个方式自己配置,就跟 cmake 一样了,这跟 option 机制没关系,它本身不会绑定域

check_xxx 只是辅助接口,目前自带 defines 启用,所以设置上更加简化,只需要一行,而不是像 cmake 一样,每个检测,要加一堆 if add

@waruqi
Copy link
Member

waruqi commented Oct 2, 2021

includes里面的 check_xxx 都是辅助接口,只是利用了 option 的特性,如果你觉得不想自带绑定 target

那么,可以自己参考 check_xxx 实现,封装自己的辅助接口,直接基于 option 搞。

@idealvin
Copy link
Author

idealvin commented Oct 3, 2021

option("HAS_BACKTRACE") add_cincludes("backtrace.h")
option("HAS_BACKTRACE_LIB") add_clinks("backtrace")
option("HAS_CXXABI") add_cxxincludes("cxxabi.h")

target("co")
if has_config("HAS_BACKTRACE") and has_config("HAS_BACKTRACE_LIB") then
    add_defines("HAS_BACKTRACE_H")
    add_syslinks("backtrace")
end
if has_config("HAS_CXXABI") then
    add_defines("HAS_CXXABI_H", {public = true})
end

I'm not sure if the following simpler code works

option("HAS_BACKTRACE") add_cincludes("backtrace.h")
option("HAS_BACKTRACE_LIB") add_clinks("backtrace")
option("HAS_CXXABI")
    add_cxxincludes("cxxabi.h")
    add_defines("HAS_CXXABI_H", {public = true})

target("co")
	if has_config("HAS_BACKTRACE") and has_config("HAS_BACKTRACE_LIB") then
	    add_defines("HAS_BACKTRACE_H")
	    add_syslinks("backtrace")
	end
	add_options("HAS_CXXABI")

@idealvin 你可以按照这个方式自己配置,就跟 cmake 一样了,这跟 option 机制没关系,它本身不会绑定域

check_xxx 只是辅助接口,目前自带 defines 启用,所以设置上更加简化,只需要一行,而不是像 cmake 一样,每个检测,要加一堆 if add

这个可以,我用这个吧

@idealvin idealvin closed this as completed Oct 3, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants