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

Add support for external/system include directories #1017

Closed
SirLynix opened this issue Nov 7, 2020 · 3 comments
Closed

Add support for external/system include directories #1017

SirLynix opened this issue Nov 7, 2020 · 3 comments

Comments

@SirLynix
Copy link
Member

SirLynix commented Nov 7, 2020

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

C and C++ compilers sometimes generate warnings from libraries headers, and we cannot fix them ourselves. For example I have a warning generated from Qt which shows up everytime I'm compiling and I cannot do anything against that (except wait for a fix or disable the warning for the whole file or use some crappy #pragma directives)

However, we can tell the compiler to not generate warnings from libraries headers found in some includes directories, those are called system includes directories and behave the same way as regular include directories (add_includedirs).
Others build system like CMake (SYSTEM) and Premake do support this solution.

For GCC and Clang, this means using -isystem <path> instead of -I <path> when adding those directories (see here).
For MSVC, there's the experimental /external:I <path> command-line (requires /experimental:external and /external:W0), see here and Broken Warnings Theory from Microsoft.

System includes directories behave like regular includes directories except they don't generate warnings (and have the least priority), they can be handled as regular includes directories when not supported by the compiler (They should however be passed after all the regular includes directories).

Describe the solution you'd like

From xmake point of view, adding the add_sysincludedirs would be a start, but I think all dependencies include directories should by default use this (if this bothers you, maybe make it an option?)

Also: maybe external header instead of system header would be a better naming? add_extincludedirs? Or an option to add_includedirs?

@SirLynix SirLynix changed the title Add support for system include directories Add support for external/system include directories Nov 7, 2020
@waruqi
Copy link
Member

waruqi commented Nov 7, 2020

In order to be consistent with the existing add_syslinks naming style, I would consider adding add_sysincludedirs() api to support it, and switch the external package to system includedirs if supported by the compiler.

@waruqi waruqi added this to the v2.3.9 milestone Nov 7, 2020
@waruqi
Copy link
Member

waruqi commented Nov 10, 2020

I have added add_sysincludedirs for target on dev branch.

target("test")
    set_kind("binary")
    add_files("src/*.cpp")
    add_sysincludedirs("sys", "sys2")

And I have switched to -isystem for packages/toolchains by default.

add_requires("libcurl")
target("test")
    set_kind("binary")
    add_files("src/*.cpp")
    add_packages("libcurl") -- we use -isystem/-external:I

And, we can also disable external/system directories for the given packages.

add_requires("libcurl", {external = false}) 
target("test")
    set_kind("binary")
    add_files("src/*.cpp")
    add_packages("libcurl")  -- we use -I/xxx

@waruqi waruqi closed this as completed Nov 10, 2020
@SirLynix
Copy link
Member Author

Thanks a lot!

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

2 participants