Print

Please login or register.
1 Hour 1 Day 1 Week 1 Month Forever
Login with username, password and session length

[linuxfever]

I've compiled aMule with UPnP support (Universal Binary, from Leopard and all that). The only problem is that is a special version (AdunanzA) so it won't work on normal Kad networks. What I did anyway was edit UPnP.cc (or cpp?) making it load from "aMule.app/Contents/Frameworks/libupnp.3.dylib", "aMule.app/Contents/Frameworks/libupnp.2.dylib" "aMule.app/Contents/Frameworks/libixml.2.dylib" (upnp 2 not really needed). This breaks as soon as you rename aMule.app to something else, but for some reason the current directory for bundled apps is the parent of the .app. What is "special" about this package is that i called the appropriate install_name_tool  to the .dylib files so they load flawlessly with dlopen(). I'm posting the link to this version. Again do not use this version directly if you aren't a Fastweb user, but copy the Frameworks directory to your bundle (after you do the UPnP.cc patch)

http://www.mediafire.com/?5meugzjfmjk

[wuischke]

This sounds nice, I hope we can change this in aMule as well.

[kreegee]

thx

[phoenix]

The link is broken for me, can someone post it again?

[linuxfever]

A mirror, actually a new version is here. If nobody is going to do the needed #ifdef's I can do the patch and submit it as soon as I've got some free time

http://www.lobortis.com/n/5298239596

[phoenix]

wuischke,

I am sorry, this is a binary. What exactly do you want me to do with it? What is that you wanted to incorporate in aMule?

Cheers!

[wuischke]

Hi phoenix,

currently we use hardcoded (*nix) library names. These do not work on Mac (and most probably Windows as well). Lionel and I have been trying to get this to work once, but we both didn't really know about dynamic libraries on Mac. So now we finally know how to call the necessary libraries to load, that's all I wanted to point out.

making it load from "aMule.app/Contents/Frameworks/libupnp.3.dylib", "aMule.app/Contents/Frameworks/libupnp.2.dylib" "aMule.app/Contents/Frameworks/libixml.2.dylib"

cya

Edit: See http://forum.amule.org/index.php?topic=13741.0

[phoenix]

wuischke,

Ok, now I understand. The patch seems ok, I just cannot test it here. If you say it works, then it is already committed

Cheers!

[linuxfever]

Ok, although I was too lazy to even create my own account on the forum, if you want I could do a nicer patch to post here, as long as I'm allowed to use CoreFoundation functions (obviously in #ifdef's) to avoid problems when renaming aMule.app to something else. Also make sure you use the Frameworks directory included in my binary distribution, if you compile it yourself it will want to be in /usr/local/lib.

If you really want to compile libupnp yourself you have to run the following command for each library to be bundled

install_name_tool -id @executable_path/../Frameworks/<libname>.x.dylib

and for dylib, for each referenced library (seen via otool -L)
install_nae_tool -change /old/library/name.dylib to @executable_path/../Frameworks/<libname.x.dylib>

[phoenix]

linuxfever,

I must say I don't understand many of the things you said. Nevermind, I don't have a Mac, maybe some day...

But your patches are surely welcome.

Cheers!

[brainnolo]

Ok, I attached a temporary patch, although it would be great if it were committed already. It still requires the application name to be aMule.app, the commented code crashes, but I will try to see what's wrong with that.

I'm "linuxfever", that was a bugmenot account, I now registered my own to post this.

[brainnolo]

Forget about the previous patch, this one works, and one can rename aMule.app to whatever he wants without problems. It also offers a simple way to add Windows support eventually (but I cannot do that part).

BTW: I saw the other posted patch, that one works only if you install UPnP libs system wide, this one is instead meant to have them embedded in the application bundle, which is the standard OS X way of doing this kind of things. My patch unfortunately is based on UPnP.cpp version just before that patch were applied, so you need to revert to that version before applying this

[phoenix]

Hi brainolo,

Why do you attach a library to an application? That seems strange to me. Why is it better to use a library just for this application when it could be used by all the applications in the system?

Anyway, there are some problems.
1) Your patch leaks memory. But that is not a big problem, can be easily fixed.
2) wxStrings are not used in this module, you need to use std::strings. Also not a problem, easy to fix.
2) You use wxStandardPaths and wxStandardPathsBase. I would preffer that you used something system specific here, once we are already using an ifdef for DARWIN.

So, if you can come up with a solution for item 3), I can fix the rest.

PS: You had an error in the first patch, the last return is a wrong variable (it is upnp3Path, should be upnp2Path):

Code: [Select]

+const char *getUPnPv2()
+{
+       if(upnp2Path)
+               return upnp2Path;
+       upnp2Path = getLibInFrameworkDir(CFSTR("libupnp.2.dylib"));
+       return upnp3Path;
+}

[brainnolo]

Yeah I noticed that error in first patch, it was in commented code however.

I'm not a C++ coder, nor I know aMule internals, nor I know wxWidgets at all, thats why you get the leaks there.

However if by leak you are talking about the malloc'ed memory that's never freed, is not a bug! As you see the pointer is cached, so the function will malloc it only once and that memory needs to remain alive in case it needs to unload/reload the libs. Is a few bytes, anyone can afford that.

I do not understand the concern about using wxStandardPaths, it does exactly what is needed and makes the code smaller. Also the mac implementation is really small and does nothing more than I would have to do myself in that piece of code. The first patch was done using CoreFoundation functions and it sucked big time This doesn't mean I'm not going to do that ever, but I'd like to know a real reason for reinventing the mighty wheel

The libraries are bundled to avoid version incompatibilities between applications, as in OS X you most likely cannot recompile the applications you get. Also, the installation becomes way easier (matter of drag/drop instead of running installers of any kind).

[phoenix]

brainnolo,

The leak is the malloc. In spite of the caching of the pointer, this is not good style. The caching is done externally to the function, so anyone using this function must also be aware that it leaks, in this sense, you allocate a resource that should be freed in the future. I agree, it is just a few bytes, but it is ugly and unnecessary. And that is not the problem we are discussing here

I understand now your reasons to include the library in the bundle.

As for the wxStandardPaths, I don't want wx dependencies in the UPnP module. There is currently one dependency, a wxMutex, used for lazyness which will be removed soon. I wish to keep wx out of UPnP, so that the code can be reused independently.

BTW, it looks quite cumbersome to find a library like this. Isn't there a PATH or LD_LIBRARY_PATH equivalent? Why do you need a path? In the system wide version, there seems to be a working path. Can't you direct this path to the bundle directory?

aMule UPnP was designed to dlopen() so that it did not introduce a new library dependency and at the same time would be simple to test in CVS code, but maybe it is time to create a configure option and link UPnP directly, what do you think?