[PATCH dwarves] Respect CMAKE_INSTALL_LIBDIR

[PATCH dwarves] Respect CMAKE_INSTALL_LIBDIR

[Ben Olson]

This patch changes the `cmake` configuration to honor `CMAKE_INSTALL_LIBDIR`
and use `lib` by default so that installations match the conventional placement
of libraries. For example, it will now install `libdwarves.so` into
 `/usr/local/lib` instead of `/usr/local` directly.

Signed-off-by: Brandon Kammerdiener <brandon.kammerdiener@intel.com>
Signed-off-by: Ben Olson <matthew.olson@intel.com>
---

This patch addresses this issue: https://github.com/acmel/dwarves/issues/48

 CMakeLists.txt | 17 ++++-------------
 README         |  3 +--
 2 files changed, 5 insertions(+), 15 deletions(-)

diff --git a/CMakeLists.txt b/CMakeLists.txt
index 8ca1bf2..b2c8057 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -21,18 +21,7 @@ else()
 	LINK_DIRECTORIES(${LIBBPF_LIBRARY_DIRS})
 endif()
 
-# Try to parse this later, Helio just showed me a KDE4 example to support
-# x86-64 builds.
-# the following are directories where stuff will be installed to
-set(__LIB "" CACHE STRING "Define suffix of directory name (32/64)" )
-
-macro(_set_fancy _var _value _comment)
-	if (NOT DEFINED ${_var})
-		set(${_var} ${_value})
-	else (NOT DEFINED ${_var})
-		set(${_var} "${${_var}}" CACHE PATH "${_comment}")
-	endif (NOT DEFINED ${_var})
-endmacro(_set_fancy)
+set(CMAKE_INSTALL_LIBDIR "lib" CACHE STRING "libdir name")
 
 # where to look first for cmake modules,
 # before ${CMAKE_ROOT}/Modules/ is checked
@@ -84,7 +73,9 @@ if(NOT LIBBPF_FOUND AND NOT EXISTS "${PROJECT_SOURCE_DIR}/lib/bpf/src/btf.h")
 	message(FATAL_ERROR "The submodules were not downloaded! GIT_SUBMODULE was turned off or failed. Please update submodules and try again.")
 endif()
 
-_set_fancy(LIB_INSTALL_DIR "${EXEC_INSTALL_PREFIX}${CMAKE_INSTALL_PREFIX}/${__LIB}" "libdir")
+if (NOT DEFINED LIB_INSTALL_DIR)
+    set(LIB_INSTALL_DIR "${EXEC_INSTALL_PREFIX}${CMAKE_INSTALL_PREFIX}/${CMAKE_INSTALL_LIBDIR}")
+endif()
 
 # libbpf uses reallocarray, which is not available in all versions of glibc
 # libbpf's include/tools/libc_compat.h provides implementation, but needs
diff --git a/README b/README
index f9aeef7..0627872 100644
--- a/README
+++ b/README
@@ -3,8 +3,7 @@ Build instructions:
 1. install cmake
 2. mkdir build
 3. cd build
-4. cmake -D__LIB=lib ..
-5. make install
+4. make install
 
 cmake Options:
   -DBUILD_SHARED_LIBS
-- 
2.47.0

Re: [PATCH dwarves] Respect CMAKE_INSTALL_LIBDIR

[Alan Maguire]

On 25/11/2024 15:51, Ben Olson wrote:
> This patch changes the `cmake` configuration to honor `CMAKE_INSTALL_LIBDIR`
> and use `lib` by default so that installations match the conventional placement
> of libraries. For example, it will now install `libdwarves.so` into
>  `/usr/local/lib` instead of `/usr/local` directly.
> 

What about distros that use /usr/[local/]/lib64 for 64-bit libraries? It
seems like CMAKE_INSTALL_LIBDIR does the right thing there also by
default, can you confirm? If that is indeed the case, it would be good
to amend the commit description to not be "lib"-specific, and say
something like "CMAKE_INSTALL_LIBDIR ensures installations match the
conventional placement of libraries - /usr/local/lib on some systems,
/usr/local/li64 on others. Otherwise folks might think they need to
override this when they probably don't. Thanks!

Alan

> Signed-off-by: Brandon Kammerdiener <brandon.kammerdiener@intel.com>
> Signed-off-by: Ben Olson <matthew.olson@intel.com>
> ---
> 
> This patch addresses this issue: https://github.com/acmel/dwarves/issues/48
> 
>  CMakeLists.txt | 17 ++++-------------
>  README         |  3 +--
>  2 files changed, 5 insertions(+), 15 deletions(-)
> 
> diff --git a/CMakeLists.txt b/CMakeLists.txt
> index 8ca1bf2..b2c8057 100644
> --- a/CMakeLists.txt
> +++ b/CMakeLists.txt
> @@ -21,18 +21,7 @@ else()
>  	LINK_DIRECTORIES(${LIBBPF_LIBRARY_DIRS})
>  endif()
>  
> -# Try to parse this later, Helio just showed me a KDE4 example to support
> -# x86-64 builds.
> -# the following are directories where stuff will be installed to
> -set(__LIB "" CACHE STRING "Define suffix of directory name (32/64)" )
> -
> -macro(_set_fancy _var _value _comment)
> -	if (NOT DEFINED ${_var})
> -		set(${_var} ${_value})
> -	else (NOT DEFINED ${_var})
> -		set(${_var} "${${_var}}" CACHE PATH "${_comment}")
> -	endif (NOT DEFINED ${_var})
> -endmacro(_set_fancy)
> +set(CMAKE_INSTALL_LIBDIR "lib" CACHE STRING "libdir name")
>  
>  # where to look first for cmake modules,
>  # before ${CMAKE_ROOT}/Modules/ is checked
> @@ -84,7 +73,9 @@ if(NOT LIBBPF_FOUND AND NOT EXISTS "${PROJECT_SOURCE_DIR}/lib/bpf/src/btf.h")
>  	message(FATAL_ERROR "The submodules were not downloaded! GIT_SUBMODULE was turned off or failed. Please update submodules and try again.")
>  endif()
>  
> -_set_fancy(LIB_INSTALL_DIR "${EXEC_INSTALL_PREFIX}${CMAKE_INSTALL_PREFIX}/${__LIB}" "libdir")
> +if (NOT DEFINED LIB_INSTALL_DIR)
> +    set(LIB_INSTALL_DIR "${EXEC_INSTALL_PREFIX}${CMAKE_INSTALL_PREFIX}/${CMAKE_INSTALL_LIBDIR}")
> +endif()
>  
>  # libbpf uses reallocarray, which is not available in all versions of glibc
>  # libbpf's include/tools/libc_compat.h provides implementation, but needs
> diff --git a/README b/README
> index f9aeef7..0627872 100644
> --- a/README
> +++ b/README
> @@ -3,8 +3,7 @@ Build instructions:
>  1. install cmake
>  2. mkdir build
>  3. cd build
> -4. cmake -D__LIB=lib ..
> -5. make install
> +4. make install
>  
>  cmake Options:
>    -DBUILD_SHARED_LIBS

Re: [PATCH dwarves] Respect CMAKE_INSTALL_LIBDIR

[Olson, Matthew]

On Mon, Nov 25, 2024 at 05:04:53PM +0000, Alan Maguire wrote:
> On 25/11/2024 15:51, Ben Olson wrote:
> > This patch changes the `cmake` configuration to honor `CMAKE_INSTALL_LIBDIR`
> > and use `lib` by default so that installations match the conventional placement
> > of libraries. For example, it will now install `libdwarves.so` into
> >  `/usr/local/lib` instead of `/usr/local` directly.
> > 
> 
> What about distros that use /usr/[local/]/lib64 for 64-bit libraries? It
> seems like CMAKE_INSTALL_LIBDIR does the right thing there also by
> default, can you confirm? If that is indeed the case, it would be good
> to amend the commit description to not be "lib"-specific, and say
> something like "CMAKE_INSTALL_LIBDIR ensures installations match the
> conventional placement of libraries - /usr/local/lib on some systems,
> /usr/local/li64 on others. Otherwise folks might think they need to
> override this when they probably don't. Thanks!

Yes, it should respect `lib64` - we've just tested on both Arch and Fedora.
On Arch, `lib64` is just a symlink to `lib`, and this patch places things
in `lib`. On Fedora, it places things in `lib64`. Just noticed that we do
have to include a built-in CMake module for it to work properly, though.
I'll add that in v2 along with the commit message change.

Thank you!

> 
> Alan
> 
> > Signed-off-by: Brandon Kammerdiener <brandon.kammerdiener@intel.com>
> > Signed-off-by: Ben Olson <matthew.olson@intel.com>
> > ---
> > 
> > This patch addresses this issue: https://github.com/acmel/dwarves/issues/48
> > 
> >  CMakeLists.txt | 17 ++++-------------
> >  README         |  3 +--
> >  2 files changed, 5 insertions(+), 15 deletions(-)
> > 
> > diff --git a/CMakeLists.txt b/CMakeLists.txt
> > index 8ca1bf2..b2c8057 100644
> > --- a/CMakeLists.txt
> > +++ b/CMakeLists.txt
> > @@ -21,18 +21,7 @@ else()
> >  	LINK_DIRECTORIES(${LIBBPF_LIBRARY_DIRS})
> >  endif()
> >  
> > -# Try to parse this later, Helio just showed me a KDE4 example to support
> > -# x86-64 builds.
> > -# the following are directories where stuff will be installed to
> > -set(__LIB "" CACHE STRING "Define suffix of directory name (32/64)" )
> > -
> > -macro(_set_fancy _var _value _comment)
> > -	if (NOT DEFINED ${_var})
> > -		set(${_var} ${_value})
> > -	else (NOT DEFINED ${_var})
> > -		set(${_var} "${${_var}}" CACHE PATH "${_comment}")
> > -	endif (NOT DEFINED ${_var})
> > -endmacro(_set_fancy)
> > +set(CMAKE_INSTALL_LIBDIR "lib" CACHE STRING "libdir name")
> >  
> >  # where to look first for cmake modules,
> >  # before ${CMAKE_ROOT}/Modules/ is checked
> > @@ -84,7 +73,9 @@ if(NOT LIBBPF_FOUND AND NOT EXISTS "${PROJECT_SOURCE_DIR}/lib/bpf/src/btf.h")
> >  	message(FATAL_ERROR "The submodules were not downloaded! GIT_SUBMODULE was turned off or failed. Please update submodules and try again.")
> >  endif()
> >  
> > -_set_fancy(LIB_INSTALL_DIR "${EXEC_INSTALL_PREFIX}${CMAKE_INSTALL_PREFIX}/${__LIB}" "libdir")
> > +if (NOT DEFINED LIB_INSTALL_DIR)
> > +    set(LIB_INSTALL_DIR "${EXEC_INSTALL_PREFIX}${CMAKE_INSTALL_PREFIX}/${CMAKE_INSTALL_LIBDIR}")
> > +endif()
> >  
> >  # libbpf uses reallocarray, which is not available in all versions of glibc
> >  # libbpf's include/tools/libc_compat.h provides implementation, but needs
> > diff --git a/README b/README
> > index f9aeef7..0627872 100644
> > --- a/README
> > +++ b/README
> > @@ -3,8 +3,7 @@ Build instructions:
> >  1. install cmake
> >  2. mkdir build
> >  3. cd build
> > -4. cmake -D__LIB=lib ..
> > -5. make install
> > +4. make install
> >  
> >  cmake Options:
> >    -DBUILD_SHARED_LIBS
> 

Re: [PATCH dwarves] Respect CMAKE_INSTALL_LIBDIR

[Alan Maguire]

On 25/11/2024 18:44, Olson, Matthew wrote:
> On Mon, Nov 25, 2024 at 05:04:53PM +0000, Alan Maguire wrote:
>> On 25/11/2024 15:51, Ben Olson wrote:
>>> This patch changes the `cmake` configuration to honor `CMAKE_INSTALL_LIBDIR`
>>> and use `lib` by default so that installations match the conventional placement
>>> of libraries. For example, it will now install `libdwarves.so` into
>>>  `/usr/local/lib` instead of `/usr/local` directly.
>>>
>>
>> What about distros that use /usr/[local/]/lib64 for 64-bit libraries? It
>> seems like CMAKE_INSTALL_LIBDIR does the right thing there also by
>> default, can you confirm? If that is indeed the case, it would be good
>> to amend the commit description to not be "lib"-specific, and say
>> something like "CMAKE_INSTALL_LIBDIR ensures installations match the
>> conventional placement of libraries - /usr/local/lib on some systems,
>> /usr/local/li64 on others. Otherwise folks might think they need to
>> override this when they probably don't. Thanks!
> 
> Yes, it should respect `lib64` - we've just tested on both Arch and Fedora.
> On Arch, `lib64` is just a symlink to `lib`, and this patch places things
> in `lib`. On Fedora, it places things in `lib64`. Just noticed that we do
> have to include a built-in CMake module for it to work properly, though.
> I'll add that in v2 along with the commit message change.
> 
> Thank you!
>

Great, thanks for confirming!


>>
>> Alan
>>
>>> Signed-off-by: Brandon Kammerdiener <brandon.kammerdiener@intel.com>
>>> Signed-off-by: Ben Olson <matthew.olson@intel.com>
>>> ---
>>>
>>> This patch addresses this issue: https://github.com/acmel/dwarves/issues/48
>>>
>>>  CMakeLists.txt | 17 ++++-------------
>>>  README         |  3 +--
>>>  2 files changed, 5 insertions(+), 15 deletions(-)
>>>
>>> diff --git a/CMakeLists.txt b/CMakeLists.txt
>>> index 8ca1bf2..b2c8057 100644
>>> --- a/CMakeLists.txt
>>> +++ b/CMakeLists.txt
>>> @@ -21,18 +21,7 @@ else()
>>>  	LINK_DIRECTORIES(${LIBBPF_LIBRARY_DIRS})
>>>  endif()
>>>  
>>> -# Try to parse this later, Helio just showed me a KDE4 example to support
>>> -# x86-64 builds.
>>> -# the following are directories where stuff will be installed to
>>> -set(__LIB "" CACHE STRING "Define suffix of directory name (32/64)" )
>>> -
>>> -macro(_set_fancy _var _value _comment)
>>> -	if (NOT DEFINED ${_var})
>>> -		set(${_var} ${_value})
>>> -	else (NOT DEFINED ${_var})
>>> -		set(${_var} "${${_var}}" CACHE PATH "${_comment}")
>>> -	endif (NOT DEFINED ${_var})
>>> -endmacro(_set_fancy)
>>> +set(CMAKE_INSTALL_LIBDIR "lib" CACHE STRING "libdir name")
>>>  
>>>  # where to look first for cmake modules,
>>>  # before ${CMAKE_ROOT}/Modules/ is checked
>>> @@ -84,7 +73,9 @@ if(NOT LIBBPF_FOUND AND NOT EXISTS "${PROJECT_SOURCE_DIR}/lib/bpf/src/btf.h")
>>>  	message(FATAL_ERROR "The submodules were not downloaded! GIT_SUBMODULE was turned off or failed. Please update submodules and try again.")
>>>  endif()
>>>  
>>> -_set_fancy(LIB_INSTALL_DIR "${EXEC_INSTALL_PREFIX}${CMAKE_INSTALL_PREFIX}/${__LIB}" "libdir")
>>> +if (NOT DEFINED LIB_INSTALL_DIR)
>>> +    set(LIB_INSTALL_DIR "${EXEC_INSTALL_PREFIX}${CMAKE_INSTALL_PREFIX}/${CMAKE_INSTALL_LIBDIR}")
>>> +endif()
>>>  
>>>  # libbpf uses reallocarray, which is not available in all versions of glibc
>>>  # libbpf's include/tools/libc_compat.h provides implementation, but needs
>>> diff --git a/README b/README
>>> index f9aeef7..0627872 100644
>>> --- a/README
>>> +++ b/README
>>> @@ -3,8 +3,7 @@ Build instructions:
>>>  1. install cmake
>>>  2. mkdir build
>>>  3. cd build
>>> -4. cmake -D__LIB=lib ..
>>> -5. make install
>>> +4. make install
>>>  
>>>  cmake Options:
>>>    -DBUILD_SHARED_LIBS
>>