Issue #2064 and libsvn_wc/diff.c

[Pe...@sorman.com]

Hi,

This wants a libsvn_wc guru:-)

Working my way through issue #2064 and the redesign of the wc_diff_callbaks, I'm currently working on diff.c in libsvn_wc. There, we have code to do either WC -> WC comparisons or WC -> repos dito. In many places in file_diff and directory_elements_diff, there is code to check for modifications between WC and pristine copy before calling the diff callbacks supplied by the user. Then, but only if use_text_base is TRUE, the callbacks are actually called. I would say that file_diffs has no vissible side effects if use_text_base is true. Also, dir_elements_diff will crawl through the directory structure, checking for files or directories that weren't already diffed (i.e. that weren't different in the repository revision) even if use_text_base is false. I don't see why this is happening, since if use_text_base is false, local modifications are uninteresting anyway.

I would like to clean this up if it is correct, but I ask here first, since I don't know if this code have some subtle, but essential, side effects in the !use_text_base case that I don't see.

Any ideas?

Regards,
//Peter

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

Re: Issue #2064 and libsvn_wc/diff.c

["Peter N. Lundblad" <pe...@famlundblad.se>]

On Thu, 4 Nov 2004, Philip Martin wrote:

> <Pe...@sorman.com> writes:
>
> > Working my way through issue #2064 and the redesign of the
> > wc_diff_callbaks, I'm currently working on diff.c in libsvn_wc. There,
> > we have code to do either WC -> WC comparisons or WC -> repos dito. In
> > many places in file_diff and directory_elements_diff, there is code to
> > check for modifications between WC and pristine copy before calling
> > the diff callbacks supplied by the user. Then, but only if
> > use_text_base is TRUE,
>
> I think you mean FALSE (or !use_text_base is TRUE).
>
Heh, I wrote that after a day of bug-hunting at my day job. Ofcourse, I
meant the oposite.

> As I understand it (I didn't write use_text_base) things work as
> follows:
>
> file_diff and directory_elements_diff (which I did write) are used to
> generate BASE-to-WORKING diffs ("svn diff" or "svn diff -rBASE") which
> does not require repository contact.  They are also used during the
> generation of REVISION-to-WORKING diffs ("svn diff -rNNN") since
> REVISION-to-WORKING is obtained by combining BASE-to-WORKING with
> REVISION-to-BASE, but again file_diff and directory_elements_diff do
> not contact the repository.
>
> The flag use_text_base is TRUE when generating BASE-to-REVISION diffs
> ("svn diff -rBASE:NNN") which is a special case of the more usual
> REVISION-to-REVISION ("svn diff -rNNN:MMM") where the base revision
> varies across the wc.  In this case the BASE-to-WORKING diffs are of
> no interest, the working copy is just used to provide the mixed
> revision BASE. (The usual REVISION-to-REVISION for fixed revisions is
> handled by repos_diff.c in libsvn_client.)
>
Yes, this looks like my interpretation of the code. The clean-up isn't the
most common case, but it is just that, a cleanup that I want to make while
reorganizing some of the code anyway.

> > I would like to clean this up if it is correct, but I ask here first,
> > since I don't know if this code have some subtle, but essential, side
> > effects in the !use_text_base case that I don't see.
>
> Aside from the mixup over use_text_base and !use_text_base (just in
> this email?) I think you have understood it.
>
Good. Thanks for checking and clarifying.

Regards,
//Peter

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

Re: Issue #2064 and libsvn_wc/diff.c

[Philip Martin <ph...@codematters.co.uk>]

<Pe...@sorman.com> writes:

> Working my way through issue #2064 and the redesign of the
> wc_diff_callbaks, I'm currently working on diff.c in libsvn_wc. There,
> we have code to do either WC -> WC comparisons or WC -> repos dito. In
> many places in file_diff and directory_elements_diff, there is code to
> check for modifications between WC and pristine copy before calling
> the diff callbacks supplied by the user. Then, but only if
> use_text_base is TRUE,

I think you mean FALSE (or !use_text_base is TRUE).

> the callbacks are actually called. I would say
> that file_diffs has no vissible side effects if use_text_base is
> true. Also, dir_elements_diff will crawl through the directory
> structure, checking for files or directories that weren't already
> diffed (i.e. that weren't different in the repository revision) even
> if use_text_base is false. I don't see why this is happening, since if
> use_text_base is false, local modifications are uninteresting anyway.

I think you mean local modifications are of no interest if
use_text_base is TRUE (or !use_text_base is FALSE).

As I understand it (I didn't write use_text_base) things work as
follows:

file_diff and directory_elements_diff (which I did write) are used to
generate BASE-to-WORKING diffs ("svn diff" or "svn diff -rBASE") which
does not require repository contact.  They are also used during the
generation of REVISION-to-WORKING diffs ("svn diff -rNNN") since
REVISION-to-WORKING is obtained by combining BASE-to-WORKING with
REVISION-to-BASE, but again file_diff and directory_elements_diff do
not contact the repository.

The flag use_text_base is TRUE when generating BASE-to-REVISION diffs
("svn diff -rBASE:NNN") which is a special case of the more usual
REVISION-to-REVISION ("svn diff -rNNN:MMM") where the base revision
varies across the wc.  In this case the BASE-to-WORKING diffs are of
no interest, the working copy is just used to provide the mixed
revision BASE. (The usual REVISION-to-REVISION for fixed revisions is
handled by repos_diff.c in libsvn_client.)


> I would like to clean this up if it is correct, but I ask here first,
> since I don't know if this code have some subtle, but essential, side
> effects in the !use_text_base case that I don't see.

Aside from the mixup over use_text_base and !use_text_base (just in
this email?) I think you have understood it.

-- 
Philip Martin

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