From 78027a8e5bd8872ee5c2c9e4a9151d21dd9cf689 Mon Sep 17 00:00:00 2001 From: Yat Ho Date: Sun, 2 Jun 2024 03:10:52 +0800 Subject: [PATCH] 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 | 8 -- Transmission.xcodeproj/project.pbxproj | 20 ++--- cmake/FindMINIUPNPC.cmake | 98 +------------------------ libtransmission/CMakeLists.txt | 1 - libtransmission/port-forwarding-upnp.cc | 40 ++-------- 5 files changed, 20 insertions(+), 147 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index ac786853f..1c997db12 100644 --- a/CMakeLists.txt +++ b/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 - $<$: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 diff --git a/Transmission.xcodeproj/project.pbxproj b/Transmission.xcodeproj/project.pbxproj index 8c0054cf2..dd978e0dc 100644 --- a/Transmission.xcodeproj/project.pbxproj +++ b/Transmission.xcodeproj/project.pbxproj @@ -2716,8 +2716,8 @@ isa = PBXNativeTarget; buildConfigurationList = BE11834C0CE160A80002D0F3 /* Build configuration list for PBXNativeTarget "miniupnp" */; buildPhases = ( - A2305097100C0293003FDB0C /* ShellScript */, - C12F197C1E1AE55A0005E93F /* ShellScript */, + A2305097100C0293003FDB0C /* updateminiupnpcstrings */, + 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", diff --git a/cmake/FindMINIUPNPC.cmake b/cmake/FindMINIUPNPC.cmake index 99e19068c..e15af6785 100644 --- a/cmake/FindMINIUPNPC.cmake +++ b/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 - #include - #include - #include - 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 - #include - #include - 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_API_VERSION - VERSION_VAR MINIUPNPC_VERSION) + MINIUPNPC_INCLUDE_DIR) mark_as_advanced(MINIUPNPC_INCLUDE_DIR MINIUPNPC_LIBRARY) diff --git a/libtransmission/CMakeLists.txt b/libtransmission/CMakeLists.txt index 170760da8..d7ec25f22 100644 --- a/libtransmission/CMakeLists.txt +++ b/libtransmission/CMakeLists.txt @@ -222,7 +222,6 @@ target_compile_definitions(${TR_NAME} $<$:WITH_INOTIFY> $<$:WITH_KQUEUE> $<$:WITH_UTP> - $<$:MINIUPNPC_API_VERSION=${MINIUPNPC_API_VERSION}> # API version macro was only added in 1.7 $<$:USE_SYSTEM_B64> $<$:HAVE_SO_REUSEPORT=1> PUBLIC diff --git a/libtransmission/port-forwarding-upnp.cc b/libtransmission/port-forwarding-upnp.cc index 15c248c5d..6d7bbc7f7 100644 --- a/libtransmission/port-forwarding-upnp.cc +++ b/libtransmission/port-forwarding-upnp.cc @@ -15,13 +15,8 @@ #include -#ifdef SYSTEM_MINIUPNP #include #include -#else -#include -#include -#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) {