You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Ivan Zhakov <ch...@gmail.com> on 2007/06/30 21:45:41 UTC
[PATCH] Remove APR ICONV dependency on Windows (was SVN Win32 Developers -- need some help)
On 6/30/07, Mark Phippard <ma...@gmail.com> wrote:
> On 6/30/07, D.J. Heap <dj...@gmail.com> wrote:
> > On 6/5/07, eg <eg...@gmail.com> wrote:
> > > Mark Phippard wrote:
> > >
> > > >
> > > > Even in his objection Brane said "if this is true". All I (and
> > > > Stefan) am saying is we need a more specific objection. If there are
> > > > real problems that we cannot resolve, then I am sure we would all be
> > > > against this. But we know there are problems with apr_iconv too, so
> > > > let's just understand what the problems are.
> > > >
> > >
> > > In an attempt to help here, I have done a bit of hunting around to see
> > > if I could substantiate any publicly reported issues. I have mentioned
> > > the few that I have found in other posts. Stefan has already responded
> > > that these should not be issues for Subversion.
> > >
> > > The only other thing I could find for utf8 conversion, relates to
> > > handling of "bad inputs" related to lone surrogate halves when passing
> > > them to WideCharToMultiByte() on older versions of Windows (alluded to
> > > in blog comments from:
> > > http://blogs.msdn.com/michkap/archive/2006/01/20/515238.aspx).
> > > Apparently this is tightened up now (on Vista?).
> > >
> > > I suspect this is also not an issue for this usage in any case, but
> > > someone else more familiar with svn usage should confirm that.
> > >
> > > If there are any issues related to using Win32
> > > MultiByteToWideChar()/WideCharToMultiByte() in place of iconv() under
> > > Windows, I cannot find them. Older versions of Windows (95/98/Me) will
> > > need to install and use the MSLU (Microsoft Layer for unicode).
> >
> >
> > So, I have not been following along very carefully, but what needs to
> > happen now? Do a build and have people using different languages test
> > it?
> >
> > I can apply the patch and build the binary zips if other people have
> > the means to test them out a bit.
> >
> > Or is there something else that needs to be done?
>
> Does the test suite have any tests that exercise this?
>
> Ivan had said he was going to look into it, but I do not think he has.
> Ideally, I think someone should put together the patch that they
> think works completely and just commit it. Then we can bang on it and
> see if there are issues.
>
Sorry, I was busy and I have not time to report my research results.
My patch almost complete and I need somebody's eyes to look and
confirm that I'm on right way (or say that I'm on wrong way :)
I used Windows MIME database to convert page name to Windows page identifier.
And most important question is it acceptable to read Windows MIME
database directly from registry instead of using MLang API [1]? I
didn't use MLang API, because it requires
CoInitialize()/CoUninitialize().
Other notes:
- Probably this new win32_xlate stuff should go to separate file?
- For present time charset aliases isn't implemented.
[1] http://msdn2.microsoft.com/En-US/library/aa767865.aspx
[[[
!! This patch is NOT complete. !!!
Remove APR ICONV dependency on Windows and use native Windows
character conversion API (MultiByteToWideChar/WideCharToMultiByte).
* subversion/tests/libsvn_subr/utf-test.c
(test_utf_cstring_to_utf8_ex2): New test for conversion from
different codepages to utf8.
(test_utf_cstring_from_utf8_ex2): New test for conversion to
different codepages from utf8.
(test_funcs): Add two tests.
* subversion/libsvn_subr/utf8.c
(win32_xlate_t): New structure for holding Windows xlate data.
(discover_windows_codepage_id): New helper for converting page name to Windows
page identifier.
(win32_xlate_open): New function to open Windows xlate.
(win32_convert_to_string_buf): Converts string from one codepage to
another using
Windows API.
(get_xlate_handle_node): Use win32_xlate_open() on Windows.
(convert_to_stringbuf): Use win32_convert_to_stringbuf() on Windows.
]]]
--
Ivan Zhakov
Re: [PATCH] Remove APR ICONV dependency on Windows (was SVN Win32 Developers -- need some help)
Posted by Ivan Zhakov <ch...@gmail.com>.
On 7/5/07, Mark Phippard <ma...@gmail.com> wrote:
> On 7/5/07, Ivan Zhakov <ch...@gmail.com> wrote:
> > On 7/3/07, D.J. Heap <dj...@gmail.com> wrote:
> > > On 7/2/07, Ivan Zhakov <ch...@gmail.com> wrote:
> > > [snip]
> > > > Here is revisited patch. Major vhanges from previous version:
> > > > - Use MLang API for converting page name to page id.
> > > > - All Windows stuff moved to separate file win32_xlate.c.
> > > >
> > > > [[[
> > > > Remove APR ICONV dependency on Windows and use native Windows
> > > > character conversion API (MultiByteToWideChar/WideCharToMultiByte).
> > > >
> > >
> > >
> > > I've built and posted binaries with this patch at:
> > >
> > > http://subversion.tigris.org/servlets/ProjectDocumentList?folderID=8682&expandFolder=8682&folderID=8682
> > >
> > > I still get 2 fs-test failures with VS6 -- haven't had time to dig
> > > into them yet.
> > >
> > > Also, I think we should probably check the return of CoInit and not
> > > call CoUninit if it fails (because CoInit/CoInitEx has already been
> > > called with a different apartment or something).
> > >
> > Good point! I've added check for return value of CoInitialize() and
> > also added logic to detect when COM already initialized for different
> > apartment.
> >
> > I think that patch is acceptable for trunk, thus I've committed it in
> > r25640. Review more than welcome :)
> >
> > On 7/3/07, Daniel Rall <dl...@collab.net> wrote:
> > > Just a nitpick, but some of the formatting is pretty inconsistent with
> > > the rest of the code base (use of Hungarian notation, lack of
> > > whitespace between data type and value in type casts, etc.).
> > Thanks, I've checked my patch again and fixed what I found.
>
> Thanks Ivan.
>
> Is it possible to build apr-util without iconv now? Or do we still
> have to build apr-iconv and ship the DLL and just know that we are now
> not actually using it?
>
Subversion don't build apr, apr-util and apr-iconv since r24859.
Probably we have to fix packaging scripts.
--
Ivan Zhakov
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] Remove APR ICONV dependency on Windows (was SVN Win32 Developers -- need some help)
Posted by Stefan Küng <to...@gmail.com>.
On 7/5/07, Mark Phippard <ma...@gmail.com> wrote:
> On 7/5/07, Ivan Zhakov <ch...@gmail.com> wrote:
> > On 7/3/07, D.J. Heap <dj...@gmail.com> wrote:
> > > On 7/2/07, Ivan Zhakov <ch...@gmail.com> wrote:
> > > [snip]
> > > > Here is revisited patch. Major vhanges from previous version:
> > > > - Use MLang API for converting page name to page id.
> > > > - All Windows stuff moved to separate file win32_xlate.c.
> > > >
> > > > [[[
> > > > Remove APR ICONV dependency on Windows and use native Windows
> > > > character conversion API (MultiByteToWideChar/WideCharToMultiByte).
> > > >
> > >
> > >
> > > I've built and posted binaries with this patch at:
> > >
> > > http://subversion.tigris.org/servlets/ProjectDocumentList?folderID=8682&expandFolder=8682&folderID=8682
> > >
> > > I still get 2 fs-test failures with VS6 -- haven't had time to dig
> > > into them yet.
> > >
> > > Also, I think we should probably check the return of CoInit and not
> > > call CoUninit if it fails (because CoInit/CoInitEx has already been
> > > called with a different apartment or something).
> > >
> > Good point! I've added check for return value of CoInitialize() and
> > also added logic to detect when COM already initialized for different
> > apartment.
> >
> > I think that patch is acceptable for trunk, thus I've committed it in
> > r25640. Review more than welcome :)
> >
> > On 7/3/07, Daniel Rall <dl...@collab.net> wrote:
> > > Just a nitpick, but some of the formatting is pretty inconsistent with
> > > the rest of the code base (use of Hungarian notation, lack of
> > > whitespace between data type and value in type casts, etc.).
> > Thanks, I've checked my patch again and fixed what I found.
>
> Thanks Ivan.
>
> Is it possible to build apr-util without iconv now? Or do we still
> have to build apr-iconv and ship the DLL and just know that we are now
> not actually using it?
AFAIK Subversion doesn't build apr/apr-util anymore?
Anyway: you must patch the apr build a little bit to make sure that
the apr-iconv dll isn't needed anymore:
there's a file 'apu.hw' which gets copied to 'apu.h' in the build
process. Depending on where you got the apr sources (repository
checkout or tarball), you have to edit the hw or the h file.
You have to change the defines:
APU_HAVE_APR_ICONV and APU_HAVE_ICONV to be both '0', not '1'.
Then you must of course do a rebuild of apr (and I think apr-util too).
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: Re: [PATCH] Remove APR ICONV dependency on Windows (was SVN Win32 Developers -- need some help)
Posted by Mark Phippard <ma...@gmail.com>.
On 7/5/07, Mark Phippard <ma...@gmail.com> wrote:
> On 7/5/07, Paul Burba <pb...@collab.net> wrote:
> > Mark,
> >
> > I did a build of trunk on my MSVC6 VM this morning and encountered the same problem you did. I think this explains the problem:
> >
> > http://trolltech.com/developer/knowledgebase/faq.2007-05-04.4776834989/
> >
> > After replacing this line in my build script:
> >
> > set SDKLIB=C:\Program Files\Microsoft Platform SDK\Lib
> >
> > with this:
> >
> > set SDKLIB=C:\Program Files\Microsoft Visual Studio\VC98\Lib;C:\Program Files\Microsoft Platform SDK\Lib
> >
> > Everything worked fine.
>
> I will give that a try, but I seem to recall I had to do the exact
> opposite for APR 1.2.8 to build because of needing newer LDAP
> libraries or something like that. I suppose since APR is built
> separate this might not be too big of an issue.
OK, this worked for getting it to build. Now win-tests.py bombs out
before it runs because it cannot find libapriconv.dll to copy it to
the Release folder.
--
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: Re: [PATCH] Remove APR ICONV dependency on Windows (was SVN Win32 Developers -- need some help)
Posted by Mark Phippard <ma...@gmail.com>.
On 7/5/07, Paul Burba <pb...@collab.net> wrote:
> Mark,
>
> I did a build of trunk on my MSVC6 VM this morning and encountered the same problem you did. I think this explains the problem:
>
> http://trolltech.com/developer/knowledgebase/faq.2007-05-04.4776834989/
>
> After replacing this line in my build script:
>
> set SDKLIB=C:\Program Files\Microsoft Platform SDK\Lib
>
> with this:
>
> set SDKLIB=C:\Program Files\Microsoft Visual Studio\VC98\Lib;C:\Program Files\Microsoft Platform SDK\Lib
>
> Everything worked fine.
I will give that a try, but I seem to recall I had to do the exact
opposite for APR 1.2.8 to build because of needing newer LDAP
libraries or something like that. I suppose since APR is built
separate this might not be too big of an issue.
--
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: Re: [PATCH] Remove APR ICONV dependency on Windows (was SVN Win32 Developers -- need some help)
Posted by Paul Burba <pb...@collab.net>.
Mark,
I did a build of trunk on my MSVC6 VM this morning and encountered the same problem you did. I think this explains the problem:
http://trolltech.com/developer/knowledgebase/faq.2007-05-04.4776834989/
After replacing this line in my build script:
set SDKLIB=C:\Program Files\Microsoft Platform SDK\Lib
with this:
set SDKLIB=C:\Program Files\Microsoft Visual Studio\VC98\Lib;C:\Program Files\Microsoft Platform SDK\Lib
Everything worked fine.
Hope that helps.
Paul
> -----Original Message-----
> From: Stefan Küng [mailto:tortoisesvn@gmail.com]
> Sent: Thursday, July 05, 2007 9:43 AM
> To: Mark Phippard
> Cc: Ivan Zhakov; D.J. Heap; Daniel Rall; dev@subversion.tigris.org
> Subject: Re: [PATCH] Remove APR ICONV dependency on Windows
> (was SVN Win32 Developers -- need some help)
>
> On 7/5/07, Mark Phippard <ma...@gmail.com> wrote:
> > On 7/5/07, Ivan Zhakov <ch...@gmail.com> wrote:
> > > On 7/5/07, Mark Phippard <ma...@gmail.com> wrote:
> > > > On 7/5/07, Stefan Küng <to...@gmail.com> wrote:
> > > > > > Now when I build SVN it cannot create the
> libsvn_subr-1.dll
> > > > > > The reason is that it cannot resolve the symbols
> for these new functions
> > > > > > we are calling ... CoInitialize etc... I imagine
> there is some new
> > > > > > .lib that we have to make available during the
> compile? I am
> > > > > > using MSVC 6.0 with the latest SDK.
> > > > >
> > > > > Both CoInitialize() and CoUninitialize() are declared in
> > > > > objbase.h and you must link against ole32.lib.
> > > >
> > > > Thanks. Isn't this detail usually handled by
> gen-make.py? I did
> > > > re-run that before building. Or are we expecting that to be a
> > > > default library provided by the IDE?
> > > >
> > > Hi Mark,
> > > It builds for me with Visual Studio 2005. Could you try to apply
> > > following patch, run gen-make.py and build Subversion.
> >
> > That gets me a different error. I made sure that I blew away the
> > Release folder and tried again, but still get the same:
> >
> > --------------------Configuration: libsvn_subr_dll - Win32
> > Release--------------
> > ------
> > Compiling resources...
> > Compiling...
> > empty.c
> > Linking...
> > .\libsvn_subr.def : warning LNK4087: CONSTANT keyword is
> obsolete; use DATA
> > Creating library
> > ..\..\..\Release\subversion\libsvn_subr/libsvn_subr-1.lib
> an d object
> > ..\..\..\Release\subversion\libsvn_subr/libsvn_subr-1.exp
> > uuid.lib(mlang_i.obj) : fatal error LNK1103: debugging information
> > corrupt; reco mpile module Error executing link.exe.
> >
> > __ALL_TESTS__ - 1 error(s), 3 warning(s)
>
> Since the linker is complaining about mlang_i.obj, telling
> you that the debug info in there is corrupt, I strongly
> recommend you download and install the Windows SDK again.
> Seems your copy is corrupt somehow (which would explain your
> crashes with my patch some time ago).
>
> 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
>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] Remove APR ICONV dependency on Windows (was SVN Win32 Developers -- need some help)
Posted by Stefan Küng <to...@gmail.com>.
On 7/5/07, Mark Phippard <ma...@gmail.com> wrote:
> On 7/5/07, Ivan Zhakov <ch...@gmail.com> wrote:
> > On 7/5/07, Mark Phippard <ma...@gmail.com> wrote:
> > > On 7/5/07, Stefan Küng <to...@gmail.com> wrote:
> > > > > Now when I build SVN it cannot create the libsvn_subr-1.dll The
> > > > > reason is that it cannot resolve the symbols for these new functions
> > > > > we are calling ... CoInitialize etc... I imagine there is some new
> > > > > .lib that we have to make available during the compile? I am using
> > > > > MSVC 6.0 with the latest SDK.
> > > >
> > > > Both CoInitialize() and CoUninitialize() are declared in objbase.h and
> > > > you must link against ole32.lib.
> > >
> > > Thanks. Isn't this detail usually handled by gen-make.py? I did
> > > re-run that before building. Or are we expecting that to be a default
> > > library provided by the IDE?
> > >
> > Hi Mark,
> > It builds for me with Visual Studio 2005. Could you try to apply
> > following patch, run gen-make.py and build Subversion.
>
> That gets me a different error. I made sure that I blew away the
> Release folder and tried again, but still get the same:
>
> --------------------Configuration: libsvn_subr_dll - Win32 Release--------------
> ------
> Compiling resources...
> Compiling...
> empty.c
> Linking...
> .\libsvn_subr.def : warning LNK4087: CONSTANT keyword is obsolete; use DATA
> Creating library ..\..\..\Release\subversion\libsvn_subr/libsvn_subr-1.lib an
> d object ..\..\..\Release\subversion\libsvn_subr/libsvn_subr-1.exp
> uuid.lib(mlang_i.obj) : fatal error LNK1103: debugging information corrupt; reco
> mpile module
> Error executing link.exe.
>
> __ALL_TESTS__ - 1 error(s), 3 warning(s)
Since the linker is complaining about mlang_i.obj, telling you that
the debug info in there is corrupt, I strongly recommend you download
and install the Windows SDK again. Seems your copy is corrupt somehow
(which would explain your crashes with my patch some time ago).
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: [PATCH] Remove APR ICONV dependency on Windows (was SVN Win32 Developers -- need some help)
Posted by Mark Phippard <ma...@gmail.com>.
On 7/5/07, Ivan Zhakov <ch...@gmail.com> wrote:
> On 7/5/07, Mark Phippard <ma...@gmail.com> wrote:
> > On 7/5/07, Stefan Küng <to...@gmail.com> wrote:
> > > > Now when I build SVN it cannot create the libsvn_subr-1.dll The
> > > > reason is that it cannot resolve the symbols for these new functions
> > > > we are calling ... CoInitialize etc... I imagine there is some new
> > > > .lib that we have to make available during the compile? I am using
> > > > MSVC 6.0 with the latest SDK.
> > >
> > > Both CoInitialize() and CoUninitialize() are declared in objbase.h and
> > > you must link against ole32.lib.
> >
> > Thanks. Isn't this detail usually handled by gen-make.py? I did
> > re-run that before building. Or are we expecting that to be a default
> > library provided by the IDE?
> >
> Hi Mark,
> It builds for me with Visual Studio 2005. Could you try to apply
> following patch, run gen-make.py and build Subversion.
That gets me a different error. I made sure that I blew away the
Release folder and tried again, but still get the same:
--------------------Configuration: libsvn_subr_dll - Win32 Release--------------
------
Compiling resources...
Compiling...
empty.c
Linking...
.\libsvn_subr.def : warning LNK4087: CONSTANT keyword is obsolete; use DATA
Creating library ..\..\..\Release\subversion\libsvn_subr/libsvn_subr-1.lib an
d object ..\..\..\Release\subversion\libsvn_subr/libsvn_subr-1.exp
uuid.lib(mlang_i.obj) : fatal error LNK1103: debugging information corrupt; reco
mpile module
Error executing link.exe.
__ALL_TESTS__ - 1 error(s), 3 warning(s)
--
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: [PATCH] Remove APR ICONV dependency on Windows (was SVN Win32 Developers -- need some help)
Posted by Ivan Zhakov <ch...@gmail.com>.
On 7/5/07, Mark Phippard <ma...@gmail.com> wrote:
> On 7/5/07, Stefan Küng <to...@gmail.com> wrote:
> > > Now when I build SVN it cannot create the libsvn_subr-1.dll The
> > > reason is that it cannot resolve the symbols for these new functions
> > > we are calling ... CoInitialize etc... I imagine there is some new
> > > .lib that we have to make available during the compile? I am using
> > > MSVC 6.0 with the latest SDK.
> >
> > Both CoInitialize() and CoUninitialize() are declared in objbase.h and
> > you must link against ole32.lib.
>
> Thanks. Isn't this detail usually handled by gen-make.py? I did
> re-run that before building. Or are we expecting that to be a default
> library provided by the IDE?
>
Hi Mark,
It builds for me with Visual Studio 2005. Could you try to apply
following patch, run gen-make.py and build Subversion.
Index: build.conf
===================================================================
--- build.conf (revision 25654)
+++ build.conf (working copy)
@@ -311,7 +311,7 @@
install = fsmod-lib
path = subversion/libsvn_subr
libs = aprutil apriconv apr xml zlib
-msvc-libs = advapi32.lib shfolder.lib
+msvc-libs = advapi32.lib shfolder.lib ole32.lib
msvc-export = svn_auth.h svn_base64.h svn_cmdline.h svn_config.h
svn_ctype.h svn_dso.h svn_error.h svn_hash.h svn_io.h svn_md5.h
svn_nls.h svn_opt.h svn_mergeinfo.h svn_path.h svn_pools.h svn_props.h
svn_quoprint.h svn_sorts.h svn_string.h svn_subst.h svn_time.h
svn_types.h svn_user.h svn_utf.h svn_version.h svn_xml.h
private\svn_atomic.h private\svn_mergeinfo_private.h
# Low-level grab bag of utilities
--
Ivan Zhakov
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] Remove APR ICONV dependency on Windows (was SVN Win32 Developers -- need some help)
Posted by Mark Phippard <ma...@gmail.com>.
On 7/5/07, Stefan Küng <to...@gmail.com> wrote:
> > Now when I build SVN it cannot create the libsvn_subr-1.dll The
> > reason is that it cannot resolve the symbols for these new functions
> > we are calling ... CoInitialize etc... I imagine there is some new
> > .lib that we have to make available during the compile? I am using
> > MSVC 6.0 with the latest SDK.
>
> Both CoInitialize() and CoUninitialize() are declared in objbase.h and
> you must link against ole32.lib.
Thanks. Isn't this detail usually handled by gen-make.py? I did
re-run that before building. Or are we expecting that to be a default
library provided by the IDE?
--
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: [PATCH] Remove APR ICONV dependency on Windows (was SVN Win32 Developers -- need some help)
Posted by Stefan Küng <to...@gmail.com>.
On 7/5/07, Mark Phippard <ma...@gmail.com> wrote:
> On 7/5/07, Mark Phippard <ma...@gmail.com> wrote:
> > Thanks Ivan.
> >
> > Is it possible to build apr-util without iconv now? Or do we still
> > have to build apr-iconv and ship the DLL and just know that we are now
> > not actually using it?
>
> I figured this out. Just had to edit apu.h.
>
> Now when I build SVN it cannot create the libsvn_subr-1.dll The
> reason is that it cannot resolve the symbols for these new functions
> we are calling ... CoInitialize etc... I imagine there is some new
> .lib that we have to make available during the compile? I am using
> MSVC 6.0 with the latest SDK.
Both CoInitialize() and CoUninitialize() are declared in objbase.h and
you must link against ole32.lib.
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: [PATCH] Remove APR ICONV dependency on Windows (was SVN Win32 Developers -- need some help)
Posted by Mark Phippard <ma...@gmail.com>.
On 7/5/07, Mark Phippard <ma...@gmail.com> wrote:
> Thanks Ivan.
>
> Is it possible to build apr-util without iconv now? Or do we still
> have to build apr-iconv and ship the DLL and just know that we are now
> not actually using it?
I figured this out. Just had to edit apu.h.
Now when I build SVN it cannot create the libsvn_subr-1.dll The
reason is that it cannot resolve the symbols for these new functions
we are calling ... CoInitialize etc... I imagine there is some new
.lib that we have to make available during the compile? I am using
MSVC 6.0 with the latest SDK.
--
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: [PATCH] Remove APR ICONV dependency on Windows (was SVN Win32 Developers -- need some help)
Posted by Mark Phippard <ma...@gmail.com>.
On 7/5/07, Ivan Zhakov <ch...@gmail.com> wrote:
> On 7/3/07, D.J. Heap <dj...@gmail.com> wrote:
> > On 7/2/07, Ivan Zhakov <ch...@gmail.com> wrote:
> > [snip]
> > > Here is revisited patch. Major vhanges from previous version:
> > > - Use MLang API for converting page name to page id.
> > > - All Windows stuff moved to separate file win32_xlate.c.
> > >
> > > [[[
> > > Remove APR ICONV dependency on Windows and use native Windows
> > > character conversion API (MultiByteToWideChar/WideCharToMultiByte).
> > >
> >
> >
> > I've built and posted binaries with this patch at:
> >
> > http://subversion.tigris.org/servlets/ProjectDocumentList?folderID=8682&expandFolder=8682&folderID=8682
> >
> > I still get 2 fs-test failures with VS6 -- haven't had time to dig
> > into them yet.
> >
> > Also, I think we should probably check the return of CoInit and not
> > call CoUninit if it fails (because CoInit/CoInitEx has already been
> > called with a different apartment or something).
> >
> Good point! I've added check for return value of CoInitialize() and
> also added logic to detect when COM already initialized for different
> apartment.
>
> I think that patch is acceptable for trunk, thus I've committed it in
> r25640. Review more than welcome :)
>
> On 7/3/07, Daniel Rall <dl...@collab.net> wrote:
> > Just a nitpick, but some of the formatting is pretty inconsistent with
> > the rest of the code base (use of Hungarian notation, lack of
> > whitespace between data type and value in type casts, etc.).
> Thanks, I've checked my patch again and fixed what I found.
Thanks Ivan.
Is it possible to build apr-util without iconv now? Or do we still
have to build apr-iconv and ship the DLL and just know that we are now
not actually using it?
--
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: [PATCH] Remove APR ICONV dependency on Windows (was SVN Win32 Developers -- need some help)
Posted by Ivan Zhakov <ch...@gmail.com>.
On 7/3/07, D.J. Heap <dj...@gmail.com> wrote:
> On 7/2/07, Ivan Zhakov <ch...@gmail.com> wrote:
> [snip]
> > Here is revisited patch. Major vhanges from previous version:
> > - Use MLang API for converting page name to page id.
> > - All Windows stuff moved to separate file win32_xlate.c.
> >
> > [[[
> > Remove APR ICONV dependency on Windows and use native Windows
> > character conversion API (MultiByteToWideChar/WideCharToMultiByte).
> >
>
>
> I've built and posted binaries with this patch at:
>
> http://subversion.tigris.org/servlets/ProjectDocumentList?folderID=8682&expandFolder=8682&folderID=8682
>
> I still get 2 fs-test failures with VS6 -- haven't had time to dig
> into them yet.
>
> Also, I think we should probably check the return of CoInit and not
> call CoUninit if it fails (because CoInit/CoInitEx has already been
> called with a different apartment or something).
>
Good point! I've added check for return value of CoInitialize() and
also added logic to detect when COM already initialized for different
apartment.
I think that patch is acceptable for trunk, thus I've committed it in
r25640. Review more than welcome :)
On 7/3/07, Daniel Rall <dl...@collab.net> wrote:
> Just a nitpick, but some of the formatting is pretty inconsistent with
> the rest of the code base (use of Hungarian notation, lack of
> whitespace between data type and value in type casts, etc.).
Thanks, I've checked my patch again and fixed what I found.
--
Ivan Zhakov
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] Remove APR ICONV dependency on Windows (was SVN Win32 Developers -- need some help)
Posted by "D.J. Heap" <dj...@gmail.com>.
On 7/2/07, Ivan Zhakov <ch...@gmail.com> wrote:
[snip]
> Here is revisited patch. Major vhanges from previous version:
> - Use MLang API for converting page name to page id.
> - All Windows stuff moved to separate file win32_xlate.c.
>
> [[[
> Remove APR ICONV dependency on Windows and use native Windows
> character conversion API (MultiByteToWideChar/WideCharToMultiByte).
>
I've built and posted binaries with this patch at:
http://subversion.tigris.org/servlets/ProjectDocumentList?folderID=8682&expandFolder=8682&folderID=8682
I still get 2 fs-test failures with VS6 -- haven't had time to dig
into them yet.
Also, I think we should probably check the return of CoInit and not
call CoUninit if it fails (because CoInit/CoInitEx has already been
called with a different apartment or something).
DJ
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] Remove APR ICONV dependency on Windows (was SVN Win32 Developers -- need some help)
Posted by Ivan Zhakov <ch...@gmail.com>.
On 7/1/07, Stefan Küng <to...@gmail.com> wrote:
> Ivan Zhakov wrote:
> > On 7/1/07, steveking <to...@gmail.com> wrote:
> >> Ivan Zhakov wrote:
> >>
> >> > Sorry, I was busy and I have not time to report my research results.
> >> >
> >> > My patch almost complete and I need somebody's eyes to look and
> >> > confirm that I'm on right way (or say that I'm on wrong way :)
> >> >
> >> > I used Windows MIME database to convert page name to Windows page
> >> > identifier.
> >> > And most important question is it acceptable to read Windows MIME
> >> > database directly from registry instead of using MLang API [1]? I
> >> > didn't use MLang API, because it requires
> >> > CoInitialize()/CoUninitialize().
> >>
> >> I think it would be better to use the API instead of reading the
> >> registry. Because the registry is not documented and could change any
> >> time.
> >> About CoInitialize(): we could call that in svn_utf_initialize() and
> >> call CoUninitialize in an apr cleanup handler.
> > Actually registry is documented, at least for Windows CE :)
> > I agree that using API is better, but it's not too easy. We have to
> > call CoInitalize() in each thread that using COM. Another problem that
> > we don't require user libraries to call svn_utf8_initalize().
> >
> > As workaround of problems above we can call to
> > CoInitialize()/CoUninitalize() every time in win32_open_xlate(). This
> > function get called only once for each page identifiers, so it
> > shouldn't cause performance degradation. What do you think about it?
>
> I forgot about the backward compatibility. Otherwise we could just
> document that CoInitialize() must be called for every thread.
>
> If the registry is documented, then we can of course use that. I don't
> think it will be faster than using the API since registry accesses are
> expensive too, but at least for most conversions it's not needed anyway.
>
Here is revisited patch. Major vhanges from previous version:
- Use MLang API for converting page name to page id.
- All Windows stuff moved to separate file win32_xlate.c.
[[[
Remove APR ICONV dependency on Windows and use native Windows
character conversion API (MultiByteToWideChar/WideCharToMultiByte).
* subversion/tests/libsvn_subr/utf-test.c
(test_utf_cstring_to_utf8_ex2): New test for conversion from
different codepages
to utf8.
(test_utf_cstring_from_utf8_ex2): New test for conversion to
different codepages
from utf8.
(test_funcs): Add two tests.
* subversion/libsvn_subr/utf8.c
(get_xlate_handle_node): Use svn_subr__win32_xlate_open() on Windows.
(convert_to_stringbuf): Use svn_subr__win32_convert_to_stringbuf() on Windows.
* subversion/libsvn_subr/win32_xlate.c
* subversion/libsvn_subr/win32_xlate.h
(win32_xlate_t): Structure for holding Windows xlate data.
(get_page_id_from_name): Helper for converting page name to page identifier.
(svn_subr__win32_xlate_open): Opens Windows xlate to convert one codepage to
another.
(svn_subr__win32_convert_to_string_buf): Converts string from one codepage to
another using Windows API.
]]]
--
Ivan Zhakov
Re: [PATCH] Remove APR ICONV dependency on Windows (was SVN Win32 Developers -- need some help)
Posted by Ivan Zhakov <ch...@gmail.com>.
On 7/1/07, Stefan Küng <to...@gmail.com> wrote:
> Ivan Zhakov wrote:
> > On 7/1/07, steveking <to...@gmail.com> wrote:
> >> Ivan Zhakov wrote:
> >>
> >> > Sorry, I was busy and I have not time to report my research results.
> >> >
> >> > My patch almost complete and I need somebody's eyes to look and
> >> > confirm that I'm on right way (or say that I'm on wrong way :)
> >> >
> >> > I used Windows MIME database to convert page name to Windows page
> >> > identifier.
> >> > And most important question is it acceptable to read Windows MIME
> >> > database directly from registry instead of using MLang API [1]? I
> >> > didn't use MLang API, because it requires
> >> > CoInitialize()/CoUninitialize().
> >>
> >> I think it would be better to use the API instead of reading the
> >> registry. Because the registry is not documented and could change any
> >> time.
> >> About CoInitialize(): we could call that in svn_utf_initialize() and
> >> call CoUninitialize in an apr cleanup handler.
> > Actually registry is documented, at least for Windows CE :)
> > I agree that using API is better, but it's not too easy. We have to
> > call CoInitalize() in each thread that using COM. Another problem that
> > we don't require user libraries to call svn_utf8_initalize().
> >
> > As workaround of problems above we can call to
> > CoInitialize()/CoUninitalize() every time in win32_open_xlate(). This
> > function get called only once for each page identifiers, so it
> > shouldn't cause performance degradation. What do you think about it?
>
> I forgot about the backward compatibility. Otherwise we could just
> document that CoInitialize() must be called for every thread.
>
> If the registry is documented, then we can of course use that. I don't
> think it will be faster than using the API since registry accesses are
> expensive too, but at least for most conversions it's not needed anyway.
>
> [snip]
> >> I know this is the correct way to do this, but I don't like it. You
> >> allocate memory here which is needed only for a few lines of code. That
> >> memory is allocated in the apr pool, which we don't know when it gets
> >> cleared again (maybe not until the app exits because it's the same pool
> >> that holds the translated strings). That means that a lot of memory is
> >> 'wasted'.
> >> For example, assume you do an
> >> svn log -v url/to/kde/repository/root
> >> and you'd have to keep that log information alive until your app exits.
> >> With using an apr pool to reserve memory here that would mean that a
> >> *lot* of memory is kept allocated but not used anymore.
> >> That's why I used calloc() and free() in my patch - which isn't really
> >> good either in terms of platform independence, but it won't consume a
> >> lot of memory for nothing. Question is: do we really need apr pools here
> >> for this Windows-specific code?
> >> The correct way here may be to create a subpool and destroy that subpool
> >> before the function exits. But that's an expensive operation.
> >>
> >> In case you're wondering why someone would do such a thing (get the log
> >> info for a whole repository and keep it until the app exits), the
> >> revision graph in TSVN needs to do exactly that.
> > Yes, I know about this problem. I propose to discuss it separately
> > after committing simplest implementation. Because we can made several
> > optimizations: for example try to allocate small buffers on stack.
>
> Yes, that would improve the performance a lot. Since most conversions
> are done on paths, I suggest to allocate buffers smaller than
> MAX_PATH*sizeof(WCHAR) on the stack, the rest could use an apr subpool.
>
Hi Stefan,
I've implemented allocation of small buffers on stack in r26357. Now
buffers smaller than MAX_PATH allocated on stack.
I'm not sure about temporary allocating buffer using apr
subpool/malloc, so I didn't implement that. Feel free to propose patch
and/or open this topic on list if you want.
--
Ivan Zhakov
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] Remove APR ICONV dependency on Windows (was SVN Win32
Developers -- need some help)
Posted by Stefan Küng <to...@gmail.com>.
Ivan Zhakov wrote:
> On 7/1/07, steveking <to...@gmail.com> wrote:
>> Ivan Zhakov wrote:
>>
>> > Sorry, I was busy and I have not time to report my research results.
>> >
>> > My patch almost complete and I need somebody's eyes to look and
>> > confirm that I'm on right way (or say that I'm on wrong way :)
>> >
>> > I used Windows MIME database to convert page name to Windows page
>> > identifier.
>> > And most important question is it acceptable to read Windows MIME
>> > database directly from registry instead of using MLang API [1]? I
>> > didn't use MLang API, because it requires
>> > CoInitialize()/CoUninitialize().
>>
>> I think it would be better to use the API instead of reading the
>> registry. Because the registry is not documented and could change any
>> time.
>> About CoInitialize(): we could call that in svn_utf_initialize() and
>> call CoUninitialize in an apr cleanup handler.
> Actually registry is documented, at least for Windows CE :)
> I agree that using API is better, but it's not too easy. We have to
> call CoInitalize() in each thread that using COM. Another problem that
> we don't require user libraries to call svn_utf8_initalize().
>
> As workaround of problems above we can call to
> CoInitialize()/CoUninitalize() every time in win32_open_xlate(). This
> function get called only once for each page identifiers, so it
> shouldn't cause performance degradation. What do you think about it?
I forgot about the backward compatibility. Otherwise we could just
document that CoInitialize() must be called for every thread.
If the registry is documented, then we can of course use that. I don't
think it will be faster than using the API since registry accesses are
expensive too, but at least for most conversions it's not needed anyway.
[snip]
>> I know this is the correct way to do this, but I don't like it. You
>> allocate memory here which is needed only for a few lines of code. That
>> memory is allocated in the apr pool, which we don't know when it gets
>> cleared again (maybe not until the app exits because it's the same pool
>> that holds the translated strings). That means that a lot of memory is
>> 'wasted'.
>> For example, assume you do an
>> svn log -v url/to/kde/repository/root
>> and you'd have to keep that log information alive until your app exits.
>> With using an apr pool to reserve memory here that would mean that a
>> *lot* of memory is kept allocated but not used anymore.
>> That's why I used calloc() and free() in my patch - which isn't really
>> good either in terms of platform independence, but it won't consume a
>> lot of memory for nothing. Question is: do we really need apr pools here
>> for this Windows-specific code?
>> The correct way here may be to create a subpool and destroy that subpool
>> before the function exits. But that's an expensive operation.
>>
>> In case you're wondering why someone would do such a thing (get the log
>> info for a whole repository and keep it until the app exits), the
>> revision graph in TSVN needs to do exactly that.
> Yes, I know about this problem. I propose to discuss it separately
> after committing simplest implementation. Because we can made several
> optimizations: for example try to allocate small buffers on stack.
Yes, that would improve the performance a lot. Since most conversions
are done on paths, I suggest to allocate buffers smaller than
MAX_PATH*sizeof(WCHAR) on the stack, the rest could use an apr subpool.
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: [PATCH] Remove APR ICONV dependency on Windows (was SVN Win32 Developers -- need some help)
Posted by Ivan Zhakov <ch...@gmail.com>.
On 7/1/07, steveking <to...@gmail.com> wrote:
> Ivan Zhakov wrote:
>
> > Sorry, I was busy and I have not time to report my research results.
> >
> > My patch almost complete and I need somebody's eyes to look and
> > confirm that I'm on right way (or say that I'm on wrong way :)
> >
> > I used Windows MIME database to convert page name to Windows page
> > identifier.
> > And most important question is it acceptable to read Windows MIME
> > database directly from registry instead of using MLang API [1]? I
> > didn't use MLang API, because it requires
> > CoInitialize()/CoUninitialize().
>
> I think it would be better to use the API instead of reading the
> registry. Because the registry is not documented and could change any time.
> About CoInitialize(): we could call that in svn_utf_initialize() and
> call CoUninitialize in an apr cleanup handler.
Actually registry is documented, at least for Windows CE :)
I agree that using API is better, but it's not too easy. We have to
call CoInitalize() in each thread that using COM. Another problem that
we don't require user libraries to call svn_utf8_initalize().
As workaround of problems above we can call to
CoInitialize()/CoUninitalize() every time in win32_open_xlate(). This
function get called only once for each page identifiers, so it
shouldn't cause performance degradation. What do you think about it?
>
>
> [snip]
> +static apr_status_t
> +win32_convert_to_stringbuf(win32_xlate_t *handle, const char *src_data,
> + apr_size_t src_length, svn_stringbuf_t **dest,
> + apr_pool_t *pool)
> +{
> + WCHAR * wide_str;
> + int retval, wide_size;
> +
> + *dest = svn_stringbuf_create("", pool);
> +
> + if (src_length == 0)
> + return APR_SUCCESS;
> +
> + retval = MultiByteToWideChar(handle->from_page_id, 0, src_data,
> src_length,
> + NULL, 0);
> + if (retval == 0)
> + return APR_FROM_OS_ERROR(GetLastError());
> +
> + wide_size = retval;
> + wide_str = apr_palloc(pool, wide_size * sizeof(WCHAR));
>
> I know this is the correct way to do this, but I don't like it. You
> allocate memory here which is needed only for a few lines of code. That
> memory is allocated in the apr pool, which we don't know when it gets
> cleared again (maybe not until the app exits because it's the same pool
> that holds the translated strings). That means that a lot of memory is
> 'wasted'.
> For example, assume you do an
> svn log -v url/to/kde/repository/root
> and you'd have to keep that log information alive until your app exits.
> With using an apr pool to reserve memory here that would mean that a
> *lot* of memory is kept allocated but not used anymore.
> That's why I used calloc() and free() in my patch - which isn't really
> good either in terms of platform independence, but it won't consume a
> lot of memory for nothing. Question is: do we really need apr pools here
> for this Windows-specific code?
> The correct way here may be to create a subpool and destroy that subpool
> before the function exits. But that's an expensive operation.
>
> In case you're wondering why someone would do such a thing (get the log
> info for a whole repository and keep it until the app exits), the
> revision graph in TSVN needs to do exactly that.
Yes, I know about this problem. I propose to discuss it separately
after committing simplest implementation. Because we can made several
optimizations: for example try to allocate small buffers on stack.
--
Ivan Zhakov
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] Remove APR ICONV dependency on Windows (was SVN Win32
Developers -- need some help)
Posted by steveking <to...@gmail.com>.
Ivan Zhakov wrote:
> Sorry, I was busy and I have not time to report my research results.
>
> My patch almost complete and I need somebody's eyes to look and
> confirm that I'm on right way (or say that I'm on wrong way :)
>
> I used Windows MIME database to convert page name to Windows page
> identifier.
> And most important question is it acceptable to read Windows MIME
> database directly from registry instead of using MLang API [1]? I
> didn't use MLang API, because it requires
> CoInitialize()/CoUninitialize().
I think it would be better to use the API instead of reading the
registry. Because the registry is not documented and could change any time.
About CoInitialize(): we could call that in svn_utf_initialize() and
call CoUninitialize in an apr cleanup handler.
[snip]
+static apr_status_t
+win32_convert_to_stringbuf(win32_xlate_t *handle, const char *src_data,
+ apr_size_t src_length, svn_stringbuf_t **dest,
+ apr_pool_t *pool)
+{
+ WCHAR * wide_str;
+ int retval, wide_size;
+
+ *dest = svn_stringbuf_create("", pool);
+
+ if (src_length == 0)
+ return APR_SUCCESS;
+
+ retval = MultiByteToWideChar(handle->from_page_id, 0, src_data,
src_length,
+ NULL, 0);
+ if (retval == 0)
+ return APR_FROM_OS_ERROR(GetLastError());
+
+ wide_size = retval;
+ wide_str = apr_palloc(pool, wide_size * sizeof(WCHAR));
I know this is the correct way to do this, but I don't like it. You
allocate memory here which is needed only for a few lines of code. That
memory is allocated in the apr pool, which we don't know when it gets
cleared again (maybe not until the app exits because it's the same pool
that holds the translated strings). That means that a lot of memory is
'wasted'.
For example, assume you do an
svn log -v url/to/kde/repository/root
and you'd have to keep that log information alive until your app exits.
With using an apr pool to reserve memory here that would mean that a
*lot* of memory is kept allocated but not used anymore.
That's why I used calloc() and free() in my patch - which isn't really
good either in terms of platform independence, but it won't consume a
lot of memory for nothing. Question is: do we really need apr pools here
for this Windows-specific code?
The correct way here may be to create a subpool and destroy that subpool
before the function exits. But that's an expensive operation.
In case you're wondering why someone would do such a thing (get the log
info for a whole repository and keep it until the app exits), the
revision graph in TSVN needs to do exactly that.
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: [PATCH] Remove APR ICONV dependency on Windows (was SVN Win32 Developers -- need some help)
Posted by Mark Phippard <ma...@gmail.com>.
On 7/5/07, D.J. Heap <dj...@gmail.com> wrote:
> On 7/5/07, Ivan Zhakov <ch...@gmail.com> wrote:
> [snip]
> > I committed my patch and found that these tests fails on your buildbot
> > slave even with VS2005. But they work on my computer. Could you see
> > what happens on your buildbot slave?
>
>
> Yes I will as soon as I can -- probably this Saturday, I'm swamped
> until then, sorry. :(
Just an fyi, but my MSVC 6.0 build passed all tests (ra_local + fsfs).
There is still the problem that win-tests.py will not run if the
apriconv.dll is not present.
--
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: [PATCH] Remove APR ICONV dependency on Windows (was SVN Win32 Developers -- need some help)
Posted by "D.J. Heap" <dj...@gmail.com>.
On 7/5/07, Ivan Zhakov <ch...@gmail.com> wrote:
[snip]
> I committed my patch and found that these tests fails on your buildbot
> slave even with VS2005. But they work on my computer. Could you see
> what happens on your buildbot slave?
Yes I will as soon as I can -- probably this Saturday, I'm swamped
until then, sorry. :(
DJ
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] Remove APR ICONV dependency on Windows (was SVN Win32 Developers -- need some help)
Posted by "D.J. Heap" <dj...@gmail.com>.
On 7/7/07, Ivan Zhakov <ch...@gmail.com> wrote:
[snip]
> Is it happens only with 'Debug' build, but doesn't happen with 'Release'?
It fails quite consistently with Release, somewhat sporadically with Debug.
>
> May be there are some problems with DLLs? Did you try to build with
> --disable-shared? Another source of problems may be that there are
> different APR libraries installed somewhere.
Haven't tried --disable-shared (the fs-test.exe is statically linked
anyway), but I'm fairly certain the APR libraries are the correct ones
-- while debugging I watched which ones were loaded.
>
> Also could you check how is your APR built: Is it built with
> APR_HAS_UNICODE_FS or not?
Yes it is.
>
> Do you have some antivirus software installed?
No.
I will back out the patch and see what happens...The buildbot had been
running pretty clean up until that day, as I recall, though -- and I
got the same failures on a different VM with the VC6 build as well.
DJ
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] Remove APR ICONV dependency on Windows (was SVN Win32 Developers -- need some help)
Posted by Ivan Zhakov <ch...@gmail.com>.
On 7/8/07, D.J. Heap <dj...@gmail.com> wrote:
> On 7/7/07, Ivan Zhakov <ch...@gmail.com> wrote:
> [snip]
> > And of course primary question: Does it work without my patch? Because
> > these failures can be related to new txn generation code, committed by
> > Blair in r25430.
> >
>
>
> I'm building with trunk r25679 and after backing out r25650, the
> fs-test.exe passes all tests again. So, either the patch is bringing
> out some lurking problem, or it is affecting the fs stuff in some way
> I can't seem to figure out.
>
It seems I found way to reproduce problem even without my patch. Many
fs-test will fail on Windows if you just remove time part from
generated transaction name in fs_fs. See patch:
Index: subversion/libsvn_fs_fs/fs_fs.c
===================================================================
--- subversion/libsvn_fs_fs/fs_fs.c (revision 25654)
+++ subversion/libsvn_fs_fs/fs_fs.c (working copy)
@@ -3195,7 +3195,7 @@
*p = '-';
process_id = getpid();
- now = apr_time_now();
+ now = 0; /* It's for testing purposes only! should apr_time_now(). */
unique_basename = apr_psprintf(pool, "%s-%05d-%" APR_TIME_T_FMT,
hostname_str, process_id, now);
Index: subversion/libsvn_subr/utf.c
===================================================================
--- subversion/libsvn_subr/utf.c (revision 25654)
+++ subversion/libsvn_subr/utf.c (working copy)
@@ -253,7 +253,7 @@
pool = apr_hash_pool_get(xlate_handle_hash);
/* Try to create a handle. */
-#if defined( WIN32)
+#if 0
apr_err = svn_subr__win32_xlate_open(&handle, topage, frompage, pool);
#elif defined(AS400)
apr_err = apr_xlate_open(&handle, (int)topage, (int)frompage, pool);
So it seems the problem of my patch that it made running Subversion
faster and we got two transactions in the same time :)
Now I'm trying to figure out why this transaction generation code
fails on Windows.
--
Ivan Zhakov
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] Remove APR ICONV dependency on Windows (was SVN Win32 Developers -- need some help)
Posted by "D.J. Heap" <dj...@gmail.com>.
On 7/7/07, Ivan Zhakov <ch...@gmail.com> wrote:
[snip]
> And of course primary question: Does it work without my patch? Because
> these failures can be related to new txn generation code, committed by
> Blair in r25430.
>
I'm building with trunk r25679 and after backing out r25650, the
fs-test.exe passes all tests again. So, either the patch is bringing
out some lurking problem, or it is affecting the fs stuff in some way
I can't seem to figure out.
DJ
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] Remove APR ICONV dependency on Windows (was SVN Win32 Developers -- need some help)
Posted by Ivan Zhakov <ch...@gmail.com>.
On 7/8/07, Ivan Zhakov <ch...@gmail.com> wrote:
> On 7/7/07, D.J. Heap <dj...@gmail.com> wrote:
> > On 7/5/07, Ivan Zhakov <ch...@gmail.com> wrote:
> > [snip]
> > > > svn_tests: Reference to non-existent node
> > > > '1.0.tDJ-VM-XP-VS6-02704-1183257584562500-1' in filesystem
> > > > 'test-repo-check-root-revision'
> > > > FAIL: fs-test.exe 26: ensure accurate storage of root node
> > [snip]
> > > I committed my patch and found that these tests fails on your buildbot
> > > slave even with VS2005. But they work on my computer. Could you see
> > > what happens on your buildbot slave?
> >
> >
> > I've spent a few hours trying to debug through this now and haven't
> > found out exactly what is going on. It doesn't happen when I step
> > through the code with a debugger, and it is somewhat sporadic in the
> > debug build anyway. So I've had to put in sleeps, printf traces, etc.
> > to get anywhere and those sometimes stop the failure from happening as
> > well.
> >
> > But in all the failures I've seen, it comes down to a transaction
> > child-node file not being there -- whether it was never created or has
> > been deleted or something, I'm not sure yet. Any ideas or suggestions
> > on where to look or what to look for?
> >
> Is it happens only with 'Debug' build, but doesn't happen with 'Release'?
>
> May be there are some problems with DLLs? Did you try to build with
> --disable-shared? Another source of problems may be that there are
> different APR libraries installed somewhere.
>
> Also could you check how is your APR built: Is it built with
> APR_HAS_UNICODE_FS or not?
>
> Do you have some antivirus software installed?
>
And of course primary question: Does it work without my patch? Because
these failures can be related to new txn generation code, committed by
Blair in r25430.
--
Ivan Zhakov
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] Remove APR ICONV dependency on Windows (was SVN Win32 Developers -- need some help)
Posted by Ivan Zhakov <ch...@gmail.com>.
On 7/7/07, D.J. Heap <dj...@gmail.com> wrote:
> On 7/5/07, Ivan Zhakov <ch...@gmail.com> wrote:
> [snip]
> > > svn_tests: Reference to non-existent node
> > > '1.0.tDJ-VM-XP-VS6-02704-1183257584562500-1' in filesystem
> > > 'test-repo-check-root-revision'
> > > FAIL: fs-test.exe 26: ensure accurate storage of root node
> [snip]
> > I committed my patch and found that these tests fails on your buildbot
> > slave even with VS2005. But they work on my computer. Could you see
> > what happens on your buildbot slave?
>
>
> I've spent a few hours trying to debug through this now and haven't
> found out exactly what is going on. It doesn't happen when I step
> through the code with a debugger, and it is somewhat sporadic in the
> debug build anyway. So I've had to put in sleeps, printf traces, etc.
> to get anywhere and those sometimes stop the failure from happening as
> well.
>
> But in all the failures I've seen, it comes down to a transaction
> child-node file not being there -- whether it was never created or has
> been deleted or something, I'm not sure yet. Any ideas or suggestions
> on where to look or what to look for?
>
Is it happens only with 'Debug' build, but doesn't happen with 'Release'?
May be there are some problems with DLLs? Did you try to build with
--disable-shared? Another source of problems may be that there are
different APR libraries installed somewhere.
Also could you check how is your APR built: Is it built with
APR_HAS_UNICODE_FS or not?
Do you have some antivirus software installed?
--
Ivan Zhakov
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] Remove APR ICONV dependency on Windows (was SVN Win32 Developers -- need some help)
Posted by "D.J. Heap" <dj...@gmail.com>.
On 7/5/07, Ivan Zhakov <ch...@gmail.com> wrote:
[snip]
> > svn_tests: Reference to non-existent node
> > '1.0.tDJ-VM-XP-VS6-02704-1183257584562500-1' in filesystem
> > 'test-repo-check-root-revision'
> > FAIL: fs-test.exe 26: ensure accurate storage of root node
[snip]
> I committed my patch and found that these tests fails on your buildbot
> slave even with VS2005. But they work on my computer. Could you see
> what happens on your buildbot slave?
I've spent a few hours trying to debug through this now and haven't
found out exactly what is going on. It doesn't happen when I step
through the code with a debugger, and it is somewhat sporadic in the
debug build anyway. So I've had to put in sleeps, printf traces, etc.
to get anywhere and those sometimes stop the failure from happening as
well.
But in all the failures I've seen, it comes down to a transaction
child-node file not being there -- whether it was never created or has
been deleted or something, I'm not sure yet. Any ideas or suggestions
on where to look or what to look for?
DJ
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] Remove APR ICONV dependency on Windows (was SVN Win32 Developers -- need some help)
Posted by Ivan Zhakov <ch...@gmail.com>.
On 7/1/07, D.J. Heap <dj...@gmail.com> wrote:
> On 6/30/07, D.J. Heap <dj...@gmail.com> wrote:
> > On 6/30/07, Ivan Zhakov <ch...@gmail.com> wrote:
> > [snip]
> > > Sorry, I was busy and I have not time to report my research results.
> > >
> > > My patch almost complete and I need somebody's eyes to look and
> > > confirm that I'm on right way (or say that I'm on wrong way :)
> >
> >
> > I've built trunk r25598 + Stefan/Ivan's patch and posted the binaries at:
> >
> > http://subversion.tigris.org/servlets/ProjectDocumentList?folderID=8682&expandFolder=8682&folderID=8682
> >
> > so people with multi-language setups can test them a bit.
> >
>
>
> FYI, I get two fs-test failures with this patch on WinXP with VC6:
>
> PASS: fs-test.exe 25: create and modify large file (seed=2683120967)
> CLEANUP: C:/Temp/tmp/tests/subversion/tests/libsvn_fs/test-repo-large-file-integrity
> svn_tests: Reference to non-existent node
> '1.0.tDJ-VM-XP-VS6-02704-1183257584562500-1' in filesystem
> 'test-repo-check-root-revision'
> FAIL: fs-test.exe 26: ensure accurate storage of root node
> PASS: fs-test.exe 27: svn_fs_node_created_rev test
> CLEANUP: C:/Temp/tmp/tests/subversion/tests/libsvn_fs/test-repo-node-created-rev
> svn_tests: Reference to non-existent node
> '2.0.tDJ-VM-XP-VS6-02704-1183257584937500-1' in filesystem
> 'test-repo-check-related'
> FAIL: fs-test.exe 28: test svn_fs_check_related
> PASS: fs-test.exe 29: test complex copies (branches)
>
I committed my patch and found that these tests fails on your buildbot
slave even with VS2005. But they work on my computer. Could you see
what happens on your buildbot slave?
--
Ivan Zhakov
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] Remove APR ICONV dependency on Windows (was SVN Win32 Developers -- need some help)
Posted by "D.J. Heap" <dj...@gmail.com>.
On 7/3/07, Daniel Rall <dl...@collab.net> wrote:
[snip]
> > I don't have any strong feelings about the temporary memory
> > management, but I would tend to prefer a subpool, I think, unless it
> > turns out to be a performance issue as Stefan thinks it might.
>
> Is this question in relation to svn_subr__win32_xlate_to_stringbuf()?
>
Right.
DJ
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] Remove APR ICONV dependency on Windows (was SVN Win32 Developers -- need some help)
Posted by Daniel Rall <dl...@collab.net>.
On Sun, 01 Jul 2007, D.J. Heap wrote:
> On 7/1/07, Ivan Zhakov <ch...@gmail.com> wrote:
...
> >Btw, what do you think about patch itself?
>
> As you and Stefan said, using the API rather than the registry will
> probably be more robust and I think the win32 specific stuff might be
> better off in its own file.
Just a nitpick, but some of the formatting is pretty inconsistent with
the rest of the code base (use of Hungarian notation, lack of
whitespace between data type and value in type casts, etc.).
> I don't have any strong feelings about the temporary memory
> management, but I would tend to prefer a subpool, I think, unless it
> turns out to be a performance issue as Stefan thinks it might.
Is this question in relation to svn_subr__win32_xlate_to_stringbuf()?
Re: [PATCH] Remove APR ICONV dependency on Windows (was SVN Win32 Developers -- need some help)
Posted by "D.J. Heap" <dj...@gmail.com>.
On 7/1/07, Ivan Zhakov <ch...@gmail.com> wrote:
[snip]
> It's strange, because it works for me: VS2005, fsfs.
Hmm, yes, strange -- VS2005 works for me too. I'll try to debug
through the fs-test failures on VC6 to see what's going on when I
can.
>
> Btw, what do you think about patch itself?
>
As you and Stefan said, using the API rather than the registry will
probably be more robust and I think the win32 specific stuff might be
better off in its own file.
I don't have any strong feelings about the temporary memory
management, but I would tend to prefer a subpool, I think, unless it
turns out to be a performance issue as Stefan thinks it might.
DJ
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] Remove APR ICONV dependency on Windows (was SVN Win32 Developers -- need some help)
Posted by Ivan Zhakov <ch...@gmail.com>.
On 7/1/07, D.J. Heap <dj...@gmail.com> wrote:
> On 6/30/07, D.J. Heap <dj...@gmail.com> wrote:
> > On 6/30/07, Ivan Zhakov <ch...@gmail.com> wrote:
> > [snip]
> > > Sorry, I was busy and I have not time to report my research results.
> > >
> > > My patch almost complete and I need somebody's eyes to look and
> > > confirm that I'm on right way (or say that I'm on wrong way :)
> >
> >
> > I've built trunk r25598 + Stefan/Ivan's patch and posted the binaries at:
> >
> > http://subversion.tigris.org/servlets/ProjectDocumentList?folderID=8682&expandFolder=8682&folderID=8682
> >
> > so people with multi-language setups can test them a bit.
> >
>
>
> FYI, I get two fs-test failures with this patch on WinXP with VC6:
>
> PASS: fs-test.exe 25: create and modify large file (seed=2683120967)
> CLEANUP: C:/Temp/tmp/tests/subversion/tests/libsvn_fs/test-repo-large-file-integrity
> svn_tests: Reference to non-existent node
> '1.0.tDJ-VM-XP-VS6-02704-1183257584562500-1' in filesystem
> 'test-repo-check-root-revision'
> FAIL: fs-test.exe 26: ensure accurate storage of root node
> PASS: fs-test.exe 27: svn_fs_node_created_rev test
> CLEANUP: C:/Temp/tmp/tests/subversion/tests/libsvn_fs/test-repo-node-created-rev
> svn_tests: Reference to non-existent node
> '2.0.tDJ-VM-XP-VS6-02704-1183257584937500-1' in filesystem
> 'test-repo-check-related'
> FAIL: fs-test.exe 28: test svn_fs_check_related
> PASS: fs-test.exe 29: test complex copies (branches)
>
>
> All other ra_local tests passed.
>
It's strange, because it works for me: VS2005, fsfs.
Btw, what do you think about patch itself?
--
Ivan Zhakov
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] Remove APR ICONV dependency on Windows (was SVN Win32 Developers -- need some help)
Posted by "D.J. Heap" <dj...@gmail.com>.
On 6/30/07, D.J. Heap <dj...@gmail.com> wrote:
> On 6/30/07, Ivan Zhakov <ch...@gmail.com> wrote:
> [snip]
> > Sorry, I was busy and I have not time to report my research results.
> >
> > My patch almost complete and I need somebody's eyes to look and
> > confirm that I'm on right way (or say that I'm on wrong way :)
>
>
> I've built trunk r25598 + Stefan/Ivan's patch and posted the binaries at:
>
> http://subversion.tigris.org/servlets/ProjectDocumentList?folderID=8682&expandFolder=8682&folderID=8682
>
> so people with multi-language setups can test them a bit.
>
FYI, I get two fs-test failures with this patch on WinXP with VC6:
PASS: fs-test.exe 25: create and modify large file (seed=2683120967)
CLEANUP: C:/Temp/tmp/tests/subversion/tests/libsvn_fs/test-repo-large-file-integrity
svn_tests: Reference to non-existent node
'1.0.tDJ-VM-XP-VS6-02704-1183257584562500-1' in filesystem
'test-repo-check-root-revision'
FAIL: fs-test.exe 26: ensure accurate storage of root node
PASS: fs-test.exe 27: svn_fs_node_created_rev test
CLEANUP: C:/Temp/tmp/tests/subversion/tests/libsvn_fs/test-repo-node-created-rev
svn_tests: Reference to non-existent node
'2.0.tDJ-VM-XP-VS6-02704-1183257584937500-1' in filesystem
'test-repo-check-related'
FAIL: fs-test.exe 28: test svn_fs_check_related
PASS: fs-test.exe 29: test complex copies (branches)
All other ra_local tests passed.
DJ
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] Remove APR ICONV dependency on Windows (was SVN Win32 Developers -- need some help)
Posted by "D.J. Heap" <dj...@gmail.com>.
On 6/30/07, Ivan Zhakov <ch...@gmail.com> wrote:
[snip]
> Sorry, I was busy and I have not time to report my research results.
>
> My patch almost complete and I need somebody's eyes to look and
> confirm that I'm on right way (or say that I'm on wrong way :)
I've built trunk r25598 + Stefan/Ivan's patch and posted the binaries at:
http://subversion.tigris.org/servlets/ProjectDocumentList?folderID=8682&expandFolder=8682&folderID=8682
so people with multi-language setups can test them a bit.
DJ
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org