Hi,

Can you please install the kdebase-kde3-dbg package and save the crash log that
comes up when ktorrent crashes? This will give us the necessary backtrace that we
can use to investigate further.

*** Bug 446 has been marked as a duplicate of this bug. ***

*** Bug 461 has been marked as a duplicate of this bug. ***

Created attachment 451 [details]
Crash backtrace

Builtin GeoIP is malfunctioning and causing the crash. But when used GeoIP library from system, it works fine. So I changed the compilation parameters to use as the default GeoIP library from the system.

Created attachment 461 [details]
Turn system_geoip on as default

I have a patch available to update ktorrent from 2.2.6 to 2.2.8. The patch is available in bug report 363.

The patch is in tar.gz format because the bug tracker won't allow a patch
larger than 1MB.

The original poster said the problem does not happen in 2.2.8. I would be grateful if regular ktorrent users would rebuild ktorrent without the geoip patch provided here to see whether the statistic module problem disappears.

If this geoip patch remains necessary after rebuilding to 2.2.8, then the patch should be revised to better inform packagers that the default is to use the external geoip database rather than built-in. The external database package is not installed on some distros as a standard package. Therefore without explicitly specifying either option the build fails likes this:

configure: error: cannot enable system geoip. GeoIP library headers were not found on your system
There was an error trying to build ktorrent.

That message could be revised to something like this;

configure: error: cannot enable system geoip. GeoIP library headers were not found on your system
There was an error trying to build ktorrent. Use the --enable-geoip build option to override the
--enable-system-geoip default option, which looks for an external system geoip database rather than
the built-in database. The external database might need to be installed from your upstream distro
sources.

(Odpověď na komentář #7)
> If this geoip patch remains necessary after rebuilding to 2.2.8, then the patch
> should be revised to better inform packagers that the default is to use the
> external geoip database rather than built-in. The external database package is
> not installed on some distros as a standard package. Therefore without
> explicitly specifying either option the build fails likes this:
> 
> configure: error: cannot enable system geoip. GeoIP library headers were not
> found on your system
> There was an error trying to build ktorrent.
> 
> That message could be revised to something like this;
> 
> configure: error: cannot enable system geoip. GeoIP library headers were not
> found on your system
> There was an error trying to build ktorrent. Use the --enable-geoip build
> option to override the
> --enable-system-geoip default option, which looks for an external system geoip
> database rather than
> the built-in database. The external database might need to be installed from
> your upstream distro
> sources.

From the changelog:

ktorrent (2.0.3+dfsg1-0ubuntu1) edgy; urgency=low

  * built with GeoIP support enabled, but removed the database file
    and country flags due to licensing restrictions (see README.Debian)

 -- Jonathan Riddell <jriddell@ubuntu.com>  Fri, 20 Oct 2006 12:56:19 +0100


In other words, the built-in support for GeoIP is not working and its use leads to falls KTorrent. Therefore, it is necessary to have a GeoIP library in the system. When I compile packages for prepared update 3.5.13 I add libgeoip-dev as build-dep in "control" file. I certainly would not mention --enable-geoip in the comment.

Ok, this is a distro licensing issue and not a build issue.

I understand to comply with Debian licensing standards that respective packagers want to physically strip the built-in database when they build the package. By doing so the default database no longer is available and hence, the desire to change the defaults.

Your patch works well for Debian based distros but not necessarily for others.

How about updating the patch with a basic test? Perhaps test whether the system is Debian or Debian based (if [ -r /etc/debian_version ] )? Or, because Debian packagers will have stripped the built-in database, test for the absence of ktorrent/plugins/infowidget/geoip/geoip.dat. In either case set the default to --enable-system-geoip. Otherwise the default remains --enable-geoip. I think that kind of approach would make all parties happy.

Of course, those who don't want any geoip support can use --disable-geoip.

(Odpověď na komentář #9)
> Ok, this is a distro licensing issue and not a build issue.
> 
> I understand to comply with Debian licensing standards that respective
> packagers want to physically strip the built-in database when they build the
> package. By doing so the default database no longer is available and hence, the
> desire to change the defaults.
> 
> Your patch works well for Debian based distros but not necessarily for others.
> 
> How about updating the patch with a basic test? Perhaps test whether the system
> is Debian or Debian based (if [ -r /etc/debian_version ] )? Or, because Debian
> packagers will have stripped the built-in database, test for the absence of
> ktorrent/plugins/infowidget/geoip/geoip.dat. In either case set the default to
> --enable-system-geoip. Otherwise the default remains --enable-geoip. I think
> that kind of approach would make all parties happy.
> 
> Of course, those who don't want any geoip support can use --disable-geoip.

So we could do so in "rules" for the compilation of ubuntu and Debian packages use --enable-system-geoip and for other remains default --enable-GeoIP?

If I understand correctly, basically "rules" is the Debian build script, or at least where the core build options are defined.

Sounds like an easy solution! Please let me know. :)

If the solution is that easy, perhaps we should we amend the README file to inform packagers of the default and add a note that the license for Debian based distros is not compatible with the default build option? Or, perhaps we add to the Trinity sources the README.Debian file you quoted above?

(Odpověď na komentář #11)
> If I understand correctly, basically "rules" is the Debian build script, or at
> least where the core build options are defined.
> 
> Sounds like an easy solution! Please let me know. :)
> 
> If the solution is that easy, perhaps we should we amend the README file to
> inform packagers of the default and add a note that the license for Debian
> based distros is not compatible with the default build option? Or, perhaps we
> add to the Trinity sources the README.Debian file you quoted above?

Yes, "rules" includes rules for building the package.

The perfect solution would be to "configure" when switched on --use-geoip check whether the data file located in the source code. And if not, then automatically switched on --use-system-geoip. A check for the presence of system GeoIP library is already done.

For debian / ubuntu packages is in any case necessary to add a dependency on libgeoip-dev.

(Odpověď na komentář #12)
> Yes, "rules" includes rules for building the package.
> 
> The perfect solution would be to "configure" when switched on --use-geoip check
> whether the data file located in the source code. And if not, then
> automatically switched on --use-system-geoip. A check for the presence of
> system GeoIP library is already done.
> 
> For debian / ubuntu packages is in any case necessary to add a dependency on
> libgeoip-dev.

I take back. Files for GeoIP are in the source code packages for debian / ubuntu, compiled, and were in the final package. But the use of a GeoIP simply leads to a crash.

Maybe he's just damaged geoip.dat?

Have you tested yet with the patch to update ktorrent to 2.2.8 (bug report 363)?

The original poster for this bug report wrote that the crash does not occur with 2.2.8.

(Odpověď na komentář #14)
> Have you tested yet with the patch to update ktorrent to 2.2.8 (bug report
> 363)?
> 
> The original poster for this bug report wrote that the crash does not occur
> with 2.2.8.

Even as I examined patch 2.2.6 => 2.2.8, so I did not see anything that should fix the problem with the built-in GeoIP. However, I gave it time, and compiled . . . . continue to fall - as I expected. Built-in support GeoIP is not working - I would say that regardless of the distribution.

Okay. Is there anything on the internet about the problem?

(Odpověď na komentář #16)
> Okay. Is there anything on the internet about the problem?

I tried searching, but a similar problem anybody others do not have. So I tried to download the package ktorrent from Ubuntu Hardy and compare geoip.dat with that in the Trinity. In Trinity is smaller by 3 bytes! When I create diff, showed three deleted "DOS" new lines...

Now that you shared this discovery, I compared the Trinity geoip.dat file to the original upstream sources. They do indeed differ. The geoip.dat files from the original 2.2.6, 2.2.7, and 2.2.8 sources are the same, but all differ from the Trinity geoip.dat file.

As nobody seems to have reported this bug anywhere online, perhaps you stumbled upon the solution. That is, there is no bug when using the original sources. Some of the sources in Trinity came from Debian repositories rather than the original upstream KDE3 sources. Possibly then, because Debian policy had packagers strip the built-in database, that the built-in database Trinity inherited was very old and therefore obsolete or corrupt all this time.

That explanation would validate the original poster's comment that the problem does not occur with 2.2.8.

My previous patch to update ktorrent to 2.2.8 did not include the geoip.dat file because my diff was text based (I did not use the diff -a option). I am posting an updated patch in bug report 363. I verified the package builds with the new patch and that the new geoip.dat file matches that in the original 2.2.8 sources.

I don't use ktorrent and I don't know how to test geoip. Would you please use the updated patch, rebuild, and test again?

(Odpověď na komentář #18)
> I don't use ktorrent and I don't know how to test geoip. Would you please use
> the updated patch, rebuild, and test again?

I confirm - after repair geoip.dat built-in GeoIP support work. But it is questionable whether even so it is not advantageous to use an external GeoIP library? Not only because of the license, but also because it will probably contain more recent data.

Good news! Thank you! I will push the 2.2.8 patch to GIT and close bug report 363.

Okay, how to handle the geoip defaults? I agree the external database is a better choice, but changing the defaults forces Trinity packagers to install that package as a dependency. We should not create non-system related external dependencies for Trinity packages. That is, when the external geoip.h file is missing the build should still continue using the internal database.

Currently the configure process looks for the external geoip.h header file. When found the configure process changes the defaults and builds against the external geoip package. When the external geoip.h file is not found then the configure process does not change the defaults and continues using the internal geoip database.

To patch this behavior the process should do exactly the opposite. That is, the proposed patch causes the configure process to look for the external geoip.h header file. When found the configure process continues and builds against the external geoip package. When the external geoip.h file is not found then the configure process should not fail but instead change the defaults and continue building but using the internal geoip database.

How does that sound? Can you update your patch to do that? Right now the proposed patch causes the build to fail when the external geoip.h file is missing.

Created attachment 503 [details]
Prefer system GeoIP against built-in - if it is installed

I have introduced for system_geoip and builtin_country_flags default value 'auto' as follows:

If the system GeoIP is installed is preferred against built-in GeoIP.

If it is not compiled built-in GeoIP, will not include country flags.

Can it be?

I tested building ktorrent with the updated patch both with the external geoip database installed and without. When the external database is installed then ktorrent builds against that and when not, uses the internal database.

Patch pushed to GIT in hash 355c6b69c69b0bc8cf10b7a3c846b5d6ca27abc4.

This resolves the bug report and thanks for the patch!