Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

After 4.0.0 headers gets installed into include/utf8 using CMake #112

Closed
diizzyy opened this issue Oct 28, 2023 · 7 comments
Closed

After 4.0.0 headers gets installed into include/utf8 using CMake #112

diizzyy opened this issue Oct 28, 2023 · 7 comments

Comments

@diizzyy
Copy link

diizzyy commented Oct 28, 2023

That doesn't seem to be intentional by looking at the rest of the source code?

@diizzyy diizzyy changed the title After 4.0.0 headers gets installed into include/utf8 After 4.0.0 headers gets installed into include/utf8 using CMake Oct 28, 2023
@nemtrif
Copy link
Owner

nemtrif commented Oct 30, 2023

It gets installed directly under include:

  • include
    • utf8.h
    • utf8
      • core.h
      • checked.h
      • ...

@diizzyy
Copy link
Author

diizzyy commented Oct 30, 2023

Before 4.0.0

include/utf8cpp/utf8.h
include/utf8cpp/utf8/checked.h
include/utf8cpp/utf8/core.h
include/utf8cpp/utf8/cpp11.h
include/utf8cpp/utf8/cpp17.h
include/utf8cpp/utf8/unchecked.h

After:

include/utf8.h
include/utf8/checked.h
include/utf8/core.h
include/utf8/cpp11.h
include/utf8/cpp17.h
include/utf8/cpp20.h
include/utf8/unchecked.h

Is that change intentional? It also seems a bit confusing as the rest of the project is referred to as utf8cpp?

cmake/utf8cppTargets.cmake (in 4.0.1)

...
set_target_properties(utf8cpp::utf8cpp PROPERTIES
  INTERFACE_INCLUDE_DIRECTORIES "${_IMPORT_PREFIX}/include/utf8cpp"

There are also distros reverting this change due to breakage with consumers

@ufleisch
Copy link
Contributor

ufleisch commented Nov 4, 2023

Unfortunately, the headers are not only installed into a different folder, the CMake build system is broken. If this gets unnoticed, it is probably because utf8.h gets installed into the standard include directory, so it is found anyway, even if the dependency to utfcpp is not correctly specified. To see the issue, we need to use a non-standard include path, e.g. using a custom prefix. This is not a theoretical problem, currently, TagLib cannot be built without hacks on macOS using Homebrew because of this.

It can be reproduced like this, first it is shown that it works correctly for version 3.2.5

git clone git@github.com:nemtrif/utfcpp.git
cd utfcpp
git checkout v3.2.5
cmake -B build -DCMAKE_INSTALL_PREFIX=$HOME/custom-prefix -DUTF8_TESTS=OFF
make -C build
make -C build install

mkdir ../utfcpp-client
cd ../utfcpp-client
cat >CMakeLists.txt <<EOF
cmake_minimum_required(VERSION 3.5.0)
project(utfcpp-client)
find_package(utf8cpp)
add_executable(client client.cpp)
target_link_libraries(client utf8::cpp)
EOF
cat >client.cpp <<EOF
#include <string>
#include <iostream>
#include <utf8.h>
int main(int argc, char *argv[]) {
  std::u16string utf16string = {0x41, 0x0448, 0x65e5, 0xd834, 0xdd1e};
  std::string u = utf8::utf16to8(utf16string);
  std::cout << u << std::endl;
  return 0;
}
EOF
cmake -B build -DCMAKE_PREFIX_PATH=$HOME/custom-prefix
make -C build
# Run build/client, it works

Now the same with 4.0.1

cd ../utfcpp
rm -rf $HOME/custom-prefix build
git checkout v4.0.1
cmake -B build -DCMAKE_INSTALL_PREFIX=$HOME/custom-prefix -DUTF8_TESTS=OFF
make -C build
make -C build install

cd ../utfcpp-client
cmake -B build -DCMAKE_PREFIX_PATH=$HOME/custom-prefix

-- Configuring done
CMake Error at CMakeLists.txt:5 (target_link_libraries):
Target "client" links to:

utf8::cpp

but the target was not found. Possible reasons include:

* There is a typo in the target name.
* A find_package call is missing for an IMPORTED target.
* An ALIAS target is missing.

Looking into $HOME/custom-prefix/share/utf8cpp/cmake/utf8cppTargets.cmake one sees that the imported target is now utf8cpp::utf8cpp and no longer utf8::cpp (as set previously in utf8cppConfig.cmake). So let's change this.

sed -i 's/utf8::cpp/utf8cpp::utf8cpp/' CMakeLists.txt

cmake -B build -DCMAKE_PREFIX_PATH=$HOME/custom-prefix

-- Configuring done
CMake Error in CMakeLists.txt:
Imported target "utf8cpp::utf8cpp" includes non-existent path

"/home/urs/custom-prefix/include/utf8cpp"

in its INTERFACE_INCLUDE_DIRECTORIES. Possible reasons include:

  • The path was deleted, renamed, or moved to another location.

  • An install or uninstall procedure did not complete successfully.

  • The installation package was faulty and references files it does not
    provide.

Another possibility would be to use utf8cpp as the name of the imported CMake target. But this will only work with v3.2.5, with v4.0.1 we get

(..)/utfcpp-client/client.cpp:3:10: fatal error: utf8.h: No such file or directory
3 | #include <utf8.h>

I would suggest the following changes to fix it:

diff --git a/CMakeLists.txt b/CMakeLists.txt
index 6fdd906..2ca50a6 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -10,7 +10,7 @@ include(GNUInstallDirs)
 
 target_include_directories(utf8cpp INTERFACE
     "$<BUILD_INTERFACE:${CMAKE_CURRENT_SOURCE_DIR}/source>"
-    $<INSTALL_INTERFACE:include/utf8cpp>
+    $<INSTALL_INTERFACE:include>
 )
 
 include(CMakePackageConfigHelpers)
diff --git a/utf8cppConfig.cmake.in b/utf8cppConfig.cmake.in
index 9c15f36..4bdb9c4 100644
--- a/utf8cppConfig.cmake.in
+++ b/utf8cppConfig.cmake.in
@@ -2,3 +2,7 @@
 
 include("${CMAKE_CURRENT_LIST_DIR}/@PROJECT_NAME@Targets.cmake")
 check_required_components("@PROJECT_NAME@")
+
+if(NOT TARGET utf8::cpp)
+    add_library(utf8::cpp ALIAS utf8cpp::utf8cpp)
+endif()

ufleisch added a commit to ufleisch/utfcpp that referenced this issue Nov 4, 2023
Also provide an imported target utf8::cpp for backward compatibility.
nemtrif added a commit that referenced this issue Nov 4, 2023
Fix CMake build system to use existing include directory (#112)
@nemtrif
Copy link
Owner

nemtrif commented Nov 4, 2023

Fixed by @ufleisch, I think.

@nemtrif nemtrif closed this as completed Nov 4, 2023
@diizzyy
Copy link
Author

diizzyy commented Nov 4, 2023

@ufleisch
Shouldn't we also use the more common lib/cmake/projectname install path rather than installing directly into share/cmake ?

@mhx
Copy link
Contributor

mhx commented Nov 30, 2023

Fixed by @ufleisch, I think.

I don't think this is actually fixed as the files are still installed to a different location as before.

I haven't found a rationale anywhere for why the files were moved from /usr/include/utf8cpp to /usr/include in the first place. The only hint is a "Rewrite CMakeLists" note in 925e714, but that doesn't even indicate that move was intentional.

mhx added a commit to mhx/utfcpp that referenced this issue Nov 30, 2023
This reverts the install path of the headers to the path that was
used before the 4.x release series, unbreaking projects that were
building against the library without using the CMake config files.
@nemtrif nemtrif reopened this Dec 3, 2023
nemtrif added a commit that referenced this issue Dec 3, 2023
fix: revert to pre-4.x install path {prefix}/utf8cpp (see #112)
@nemtrif
Copy link
Owner

nemtrif commented Dec 3, 2023

Hopefully fixed in 4.0.3

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants