Skip to content

Commit

Permalink
Support absolute path for CMAKE_INSTALL_*DIR
Browse files Browse the repository at this point in the history
Issue

When `CMAKE_INSTALL_LIBDIR` and `CMAKE_INSTALL_INCLUDEDIR` are set to absolute paths, the `msgpack-c.pc` file generated by CMake improperly configures `libdir` and `includedir`. This leads to incorrect paths that prevent the compiler from locating necessary header and library files.

How to reproduce

Build and install `msgpack-c`.

```console
% cmake -S . -B ../msgpack-c.build -DCMAKE_INSTALL_LIBDIR=/tmp/local/lib -DCMAKE_INSTALL_INCLUDEDIR=/tmp/local/include
% cmake --build ../msgpack-c.build
% sudo cmake --install ../msgpack-c.build
```

Compile `example/simple_c.c` using installed msgpack-c. The following error happens because the linker cannot find paths provided by pkg-config.

```console
% export PKG_CONFIG_PATH=/tmp/local/lib/pkgconfig:$PKG_CONFIG_PATH
% gcc -o simple_c example/simple_c.c $(pkg-config --cflags --libs msgpack-c)
/usr/bin/ld: cannot find -lmsgpack-c: No such file or directory
collect2: error: ld returned 1 exit status
```

Expected

Successfully compile `example/simple_c.c` using installed msgpack-c. We can execute `simple_c` like the following.

```console
% gcc -o simple_c example/simple_c.c $(pkg-config --cflags --libs msgpack-c)
% ./simple_c
93 01 c3 a7 65 78 61 6d 70 6c 65
[1, true, "example"]
```

Explain the problem in detail

The generated `msgpack-c.pc` file does not handle absolute paths correctly. Here is the result of the incorrect configuration in `How to reproduce` section. In the following `msgpack-c.pc` file, `libdir` and `includedir` are showing unrecognized paths, leading to incorrect paths.

```console
% cat /tmp/local/lib/pkgconfig/msgpack-c.pc
prefix=/usr/local
exec_prefix=/usr/local
libdir=${prefix}//tmp/local/lib <- Here the path is wrong. We expected `/tmp/local/lib`
includedir=${prefix}//tmp/local/include <- Here the path is wrong. We expected `/tmp/local/include`

Name: MessagePack
Description: Binary-based efficient object serialization library
Version: 6.0.1
Libs: -L${libdir} -lmsgpack-c
Cflags: -I${includedir}
```

Solution

Modify the `CMakeLists.txt` file to ensure that `libdir` and `includedir` use absolute paths. This change addresses the issue by providing correct paths to the compiler and linker.
  • Loading branch information
otegami committed May 12, 2024
1 parent c31fafb commit 4e027b7
Show file tree
Hide file tree
Showing 2 changed files with 12 additions and 4 deletions.
12 changes: 10 additions & 2 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,16 @@ LIST (APPEND CMAKE_MODULE_PATH "${PROJECT_SOURCE_DIR}/cmake/")
include(GNUInstallDirs)
SET (prefix ${CMAKE_INSTALL_PREFIX})
SET (exec_prefix ${CMAKE_INSTALL_PREFIX})
SET (libdir ${CMAKE_INSTALL_LIBDIR})
SET (includedir ${CMAKE_INSTALL_INCLUDEDIR})
IF (IS_ABSOLUTE ${CMAKE_INSTALL_LIBDIR})
SET (libdir ${CMAKE_INSTALL_LIBDIR})
ELSE ()
SET (libdir "\${prefix}/${CMAKE_INSTALL_LIBDIR}")
ENDIF ()
IF (IS_ABSOLUTE ${CMAKE_INSTALL_INCLUDEDIR})
SET (includedir ${CMAKE_INSTALL_INCLUDEDIR})
ELSE ()
SET (includedir "\${prefix}/${CMAKE_INSTALL_INCLUDEDIR}")
ENDIF ()

OPTION (MSGPACK_32BIT "32bit compile" OFF)

Expand Down
4 changes: 2 additions & 2 deletions msgpack-c.pc.in
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
prefix=@prefix@
exec_prefix=@exec_prefix@
libdir=${prefix}/@libdir@
includedir=${prefix}/@includedir@
libdir=@libdir@
includedir=@includedir@

Name: MessagePack
Description: Binary-based efficient object serialization library
Expand Down

6 comments on commit 4e027b7

@saper
Copy link

@saper saper commented on 4e027b7 Jun 10, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have question here - are there cases when CMAKE_INSTALL_LIBDIR are absolute already? In any case, would using CMAKE_INSTALL_FULL_LIBDIR and CMAKE_INSTALL_INCLUDE_DIR help? https://cmake.org/cmake/help/v3.11/module/GNUInstallDirs.html

(FreeBSD has a downstream bug about this at https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=279615)

@redboltz
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@otegami, Could you answear the question?

@otegami
Copy link
Author

@otegami otegami commented on 4e027b7 Jun 10, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm so sorry I'm late. Sure!

I have question here - are there cases when CMAKE_INSTALL_LIBDIR are absolute already?

In this change, I have made changes to the generated pkg-config file to support absolute path; however, we still face issues using absolute paths for CMAKE_INSTALL_INCLUDEDIR. Although the absolute paths for CMAKE_INSTALL_LIBDIR work fine, the header files are still installed in unexpected locations when we use absolute paths for CMAKE_INSTALL_INCLUDEDIR like the following. Addressing this issue should be our next step.

Below are the details of the commands used and the outcomes observed:

% cmake -S . -B ../msgpack-c.build -DCMAKE_INSTALL_LIBDIR=/lib -DCMAKE_INSTALL_INCLUDEDIR=/include
% cmake --build ../msgpack-c.build
% cat ../msgpack-c.build/msgpack-c.pc
prefix=/usr/local
exec_prefix=/usr/local
libdir=/lib
includedir=/include

Name: MessagePack
Description: Binary-based efficient object serialization library
Version: 6.0.1
Libs: -L${libdir} -lmsgpack-c
Cflags: -I${includedir} 
% sudo cmake --install ../msgpack-c.build
-- Install configuration: ""
-- Installing: /lib/libmsgpack-c.so.2.0.0
-- Up-to-date: /lib/libmsgpack-c.so.2
-- Up-to-date: /lib/libmsgpack-c.so
-- Installing: /lib/libmsgpack-c.a
-- Up-to-date: /usr/local/include/msgpack.h
-- Up-to-date: /usr/local/include/msgpack/fbuffer.h
-- Up-to-date: /usr/local/include/msgpack/gcc_atomic.h
-- Up-to-date: /usr/local/include/msgpack/object.h
-- Up-to-date: /usr/local/include/msgpack/pack.h
-- Up-to-date: /usr/local/include/msgpack/pack_define.h
-- Up-to-date: /usr/local/include/msgpack/sbuffer.h
-- Up-to-date: /usr/local/include/msgpack/timestamp.h
-- Up-to-date: /usr/local/include/msgpack/unpack.h
-- Up-to-date: /usr/local/include/msgpack/unpack_define.h
-- Up-to-date: /usr/local/include/msgpack/unpack_template.h
-- Up-to-date: /usr/local/include/msgpack/util.h
-- Up-to-date: /usr/local/include/msgpack/version.h
-- Up-to-date: /usr/local/include/msgpack/version_master.h
-- Up-to-date: /usr/local/include/msgpack/vrefbuffer.h
-- Up-to-date: /usr/local/include/msgpack/zbuffer.h
-- Up-to-date: /usr/local/include/msgpack/zone.h
-- Installing: /usr/local/include/msgpack/pack_template.h
-- Installing: /usr/local/include/msgpack/sysdep.h
-- Installing: /lib/pkgconfig/msgpack-c.pc
-- Old export file "/lib/cmake/msgpack-c/msgpack-c-targets.cmake" will be replaced.  Removing files [/lib/cmake/msgpack-c/msgpack-c-targets-noconfig.cmake].
-- Installing: /lib/cmake/msgpack-c/msgpack-c-targets.cmake
-- Installing: /lib/cmake/msgpack-c/msgpack-c-targets-noconfig.cmake
-- Installing: /lib/cmake/msgpack-c/msgpack-c-config.cmake
-- Installing: /lib/cmake/msgpack-c/msgpack-c-config-version.cmake

(FreeBSD has a downstream bug about this at https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=279615)

I have a sample problem with your case. So I'm trying to support an absolute path now.

In any case, would using CMAKE_INSTALL_FULL_LIBDIR and CMAKE_INSTALL_INCLUDE_DIR help? https://cmake.org/cmake/help/v3.11/module/GNUInstallDirs.html

I didn't use the CMAKE_INSTALL_FULL_* based on the following tips.

The Apache Arrow project also supports it in this way.

@saper
Copy link

@saper saper commented on 4e027b7 Jun 10, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think one "snip" above is the better solution: https://github.com/jtojnar/cmake-snips?tab=readme-ov-file#assuming-cmake_install_dir-is-relative-path

Assuming CMAKE_INSTALL_

is relative path

There is no guarantee that variables like CMAKE_INSTALL_LIBDIR or CMAKE_INSTALL_INCLUDEDIR are relative paths, or even located under CMAKE_INSTALL_PREFIX. In fact, the documentation explicitly mentions that they can be absolute several times.

If you need absolute paths, never concatenate CMAKE_INSTALL_

to CMAKE_INSTALL_PREFIX manually. Use CMAKE_INSTALL_FULL_ instead.

Also there should be no need to set the default paths like this:

IF (NOT DEFINED CMAKE_INSTALL_LIBDIR)
    SET(CMAKE_INSTALL_LIBDIR lib)
ENDIF ()

We have already GNUInstallDirs included, maybe we should use it?

@saper
Copy link

@saper saper commented on 4e027b7 Jun 10, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have opened #1124 to discuss this further. For example let's clarify what is special about macos there.

@saper
Copy link

@saper saper commented on 4e027b7 Jun 11, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please sign in to comment.