Opened 10 years ago

Closed 7 years ago

Last modified 7 years ago

#4323 closed Enhancement (fixed)

Allow usage of system miniupnpc

Reported by: pva0xd Owned by: jordan
Priority: Normal Milestone: 2.83
Component: Transmission Version: 2.31
Severity: Normal Keywords:
Cc: jbicha@…

Description

Hi. Patchset below does two things: fixes build issue in gentoo and allows usage of system miniupnpc. I guess later other libraries in third-party directory will following since all distributions strive for separated libraries.

So what following patches do:

  1. 0001-configure.ac-Drop-redudant-code-indentation.patch - drops redudant code/indentation in configure.ac
  2. 0002-Add-qt-config-to-make-qmake-aware-about-.-configure-.patch - Add qt/config to make qmake aware about ./configure results. This simplifies use of system third-party tools. Fixes build issue caused by ordering of make/qmake run: bugs.gentoo.org/368523.
  3. 0003-Allow-usage-of-system-miniupnp.-Rename-miniupnp-mini.patch - Allow usage of system miniupnp. Rename miniupnp -> miniupnpc since upstream suggests miniupnpc name for headers.

Attachments (8)

0002-Add-qt-config-to-make-qmake-aware-about-.-configure-.patch (1.9 KB) - added by pva0xd 10 years ago.
0002-Add-qt-config-to-make-qmake-aware-about-.-configure-.patch
0003-Allow-usage-of-system-miniupnp.-Rename-miniupnp-mini.patch (21.6 KB) - added by pva0xd 10 years ago.
0003-Allow-usage-of-system-miniupnp.-Rename-miniupnp-mini.patch
0001-configure.ac-Drop-redudant-code-indentation.patch (1.5 KB) - added by pva0xd 10 years ago.
0001-configure.ac-Drop-redudant-code-indentation.patch
transmission-2.31-0002-config.in-4-qt.pro.patch (2.4 KB) - added by pva0xd 10 years ago.
0002-Add-qt-config-to-make-qmake-aware-about-.-configure-.patch
transmission-2.31-0003-system-miniupnpc.patch (21.8 KB) - added by pva0xd 10 years ago.
0003-Allow-usage-of-system-miniupnp.-Rename-miniupnp-mini.patch
0001-Allow-usage-of-system-miniupnp.-Rename-miniupnp-mini.patch (25.9 KB) - added by pva0xd 10 years ago.
0001-Allow-usage-of-system-miniupnp.-Rename-miniupnp-mini.patch
transmission-2.33-0003-system-miniupnpc.patch (10.2 KB) - added by pva0xd 10 years ago.
transmission-2.33-0003-system-miniupnpc.patch
001-upnp.c.diff (1.0 KB) - added by rb07 7 years ago.
Update to miniupnp version 1.9

Download all attachments as: .zip

Change History (28)

Changed 10 years ago by pva0xd

0002-Add-qt-config-to-make-qmake-aware-about-.-configure-.patch

Changed 10 years ago by pva0xd

0003-Allow-usage-of-system-miniupnp.-Rename-miniupnp-mini.patch

Changed 10 years ago by pva0xd

0001-configure.ac-Drop-redudant-code-indentation.patch

Changed 10 years ago by pva0xd

0002-Add-qt-config-to-make-qmake-aware-about-.-configure-.patch

Changed 10 years ago by pva0xd

0003-Allow-usage-of-system-miniupnp.-Rename-miniupnp-mini.patch

comment:1 Changed 10 years ago by rb07

The idea is good, but in practice MiniUPnP changed API between versions 1.4.x and 1.5.x, Transmission ships with the former so it has to be changed to use the later.

The change is minor, but you would have to apply a patch depending on the system's installed version of MiniUPnP.

For reference, this is what I use with MiniUPnP 1.5:

--- libtransmission/upnp.c      (revision 12439)
+++ libtransmission/upnp.c      (working copy)
@@ -97,7 +97,7 @@
     {
         struct UPNPDev * devlist;
         errno = 0;
-        devlist = upnpDiscover( 2000, NULL, NULL, 0 );
+        devlist = upnpDiscover( 2000, NULL, NULL, 0, &errno );
         if( devlist == NULL )
         {
             tr_ndbg(
@@ -144,9 +144,9 @@
 
         tr_snprintf( portStr, sizeof( portStr ), "%d", handle->port );
         if( UPNP_GetSpecificPortMappingEntry( handle->urls.controlURL, handle->data.first.servicetype,
-            portStr, "TCP", intClient, intPort ) != UPNPCOMMAND_SUCCESS  ||
+            portStr, "TCP", intClient, intPort, NULL, NULL, NULL ) != UPNPCOMMAND_SUCCESS  ||
             UPNP_GetSpecificPortMappingEntry( handle->urls.controlURL, handle->data.first.servicetype,
-            portStr, "UDP", intClient, intPort ) != UPNPCOMMAND_SUCCESS )
+            portStr, "UDP", intClient, intPort, NULL, NULL, NULL ) != UPNPCOMMAND_SUCCESS )
         {
             tr_ninf( getKey( ), _( "Port %d isn't forwarded" ), handle->port );
             handle->isMapped = false;
@@ -198,7 +198,7 @@
             err_tcp = UPNP_AddPortMapping( handle->urls.controlURL,
                                        handle->data.first.servicetype,
                                        portStr, portStr, handle->lanaddr,
-                                       desc, "TCP", NULL );
+                                       desc, "TCP", NULL, NULL );
             if( err_tcp )
                 tr_ndbg( getKey( ), "TCP Port forwarding failed with error %d (errno %d - %s)",
                          err_tcp, errno, tr_strerror( errno ) );
@@ -207,7 +207,7 @@
             err_udp = UPNP_AddPortMapping( handle->urls.controlURL,
                                        handle->data.first.servicetype,
                                        portStr, portStr, handle->lanaddr,
-                                       desc, "UDP", NULL );
+                                       desc, "UDP", NULL, NULL );
             if( err_udp )
                 tr_ndbg( getKey( ), "UDP Port forwarding failed with error %d (errno %d - %s)",
                          err_udp, errno, tr_strerror( errno ) );

comment:3 Changed 10 years ago by pva0xd

  • Status changed from new to reopened

Guys, thank you for mentioning #2071#comment:8: as suggested there "the only reliable way is to try and compile the different function APIs and see which ones survive the compile." I'll see if I'll take this step.

Still first two patches have nothing to do with system miniupnpc and they are worth to apply: 0001-configure.ac-Drop-redudant-code-indentation.patch - just style fixes, nothing functional. transmission-2.31-0002-config.in-4-qt.pro.patch - fixes real bug https://bugs.gentoo.org/368523

Changed 10 years ago by pva0xd

0001-Allow-usage-of-system-miniupnp.-Rename-miniupnp-mini.patch

comment:4 Changed 10 years ago by pva0xd

And last submittion is patch that tries detect miniupnp version and works around it in code. Currently 1.5 and 1.5.20110618 library versions are supported. But it's not hard to add support for other versions as well.

comment:5 Changed 10 years ago by pva0xd

Ok now I've got response from upstream developer:

"I'm trying to upgrade soname when API become incompatible."

So it looks like if library API became incompatible but soname is unchanged - that's bug to be reported.

comment:6 Changed 10 years ago by pva0xd

Also I'd like to know your attitude to allow transmission be compiled with libnatpmp. I've got bug on this: https://bugs.gentoo.org/show_bug.cgi?id=376647 and I'd like to have some ideas about possibility to have patch upstream before I start working on this. Thank you in advance!

comment:7 Changed 10 years ago by charles

It seems like overkill for a library that's only 1000 loc, but I'm not opposed to the idea.

Also, miniupnp 1.6 is out and we're going to use it by default in the next version of Transmission. Could you update your miniupnp patch?

comment:8 Changed 10 years ago by jordan

  • Milestone changed from None Set to 2.40
  • Owner set to jordan
  • Status changed from reopened to new

comment:9 Changed 10 years ago by jordan

  • Status changed from new to assigned

comment:10 Changed 10 years ago by jordan

pva0xd: ping

Changed 10 years ago by pva0xd

transmission-2.33-0003-system-miniupnpc.patch

comment:11 Changed 10 years ago by pva0xd

Ok, here is updated version of the patch. I've decided not to move miniupnp directory into miniupnpc as this will allow to use different pathes for system headers and local headers:

+#ifdef SYSTEM_MINIUPNP +#include <miniupnpc/miniupnpc.h> +#include <miniupnpc/upnpcommands.h> +#else

#include <miniupnp/miniupnpc.h> #include <miniupnp/upnpcommands.h>

+#endif

And sorry for delay, I'm jumping from one conference to another... That's summer! :)

comment:12 Changed 10 years ago by jbicha

  • Cc jbicha@… added
  • Version changed from 2.31 to 2.33+

Hi, I just wanted to comment that Debian & Ubuntu are also patching Transmission to use the system libnatpmp and miniupnpc, and we're using relatively recent versions of the libraries (we haven't packaged 1.6 yet but that will probably happen soon enough).

comment:13 Changed 10 years ago by livings124

  • Version changed from 2.33+ to 2.31

comment:14 Changed 10 years ago by jordan

  • Type changed from Bug to Enhancement

comment:15 Changed 10 years ago by livings124

  • Milestone changed from 2.40 to Sometime

comment:16 Changed 10 years ago by jordan

  • Milestone changed from Sometime to 2.41

comment:17 Changed 10 years ago by livings124

  • Milestone changed from 2.41 to 2.42

comment:18 Changed 10 years ago by jordan

  • Milestone changed from 2.42 to 2.50
  • Resolution set to fixed
  • Status changed from assigned to closed

Patch added in r12957.

I made two main changes -- (1) instead of relying on configure-time command line arguments, always use the system libraries if they're available, and (2) added wrapper functions for the UPNP API s.t. we can avoid some excess #ifdefs.

This revision seems to be working correctly both for 1.5 and 1.6. Please give me a yell if I've missed anything :)

Changed 7 years ago by rb07

Update to miniupnp version 1.9

comment:19 Changed 7 years ago by rb07

  • Resolution fixed deleted
  • Status changed from closed to reopened

Currently using Transmission 2.82 & miniupnp 1.9, the attached patch was needed to build; tested fine.

comment:20 Changed 7 years ago by jordan

  • Resolution set to fixed
  • Status changed from reopened to closed

I've added miniupnp 1.9 and better version detection in r14262.

This revision changes the upnp.c code to #ifdef based off of MINIUPNP_API_VERSION rather than the library version, so this should automatically support future versions where the api doesn't get bumped.

comment:21 Changed 7 years ago by jordan

  • Milestone changed from 2.50 to 2.83
Note: See TracTickets for help on using tickets.