asmaloney / libE57Format

Library for reading & writing the E57 file format
Boost Software License 1.0
137 stars 66 forks source link

Disabling tests breaks cmake files installation dir #314

Closed adi64 closed 1 day ago

adi64 commented 4 days ago

Hi, first off, Thank you for this library.

I build libE57Format locally like this:

cmake -B build -S libE57Format -DCMAKE_INSTALL_PREFIX=/usr -DE57_BUILD_SHARED=ON -DE57_BUILD_TEST=OFF

This works with version 3.1.1, however with version 3.2.0, somehow, setting -DE57_BUILD_TEST=OFF breaks the path generation for the cmake files, which causes it to install them to /cmake instead of /usr/lib/cmake. Setting -DE57_BUILD_TEST=ON gives me the correct path again.

asmaloney commented 4 days ago

This change uses CMAKE_INSTALL_LIBDIR to set the default path and allow the whole thing to be overridden by E57_INSTALL_CMAKEDIR.

There's a STATUS message that outputs this info. Can you please add the cmake output (everything from -- Using CMake x.y.z?

What OS are you using?

I can't see anything in the testing which changes/overrides any of this and I cannot reproduce it here with CMake 3.31.1 on macOS.

adi64 commented 4 days ago

I'm using an Arch Linux. This is part of a package build.

A bit of context on the directory structure:

/home/adrian/builds/e57format-git
├── pkg              <-- this is where the build should be installed to. This is the package root. It should contain `usr/` etc.
└── src
    ├── build        <-- This is where CMake should build
    └── libE57Format <-- This is the checked out git repo

First step:

cmake -B build -S libE57Format -DCMAKE_INSTALL_PREFIX=/usr -DE57_BUILD_SHARED=ON -DE57_BUILD_TEST=OFF

Output:

- Using CMake 3.31.1
-- The CXX compiler identification is GNU 14.2.1
-- Detecting CXX compiler ABI info
-- Detecting CXX compiler ABI info - done
-- Check for working CXX compiler: /usr/lib/ccache/bin/c++ - skipped
-- Detecting CXX compile features
-- Detecting CXX compile features - done
-- Performing Test CMAKE_HAVE_LIBC_PTHREAD
-- Performing Test CMAKE_HAVE_LIBC_PTHREAD - Success
-- Found Threads: TRUE
-- Found XercesC: /usr/lib/libxerces-c.so (found version "3.2.5")
-- [E57Format] Revision ID: E57Format-3.2.0-x86_64-gcc142 (git v3.2.0)
-- [E57Format] Building shared library
-- Performing Test COMPILER_HAS_HIDDEN_VISIBILITY
-- Performing Test COMPILER_HAS_HIDDEN_VISIBILITY - Success
-- Performing Test COMPILER_HAS_HIDDEN_INLINE_VISIBILITY
-- Performing Test COMPILER_HAS_HIDDEN_INLINE_VISIBILITY - Success
-- Performing Test COMPILER_HAS_DEPRECATED_ATTR
-- Performing Test COMPILER_HAS_DEPRECATED_ATTR - Success
-- Submodule update using git (/usr/bin/git)
-- Submodule update directory: /home/adrian/builds/e57format-git/src/libE57Format
-- Submodule update complete
-- [E57Format] Setting validation level to 1
-- [E57Format] Using ccache: /usr/bin/ccache
-- [E57Format] Using clang-format: /usr/bin/clang-format
-- [E57Format] CMake files install to /usr//cmake/E57Format
-- Configuring done (1.0s)
-- Generating done (0.0s)
-- Build files have been written to: /home/adrian/builds/e57format-git/src/build

Interesting to me is the line -- [E57Format] CMake files install to /usr//cmake/E57Format. That should've been /usr/lib/cmake/E57Format, I guess.

Second step is cmake --build build which outputs just CMake build progress and nothing else.

The install step for the package is

DESTDIR="/home/adrian/builds/e57format-git/pkg/e57format-git" cmake --install build

which outputs:

-- Install configuration: ""
-- Installing: /home/adrian/builds/e57format-git/pkg/e57format-git/usr/include/E57Format/E57Export.h
-- Installing: /home/adrian/builds/e57format-git/pkg/e57format-git/usr/include/E57Format/E57Format.h
-- Installing: /home/adrian/builds/e57format-git/pkg/e57format-git/usr/include/E57Format/E57Exception.h
-- Installing: /home/adrian/builds/e57format-git/pkg/e57format-git/usr/include/E57Format/E57SimpleData.h
-- Installing: /home/adrian/builds/e57format-git/pkg/e57format-git/usr/include/E57Format/E57SimpleReader.h
-- Installing: /home/adrian/builds/e57format-git/pkg/e57format-git/usr/include/E57Format/E57SimpleWriter.h
-- Installing: /home/adrian/builds/e57format-git/pkg/e57format-git/usr/include/E57Format/E57Version.h
-- Installing: /home/adrian/builds/e57format-git/pkg/e57format-git/usr/lib/libE57Format.so.3.2.0
-- Installing: /home/adrian/builds/e57format-git/pkg/e57format-git/usr/lib/libE57Format.so.3
-- Installing: /home/adrian/builds/e57format-git/pkg/e57format-git/usr/lib/libE57Format.so
-- Installing: /home/adrian/builds/e57format-git/pkg/e57format-git/cmake/E57Format/E57Format-export.cmake
-- Installing: /home/adrian/builds/e57format-git/pkg/e57format-git/cmake/E57Format/E57Format-export-noconfig.cmake
-- Installing: /home/adrian/builds/e57format-git/pkg/e57format-git/cmake/E57Format/e57format-config.cmake

This is correct for the includes and the built library files, but the cmake files are in the wrong path.

adi64 commented 4 days ago

If I set -DE57_BUILD_TEST=ON:

-- Using CMake 3.31.1
-- The CXX compiler identification is GNU 14.2.1
-- Detecting CXX compiler ABI info
-- Detecting CXX compiler ABI info - done
-- Check for working CXX compiler: /usr/lib/ccache/bin/c++ - skipped
-- Detecting CXX compile features
-- Detecting CXX compile features - done
-- Performing Test CMAKE_HAVE_LIBC_PTHREAD
-- Performing Test CMAKE_HAVE_LIBC_PTHREAD - Success
-- Found Threads: TRUE
-- Found XercesC: /usr/lib/libxerces-c.so (found version "3.2.5")
-- [E57Format] Revision ID: E57Format-3.2.0-x86_64-gcc142 (git v3.2.0)
-- [E57Format] Building shared library
-- Performing Test COMPILER_HAS_HIDDEN_VISIBILITY
-- Performing Test COMPILER_HAS_HIDDEN_VISIBILITY - Success
-- Performing Test COMPILER_HAS_HIDDEN_INLINE_VISIBILITY
-- Performing Test COMPILER_HAS_HIDDEN_INLINE_VISIBILITY - Success
-- Performing Test COMPILER_HAS_DEPRECATED_ATTR
-- Performing Test COMPILER_HAS_DEPRECATED_ATTR - Success
-- Submodule update using git (/usr/bin/git)
-- Submodule update directory: /home/adrian/builds/e57format-git/src/libE57Format
Submodule path 'test/extern/googletest': checked out 'fa6de7f4382f5c8fb8b9e32eea28a2eb44966c32'
-- Submodule update complete
-- [E57Format] Setting validation level to 1
-- [E57Format] Using ccache: /usr/bin/ccache
-- [E57Format] Using clang-format: /usr/bin/clang-format
-- [E57Format] Testing enabled
-- [testE57] Enabling address sanitizer (ASan)
-- [testE57] Enabling undefined behaviour sanitizer (UBSan)
-- [testE57] Using ccache: /usr/bin/ccache
CMake Warning at test/CMakeLists.txt:86 (message):
  [E57 Test] Test data not found.  Please set E57_TEST_DATA_PATH to the
  directory containing libE57Format-test-data.

-- The C compiler identification is GNU 14.2.1
-- Detecting C compiler ABI info
-- Detecting C compiler ABI info - done
-- Check for working C compiler: /usr/lib/ccache/bin/cc - skipped
-- Detecting C compile features
-- Detecting C compile features - done
-- [gtest_main] Enabling address sanitizer (ASan)
-- [gtest_main] Enabling undefined behaviour sanitizer (UBSan)
-- [E57Format] CMake files install to /usr/lib/cmake/E57Format
-- Configuring done (3.5s)
-- Generating done (0.0s)
-- Build files have been written to: /home/adrian/builds/e57format-git/src/build
asmaloney commented 4 days ago

Interesting to me is the line -- [E57Format] CMake files install to /usr//cmake/E57Format. That should've been /usr/lib/cmake/E57Format, I guess.

Yes, exactly. That seems to indicate that CMAKE_INSTALL_LIBDIR is somehow empty. The only ref I found w.r.t. Arch with a quick search is this one which implies that it should be set properly.

Could you please output CMAKE_INSTALL_LIBDIR at the beginning of the cmake file so we can see what the default is?

message( STATUS "[CMAKE_INSTALL_PREFIX] ${CMAKE_INSTALL_PREFIX}" )

(And then maybe try it again with the "working" options E57_BUILD_TEST=ON to see if it's different.)

The second issue is that the install doesn't seem to be using the DESTINATION properly.

I don't know a lot about the CMake options for cmake installation since I don't ever do that, but the options are the same for the lib and for the cmake file so I would have expected them to end up in the same place... Maybe a difference between EXPORT and FILES? Maybe there's a clue in here? I don't have time to look through that at the moment.

adi64 commented 2 days ago

CMAKE_INSTALL_LIBDIR is indeed empty. I noticed that this is part of the GNUInstallDirs module. A quick search showed that this module is only included in googletest (libE57Format/test/extern/googletest/CMakeLists.txt). That explains it: The module is only loaded via googletest and therefore only if tests are enabled. Otherwise it's not loaded at all, and that's why CMAKE_INSTALL_LIBDIR is never set. I created PR #315 which should fix this :)

asmaloney commented 1 day ago

Great catch! Thank you.

adi64 commented 1 day ago

Thank you for looking into this so quickly and guiding me in the correct direction! :)