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

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

Closed
M-Gjerde opened this issue Oct 18, 2023 · 25 comments · Fixed by #803
Closed

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

M-Gjerde opened this issue Oct 18, 2023 · 25 comments · Fixed by #803
Assignees

Comments

@M-Gjerde
Copy link
Contributor

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
Copy link
Collaborator

@aqnuep what is your recommendation?

@MarkCallow
Copy link
Collaborator

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
Copy link
Collaborator

@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
Copy link
Contributor Author

@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
Copy link
Collaborator

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
Copy link
Contributor Author

Thank you, I appreciate that.

@MarkCallow
Copy link
Collaborator

Re-opening to remind me to fix the problem.

@MarkCallow MarkCallow reopened this Oct 30, 2023
@St0wy
Copy link

St0wy commented Nov 7, 2023

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
Copy link
Collaborator

aqnuep commented Nov 7, 2023

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
Copy link
Collaborator

MarkCallow commented Nov 8, 2023

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
Copy link

VVD commented Feb 27, 2024

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

@VVD
Copy link

VVD commented Feb 27, 2024

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
Copy link
Contributor Author

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

@VVD
Copy link

VVD commented Feb 27, 2024

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

@MarkCallow
Copy link
Collaborator

MarkCallow commented Feb 28, 2024

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
Copy link

VVD commented Feb 28, 2024

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
Copy link
Collaborator

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
Copy link

VVD commented Feb 29, 2024

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
Copy link

VVD commented Feb 29, 2024

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
Copy link
Collaborator

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
Copy link

VVD commented Feb 29, 2024

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
Copy link
Collaborator

Please try adding

set(FMT_INSTALL OFF)

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

@VVD
Copy link

VVD commented Mar 1, 2024

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
Copy link
Collaborator

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
Copy link

VVD commented Mar 1, 2024

Done.
Thanks!

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

Successfully merging a pull request may close this issue.

5 participants