aMule Forum
English => Multiplatform => Mac OSX => Topic started by: linuxfever on November 13, 2007, 06:16:10 PM
-
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
-
This sounds nice, I hope we can change this in aMule as well.
-
thx :)
-
The link is broken for me, can someone post it again?
-
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
-
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!
-
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
-
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!
-
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>
-
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!
-
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.
-
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
-
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):
+const char *getUPnPv2()
+{
+ if(upnp2Path)
+ return upnp2Path;
+ upnp2Path = getLibInFrameworkDir(CFSTR("libupnp.2.dylib"));
+ return upnp3Path;
+}
-
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 :D
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).
-
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?
-
Yeah I know that isn't good coding at all, but was the fastest non-crashing patch I could output :D (btw: returning a pointer you can either free or not is actually not too bad as long as is documented, is not a leak because the pointer is returned to the caller which can then free it)
I understand now your reluctance at using wxStandardPaths, in that case, I will try to do it differently. Unfortunately DYLD_LIBRARY_PATH does not include the local framework directory, but is possible to add it at runtime instead of determining each library path..although it wouldn't plug-in as easily in the current code.
Linking directly to the UPnP library is probably the best thing to do, this would even allow to simply statically link in upnp and avoid strange binaries hanging around.
This said, I'm working on a better patch right now.
-
Ok, here is the third patch, it still has the "leak", but is now documented with a comment. You can change this however you feel it suits, I didn't change it because we might have different view of what a better solution would be :D
Now it doesn't depend anymore on wxMac at all, i used #ifdef __DARWIN__ and a few CoreFoundation functions
PS: No, there is no way to know how long an URL is before actually converting it. The documentation suggests using MAXPATHLEN sized buffer (1024 on OS X). Again, I didn't move it to a smaller buffer (which is probably much better) because I feel you are changing that part as well :D
-
Hi brainnolo,
Thank you for your patience. I have applied your patch with some changes, as you have already guessed :)
But unfortunately I am not able to test the changes I made, so I will ask you a favor, please compile tomorrow cvs and report your results.
What I did was to convert the parameter of the function which I turned into a "const char *" to a CFStringRef, with the line
CFStringRef libName = CFSTR(name);
If this line is wrong or not compiling, please post the fix.
I have also choosen not to cache the results, once we will only be using them once. Besides, it's less three redundant functions. The leak is avoided by returning an object. The object is destroyed as soon as it gets out of scope, and no memory is lost. I have to say that I like C a lot, but I have to admit that C string handling sucks.
Thanks again for your cooperation!
Cheers!
-
It shouldn't work, CFSTR only accepts literals! CFStringCreateWithCString(NULL, libName, kCFStringEncodingUTF8) will work, but then you have to CFRelease the object created. CFSTR is meant to create the object equivalent of a string literal.
-
Would you mind doing the fix, testing and posting the patch here? Like I said before, I have no Mac here.
Cheers!
-
Now I see why you didn't like the malloc :D My bad, I don't speak C++ (although I don't usually code plain C either, Objective-C is my bread and butter :D)
Attached is the patch to your patch, as I thought, the CFSTR didn't work and needed to be replaced ;)
Maybe you should offer the Frameworks directory with UPnP binaries for download and/or bundle it in your next binary distribution.
-
Ok, your latest patch is committed.
As for the Frameworks directory, are you referring to source or binary? I don't make the Mac binaries, lionel77 or wuischke do them, so I guess this is a suggestion for them :)
Thanks again, and please don't stop contributing!
Cheers!
-
I don't make the Mac binaries, lionel77 or wuischke do them, so I guess this is a suggestion for them
I haven't set up a cross-compiler on my new system yet and there are plenty of people with a Mac and the knowledge to compile around, so I'm not really motivated to do it. ;)
Anyway, it's not a matter of compilation anyway, but of the Mac packager script.
brainnolo, do you know some basic shell scripting? If yes, please have a look at the src/utils/scripts/mac_packager in the tarball and do the necessary changes to include the libraries. I believe there are no problems except warning messages when the files are not found, so integrating it to the script should be fine.
-
I already compiled Universal binary UPnP libraries which work on 10.4 and 10.5 so I have the binaries already. Yes I know shell scripting, I will post a patch here to said script asap.
EDIT: Actually if you bundle said binaries in the current aMule.app skeleton there is nothing else to modify, and if you don't there is no way to get them automagically (except if I implement autodownload, autocompile, auto-edit of said libraries, but it looks like a long and useless process to me).
EDIT2: Or you host said files somewhere and I can just auto-retrieve them in the packaging script, which would then require internet access)
EDIT3: For you convenience, I attached a zip which only contains the libupnp-binaries
-
Ok, committed the 3 UPnP library files.
Cheers!
-
I patched /src/utils/scripts/mac_packager to avoid removing libupnp libs during initial cleanup (this would defeat bundling them) . It will now remove only libwx in the Frameworks directory (which are supposed to be from an old build).
-
Ok, committed!
Thanks!
-
Sorry but now, if I want to compile the CVS on Mac and have the UPnP support, what do I have to do?
I'm compiling wxMac and then I'll compile aMule CVS from the last tarball (20/11). Then?
Thanks! ;)
-
Hi apo758,
This one I hope brainnolo will answer, I know nothing about Mac compilation environment. :(
Maybe we should have a doc explaining it in the tarball, or maybe some place in the wiki (if there isn't one already).
-
Hi I just downloaded the current CVS and still having the library issue, any ideas?
008-01-02 12:20:44: Credit file loaded, 2156 clients are known
2008-01-02 12:20:45: - This is aMule CVS using wxMac v2.7.0 (Snapshot: rev. 6921) based on eMule.
2008-01-02 12:20:45: Running on MacOS (Darwin 8.7.0 Power Macintosh)
2008-01-02 12:20:45: - Visit http://www.amule.org to check if a new version is available.
2008-01-02 12:20:45: Loading server.met file: /Users/joe/Library/Application Support/aMule/server.met
2008-01-02 12:20:45: 144 servers in server.met found
2008-01-02 12:20:45: Found 2 part files
2008-01-02 12:20:45: External connections disabled in config file
2008-01-02 12:20:45: MuleUDPSocket: Created Server UDP-Socket at port 4665
2008-01-02 12:20:45: MuleUDPSocket: Created Client UDP-Socket at port 4672
2008-01-02 12:20:45: error(CDynamicLibHandle): Unable to dlopen libixml.so. Check PATH and LD_LIBRARY_PATH.
2008-01-02 12:20:45: The URL to download can't be empty
2008-01-02 12:20:45: CUPnPException: error(CDynamicLibHandle): Unable to dlopen libixml.so. Check PATH and LD_LIBRARY_PATH.
2008-01-02 12:20:45: Universal Plug and Play: CUPnPException: error(CDynamicLibHandle): Unable to dlopen libixml.so. Check PATH and LD_LIBRARY_PATH.
2008-01-02 12:20:45: Found 355 known shared files, 2 unknown
2008-01-02 12:20:45: Connecting
2008-01-02 12:20:45: Servers: Trying to connect
2008-01-02 12:20:45: Connecting to Razorback 3.0 (85.17.52.92 - 85.17.52.92:443)
2008-01-02 12:20:48: Read 200 Kad contacts
2008-01-02 12:20:48: webserver running on pid 948
2008-01-02 12:20:48: Loading IP-filters 'ipfilter.dat' and 'ipfilter_static.dat'.
2008-01-02 12:20:48: Loaded 0 IP-ranges from 'ipfilter.dat'. 0 malformed lines were discarded.
2008-01-02 12:20:48: Loaded 0 IP-ranges from 'ipfilter_static.dat'. 0 malformed lines were discarded.
2008-01-02 12:20:48: ThreadScheduler: Completed task 'Load IPFilter', 2 tasks remaining.
2008-01-02 12:20:48: Hasher: Starting to create MD4 and AICH hash for file: aMule-Mac-1.CVS.2006-08-10.noDebug_UniversalBinary_TigerOnly.zip
2008-01-02 12:20:48: Failed to download the server list from
2008-01-02 12:20:48: Connected to Razorback 3.0 (85.17.52.92:5000)
2008-01-02 12:20:48: Your copy of aMule is up to date.
2008-01-02 12:20:48: ThreadScheduler: Completed task 'Hashing - /Users/joe/downloads/aMule-Mac-1.CVS.2006-08-10.noDebug_UniversalBinary_TigerOnly.zip', 1 tasks remaining.
2008-01-02 12:20:48: Hasher: Starting to create MD4 and AICH hash for file: aMule.zip
2008-01-02 12:20:48: Servers: Connected
2008-01-02 12:20:48: Connection established with: Razorback 3.0
2008-01-02 12:20:48: New client ID is 3359221575
2008-01-02 12:20:49: Received 1 new servers
2008-01-02 12:20:49: ThreadScheduler: Completed task 'Hashing - /Users/joe/downloads/aMule.zip', 0 tasks remaining.
2008-01-02 12:20:49: Saving of server-list completed.
2008-01-02 12:20:50: ThreadScheduler: Completed task 'AICH Syncronizing', 0 tasks remaining.
-
Your version is old (I guess 6-12 months). Please get a more recent one.
-
I have compiled the UPnP 1.6.6 binaries which would be ok for distribution. However I was wondering if UPnP is now linked instead of dynamically loaded. In the first case you could remove those old files (the dylibs)
-
I have compiled the UPnP 1.6.6 binaries which would be ok for distribution. However I was wondering if UPnP is now linked instead of dynamically loaded. In the first case you could remove those old files (the dylibs)
Yes, it's linked now (dinamically) so the 3 dylibs in the AppBundle Frameworks can be removed.
PS: I think it's time that someone rewrite the mac_packager script, too.
I have a my version, but the current one is too much personalized for my needs...
-
Ok, then is better if those files are removed. They are quite big! I will try to take a look at the packager but I my free time is really scarce at the moment
-
Ignore what is below, apparently it has been fixed already and was a problem in my copy (which is patched). There are still those dylib files in aMule.app/Contents/Frameworks to delete!
<old>
Ok there is an huge problem in compiling aMule for Mac with UPnP support. HFS+ is case insensitive and aMule UPnP.h is included where one would expect the real upnp.h is included. A fix would be to add to the include path just the prefix of UPnP and then include the files via <upnp/upnp.h> instead of "upnp.h". Another solution would be to rename UPnP.h to something else.
</old>