Re: [gentoo-dev] [PATCH 1/2] cmake-utils.eclass: CMake argument passing rework - clean build_rules and toolchain_file files from unrelated stuff (pass to CMake directly) - move some invariant CMake options actually to invariants parts - eliminate common_config (pass to CMake directly) for better transparency

[Maciej Mrozowski <reavertm@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 6118 bytes --]

On czwartek, 3 listopada 2016 07:31:10 CET Michał Górny wrote:
> On Thu,  3 Nov 2016 00:52:16 +0100
> 
> Maciej Mrozowski <reavertm@gmail.com> wrote:
> > From: Maciej Mrozowski <reavertm@gentoo.org>
> > 
> > ---
> > 
> >  eclass/cmake-utils.eclass | 54
> >  ++++++++++++++++++----------------------------- 1 file changed, 21
> >  insertions(+), 33 deletions(-)
> > 
> > diff --git a/eclass/cmake-utils.eclass b/eclass/cmake-utils.eclass
> > index 393ee28..88d2163 100644
> > --- a/eclass/cmake-utils.eclass
> > +++ b/eclass/cmake-utils.eclass
> > @@ -517,13 +517,10 @@ enable_cmake-utils_src_configure() {
> > 
> >  		includes="<INCLUDES>"
> >  	
> >  	fi
> >  	cat > "${build_rules}" <<- _EOF_ || die
> > 
> > -		SET (CMAKE_AR $(type -P $(tc-getAR)) CACHE FILEPATH "Archive 
manager"
> > FORCE)> 
> >  		SET (CMAKE_ASM_COMPILE_OBJECT "<CMAKE_C_COMPILER> <DEFINES> $
{includes}
> >  		${CFLAGS} <FLAGS> -o <OBJECT> -c <SOURCE>" CACHE STRING "ASM 
compile
> >  		command" FORCE) SET (CMAKE_C_COMPILE_OBJECT "<CMAKE_C_COMPILER>
> >  		<DEFINES> ${includes} ${CPPFLAGS} <FLAGS> -o <OBJECT> -c 
<SOURCE>"
> >  		CACHE STRING "C compile command" FORCE) SET 
(CMAKE_CXX_COMPILE_OBJECT
> >  		"<CMAKE_CXX_COMPILER> <DEFINES> ${includes} ${CPPFLAGS} <FLAGS> 
-o
> >  		<OBJECT> -c <SOURCE>" CACHE STRING "C++ compile command" FORCE) 
SET
> >  		(CMAKE_Fortran_COMPILE_OBJECT "<CMAKE_Fortran_COMPILER> 
<DEFINES>
> >  		${includes} ${FCFLAGS} <FLAGS> -o <OBJECT> -c <SOURCE>" CACHE 
STRING
> >  		"Fortran compile command" FORCE)> 
> > -		SET (CMAKE_RANLIB $(type -P $(tc-getRANLIB)) CACHE FILEPATH 
"Archive
> > index generator" FORCE) -		SET (PKG_CONFIG_EXECUTABLE $(type -P
> > $(tc-getPKG_CONFIG)) CACHE FILEPATH "pkg-config executable" FORCE)> 
> >  	_EOF_
> >  	
> >  	local toolchain_file=${BUILD_DIR}/gentoo_toolchain.cmake
> > 
> > @@ -531,6 +528,8 @@ enable_cmake-utils_src_configure() {
> > 
> >  		SET (CMAKE_C_COMPILER $(tc-getCC))
> >  		SET (CMAKE_CXX_COMPILER $(tc-getCXX))
> >  		SET (CMAKE_Fortran_COMPILER $(tc-getFC))
> > 
> > +		SET (CMAKE_AR $(type -P $(tc-getAR)) CACHE FILEPATH "Archive 
manager"
> > FORCE) +		SET (CMAKE_RANLIB $(type -P $(tc-getRANLIB)) CACHE FILEPATH
> > "Archive index generator" FORCE)> 
> >  	_EOF_
> >  	
> >  	if tc-is-cross-compiler; then
> > 
> > @@ -571,32 +570,29 @@ enable_cmake-utils_src_configure() {
> > 
> >  			# in Prefix we need rpath and must ensure cmake gets our 
default
> >  			linker path # right ... except for Darwin hosts
> >  			IF (NOT APPLE)
> > 
> > -			SET (CMAKE_SKIP_RPATH OFF CACHE BOOL "" FORCE)
> > -			SET (CMAKE_PLATFORM_REQUIRED_RUNTIME_PATH
> > "${EPREFIX}/usr/${CHOST}/lib/gcc;${EPREFIX}/usr/${CHOST}/lib;${EPREFIX}/u
> > sr/$(get_libdir);${EPREFIX}/$(get_libdir)" -			CACHE STRING "" 
FORCE)
> > -
> > +				SET (CMAKE_SKIP_RPATH OFF CACHE BOOL "" FORCE)
> > +				SET (CMAKE_PLATFORM_REQUIRED_RUNTIME_PATH
> > "${EPREFIX}/usr/${CHOST}/lib/gcc;${EPREFIX}/usr/${CHOST}/lib;${EPREFIX}/u
> > sr/$(get_libdir);${EPREFIX}/$(get_libdir)" CACHE STRING "" FORCE)> 
> >  			ELSE ()
> > 
> > -
> > -			SET(CMAKE_PREFIX_PATH "${EPREFIX}${PREFIX}" CACHE STRING "" 
FORCE)
> > -			SET(CMAKE_SKIP_BUILD_RPATH OFF CACHE BOOL "" FORCE)
> > -			SET(CMAKE_SKIP_RPATH OFF CACHE BOOL "" FORCE)
> > -			SET(CMAKE_BUILD_WITH_INSTALL_RPATH TRUE CACHE BOOL "")
> > -			SET(CMAKE_INSTALL_RPATH
> > "${EPREFIX}${PREFIX}/lib;${EPREFIX}/usr/${CHOST}/lib/gcc;${EPREFIX}/usr/$
> > {CHOST}/lib;${EPREFIX}/usr/$(get_libdir);${EPREFIX}/$(get_libdir)" CACHE
> > STRING "" FORCE) -			SET(CMAKE_INSTALL_RPATH_USE_LINK_PATH 
TRUE CACHE
> > BOOL "" FORCE) -			SET(CMAKE_INSTALL_NAME_DIR "${EPREFIX}$
{PREFIX}/lib"
> > CACHE STRING "" FORCE) -
> > +				SET(CMAKE_PREFIX_PATH "${EPREFIX}${PREFIX}" CACHE 
STRING "" FORCE)
> > +				SET(CMAKE_SKIP_BUILD_RPATH OFF CACHE BOOL "" FORCE)
> > +				SET(CMAKE_SKIP_RPATH OFF CACHE BOOL "" FORCE)
> > +				SET(CMAKE_BUILD_WITH_INSTALL_RPATH TRUE CACHE BOOL "")
> > +				SET(CMAKE_INSTALL_RPATH
> > "${EPREFIX}${PREFIX}/lib;${EPREFIX}/usr/${CHOST}/lib/gcc;${EPREFIX}/usr/$
> > {CHOST}/lib;${EPREFIX}/usr/$(get_libdir);${EPREFIX}/$(get_libdir)" CACHE
> > STRING "" FORCE) +				SET(CMAKE_INSTALL_RPATH_USE_LINK_PATH 
TRUE CACHE
> > BOOL "" FORCE) +				SET(CMAKE_INSTALL_NAME_DIR "${EPREFIX}$
{PREFIX}/lib"
> > CACHE STRING "" FORCE)> 
> >  			ENDIF (NOT APPLE)
> >  		
> >  		_EOF_
> >  	
> >  	fi
> >  	
> >  	# Common configure parameters (invariants)
> > 
> > -	local common_config=${BUILD_DIR}/gentoo_common_config.cmake
> > 
> >  	local libdir=$(get_libdir)
> > 
> > -	cat > "${common_config}" <<- _EOF_ || die
> > -		SET (LIB_SUFFIX ${libdir/lib} CACHE STRING "library path suffix" 
FORCE)
> > -		SET (CMAKE_INSTALL_LIBDIR ${libdir} CACHE PATH "Output directory 
for
> > libraries") -	_EOF_
> > -	[[ "${NOCOLOR}" = true || "${NOCOLOR}" = yes ]] && echo 'SET
> > (CMAKE_COLOR_MAKEFILE OFF CACHE BOOL "pretty colors during make" FORCE)'
> > >> "${common_config}" +	local cmakeargs=(
> > +		-DLIB_SUFFIX="${libdir/lib}"
> > +		-DCMAKE_INSTALL_LIBDIR="${libdir}"
> > +		-DPKG_CONFIG_EXECUTABLE="$(type -P $(tc-getPKG_CONFIG))"
> > +		-DCMAKE_USER_MAKE_RULES_OVERRIDE="${build_rules}"
> > +		-DCMAKE_TOOLCHAIN_FILE="${toolchain_file}"
> > +	)
> 
> Wasn't the point of using a config file to avoid warnings when the
> variables aren't used?

Nah, config file was there even before CMake supported warning about unused 
variables (or at least definitely long before we enabled it in eclass).
The problem _I_ have with config file is that its contents is not visible in 
build log.
Though I admit extra warnings are not nice.
-D PKG_CONFIG_EXECUTABLE could be replaced with PKG_CONFIG env var.
But LIB_SUFFIX is some KDE early invention, populated in kde-related projects 
(otherwise GNUInstallDirs should be used or manually setting 
CMAKE_INSTALL_LIBDIR).
Not sure whether config file is worth having to eliminate one or two CMake 
warnings, let's see what others think.

-- 
regards
MM

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 163 bytes --]