Re: [cmake-developers] Questions about coding conventions
On Sun, Jun 12, 2016 at 4:26 PM, Ben Boeckel wrote: > On Fri, Jun 10, 2016 at 15:30:05 +0200, Daniel Pfeifer wrote: >> Passing and returning strings: We have `const char*`, `std::string`, >> and `std::string const&`. Unifying this can affect performance. >> Storing `const char*` in a `std::string` creates a (potentially >> unneded) copy (assuming it is not null). > > I went through and changed all `const char*` paramters to `std::string > const&` where the pointer was given to a string within the function > (unless it was an error path or such). This allows eliding another > allocation. I am aware of that. In case of passing `foo.c_str()`, this moved the creation of the unnecessary copy from inside the function to the call site, resulting in more allocations rather than less, assuming the function is called more than once. It is important to run clang-tidy after each such refactoring to remove the now unnecessary c_str() calls. >> `const char*` can distinguish >> invalid from empty. Do we actually need this distinction? > > It's used all over the place. Can you show an example? To be clear: We are looking for a function, that has a code path for `str == NULL` and a *different* codepath for `str[0] = '\0'`. -- Powered by www.kitware.com Please keep messages on-topic and check the CMake FAQ at: http://www.cmake.org/Wiki/CMake_FAQ Kitware offers various services to support the CMake community. For more information on each offering, please visit: CMake Support: http://cmake.org/cmake/help/support.html CMake Consulting: http://cmake.org/cmake/help/consulting.html CMake Training Courses: http://cmake.org/cmake/help/training.html Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html Follow this link to subscribe/unsubscribe: http://public.kitware.com/mailman/listinfo/cmake-developers
[cmake-developers] Dogfooding: clang-tidy, include-what-you-use, link-what-you-use
Hi, now that we have integrated include-what-you-use, clang-tidy, and just recently also a mechanism for link-what-you-use, it is time we start eating our own dog food. https://en.wikipedia.org/wiki/Eating_your_own_dog_food I have set up a dashboard build where all three mechanisms are enabled: https://open.cdash.org/viewBuildError.php?type=1&buildid=4408215 The buildscript can be seen here: https://github.com/purpleKarrot/dotfiles/blob/master/config/cmake/ci_cmake.cmake I have already drastically reduced the number of clang-tidy warnings in the last weeks. I am currently experimenting with include-what-you-use and will push some changes in the coming days. Who wants to resolve the link-what-you-use violations? cheers, Daniel -- Powered by www.kitware.com Please keep messages on-topic and check the CMake FAQ at: http://www.cmake.org/Wiki/CMake_FAQ Kitware offers various services to support the CMake community. For more information on each offering, please visit: CMake Support: http://cmake.org/cmake/help/support.html CMake Consulting: http://cmake.org/cmake/help/consulting.html CMake Training Courses: http://cmake.org/cmake/help/training.html Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html Follow this link to subscribe/unsubscribe: http://public.kitware.com/mailman/listinfo/cmake-developers
Re: [cmake-developers] FindFreetype patch for Windows debug lib naming
On 6/12/2016 12:33 PM, Rolf Eike Beer wrote:
Am Freitag, 10. Juni 2016, 09:03:13 schrieb Brad King:
On 06/10/2016 03:19 AM, Stuart Mentzer wrote:
On 6/10/2016 2:45 AM, Rolf Eike Beer wrote:
I'm sure I also wrote "you probably need to set/unset
FREETYPE_LIBRARY_RELEASE around SLC so it still works",
The point here is that you can't change the variable name
that is used in find_library() as it would break compatibility.
Is there someone who can just do what is needed and let us be done with
this?
Eike, IIRC what we've done for other modules is change the cache
entries to use ${pkg}_LIBRARY_{RELEASE,DEBUG} and then provided
compatibility with scripts that explicitly set ${pkg}_LIBRARY
by using the value if set and skipping the search. See FindZLIB
for an example of this.
I'll hack something together, but that might take a day or two.
Greetings,
Eike
Looking at FindZlib.cmake I think the attached should be OK and it is working
for my use case. It goes back to setting up FREETYPE_LIBRARY_RELEASE variable
and letting SLC set up FREETYPE_LIBRARY, as my first try and FindZlib do. It
also lets SLC do the mark_as_advanced for the _RELEASE and _DEBUG vars. If that
seems good to you both I'm willing to do another patch submission.
Thanks,
Stuart
#.rst:
# FindFreetype
#
#
# Locate FreeType library
#
# This module defines
#
# ::
#
# FREETYPE_LIBRARIES, the library to link against
# FREETYPE_FOUND, if false, do not try to link to FREETYPE
# FREETYPE_INCLUDE_DIRS, where to find headers.
# FREETYPE_VERSION_STRING, the version of freetype found (since CMake 2.8.8)
# This is the concatenation of the paths:
# FREETYPE_INCLUDE_DIR_ft2build
# FREETYPE_INCLUDE_DIR_freetype2
#
#
#
# $FREETYPE_DIR is an environment variable that would correspond to the
# ./configure --prefix=$FREETYPE_DIR used in building FREETYPE.
#=
# Copyright 2007-2009 Kitware, Inc.
#
# Distributed under the OSI-approved BSD License (the "License");
# see accompanying file Copyright.txt for details.
#
# This software is distributed WITHOUT ANY WARRANTY; without even the
# implied warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
# See the License for more information.
#=
# (To distribute this file outside of CMake, substitute the full
# License text for the above reference.)
# Created by Eric Wing.
# Modifications by Alexander Neundorf.
# This file has been renamed to "FindFreetype.cmake" instead of the correct
# "FindFreeType.cmake" in order to be compatible with the one from KDE4, Alex.
# Ugh, FreeType seems to use some #include trickery which
# makes this harder than it should be. It looks like they
# put ft2build.h in a common/easier-to-find location which
# then contains a #include to a more specific header in a
# more specific location (#include ).
# Then from there, they need to set a bunch of #define's
# so you can do something like:
# #include FT_FREETYPE_H
# Unfortunately, using CMake's mechanisms like include_directories()
# wants explicit full paths and this trickery doesn't work too well.
# I'm going to attempt to cut out the middleman and hope
# everything still works.
set(FREETYPE_FIND_ARGS
HINTS
ENV FREETYPE_DIR
PATHS
/usr/X11R6
/usr/local/X11R6
/usr/local/X11
/usr/freeware
ENV GTKMM_BASEPATH
[HKEY_CURRENT_USER\\SOFTWARE\\gtkmm\\2.4;Path]
[HKEY_LOCAL_MACHINE\\SOFTWARE\\gtkmm\\2.4;Path]
)
find_path(
FREETYPE_INCLUDE_DIR_ft2build
ft2build.h
${FREETYPE_FIND_ARGS}
PATH_SUFFIXES
include/freetype2
include
freetype2
)
find_path(
FREETYPE_INCLUDE_DIR_freetype2
NAMES
freetype/config/ftheader.h
config/ftheader.h
${FREETYPE_FIND_ARGS}
PATH_SUFFIXES
include/freetype2
include
freetype2
)
if(NOT FREETYPE_LIBRARY)
find_library(FREETYPE_LIBRARY_RELEASE
NAMES
freetype
libfreetype
freetype219
${FREETYPE_FIND_ARGS}
PATH_SUFFIXES
lib
)
find_library(FREETYPE_LIBRARY_DEBUG
NAMES
freetyped
libfreetyped
freetype219d
${FREETYPE_FIND_ARGS}
PATH_SUFFIXES
lib
)
include(${CMAKE_CURRENT_LIST_DIR}/SelectLibraryConfigurations.cmake)
select_library_configurations(FREETYPE)
endif()
unset(FREETYPE_FIND_ARGS)
# set the user variables
if(FREETYPE_INCLUDE_DIR_ft2build AND FREETYPE_INCLUDE_DIR_freetype2)
set(FREETYPE_INCLUDE_DIRS
"${FREETYPE_INCLUDE_DIR_ft2build};${FREETYPE_INCLUDE_DIR_freetype2}")
list(REMOVE_DUPLICATES FREETYPE_INCLUDE_DIRS)
endif()
set(FREETYPE_LIBRARIES "${FREETYPE_LIBRARY}")
if(EXISTS "${FREETYPE_INCLUDE_DIR_freetype2}/freetype/freetype.h")
set(FREETYPE_H "${FREETYPE_INCLUDE_DIR_freetype2}/freetype/freetype.h")
elseif(EXISTS "${FREETYPE_INCLUDE_DIR_freetype2}/freetype.h")
set(FREETYPE_H "${FREETYPE_INCLUDE_DIR_freetype2}/freetype.h")
endif()
if(FREETYPE_INCLUDE_DIR_fre
Re: [cmake-developers] FindFreetype patch for Windows debug lib naming
Am Freitag, 10. Juni 2016, 09:03:13 schrieb Brad King:
> On 06/10/2016 03:19 AM, Stuart Mentzer wrote:
> > On 6/10/2016 2:45 AM, Rolf Eike Beer wrote:
> >> I'm sure I also wrote "you probably need to set/unset
> >> FREETYPE_LIBRARY_RELEASE around SLC so it still works",
> >> The point here is that you can't change the variable name
> >> that is used in find_library() as it would break compatibility.
> >
> > Is there someone who can just do what is needed and let us be done with
> > this?
> Eike, IIRC what we've done for other modules is change the cache
> entries to use ${pkg}_LIBRARY_{RELEASE,DEBUG} and then provided
> compatibility with scripts that explicitly set ${pkg}_LIBRARY
> by using the value if set and skipping the search. See FindZLIB
> for an example of this.
I'll hack something together, but that might take a day or two.
Greetings,
Eike
signature.asc
Description: This is a digitally signed message part.
--
Powered by www.kitware.com
Please keep messages on-topic and check the CMake FAQ at:
http://www.cmake.org/Wiki/CMake_FAQ
Kitware offers various services to support the CMake community. For more
information on each offering, please visit:
CMake Support: http://cmake.org/cmake/help/support.html
CMake Consulting: http://cmake.org/cmake/help/consulting.html
CMake Training Courses: http://cmake.org/cmake/help/training.html
Visit other Kitware open-source projects at
http://www.kitware.com/opensource/opensource.html
Follow this link to subscribe/unsubscribe:
http://public.kitware.com/mailman/listinfo/cmake-developers
Re: [cmake-developers] Questions about coding conventions
On Fri, Jun 10, 2016 at 15:30:05 +0200, Daniel Pfeifer wrote: > Passing and returning strings: We have `const char*`, `std::string`, > and `std::string const&`. Unifying this can affect performance. > Storing `const char*` in a `std::string` creates a (potentially > unneded) copy (assuming it is not null). I went through and changed all `const char*` paramters to `std::string const&` where the pointer was given to a string within the function (unless it was an error path or such). This allows eliding another allocation. > `const char*` can distinguish > invalid from empty. Do we actually need this distinction? It's used all over the place. --Ben -- Powered by www.kitware.com Please keep messages on-topic and check the CMake FAQ at: http://www.cmake.org/Wiki/CMake_FAQ Kitware offers various services to support the CMake community. For more information on each offering, please visit: CMake Support: http://cmake.org/cmake/help/support.html CMake Consulting: http://cmake.org/cmake/help/consulting.html CMake Training Courses: http://cmake.org/cmake/help/training.html Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html Follow this link to subscribe/unsubscribe: http://public.kitware.com/mailman/listinfo/cmake-developers
[cmake-developers] [PATCH] Allow LIB_SUFFIX be used as find path
Inspired by commit 896ad25 for bug #11260, this commit allows to use the
variable LIB_SUFFIX to be used as find path as well. Allowing the find
path to be more deterministic on custom setups.
A similar idea has already been suggested in #10287 and is required for
bug #15594.
---
Help/command/find_library.rst | 5
Help/manual/cmake-properties.7.rst | 1 +
Modules/FindPkgConfig.cmake| 5 +++-
Source/cmFindLibraryCommand.cxx| 27 ++
Source/cmFindPackageCommand.cxx| 32 +++---
Source/cmFindPackageCommand.h | 1 +
Tests/CMakeOnly/find_library/CMakeLists.txt| 14 ++
.../CMakeOnly/find_library/lib/A/libXYZ/libtest3.a | 0
Tests/CMakeOnly/find_library/lib/XYZ/libtest2.a| 0
.../CMakeOnly/find_library/libXYZ/A/lib/libtest2.a | 0
.../find_library/libXYZ/A/libXYZ/libtest1.a| 0
Tests/CMakeOnly/find_library/libXYZ/A/libtest1.a | 0
Tests/CMakeOnly/find_library/libXYZ/libtest1.a | 0
13 files changed, 62 insertions(+), 23 deletions(-)
create mode 100644 Tests/CMakeOnly/find_library/lib/A/libXYZ/libtest3.a
create mode 100644 Tests/CMakeOnly/find_library/lib/XYZ/libtest2.a
create mode 100644 Tests/CMakeOnly/find_library/libXYZ/A/lib/libtest2.a
create mode 100644 Tests/CMakeOnly/find_library/libXYZ/A/libXYZ/libtest1.a
create mode 100644 Tests/CMakeOnly/find_library/libXYZ/A/libtest1.a
create mode 100644 Tests/CMakeOnly/find_library/libXYZ/libtest1.a
diff --git a/Help/command/find_library.rst b/Help/command/find_library.rst
index 1eb50f7..66eb401 100644
--- a/Help/command/find_library.rst
+++ b/Help/command/find_library.rst
@@ -49,6 +49,11 @@ path to the framework ``/A.framework``. When a
full path to a
framework is used as a library, CMake will use a ``-framework A``, and a
``-F`` to link the framework to the target.
+If the :prop_gbl:`FIND_LIBRARY_USE_CUSTOM_SUFFIX` global property is set
+all search paths will be tested as normal, with `LIB_SUFFIX` appended, and
+with all matches of ``lib/`` replaced with `lib${LIB_SUFFIX}/`. This property
+overrides both `FIND_LIBRARY_USE_LIB32_PATHS` and
`FIND_LIBRARY_USE_LIB64_PATHS`.
+
If the :prop_gbl:`FIND_LIBRARY_USE_LIB32_PATHS` global property is set
all search paths will be tested as normal, with ``32/`` appended, and
with all matches of ``lib/`` replaced with ``lib32/``. This property is
diff --git a/Help/manual/cmake-properties.7.rst
b/Help/manual/cmake-properties.7.rst
index d16f231..128f180 100644
--- a/Help/manual/cmake-properties.7.rst
+++ b/Help/manual/cmake-properties.7.rst
@@ -24,6 +24,7 @@ Properties of Global Scope
/prop_gbl/DISABLED_FEATURES
/prop_gbl/ENABLED_FEATURES
/prop_gbl/ENABLED_LANGUAGES
+ /prop_gbl/FIND_LIBRARY_USE_CUSTOM_SUFFIX
/prop_gbl/FIND_LIBRARY_USE_LIB32_PATHS
/prop_gbl/FIND_LIBRARY_USE_LIB64_PATHS
/prop_gbl/FIND_LIBRARY_USE_OPENBSD_VERSIONING
diff --git a/Modules/FindPkgConfig.cmake b/Modules/FindPkgConfig.cmake
index 33290c4..75fbef8 100644
--- a/Modules/FindPkgConfig.cmake
+++ b/Modules/FindPkgConfig.cmake
@@ -311,7 +311,10 @@ macro(_pkg_check_modules_internal _is_required _is_silent
_no_cmake_path _no_cma
if(NOT DEFINED CMAKE_SYSTEM_NAME
OR (CMAKE_SYSTEM_NAME MATCHES "^(Linux|kFreeBSD|GNU)$"
AND NOT CMAKE_CROSSCOMPILING))
-if(EXISTS "/etc/debian_version") # is this a debian system ?
+get_property(uselibcustom GLOBAL PROPERTY
FIND_LIBRARY_USE_CUSTOM_SUFFIX)
+if(uselibcustom)
+ list(APPEND _lib_dirs "lib${LIB_SUFFIX}/pkgconfig")
+elseif(EXISTS "/etc/debian_version") # is this a debian system ?
if(CMAKE_LIBRARY_ARCHITECTURE)
list(APPEND _lib_dirs
"lib/${CMAKE_LIBRARY_ARCHITECTURE}/pkgconfig")
endif()
diff --git a/Source/cmFindLibraryCommand.cxx b/Source/cmFindLibraryCommand.cxx
index 3094fcf..d86137a 100644
--- a/Source/cmFindLibraryCommand.cxx
+++ b/Source/cmFindLibraryCommand.cxx
@@ -40,20 +40,23 @@ bool
cmFindLibraryCommand::InitialPass(std::vector const& argsIn,
return true;
}
+ // add custom lib${LIB_SUFFIX} paths instead of defaults
if (this->Makefile->GetState()->GetGlobalPropertyAsBool(
-"FIND_LIBRARY_USE_LIB32_PATHS")) {
-// add special 32 bit paths if this is a 32 bit compile.
-if (this->Makefile->PlatformIs32Bit()) {
- this->AddArchitecturePaths("32");
-}
+"FIND_LIBRARY_USE_CUSTOM_SUFFIX") &&
+ this->Makefile->GetDefinition("LIB_SUFFIX")) {
+this->AddArchitecturePaths(this->Makefile->GetDefinition("LIB_SUFFIX"));
}
-
- if (this->Makefile->GetState()->GetGlobalPropertyAsBool(
-"FIND_LIBRARY_USE_LIB64_PATHS")) {
-// add special 64 bit paths if this is a 64 bit compile.
-if (this->Makefile->PlatformIs64Bit()) {
- this->AddArchitecturePaths("64");
-}
+ // add special 32 bit paths if this is a 32 bit
