From: SourceForge.net <no...@so...> - 2004-11-22 19:55:41
|
Patches item #780565, was opened at 2003-07-30 17:42 Message generated for change (Comment added) made by nosnilmot You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=300235&aid=780565&group_id=235 Category: None Group: None Status: Open Resolution: None Priority: 5 Submitted By: Benjamin Miller (bmiller) >Assigned to: Tim Ringenbach (marv_sf) Summary: Allow gaim to run from directory other than $prefix Initial Comment: I wrote this mainly so the Solaris tar.gz package would work for people who can't install in /usr/local. It might be useful for other network sorts of installations though. It adds two functions, gaim_libdir() and gaim_datadir(), and replaces all occurrences of LIBDIR and DATADIR with calls to these functions. This is a patch on gaim 0.66, but maybe it will apply to CVS also. -Ben ---------------------------------------------------------------------- >Comment By: Stu Tomlinson (nosnilmot) Date: 2004-11-22 14:55 Message: Logged In: YES user_id=309779 Tim's looking into relocatability, so I'm reassigning this to him, with a couple of comments. This patch shouldn't replace #ifdef OLD_PERL with #if 1 :) I don't think this is core/ui split. Assuming the libdir is called 'lib' is not safe, on multilib systems it may be lib64 ---------------------------------------------------------------------- Comment By: Benjamin Miller (bmiller) Date: 2003-08-22 12:26 Message: Logged In: YES user_id=144203 Here you go. I think I've made all the changes you wanted. ---------------------------------------------------------------------- Comment By: Benjamin Miller (bmiller) Date: 2003-08-13 22:00 Message: Logged In: YES user_id=144203 Okay, I guess I misunderstood. Thanks for clearing that up. I can finish changing the names of the functions and whatnot, and have an updated patch up soon. -Ben ---------------------------------------------------------------------- Comment By: Luke Schierer (lschiere) Date: 2003-08-13 21:35 Message: Logged In: YES user_id=28833 I'd like this patch to go in, ChipX86 is concerned with its effects on the core/ui split. that pretty much sumerizes the debate. ---------------------------------------------------------------------- Comment By: Benjamin Miller (bmiller) Date: 2003-08-13 20:51 Message: Logged In: YES user_id=144203 Well, frankly, if there's still a lot of debate as to whether this patch is needed or if it's done correctly, then it doesn't seem like it's worth it for me to make the rest of these changes. As the patch is now, it works perfectly for the Solaris tar.gz binary. That was the only reason I wrote it initially, but someone asked me in the forum if I could share my code (because he was having the same issue). If the debate on this ends, and you still want to use my patch, let me know, and I'll update it. Thanks. -Ben ---------------------------------------------------------------------- Comment By: Christian Hammond (chipx86) Date: 2003-08-13 16:22 Message: Logged In: YES user_id=5989 There's still a lot of debate as to whether or not this is even needed, or if it's done correctly... but I'll leave that for now. Issues so far: I've decided that these functions should be UI, not core. I wouldn't even be able to compile qpe-gaim with this patch. So, the functions should be renamed to gaim_gtk_get_lib_dir(), gaim_gtk_get_data_dir(), gaim_gtk_get_prefix(), gaim_gtk_get_binary(). The global var should be set using gaim_gtk_{set,get}_bin_name() or something. ---------------------------------------------------------------------- Comment By: Benjamin Miller (bmiller) Date: 2003-08-07 22:27 Message: Logged In: YES user_id=144203 Hi. I've fixed everything except the global variable. Still waiting to hear from someone on how I should resolve that. Let me know, and I can have a new patch up right away. ---------------------------------------------------------------------- Comment By: Benjamin Miller (bmiller) Date: 2003-07-31 23:57 Message: Logged In: YES user_id=144203 Thanks for the comments. First, the g_find_program_in_path function should deal with out-of-the-way directories fine. If you have a copy of 0.66 installed in /usr, and CVS installed in ~/gaim-cvs, and you execute the CVS version with ~/gaim-cvs/bin/gaim, then g_find_program_in_path(argv[0]) will return ~/gaim-cvs/bin/gaim. The only time it checks $PATH is if you just execute "gaim" with no leading path. (I've tested this, with a copy installed in /usr/local and one installed in /tmp/gaim-0.66.) I'll definitely change the naming, and switch to g_free and whatnot. And actually, I usually do write the return type of a function on the line above the function name. Not sure why I didn't here... As for the global variable, if you think a gaim_set_binary_name() and gaim_get_binary_name() would be a good solution, I can do that. I'm not too clear on the whole core/UI separation thing (I remember hearing about the plan a couple years ago, but that's about it), so I can't really suggest anything there. Let me know how I should handle this, and I'll submit a new version. -Ben ---------------------------------------------------------------------- Comment By: Christian Hammond (chipx86) Date: 2003-07-31 23:03 Message: Logged In: YES user_id=5989 I have several comments about this patch. First, I like the direction you're going, but a few things need some work. The biggest problem is that sometimes for development, we install gaim into some out-of-the-way directory. I use ~/cvs/gaim/*-root for my test trees. This patch will end up finding my /usr/bin/gaim, though, and proceed to use the paths based off that. It should check the prefix, DATADIR, etc. first, try to determine if they're valid, and if so, use those. If not valid, it should do the searching it currently does. The other issue is the namespace. Not a big deal, as I could change it myself, but I'd rather not :) gaim_libdir() should be gaim_get_library_dir(), gaim_datadir() should be gaim_get_data_dir(). gaim_binary() should be static, and should be renamed get_binary_path(). gaim_prefix() should also be static, and be named get_prefix(). Another issue is the global variable. Global variables are evil, and we don't want to introduce new ones. Rather than set the variable, there should be a gaim_set_binary_name() or something, but I really hate for UI authors to have to set this... Not sure of a way around this. Keep in mind that we must remain core/UI split. Last little thing... use g_free() instead of free(), and try to keep the coding style closer to the rest of gaim (like for function implementations, put the return type, newline, and then function name and then (params)). There shouldn't be spaces between function names and the opening paren. Nit-pick stuff, but it helps to keep things consistent :) The patch is looking good, but the first few items are critical before it can be accepted. Thanks. ---------------------------------------------------------------------- Comment By: Benjamin Miller (bmiller) Date: 2003-07-31 13:06 Message: Logged In: YES user_id=144203 Oops! Forgot to check the little box when I tried to upload the file. ---------------------------------------------------------------------- Comment By: Luke Schierer (lschiere) Date: 2003-07-31 09:40 Message: Logged In: YES user_id=28833 no files attached. ---------------------------------------------------------------------- You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=300235&aid=780565&group_id=235 |