refactor: cleanup build for miniupnp (#6665)

* fix: remove redundant/outdated miniupnp cmake definitions

* refactor: simplify miniupnpc includes

* fix path to miniupnp

* fix: Xcode project

* fixup! fix: Xcode project

* code review: Xcode changes from mikedld

* refactor: drop miniupnpc support below `1.7`

CMakeLists.txt

@@ -526,17 +526,9 @@ if(NOT USE_SYSTEM_MINIUPNPC)
     target_compile_definitions(miniupnpc::libminiupnpc
         INTERFACE
             MINIUPNP_STATICLIB)
-
-    set(MINIUPNPC_VERSION 2.2)
-    set(MINIUPNPC_API_VERSION 17)
 endif()
 unset(TR_MINIUPNPC_LIBNAME)
 
-target_compile_definitions(miniupnpc::libminiupnpc
-    INTERFACE
-        SYSTEM_MINIUPNP
-        $<$<VERSION_LESS:${MINIUPNPC_VERSION},1.7>:MINIUPNPC_API_VERSION=${MINIUPNPC_API_VERSION}>) # API version macro was only added in 1.7
-
 add_subdirectory(${TR_THIRD_PARTY_SOURCE_DIR}/wildmat)
 
 tr_add_external_auto_library(DHT dht dht

Transmission.xcodeproj/project.pbxproj

@@ -2716,8 +2716,8 @@
 			isa = PBXNativeTarget;
 			buildConfigurationList = BE11834C0CE160A80002D0F3 /* Build configuration list for PBXNativeTarget "miniupnp" */;
 			buildPhases = (
-				A2305097100C0293003FDB0C /* ShellScript */,
+				A2305097100C0293003FDB0C /* updateminiupnpcstrings */,
-				C12F197C1E1AE55A0005E93F /* ShellScript */,
+				C12F197C1E1AE55A0005E93F /* symlinks */,
 				BE1183440CE160960002D0F3 /* Headers */,
 				BE1183450CE160960002D0F3 /* Sources */,
 				BE1183460CE160960002D0F3 /* Frameworks */,
@@ -3053,7 +3053,7 @@
 			shellPath = /bin/sh;
 			shellScript = "sh update-version-h.sh\n";
 		};
-		A2305097100C0293003FDB0C /* ShellScript */ = {
+		A2305097100C0293003FDB0C /* updateminiupnpcstrings */ = {
 			isa = PBXShellScriptBuildPhase;
 			buildActionMask = 2147483647;
 			files = (
@@ -3063,6 +3063,7 @@
 				"third-party/miniupnp/miniupnpc/miniupnpcstrings.h.in",
 				"third-party/miniupnp/miniupnpc/updateminiupnpcstrings.sh",
 			);
+			name = updateminiupnpcstrings;
 			outputPaths = (
 				"third-party/miniupnp/miniupnpc/miniupnpcstrings.h",
 			);
@@ -3088,19 +3089,20 @@
 			shellPath = /bin/bash;
 			shellScript = "cd third-party/libevent/include/event2\n\nif [ ! -e event-config.h -a ! ../../../macosx-libevent-event-config.h -ef event-config.h ]; then\n    ln -s ../../../macosx-libevent-event-config.h event-config.h;\nfi\n\nif [ ! -e ../evconfig-private.h -a ! ../../macosx-libevent-evconfig-private.h -ef ../evconfig-private.h ]; then\n    ln -s ../../macosx-libevent-evconfig-private.h ../evconfig-private.h;\nfi\n";
 		};
-		C12F197C1E1AE55A0005E93F /* ShellScript */ = {
+		C12F197C1E1AE55A0005E93F /* symlinks */ = {
 			isa = PBXShellScriptBuildPhase;
 			buildActionMask = 2147483647;
 			files = (
 			);
 			inputPaths = (
 			);
+			name = symlinks;
 			outputPaths = (
-				"third-party/miniupnp/miniupnp",
+				"third-party/miniupnp/miniupnpc/include/miniupnpc",
 			);
 			runOnlyForDeploymentPostprocessing = 0;
 			shellPath = /bin/sh;
-			shellScript = "cd third-party/miniupnp && rm -f miniupnp && ln -s . miniupnp\n";
+			shellScript = "cd third-party/miniupnp/miniupnpc\nln -sf . include/miniupnpc\n";
 		};
 		C12F197E1E1AE6D50005E93F /* ShellScript */ = {
 			isa = PBXShellScriptBuildPhase;
@@ -4009,7 +4011,7 @@
 					"third-party/libnatpmp/*.h",
 					"third-party/libpsl/include",
 					"third-party/libutp/include",
-					"third-party/miniupnpc/*.h",
+					"third-party/miniupnp/miniupnpc/include",
 					"third-party/utfcpp/source",
 					"third-party/wide-integer",
 					"third-party/wildmat",
@@ -4254,7 +4256,7 @@
 					"third-party/libnatpmp/*.h",
 					"third-party/libpsl/include",
 					"third-party/libutp/include",
-					"third-party/miniupnpc/*.h",
+					"third-party/miniupnp/miniupnpc/include",
 					"third-party/utfcpp/source",
 					"third-party/wide-integer",
 					"third-party/wildmat",
@@ -4578,7 +4580,7 @@
 					"third-party/libnatpmp/*.h",
 					"third-party/libpsl/include",
 					"third-party/libutp/include",
-					"third-party/miniupnpc/*.h",
+					"third-party/miniupnp/miniupnpc/include",
 					"third-party/utfcpp/source",
 					"third-party/wide-integer",
 					"third-party/wildmat",

cmake/FindMINIUPNPC.cmake

@@ -21,100 +21,6 @@ find_library(MINIUPNPC_LIBRARY
         libminiupnpc
     HINTS ${_MINIUPNPC_LIBDIR})
 
-if(MINIUPNPC_INCLUDE_DIR)
-    if(_MINIUPNPC_VERSION)
-        set(MINIUPNPC_VERSION ${_MINIUPNPC_VERSION})
-    else()
-        file(STRINGS "${MINIUPNPC_INCLUDE_DIR}/miniupnpc/miniupnpc.h" MINIUPNPC_VERSION_STR
-            REGEX "^#define[\t ]+MINIUPNPC_VERSION[\t ]+\"[^\"]+\"")
-        if(MINIUPNPC_VERSION_STR MATCHES "\"([^\"]+)\"")
-            set(MINIUPNPC_VERSION "${CMAKE_MATCH_1}")
-        endif()
-
-        # Let's hope it's 1.7 or higher, since it provides
-        # MINIUPNPC_API_VERSION and we won't have to figure
-        # it out on our own
-        file(STRINGS "${MINIUPNPC_INCLUDE_DIR}/miniupnpc/miniupnpc.h" MINIUPNPC_API_VERSION_STR
-            REGEX "^#define[\t ]+MINIUPNPC_API_VERSION[\t ]+[0-9]+")
-        if(MINIUPNPC_API_VERSION_STR MATCHES "^#define[\t ]+MINIUPNPC_API_VERSION[\t ]+([0-9]+)")
-            set(MINIUPNPC_API_VERSION "${CMAKE_MATCH_1}")
-        endif()
-    endif()
-
-    if(MINIUPNPC_LIBRARY)
-        # Or maybe it's miniupnp 1.6
-        if(NOT DEFINED MINIUPNPC_API_VERSION)
-            file(WRITE ${CMAKE_BINARY_DIR}${CMAKE_FILES_DIRECTORY}/CMakeTmp/CheckMiniUPnPC_1.6.c
-                "#include <stdlib.h>
-                #include <errno.h>
-                #include <miniupnpc/miniupnpc.h>
-                #include <miniupnpc/upnpcommands.h>
-                int main()
-                {
-                    struct UPNPDev * devlist;
-                    struct UPNPUrls urls;
-                    struct IGDdatas data;
-                    char lanaddr[16];
-                    char portStr[8];
-                    char intPort[8];
-                    char intClient[16];
-                    upnpDiscover( 2000, NULL, NULL, 0, 0, &errno );
-                    UPNP_GetValidIGD( devlist, &urls, &data, lanaddr, sizeof( lanaddr ) );
-                    UPNP_GetSpecificPortMappingEntry( urls.controlURL, data.first.servicetype,
-                                        portStr, \"TCP\", intClient, intPort, NULL, NULL, NULL );
-                    return 0;
-                }")
-            try_compile(_MINIUPNPC_HAVE_VERSION_1_6
-                ${CMAKE_BINARY_DIR}
-                ${CMAKE_BINARY_DIR}${CMAKE_FILES_DIRECTORY}/CMakeTmp/CheckMiniUPnPC_1.6.c
-                COMPILE_DEFINITIONS -DINCLUDE_DIRECTORIES=${MINIUPNPC_INCLUDE_DIR}
-                LINK_LIBRARIES ${MINIUPNPC_LIBRARY}
-                OUTPUT_VARIABLE OUTPUT)
-            if(_MINIUPNPC_HAVE_VERSION_1_6)
-                if(NOT DEFINED MINIUPNPC_VERSION)
-                    set(MINIUPNPC_VERSION 1.6)
-                endif()
-                set(MINIUPNPC_API_VERSION 8)
-            endif()
-        endif()
-
-        # Or maybe it's miniupnp 1.5
-        if(NOT DEFINED MINIUPNPC_API_VERSION)
-            file(WRITE ${CMAKE_BINARY_DIR}${CMAKE_FILES_DIRECTORY}/CMakeTmp/CheckMiniUPnPC_1.5.c
-                "#include <stdlib.h>
-                #include <miniupnpc/miniupnpc.h>
-                #include <miniupnpc/upnpcommands.h>
-                int main()
-                {
-                    struct UPNPDev * devlist;
-                    struct UPNPUrls urls;
-                    struct IGDdatas data;
-                    char lanaddr[16];
-                    char portStr[8];
-                    char intPort[8];
-                    char intClient[16];
-                    upnpDiscover( 2000, NULL, NULL, 0 );
-                    UPNP_GetValidIGD( devlist, &urls, &data, lanaddr, sizeof( lanaddr ) );
-                    UPNP_GetSpecificPortMappingEntry( urls.controlURL, data.first.servicetype,
-                                        portStr, \"TCP\", intClient, intPort );
-                    return 0;
-                }")
-            try_compile(_MINIUPNPC_HAVE_VERSION_1_5
-                ${CMAKE_BINARY_DIR}
-                ${CMAKE_BINARY_DIR}${CMAKE_FILES_DIRECTORY}/CMakeTmp/CheckMiniUPnPC_1.5.c
-                COMPILE_DEFINITIONS -DINCLUDE_DIRECTORIES=${MlINIUPNPC_INCLUDE_DIR}
-                LINK_LIBRARIES ${MINIUPNPC_LIBRARY}
-                OUTPUT_VARIABLE OUTPUT)
-            if(_MINIUPNPC_HAVE_VERSION_1_5)
-                if(NOT DEFINED MINIUPNPC_VERSION)
-                    set(MINIUPNPC_VERSION 1.5)
-                endif()
-                set(MINIUPNPC_API_VERSION 5)
-            endif()
-        endif()
-    endif()
-endif()
-
 set(MINIUPNPC_INCLUDE_DIRS ${MINIUPNPC_INCLUDE_DIR})
 set(MINIUPNPC_LIBRARIES ${MINIUPNPC_LIBRARY})
 
@@ -123,9 +29,7 @@ include(FindPackageHandleStandardArgs)
 find_package_handle_standard_args(MINIUPNPC
     REQUIRED_VARS
         MINIUPNPC_LIBRARY
-        MINIUPNPC_INCLUDE_DIR
+        MINIUPNPC_INCLUDE_DIR)
-        MINIUPNPC_API_VERSION
-    VERSION_VAR MINIUPNPC_VERSION)
 
 mark_as_advanced(MINIUPNPC_INCLUDE_DIR MINIUPNPC_LIBRARY)
 

libtransmission/CMakeLists.txt

@@ -222,7 +222,6 @@ target_compile_definitions(${TR_NAME}
         $<$<BOOL:${WITH_INOTIFY}>:WITH_INOTIFY>
         $<$<BOOL:${WITH_KQUEUE}>:WITH_KQUEUE>
         $<$<BOOL:${ENABLE_UTP}>:WITH_UTP>
-        $<$<VERSION_LESS:${MINIUPNPC_VERSION},1.7>:MINIUPNPC_API_VERSION=${MINIUPNPC_API_VERSION}> # API version macro was only added in 1.7
         $<$<BOOL:${USE_SYSTEM_B64}>:USE_SYSTEM_B64>
         $<$<BOOL:${HAVE_SO_REUSEPORT}>:HAVE_SO_REUSEPORT=1>
     PUBLIC

libtransmission/port-forwarding-upnp.cc

@@ -15,13 +15,8 @@
 
 #include <fmt/core.h>
 
-#ifdef SYSTEM_MINIUPNP
 #include <miniupnpc/miniupnpc.h>
 #include <miniupnpc/upnpcommands.h>
-#else
-#include <miniupnp/miniupnpc.h>
-#include <miniupnp/upnpcommands.h>
-#endif
 
 #define LIBTRANSMISSION_PORT_FORWARDING_MODULE
 
@@ -34,6 +29,10 @@
 #include "libtransmission/tr-macros.h" // TR_ADDRSTRLEN
 #include "libtransmission/utils.h" // for _(), tr_strerror()
 
+#ifndef MINIUPNPC_API_VERSION
+#error miniupnpc >= 1.7 is required
+#endif
+
 namespace
 {
 enum class UpnpState : uint8_t
@@ -105,20 +104,16 @@ constexpr auto port_fwd_state(UpnpState upnp_state, bool is_mapped)
     UPNPDev* ret = nullptr;
     auto have_err = bool{};
 
-#if (MINIUPNPC_API_VERSION >= 8) /* adds ipv6 and error args */
+    // MINIUPNPC_API_VERSION >= 8 (adds ipv6 and error args)
     int err = UPNPDISCOVER_SUCCESS;
 
-#if (MINIUPNPC_API_VERSION >= 14) /* adds ttl */
+#if (MINIUPNPC_API_VERSION >= 14) // adds ttl
     ret = upnpDiscover(msec, bindaddr, nullptr, 0, 0, 2, &err);
 #else
     ret = upnpDiscover(msec, bindaddr, nullptr, 0, 0, &err);
 #endif
 
     have_err = err != UPNPDISCOVER_SUCCESS;
-#else
-    ret = upnpDiscover(msec, bindaddr, nullptr, 0);
-    have_err = ret == nullptr;
-#endif
 
     if (have_err)
     {
@@ -147,7 +142,7 @@ constexpr auto port_fwd_state(UpnpState upnp_state, bool is_mapped)
         nullptr /*desc*/,
         nullptr /*enabled*/,
         nullptr /*duration*/);
-#elif (MINIUPNPC_API_VERSION >= 8) /* adds desc, enabled and leaseDuration args */
+#else // MINIUPNPC_API_VERSION >= 8 (adds desc, enabled and leaseDuration args)
     int const err = UPNP_GetSpecificPortMappingEntry(
         handle->urls.controlURL,
         handle->data.first.servicetype,
@@ -158,14 +153,6 @@ constexpr auto port_fwd_state(UpnpState upnp_state, bool is_mapped)
         nullptr /*desc*/,
         nullptr /*enabled*/,
         nullptr /*duration*/);
-#else
-    int const err = UPNP_GetSpecificPortMappingEntry(
-        handle->urls.controlURL,
-        handle->data.first.servicetype,
-        port_str.c_str(),
-        proto,
-        std::data(int_client),
-        std::data(int_port));
 #endif
 
     return err;
@@ -184,7 +171,7 @@ constexpr auto port_fwd_state(UpnpState upnp_state, bool is_mapped)
     auto const advertised_port_str = std::to_string(advertised_port.host());
     auto const local_port_str = std::to_string(local_port.host());
 
-#if (MINIUPNPC_API_VERSION >= 8)
+    // MINIUPNPC_API_VERSION >= 8
     int const err = UPNP_AddPortMapping(
         handle->urls.controlURL,
         handle->data.first.servicetype,
@@ -195,17 +182,6 @@ constexpr auto port_fwd_state(UpnpState upnp_state, bool is_mapped)
         proto,
         nullptr,
         nullptr);
-#else
-    int const err = UPNP_AddPortMapping(
-        handle->urls.controlURL,
-        handle->data.first.servicetype,
-        advertised_port_str.c_str(),
-        local_port_str.c_str(),
-        handle->lanaddr.c_str(),
-        desc,
-        proto,
-        nullptr);
-#endif
 
     if (err != 0)
     {