dont use global include dir #89
Conversation
|
This is set so when installing header only libraries with |
|
I am just generally concerned with determinism, i.e. that the build of one package is not influenced by the presence of another, especcially for build caching. and also that when in cmake I use one package i dont get to see another "for free" so I can guarnatuee that only the specified dependencies are visible. header only should also be discoverable from the package dir, no? |
|
hmm, I cant find this header thing. -X is for specifying a cmake file, right? Ah, header.cmake :). Hmm, couldnt that produce a findable package? |
|
ok, I looked at header.cmake and tried to make it generate a config file, but I think its a bit too complex, also I dont really need it right now, so I will leave it alone. still i would like to avoid having all headers visible all the time, so I think I will just make this switchable |
This reverts commit 21cd83a.
|
ok, added --no-global-include to init fo this to be optional, ok? |
| @@ -56,7 +56,8 @@ def w(obj, prefix, verbose, build_path, *args, **kwargs): | |||
| @click.option('-D', '--define', multiple=True, help="Extra configuration variables to pass to CMake") | |||
| @click.option('--shared', is_flag=True, help="Set toolchain to build shared libraries by default") | |||
| @click.option('--static', is_flag=True, help="Set toolchain to build static libraries by default") | |||
| def init_command(prefix, toolchain, cc, cxx, cflags, cxxflags, ldflags, std, define, shared, static): | |||
| @click.option('--no-global-include', is_flag=True, help="Don't use global include dir (required for -X header)") | |||
There was a problem hiding this comment.
How about naming this --disable-global-include?
|
Also, are you able to add a test for this? I think a simple test like this just to get coverage could be added in test/test_cget.py: def test_disable_global_include_toolchain(d):
d.cmds([
cget_cmd('init', '--disable-global-include'),
cget_cmd('install', '--verbose --test', get_exists_path('libsimple'))
]) |
DeepCode's analysis on #9a5780 found:
Top issues
👉 View analysis in DeepCode’s Dashboard | Configure the bot |
| @@ -87,8 +90,10 @@ def init_command(prefix, toolchain, cc, cxx, cflags, cxxflags, ldflags, std, def | |||
| @click.option('--debug', is_flag=True, help="Install debug version") | |||
| @click.option('--release', is_flag=True, help="Install release version") | |||
| @click.option('--insecure', is_flag=True, help="Don't use https urls") | |||
| @click.option('--use-build-cache', is_flag=True, help="Cache builds") | |||
There was a problem hiding this comment.
This shouldn't have the build cache changes.
| # TODO: Set also PKG_CONFIG_SYSROOT_DIR | ||
| if(CMAKE_FIND_ROOT_PATH_MODE_PACKAGE STREQUAL "ONLY") | ||
| list(APPEND ${PREFIX}_BASE_ENV_COMMAND "PKG_CONFIG_LIBDIR=${${PREFIX}_PKG_CONFIG_PATH}") | ||
| endif() |
There was a problem hiding this comment.
This shouldn't skip setting pkg config paths. Either way, I would prefer these changes in a seperate PR.
| COMMAND ${CMAKE_CURRENT_SOURCE_DIR}/autogen.sh | ||
| WORKING_DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR} | ||
| ) | ||
| endif (AUTOTOOLS_RUN_AUTOGEN_SH) |
There was a problem hiding this comment.
This should be put in a seperate PR.
| set_ = cmake_set | ||
| if_ = cmake_if | ||
| else_ = cmake_else | ||
| append_ = cmake_append | ||
| yield set_('CGET_PREFIX', self.prefix) | ||
| yield set_('CMAKE_PREFIX_PATH', self.prefix) |
There was a problem hiding this comment.
Extending the CMAKE_PREFIX_PATH should not be removed.
its not neccessary and makes it harder to isolate components