You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Mark Phippard <ma...@gmail.com> on 2007/05/16 17:51:54 UTC
Remove APR_ICONV dependency on Windows
I have been running into some weird ICONV problems with Subclipse on
Windows. The problem is that we have never been able to deliver the
ICONV *.so files with Subclipse. So either the user has installed the
command line and set APR_ICONV_PATH and they get the functionality, or
they have not and they do not.
Anyway, I am currently building trunk with APR 1.2 and ICONV 1.1. If
Subclipse is run on a system with environment variable set and pointed
at 0.9.7 files, then the JVM crashes.
This reminded me of this thread from 2005:
http://subversion.tigris.org/servlets/BrowseList?list=dev&by=thread&from=375124
Stefen King submitted a patch and went through the review process on
code to remove the dependency and use the Win32 conversion functions.
I did not see any -1's and he re-submitted the patch several times.
Any chance we could revive this and get it into Subversion trunk?
He already uses it with TortoiseSVN. I could manually patch the build
I do for Subclipse but I would rather not since when I do a release I
typically use the DLL's that get posted to Subversion site.
--
Thanks
Mark Phippard
http://markphip.blogspot.com/
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Remove APR_ICONV dependency on Windows
Posted by Mark Phippard <ma...@gmail.com>.
On 5/16/07, Stefan Küng <to...@gmail.com> wrote:
> Mark Phippard wrote:
> > I have been running into some weird ICONV problems with Subclipse on
> > Windows. The problem is that we have never been able to deliver the
> > ICONV *.so files with Subclipse. So either the user has installed the
> > command line and set APR_ICONV_PATH and they get the functionality, or
> > they have not and they do not.
> >
> > Anyway, I am currently building trunk with APR 1.2 and ICONV 1.1. If
> > Subclipse is run on a system with environment variable set and pointed
> > at 0.9.7 files, then the JVM crashes.
>
> That would even happen if you could deliver the *.so files with
> Subclipse. I reported this years ago here and on the apr mailing list, I
> even sent a patch to address the issue (guess what happened). I had to
> patch the apr-iconv file to change the way it locates the *.so files for
> TSVN to avoid the issues.
> Because: depending on the compiler version (the version of the c-runtime
> to be exact) you build apr-iconv, you get crashes even if you have the
> correct version of apr-iconv. For example, if your app is built with
> VS2005 (which uses c-runtime v8) but some other app installed apr-iconv
> (the same version you're using) built with VC6 (which uses the c-runtime
> v6), you get crashes too.
What started this is that I remembered your patch. I was going to
grab it and apply it and then test delivering the .so with Subclipse.
Then I realized you have this even better patch.
--
Thanks
Mark Phippard
http://markphip.blogspot.com/
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Remove APR_ICONV dependency on Windows
Posted by Stefan Küng <to...@gmail.com>.
Mark Phippard wrote:
> I have been running into some weird ICONV problems with Subclipse on
> Windows. The problem is that we have never been able to deliver the
> ICONV *.so files with Subclipse. So either the user has installed the
> command line and set APR_ICONV_PATH and they get the functionality, or
> they have not and they do not.
>
> Anyway, I am currently building trunk with APR 1.2 and ICONV 1.1. If
> Subclipse is run on a system with environment variable set and pointed
> at 0.9.7 files, then the JVM crashes.
That would even happen if you could deliver the *.so files with
Subclipse. I reported this years ago here and on the apr mailing list, I
even sent a patch to address the issue (guess what happened). I had to
patch the apr-iconv file to change the way it locates the *.so files for
TSVN to avoid the issues.
Because: depending on the compiler version (the version of the c-runtime
to be exact) you build apr-iconv, you get crashes even if you have the
correct version of apr-iconv. For example, if your app is built with
VS2005 (which uses c-runtime v8) but some other app installed apr-iconv
(the same version you're using) built with VC6 (which uses the c-runtime
v6), you get crashes too.
The good thing is, with the patch for the utf.c file in Subversion, I
don't need apr-iconv at all anymore.
Stefan
--
___
oo // \\ "De Chelonian Mobile"
(_,\/ \_/ \ TortoiseSVN
\ \_/_\_/> The coolest Interface to (Sub)Version Control
/_/ \_\ http://tortoisesvn.net
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Remove APR_ICONV dependency on Windows
Posted by Stefan Küng <to...@gmail.com>.
Mark Phippard wrote:
>> apr-util\include\apu.hw (or apu.h if it already exists):
>> there's a define in there
>> #define APU_HAVE_APR_ICONV 1
>> #define APU_HAVE_ICONV 0
>> #define APR_HAS_XLATE (APU_HAVE_APR_ICONV || APU_HAVE_ICONV)
>> set APU_HAVE_APR_ICONV to 0 instead of 1.
>
> Stefan,
>
> I followed these instructions and built with your utf.c file and
> latest trunk. The Java bindings still pass their tests OK, but the
> command line gets an assertion failure at line 1185 of utf.c. Even
> when running svn --version.
Strange. My NAnt script also builds svn.exe so I can do some testing
with it. But 'svn --version' and all other commands work just fine with
my version.
Are you sure all the apr dll's have been rebuilt and your svn.exe is
using the new ones and not some older dll's still lying around somewhere?
Stefan
--
___
oo // \\ "De Chelonian Mobile"
(_,\/ \_/ \ TortoiseSVN
\ \_/_\_/> The coolest Interface to (Sub)Version Control
/_/ \_\ http://tortoisesvn.net
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Remove APR_ICONV dependency on Windows
Posted by Mark Phippard <ma...@gmail.com>.
On 5/16/07, Stefan Küng <to...@gmail.com> wrote:
> Mark Phippard wrote:
> >> * it uses new/delete instead of apr pools to reserve the memory
> >
> > I think you submitted a patch to the thread that does this.
>
> Yes I did. But that was some time ago:
> http://subversion.tigris.org/servlets/ReadMsg?listName=dev&msgNo=106703
>
> But then, it got rejected with an argument I simply couldn't do anything
> about:
> http://subversion.tigris.org/servlets/ReadMsg?list=dev&msgNo=106869
> I can't prove rumors wrong.
>
> So this time I decided to not send the patch at all but keep it to
> myself. Because I still can't prove that rumor wrong. So there was no
> point in even trying.
>
> >> Yes, I don't define APR_HAS_XLATE. If that isn't defined but WIN32 is,
> >> then the windows APIs are used to convert from/to utf8.
> >
> > Using the "normal" Windows build would I have to edit the generated
> > DSW files? I really need this in Subclipse and I do not need the
> > --encoding option either.
>
> Well, I'm using NAnt scripts to build all the libraries (that way,
> patching them is much easier to do, and I can customize the build the
> way I want and need to). I haven't really tried building with the *.dsp
> files (*.dsw are the workspace files, I think you meant the *.dsp files
> which are the project files for the builds).
>
> First, you need to 'patch' apr-util:
>
> apr-util\include\apu.hw (or apu.h if it already exists):
> there's a define in there
> #define APU_HAVE_APR_ICONV 1
> #define APU_HAVE_ICONV 0
> #define APR_HAS_XLATE (APU_HAVE_APR_ICONV || APU_HAVE_ICONV)
> set APU_HAVE_APR_ICONV to 0 instead of 1.
Stefan,
I followed these instructions and built with your utf.c file and
latest trunk. The Java bindings still pass their tests OK, but the
command line gets an assertion failure at line 1185 of utf.c. Even
when running svn --version.
Any ideas?
--
Thanks
Mark Phippard
http://markphip.blogspot.com/
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Remove APR_ICONV dependency on Windows
Posted by Stefan Küng <to...@gmail.com>.
Mark Phippard wrote:
>> * it uses new/delete instead of apr pools to reserve the memory
>
> I think you submitted a patch to the thread that does this.
Yes I did. But that was some time ago:
http://subversion.tigris.org/servlets/ReadMsg?listName=dev&msgNo=106703
But then, it got rejected with an argument I simply couldn't do anything
about:
http://subversion.tigris.org/servlets/ReadMsg?list=dev&msgNo=106869
I can't prove rumors wrong.
So this time I decided to not send the patch at all but keep it to
myself. Because I still can't prove that rumor wrong. So there was no
point in even trying.
>> Yes, I don't define APR_HAS_XLATE. If that isn't defined but WIN32 is,
>> then the windows APIs are used to convert from/to utf8.
>
> Using the "normal" Windows build would I have to edit the generated
> DSW files? I really need this in Subclipse and I do not need the
> --encoding option either.
Well, I'm using NAnt scripts to build all the libraries (that way,
patching them is much easier to do, and I can customize the build the
way I want and need to). I haven't really tried building with the *.dsp
files (*.dsw are the workspace files, I think you meant the *.dsp files
which are the project files for the builds).
First, you need to 'patch' apr-util:
apr-util\include\apu.hw (or apu.h if it already exists):
there's a define in there
#define APU_HAVE_APR_ICONV 1
#define APU_HAVE_ICONV 0
#define APR_HAS_XLATE (APU_HAVE_APR_ICONV || APU_HAVE_ICONV)
set APU_HAVE_APR_ICONV to 0 instead of 1.
Not sure if those defines are also set in the *.dsp files which the
Subversion build generator creates (as mentioned above, I don't use
them). If they're defined, you may have to change them too.
Just make sure that you test your build (e.g., by updating files with
non-ascii names) to make sure that the iconv files really are not
used/needed anymore.
Stefan
--
___
oo // \\ "De Chelonian Mobile"
(_,\/ \_/ \ TortoiseSVN
\ \_/_\_/> The coolest Interface to (Sub)Version Control
/_/ \_\ http://tortoisesvn.net
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Remove APR_ICONV dependency on Windows
Posted by Mark Phippard <ma...@gmail.com>.
On 5/16/07, Stefan Küng <to...@gmail.com> wrote:
> Mark Phippard wrote:
>
> >> Note: it does not work if conversions are needed from/to another locale
> >> than the local one. But since UI clients usually only convert to the
> >> current locale from/to utf8, that's not really an issue.
> >
> > I do not believe Subversion does any other kind of conversion either.
>
> Yes it does. You can for some APIs pass a param to force the output to
> be in another locale. I don't think the CL client uses those, but the
> APIs can do this.
> I haven't implemented this for two reasons:
> * I don't know how to convert a locale given as a string (e.g.,
> "iso-8859-1") into the value needed to pass to the Windows APIs.
> * TSVN doesn't use those anyway
Right, some of the commands take an --encoding option.
> > Stefan, what happened last time? Did you just get tired of the back
> > and forth and decide it was easy enough to handle in your build
> > system? It seemed like you addressed all the comments though, why did
> > this not get applied?
>
> Well, to be honest, I simply don't have the energy to deal with that
> anymore. So I just kept the patch for TSVN. And with this patch I was
> (and still am) absolutely sure that it will get rejected:
> * it doesn't work for conversions to/from a charset which is not set as
> the current one in the process/thread
It sounds like that would be needed.
> * it uses new/delete instead of apr pools to reserve the memory
I think you submitted a patch to the thread that does this.
> Yes, I don't define APR_HAS_XLATE. If that isn't defined but WIN32 is,
> then the windows APIs are used to convert from/to utf8.
Using the "normal" Windows build would I have to edit the generated
DSW files? I really need this in Subclipse and I do not need the
--encoding option either.
--
Thanks
Mark Phippard
http://markphip.blogspot.com/
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Remove APR_ICONV dependency on Windows
Posted by Stefan Küng <to...@gmail.com>.
Mark Phippard wrote:
>> Note: it does not work if conversions are needed from/to another locale
>> than the local one. But since UI clients usually only convert to the
>> current locale from/to utf8, that's not really an issue.
>
> I do not believe Subversion does any other kind of conversion either.
Yes it does. You can for some APIs pass a param to force the output to
be in another locale. I don't think the CL client uses those, but the
APIs can do this.
I haven't implemented this for two reasons:
* I don't know how to convert a locale given as a string (e.g.,
"iso-8859-1") into the value needed to pass to the Windows APIs.
* TSVN doesn't use those anyway
> Stefan, what happened last time? Did you just get tired of the back
> and forth and decide it was easy enough to handle in your build
> system? It seemed like you addressed all the comments though, why did
> this not get applied?
Well, to be honest, I simply don't have the energy to deal with that
anymore. So I just kept the patch for TSVN. And with this patch I was
(and still am) absolutely sure that it will get rejected:
* it doesn't work for conversions to/from a charset which is not set as
the current one in the process/thread
* it uses new/delete instead of apr pools to reserve the memory
And I haven't really checked, but there also might be some tabs in there
instead of spaces :)
Yes, I could have done this 'properly' by using apr pools and maybe come
up with some workaround for the non-current locale conversions. But that
would have meant more work for me, with the very high risk that after I
addressed all the concerns of the devs it would still get rejected.
> I am not a C programmer, but looking at the code, it looked like you
> conditioned it based on something you are DEFINE in your own buld
> system. Could we just make it always do this when building on WIN32?
Yes, I don't define APR_HAS_XLATE. If that isn't defined but WIN32 is,
then the windows APIs are used to convert from/to utf8.
Stefan
--
___
oo // \\ "De Chelonian Mobile"
(_,\/ \_/ \ TortoiseSVN
\ \_/_\_/> The coolest Interface to (Sub)Version Control
/_/ \_\ http://tortoisesvn.net
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Remove APR_ICONV dependency on Windows
Posted by Mark Phippard <ma...@gmail.com>.
On 5/16/07, Stefan Küng <to...@gmail.com> wrote:
> Mark Phippard wrote:
> > I have been running into some weird ICONV problems with Subclipse on
> > Windows. The problem is that we have never been able to deliver the
> > ICONV *.so files with Subclipse. So either the user has installed the
> > command line and set APR_ICONV_PATH and they get the functionality, or
> > they have not and they do not.
> >
> > Anyway, I am currently building trunk with APR 1.2 and ICONV 1.1. If
> > Subclipse is run on a system with environment variable set and pointed
> > at 0.9.7 files, then the JVM crashes.
> >
> > This reminded me of this thread from 2005:
> >
> > http://subversion.tigris.org/servlets/BrowseList?list=dev&by=thread&from=375124
> >
> >
> > Stefen King submitted a patch and went through the review process on
> > code to remove the dependency and use the Win32 conversion functions.
> > I did not see any -1's and he re-submitted the patch several times.
> > Any chance we could revive this and get it into Subversion trunk?
> >
> > He already uses it with TortoiseSVN. I could manually patch the build
> > I do for Subclipse but I would rather not since when I do a release I
> > typically use the DLL's that get posted to Subversion site.
>
> FYI: the patched file (utf.c) which I now use for TortoiseSVN can be
> found here:
> http://tortoisesvn.tigris.org/svn/tortoisesvn/trunk/ext/build/utf.c
>
> Note: it does not work if conversions are needed from/to another locale
> than the local one. But since UI clients usually only convert to the
> current locale from/to utf8, that's not really an issue.
I do not believe Subversion does any other kind of conversion either.
Stefan, what happened last time? Did you just get tired of the back
and forth and decide it was easy enough to handle in your build
system? It seemed like you addressed all the comments though, why did
this not get applied?
I am not a C programmer, but looking at the code, it looked like you
conditioned it based on something you are DEFINE in your own buld
system. Could we just make it always do this when building on WIN32?
--
Thanks
Mark Phippard
http://markphip.blogspot.com/
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Remove APR_ICONV dependency on Windows
Posted by Stefan Küng <to...@gmail.com>.
Mark Phippard wrote:
> I have been running into some weird ICONV problems with Subclipse on
> Windows. The problem is that we have never been able to deliver the
> ICONV *.so files with Subclipse. So either the user has installed the
> command line and set APR_ICONV_PATH and they get the functionality, or
> they have not and they do not.
>
> Anyway, I am currently building trunk with APR 1.2 and ICONV 1.1. If
> Subclipse is run on a system with environment variable set and pointed
> at 0.9.7 files, then the JVM crashes.
>
> This reminded me of this thread from 2005:
>
> http://subversion.tigris.org/servlets/BrowseList?list=dev&by=thread&from=375124
>
>
> Stefen King submitted a patch and went through the review process on
> code to remove the dependency and use the Win32 conversion functions.
> I did not see any -1's and he re-submitted the patch several times.
> Any chance we could revive this and get it into Subversion trunk?
>
> He already uses it with TortoiseSVN. I could manually patch the build
> I do for Subclipse but I would rather not since when I do a release I
> typically use the DLL's that get posted to Subversion site.
FYI: the patched file (utf.c) which I now use for TortoiseSVN can be
found here:
http://tortoisesvn.tigris.org/svn/tortoisesvn/trunk/ext/build/utf.c
Note: it does not work if conversions are needed from/to another locale
than the local one. But since UI clients usually only convert to the
current locale from/to utf8, that's not really an issue.
Stefan
--
___
oo // \\ "De Chelonian Mobile"
(_,\/ \_/ \ TortoiseSVN
\ \_/_\_/> The coolest Interface to (Sub)Version Control
/_/ \_\ http://tortoisesvn.net
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org