Does dir_delta need a text_deltas parameter?

[Greg Hudson <gh...@MIT.EDU>]

libsvn_repos's svn_repos_dir_delta() accepts a text_deltas parameter
which controls whether or not text deltas are generated.

As far as I know, the only reason for this parameter is so that we can
use it for a "status" operation without the overhead of text deltas.

But the parameter is not necessary for this purpose; a status editor
returns a NULL handler from apply_textdelta.  So the text_deltas
parameter would appear to be redundant.  (And dangerous; if an editor
requests deltas and doesn't get them, or vice versa, than something
is likely to go wrong.)

Does anyone know a reason why I can't nuke it?  If nobody can remember
why it's there, I'm sure the test suite will tell me if there's a
real reason for it.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: Does dir_delta need a text_deltas parameter?

[Greg Hudson <gh...@MIT.EDU>]

Trying to implement this change, I found that delta_files() (a helper
function for dir_delta()) uses the text_deltas flag to save the work of
computing a base checksum.  (On the other hand, it does not try to save
the effort of calling svn_fs_contents_changed(), which is supposed to
some day do a full text comparison, but currently doesn't.)

This exposes a flaw in the editor interface: when driving a status
editor, you shouldn't have to go to the work of computing checksums. 
But in the absence of out-of-band information, you don't find that out
until you call apply_textdelta(), at which point you've already passed
in the base checksum.

I'm leaving things as-is for now, but this is a pretty serious API wart.


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: Does dir_delta need a text_deltas parameter?

[Ben Collins-Sussman <su...@collab.net>]

Greg Hudson <gh...@MIT.EDU> writes:

> libsvn_repos's svn_repos_dir_delta() accepts a text_deltas parameter
> which controls whether or not text deltas are generated.
> 
> As far as I know, the only reason for this parameter is so that we can
> use it for a "status" operation without the overhead of text deltas.
> 
> But the parameter is not necessary for this purpose; a status editor
> returns a NULL handler from apply_textdelta.  So the text_deltas
> parameter would appear to be redundant.  (And dangerous; if an editor
> requests deltas and doesn't get them, or vice versa, than something
> is likely to go wrong.)
> 
> Does anyone know a reason why I can't nuke it?  If nobody can remember
> why it's there, I'm sure the test suite will tell me if there's a
> real reason for it.

mod_dav_svn heavily depends on this feature, I think.

ra_dav's implementation of RA->do_update, RA->do_switch, and
RA->do_diff all essentially do the same thing:  a custom deltaV REPORT
request is sent:  "please compare rev1/path1 with rev2/path2".  

When mod_dav_svn receives this request, I believe it invokes
svn_repos_dir_delta() with the "no text deltas" option.  The response
sent back to the client is essentially a "skeletal" editor drive from
_dir_delta().   The client parses this skeleton, and does an HTTP GET
requests whenever it sees an add_file() or open_file().

But what you say is true, Greg;  I don't see why mod_dav_svn couldn't
just return a NULL apply_textdelta handler.  Let's see what cmpilato
and gstein say about this.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org