KhronosGroup / KTX-Software

KTX (Khronos Texture) Library and Tools
Other
861 stars 225 forks source link

Issue using KTX-Software when fmt is already included #786

Closed M-Gjerde closed 9 months ago

M-Gjerde commented 11 months ago

So I updated KTX-Software to a later release and it seems fmt as been added as a dependency to KTX-Software. My issue is that I use KTX-Software as a submodule myself alongside fmt which makes my project break with:

[INFO] Adding FMT from directory: external/fmt CMake Error at external/fmt/CMakeLists.txt:50 (add_library): add_library cannot create target "fmt" because another target with the same name already exists. The existing target is a static library created in source directory "----/KTX-Software/other_projects/fmt". See documentation for policy CMP0002 for more details. Call Stack (most recent call first): external/fmt/CMakeLists.txt:285 (add_module_library)

Is there a way you can make fmt for KTX-Software optional? Or supply with an already built target?

Best Regards

MarkCallow commented 10 months ago

@aqnuep what is your recommendation?

MarkCallow commented 10 months ago

One way to fix this is to do a find_package for {fmt} and only if not found use the copy in this repo. @aqnuep?

MarkCallow commented 10 months ago

@M-Gjerde why are you including KTX-Software as a submodule with the CLI tools enabled? As a submodule you should only need the library so should set KTX_FEATURE_TOOLS to OFF. Without the tools, {fmt} will not be included.

M-Gjerde commented 10 months ago

@MarkCallow Thank you for getting back to me. You're right, I'm not using the CLI tools when I use KTX as a submodule. However, setting KTX_FEATURE_TOOLS to OFF still includes fmt though. Currently, I'm just linking against the fmt::fmt target built by KTX, but I'd like to control that myself in the case of 'what if' {fmt} is removed or changed in a future version.

MarkCallow commented 10 months ago

However, setting KTX_FEATURE_TOOLS to OFF still includes fmt though.

Thank you for the report. I'll fix that in the near future.

M-Gjerde commented 10 months ago

Thank you, I appreciate that.

MarkCallow commented 10 months ago

Re-opening to remind me to fix the problem.

St0wy commented 10 months ago

I fixed this in a fork by checking if the target fmt::fmt already exists :

if(NOT TARGET fmt::fmt)
    set(FMT_SYSTEM_HEADERS ON)
    add_subdirectory(other_projects/fmt)
endif()

I saw this being used in a CMake template. I could make a pull request with this change if it seems like a good way to fix it (it could also be used with other dependecies).

aqnuep commented 10 months ago

I believe such a solution could work. Although I'd certainly try out using e.g. the EXCLUDE_FROM_ALL parameter of add_subdirectory first, because your proposal would make KTX-Software use the external fmt implementation you use instead of the one included here. I don't expect that to cause any problems though.

MarkCallow commented 10 months ago

The first fix is to put the following lines in CMakeLists.txt

https://github.com/KhronosGroup/KTX-Software/blob/88fc7a6e97b2cf8c91343977abe221d054184246/CMakeLists.txt#L1021C1-L1024C41

[EDIT: Not sure why the above link isn't causing the lines to be shown here. Apologies.]

into an

if(KTX_FEATURE_TOOLS)
endif()

block. If people actually want to build the tools with other projects that are already using fmt then we can add what is proposed here:


if(KTX_FEATURE_TOOLS)
  if(NOT TARGET fmt::fmt)
    set(FMT_SYSTEM_HEADERS ON)
    add_subdirectory(other_projects/fmt)
  endif()
  if(NOT TARGET cxxopts::cxxopts
    add_subdirectory(other_projects/cxxopts)
  endif()
endif()
VVD commented 6 months ago

How to use installed in system libfmt? Why use embedded one?

VVD commented 6 months ago

My dirty hack:

--- CMakeLists.txt.orig 2024-02-27 15:40:40 UTC
+++ CMakeLists.txt
@@ -1065,7 +1065,7 @@ endif()
 # except for building the ktx library.
 if((KTX_FEATURE_TOOLS OR KTX_FEATURE_TESTS) AND NOT TARGET fmt::fmt)
     set(FMT_SYSTEM_HEADERS ON)
-    add_subdirectory(other_projects/fmt)
+#    add_subdirectory(other_projects/fmt)
 endif()
 if(KTX_FEATURE_TOOLS AND NOT TARGET cxxopts::cxxopts)
     add_subdirectory(other_projects/cxxopts)
--- tests/tests.cmake.orig      2024-02-27 15:41:38 UTC
+++ tests/tests.cmake
@@ -62,7 +62,7 @@ target_link_libraries(
     unittests
     gtest
     ktx
-    fmt::fmt
+#    fmt::fmt
     ${CMAKE_THREAD_LIBS_INIT}
 )

--- tools/imageio/CMakeLists.txt.orig   2024-02-27 15:41:01 UTC
+++ tools/imageio/CMakeLists.txt
@@ -71,4 +71,5 @@ set_target_properties(imageio PROPERTIES
     CXX_VISIBILITY_PRESET ${STATIC_APP_LIB_SYMBOL_VISIBILITY}
 )

-target_link_libraries(imageio fmt::fmt)
+#target_link_libraries(imageio fmt::fmt)
+target_link_libraries(imageio)
--- tools/ktx/CMakeLists.txt.orig       2024-02-27 15:41:19 UTC
+++ tools/ktx/CMakeLists.txt
@@ -63,7 +63,7 @@ PRIVATE
     ktx
     ${ASTCENC_LIB_TARGET}
     $<IF:$<BOOL:${WIN32}>,Pathcch,> # For PathCchRemoveFileSpec on Windows
-    fmt::fmt
+#    fmt::fmt
     cxxopts::cxxopts
 )

LDFLAGS+= -lfmt -L/usr/local/lib CFLAGS+= -I/usr/local/include

M-Gjerde commented 6 months ago

This has already been fixed, maybe an update to the latest commit on the master branch will solve this for you.

VVD commented 6 months ago

I'm using last release 4.3.1. But my problem is a bit different - build and link with installed in system libfmt.

MarkCallow commented 6 months ago

Does the standard build fail in the presence of a "system installed" libfmt or is it that you want to use the system one?

Why use embedded one?

Because not everyone has a system-installed libfmt and because, as it is part of C++20, the need for such libraries and the libraries will disappear.

VVD commented 6 months ago

Install stage failed - tried install include/fmt/*.h files which conflicts with files from system-installed libfmt.

Because not everyone has a system-installed libfmt and because, as it is part of C++20, the need for such libraries and the libraries will disappear.

Can I set -std=c++20 (-std=c++2a) for prevent use any libfmt (embedded and system-installed)?

MarkCallow commented 6 months ago

Install stage failed - tried install include/fmt/*.h files which conflicts with files from system-installed libfmt.

What exact install command did you use. What system are you using? The installable packages produced by this project do not include any fmt/*.h headers. They are not needed in order to use libktx.

Can I set -std=c++20 (-std=c++2a) for prevent use any libfmt (embedded and system-installed)?

Build with C++20 hasn't been tested. I expect it will still use the embedded {fmt} library. I'd be happy to accept a PR to make the build work with C++20 provided a build with C++11 still works.

VVD commented 6 months ago

FreeBSD 13.2-p10 amd64. I create port graphics/khronos-texture: https://reviews.freebsd.org/D44120

Log of the install stage:

cd /tmp/work/usr/ports/graphics/khronos-texture/work/.build && /usr/local/bin/cmake -DCMAKE_INSTALL_DO_STRIP=1 -P cmake_install.cmake
-- Install configuration: "Release"
-- Installing: /tmp/work/usr/ports/graphics/khronos-texture/work/stage/usr/local/lib/libfmt.a
-- Installing: /tmp/work/usr/ports/graphics/khronos-texture/work/stage/usr/local/include/fmt/args.h
-- Installing: /tmp/work/usr/ports/graphics/khronos-texture/work/stage/usr/local/include/fmt/chrono.h
-- Installing: /tmp/work/usr/ports/graphics/khronos-texture/work/stage/usr/local/include/fmt/color.h
-- Installing: /tmp/work/usr/ports/graphics/khronos-texture/work/stage/usr/local/include/fmt/compile.h
-- Installing: /tmp/work/usr/ports/graphics/khronos-texture/work/stage/usr/local/include/fmt/core.h
-- Installing: /tmp/work/usr/ports/graphics/khronos-texture/work/stage/usr/local/include/fmt/format.h
-- Installing: /tmp/work/usr/ports/graphics/khronos-texture/work/stage/usr/local/include/fmt/format-inl.h
-- Installing: /tmp/work/usr/ports/graphics/khronos-texture/work/stage/usr/local/include/fmt/os.h
-- Installing: /tmp/work/usr/ports/graphics/khronos-texture/work/stage/usr/local/include/fmt/ostream.h
-- Installing: /tmp/work/usr/ports/graphics/khronos-texture/work/stage/usr/local/include/fmt/printf.h
-- Installing: /tmp/work/usr/ports/graphics/khronos-texture/work/stage/usr/local/include/fmt/ranges.h
-- Installing: /tmp/work/usr/ports/graphics/khronos-texture/work/stage/usr/local/include/fmt/std.h
-- Installing: /tmp/work/usr/ports/graphics/khronos-texture/work/stage/usr/local/include/fmt/xchar.h
-- Installing: /tmp/work/usr/ports/graphics/khronos-texture/work/stage/usr/local/lib/cmake/fmt/fmt-config.cmake
-- Installing: /tmp/work/usr/ports/graphics/khronos-texture/work/stage/usr/local/lib/cmake/fmt/fmt-config-version.cmake
-- Installing: /tmp/work/usr/ports/graphics/khronos-texture/work/stage/usr/local/lib/cmake/fmt/fmt-targets.cmake
-- Installing: /tmp/work/usr/ports/graphics/khronos-texture/work/stage/usr/local/lib/cmake/fmt/fmt-targets-release.cmake
-- Installing: /tmp/work/usr/ports/graphics/khronos-texture/work/stage/usr/local/lib/pkgconfig/fmt.pc
-- Installing: /tmp/work/usr/ports/graphics/khronos-texture/work/stage/usr/local/bin/ktx
-- Set non-toolchain portion of runtime path of "/tmp/work/usr/ports/graphics/khronos-texture/work/stage/usr/local/bin/ktx" to "$ORIGIN:$ORIGIN/../lib"
-- Installing: /tmp/work/usr/ports/graphics/khronos-texture/work/stage/usr/local/bin/ktx2check
-- Set non-toolchain portion of runtime path of "/tmp/work/usr/ports/graphics/khronos-texture/work/stage/usr/local/bin/ktx2check" to "$ORIGIN:$ORIGIN/../lib"
-- Installing: /tmp/work/usr/ports/graphics/khronos-texture/work/stage/usr/local/bin/ktx2ktx2
-- Set non-toolchain portion of runtime path of "/tmp/work/usr/ports/graphics/khronos-texture/work/stage/usr/local/bin/ktx2ktx2" to "$ORIGIN:$ORIGIN/../lib"
-- Installing: /tmp/work/usr/ports/graphics/khronos-texture/work/stage/usr/local/bin/ktxinfo
-- Set non-toolchain portion of runtime path of "/tmp/work/usr/ports/graphics/khronos-texture/work/stage/usr/local/bin/ktxinfo" to "$ORIGIN:$ORIGIN/../lib"
-- Installing: /tmp/work/usr/ports/graphics/khronos-texture/work/stage/usr/local/bin/ktxsc
-- Set non-toolchain portion of runtime path of "/tmp/work/usr/ports/graphics/khronos-texture/work/stage/usr/local/bin/ktxsc" to "$ORIGIN:$ORIGIN/../lib"
-- Installing: /tmp/work/usr/ports/graphics/khronos-texture/work/stage/usr/local/bin/toktx
-- Set non-toolchain portion of runtime path of "/tmp/work/usr/ports/graphics/khronos-texture/work/stage/usr/local/bin/toktx" to "$ORIGIN:$ORIGIN/../lib"
-- Installing: /tmp/work/usr/ports/graphics/khronos-texture/work/stage/usr/local/lib/libktx.so.0.0.0
-- Installing: /tmp/work/usr/ports/graphics/khronos-texture/work/stage/usr/local/lib/libktx.so.0
-- Installing: /tmp/work/usr/ports/graphics/khronos-texture/work/stage/usr/local/include/ktx.h
-- Installing: /tmp/work/usr/ports/graphics/khronos-texture/work/stage/usr/local/include/ktxvulkan.h
-- Installing: /tmp/work/usr/ports/graphics/khronos-texture/work/stage/usr/local/lib/libktx.so
-- Installing: /tmp/work/usr/ports/graphics/khronos-texture/work/stage/usr/local/include/KHR/khr_df.h
-- Installing: /tmp/work/usr/ports/graphics/khronos-texture/work/stage/usr/local/lib/cmake/ktx/KtxTargets.cmake
-- Installing: /tmp/work/usr/ports/graphics/khronos-texture/work/stage/usr/local/lib/cmake/ktx/KtxTargets-release.cmake
-- Installing: /tmp/work/usr/ports/graphics/khronos-texture/work/stage/usr/local/lib/cmake/ktx/KtxConfig.cmake
-- Installing: /tmp/work/usr/ports/graphics/khronos-texture/work/stage/usr/local/lib/cmake/ktx/KtxConfigVersion.cmake

/tmp/work/usr/ports/graphics/khronos-texture/work/.build/cmake_install.cmake have lines:

if(NOT CMAKE_INSTALL_LOCAL_ONLY)
  # Include the install script for the subdirectory.
  include("/tmp/work/usr/ports/graphics/khronos-texture/work/.build/other_projects/fmt/cmake_install.cmake")
endif()

/tmp/work/usr/ports/graphics/khronos-texture/work/.build/other_projects/fmt/cmake_install.cmake have lines:

if(CMAKE_INSTALL_COMPONENT STREQUAL "Unspecified" OR NOT CMAKE_INSTALL_COMPONENT)
  file(INSTALL DESTINATION "${CMAKE_INSTALL_PREFIX}/include/fmt" TYPE FILE FILES
    "/tmp/work/usr/ports/graphics/khronos-texture/work/KTX-Software-4.3.1/other_projects/fmt/include/fmt/args.h"
    "/tmp/work/usr/ports/graphics/khronos-texture/work/KTX-Software-4.3.1/other_projects/fmt/include/fmt/chrono.h"
    "/tmp/work/usr/ports/graphics/khronos-texture/work/KTX-Software-4.3.1/other_projects/fmt/include/fmt/color.h"
    "/tmp/work/usr/ports/graphics/khronos-texture/work/KTX-Software-4.3.1/other_projects/fmt/include/fmt/compile.h"
    "/tmp/work/usr/ports/graphics/khronos-texture/work/KTX-Software-4.3.1/other_projects/fmt/include/fmt/core.h"
    "/tmp/work/usr/ports/graphics/khronos-texture/work/KTX-Software-4.3.1/other_projects/fmt/include/fmt/format.h"
    "/tmp/work/usr/ports/graphics/khronos-texture/work/KTX-Software-4.3.1/other_projects/fmt/include/fmt/format-inl.h"
    "/tmp/work/usr/ports/graphics/khronos-texture/work/KTX-Software-4.3.1/other_projects/fmt/include/fmt/os.h"
    "/tmp/work/usr/ports/graphics/khronos-texture/work/KTX-Software-4.3.1/other_projects/fmt/include/fmt/ostream.h"
    "/tmp/work/usr/ports/graphics/khronos-texture/work/KTX-Software-4.3.1/other_projects/fmt/include/fmt/printf.h"
    "/tmp/work/usr/ports/graphics/khronos-texture/work/KTX-Software-4.3.1/other_projects/fmt/include/fmt/ranges.h"
    "/tmp/work/usr/ports/graphics/khronos-texture/work/KTX-Software-4.3.1/other_projects/fmt/include/fmt/std.h"
    "/tmp/work/usr/ports/graphics/khronos-texture/work/KTX-Software-4.3.1/other_projects/fmt/include/fmt/xchar.h"
    )
endif()

Will -DCMAKE_INSTALL_COMPONENT:BOOL=OFF fix this? Update: tested - no.

VVD commented 6 months ago

With -D KTX_FEATURE_TOOLS:BOOL=OFF it install all libfmt includes, cmake, pc, static libfmt.a and doesn't build:

bin/ktx
bin/ktx2check
bin/ktx2ktx2
bin/ktxinfo
bin/ktxsc
bin/toktx
MarkCallow commented 6 months ago

It looks like this is happening because of the include of the fmt directory in the KTX-Software CMakeLists.txt. We need to find a way to prevent the fmt install.

I'm not familiar with -P cmake_install.cmake. What happens if you use

/usr/local/bin/cmake -DCMAKE_INSTALL_DO_STRIP=1 --target install

This is what is run by the package builder and the packages do not include the fmt includes.

VVD commented 6 months ago

cmake -DCMAKE_INSTALL_DO_STRIP=1 -P cmake_install.cmake run automatically by ports infrastructure - possible get it from other cmake-files of the KTX project. I don't know how to change this behavior.

MarkCallow commented 6 months ago

Please try adding

set(FMT_INSTALL OFF)

just before line 1073 in KTX-Software's root CMakeList.txt and report back.

VVD commented 6 months ago

Thanks! This patch fixed the issue:

--- CMakeLists.txt.orig
+++ CMakeLists.txt
@@ -1064,6 +1064,7 @@ endif()
 # this CMakeLists is included in another project which is unlikely
 # except for building the ktx library.
 if((KTX_FEATURE_TOOLS OR KTX_FEATURE_TESTS) AND NOT TARGET fmt::fmt)
+    set(FMT_INSTALL OFF)
     set(FMT_SYSTEM_HEADERS ON)
     add_subdirectory(other_projects/fmt)
 endif()

Or build with -DFMT_INSTALL:BOOL=FALSE.

MarkCallow commented 6 months ago

This patch fixed the issue:

Great. Please open a new issue with a title something like "Project install installs fmt lib", reference the patch in the comment above and the comment with the install log. I'll add the fix to the next release. The issue is to make sure I don't forget.

VVD commented 6 months ago

Done. Thanks!