Steve,

  This is exactly my desired solution.  If you would like to
submit your patch to the list (submissions by reference aren't
accepted without extra hassles) I would very much like to adopt 
it.  (Especially as opposed to reinventing it.)

  My proposal of LD_LIBRARY_PATH corresponds to the fact that
Unix doesn't have the GetModuleFileName concept to figure out
where the absolute path of the libapriconv-1.so module lies.
Since we have to make exceptions for Win32, and HP/UX, I see
no reason not to use the native solution to Win32.  

  Incidentally, do we need to search DYLD_LIBRARY_PATH or 
LD_LIBRARY_PATH on OSX?

Bill

At 09:49 AM 3/30/2005, SteveKing wrote:
>Branko Čibej wrote:
>>William A. Rowe, Jr. wrote:
>>
>>>At 12:36 AM 3/30/2005, Curt Arnold wrote:
>>> 
>>>
>>>>However, it would be good if we could get some comment from a Subversion 
>>>>developer.
>>>>  
>>>
>>>Agreed, and Branko's watching this thread.
>>> 
>>Yup.
>>It might also be a good thing to ping Steve King of TortoiseSVN fame; TSVN 
>>also ships a hacked version of apr-iconv, I believe the hack is to ignore 
>>APR_ICONV_PATH. The issue IIRC is collision between memory allocators in 
>>different runtime librararies on Windows; but anyway, Steve should be able to 
>>explain.
>
>The reasons why TSVN ships a patched version of apr-iconv:
>
>- the *.so files are actually dll's which require a specific C-runtime. This 
>isn't a big problem if APR_ICONV_PATH points to a iconv compiled with VC6. But 
>if it points to an iconv compiled with >VC7, then programs not using the new 
>c-runtime (> version 7) won't run anymore. A 'solution' to that problem would 
>be to install the runtime in e.g. the system dir on windows, but even MS tells 
>to _not_ do that.
>
>- every program which installs its own iconv lib might change the 
>APR_ICONV_PATH variable (believe me: most programs _don't_ check if it's 
>already set) and overwrite this with maybe an older version of the lib!
>
>- looking up the env variable isn't very fast, i.e. it takes more time to load 
>the *.so modules than other methods.
>
>The changes I made to apr-iconv for TSVN:
>
>- add a DllMain() to iconv_module which caches the path to the *.so modules 
>(i.e. the path is determined only once when the iconv dll is loaded, not for 
>every *.so module) - this is a _very_ big speed improvement for Subversion 
>which makes intensive use of *.so modules.
>
>- the path is determined like this now:
>  (in the following description, THISPATH refers to the path where the 
> libapriconv.dll is located, fetched via GetModuleFileName() API)
>  1. path exists THISPATH\iconv
>  2. path exists ..\THISPATH\iconv
>  3. use APR_ICONV_PATH
>  by doing this, it's assured that a program like TSVN always uses the iconv 
> lib it has installed itself. Only if it doesn't install the iconv lib itself 
> or it can't be found in 1) or 2), then the APR_ICONV_PATH env variable is 
> used to find the *.so modules.
>
>You can find the patched iconv_module.c file here:
>https://svn.collab.net/repos/tortoisesvn/trunk/ext/apr-iconv_patch/lib/iconv_module.c
>
>Ever since TSVN ships with this patched iconv version, we haven't had any more 
>problems with other Subversion clients interfering or missing c-runtime dll's 
>or memory leaks due to incompatible c-runtimes (according to MS, the version 6 
>and 7 of the c-runtime are not compatible and use different memory allocators 
>which leads to memory leaks and even heap corruptions if a program uses the 
>wrong version).
>
>So I'm not sure what you guys have planned to change in apr-iconv, but I 
>strongly recommend that if you change something, get rid of the APR_ICONV_PATH 
>(on windows at least), or at least use it as the last option like TSVN does 
>now.
>
>Stefan
>
>-- 
>       ___
>  oo  // \\      "De Chelonian Mobile"
> (_,\/ \_/ \     TortoiseSVN
>   \ \_/_\_/>    The coolest Interface to (Sub)Version Control
>   /_/   \_\     http://tortoisesvn.tigris.org


Reply via email to