[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: Compressed text-base patch

From: Eric Dorland <eric.dorland_at_mail.mcgill.ca>
Date: 2003-04-28 22:59:41 CEST

* Philip Martin (philip@codematters.co.uk) wrote:
> Eric Dorland <eric.dorland@mail.mcgill.ca> writes:
>
> > After a few weeks of work on and off, here's my first stab at a
> > compressed text-base patch.
>
> Great! I was wondering if anything was happening on this front.
>
> > To enable this code you need to ./configure with --with-zlib of
> > course, and add compress-text-base = yes in the [miscellany] section
> > of your .subversion/config file. Thanks all, I look forward to your
> > comments and, hopefully, your bug fixes :)
>
> I see the code adds a .gz suffix on to the name of compressed text
> bases, and then uses svn_io_check_path in some places, and strcmp in
> other places, to determine whether a text base is compressed or not.
> That looks a little ugly to me, did you consider storing some sort of
> compressed flag in the entries file? One problem I have with your
> current implementation is that I would want to be able to control text
> base compression on a per working copy basis. Hmm, perhaps an 'svn
> compress' command to convert a working copy would be useful?

No I didn't consider that, and it does sound like a good idea. I went
with the extension because it makes it obvious that the file is a
compressed file, but since it's in the .svn directory that probably
isn't that important.
 
> As to why your commits fail, the problem is your use of
> apr_file_data_get/apr_file_data_set. I do not know whether your
> expectations are wrong, or whether the APR implementation is wrong,
> but it appears that the data gets associated with the pool used by the
> file, rather than with the file itself. This causes problems when two
> files use the same pool.

Yes I figured that out last night myself :) I think it's APR that's in
the wrong here, since you would except the data to be associated with
the file handle, not it's memory pool. Are there any APR developers
on the list who think I'm wrong?

> I could get a simple commit to work using
>
> {
> apr_pool_t *subpool = svn_pool_create (pool);
> /* Open a filehandle for tmp text-base. */
> SVN_ERR_W (svn_io_file_open (&localfile, tmp_base,
> APR_READ, APR_OS_DEFAULT, subpool),
> "svn_wc_transmit_text_deltas: error opening local file");
> }
>
> around line 617 in adm_crawler.c:svn_wc_transmit_text_deltas but this
> is obviously a hack and lots of regression tests still fail. Perhaps
> if the entries file stored a compressed flag this use of APR file data
> would not be required?

Well the data is there to transparently let the stream functions know
whether the data is compressed or not. It's sort of a hacked
polymorphism, so that we don't have to check whether the text-base is
compressed or not deep in the code. You just call
svn_stream_from_apr_file, and it knows whether to wrap it in a
compressed stream or not (from the file data). So storing the
compression flag in the entries wouldn't make this go away. So either
I'll fix APR or use a better hack than the one above :-D I have one in
mind that wouldn't be too gruesome.

-- 
Eric Dorland <eric.dorland@mail.mcgill.ca>
ICQ: #61138586, Jabber: hooty@jabber.com
1024D/16D970C6 097C 4861 9934 27A0 8E1C  2B0A 61E9 8ECF 16D9 70C6
-----BEGIN GEEK CODE BLOCK-----
Version: 3.12
GCS d- s++: a-- C+++ UL+++ P++ L++ E++ W++ N+ o K- w+ 
O? M++ V-- PS+ PE Y+ PGP++ t++ 5++ X+ R tv++ b+++ DI+ D+ 
G e h! r- y+ 
------END GEEK CODE BLOCK------
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Mon Apr 28 23:00:46 2003

This is an archived mail posted to the Subversion Dev mailing list.

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.