paceholder / nodeeditor

Qt Node Editor. Dataflow programming framework
BSD 3-Clause "New" or "Revised" License
2.94k stars 796 forks source link

Fix vcpkg conflict and add documentation #324

Closed arsdever closed 1 year ago

arsdever commented 1 year ago

Resolves #321

arsdever commented 1 year ago

I need someone to test this without vcpkg.

paceholder commented 1 year ago

Thanks for your pull request. Unfortunately, it broke the build. I tried to fix it based on your changes. Waiting for the build results now... https://github.com/paceholder/nodeeditor/pull/329

arsdever commented 1 year ago

@paceholder Hi. I was also trying to look through the PR again. I found something strange and wanted to clarify. On line external/CMakeLists.txt:2 you require a version 2.13.7 for the Catch2 package, but on this test/CMakeLists.txt:1 line you require a version 2.3.0 for the same package. Is there a reason for that?

Anyways, with the following patch applied, it works fine with the CMake's configuration command when the -DCMAKE_TOOLCHAIN_FILE=$VCPKG_DIR/scripts/buildsystems/vcpkg.cmake flag is passed.

diff --git a/external/CMakeLists.txt b/external/CMakeLists.txt
index 49ee9d2..d7fbfcc 100644
--- a/external/CMakeLists.txt
+++ b/external/CMakeLists.txt
@@ -1,15 +1,7 @@
 if(BUILD_TESTING)
-  find_package(Catch2 2.13.7 QUIET)
+  find_package(Catch2 QUIET)

   if(NOT Catch2_FOUND)
     add_subdirectory(Catch2)
   endif()
 endif()
-
-if(NOT COMMAND find_package)
-  macro(find_package pkg)
-    if(NOT TARGET "${pkg}")
-      _find_package(${ARGV})
-    endif()
-  endmacro()
-endif()
diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt
index dffa83b..6ee51b2 100644
--- a/test/CMakeLists.txt
+++ b/test/CMakeLists.txt
@@ -1,4 +1,4 @@
-find_package(Catch2 2.3.0 REQUIRED)
+find_package(Catch2 REQUIRED)

 if (Qt6_FOUND)
   find_package(Qt6 COMPONENTS Test)
paceholder commented 1 year ago

This part of the code wasn't originally written by me. That's why I can't give a detailed explanation. I think after some fixes and modernization one version was updated and the other one left untouched.

paceholder commented 1 year ago

I merged my corrected branch. Thanks for your idea and let me know if something pops up again.