You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Malcolm Rowe <ma...@farside.org.uk> on 2005/10/07 21:30:49 UTC

[PATCH] Note latent bug in libsvn_wc/diff.c

I've just spotted what I'm almost certain is a bug in libsvn_wc/diff.c,
and I'd like to make sure it's not forgotten.

So why not add a regression test? The precise bug is that svn_client_diff3()
will, for wc->repos diffs, report deleted directories as adds. However,
BASE<->repos diffs don't currently report deleted directories at all
(that's another bug), and there's no way to get the command-line client
to request a WORKING->repos diff, only a repos->WORKING diff (which in
this situation, produces identical - but this time, correct - output).

It should be possible to reproduce this by calling svn_client_diff3()
directly, of course, but I don't have the time (or experience) to be
able to create a regression test that doesn't just drive the command-line
client, so this seemed like the next-best thing.

[[[
* subversion/libsvn_wc/diff.c
  (delete_entry): Add a comment noting that we don't correctly handle
    deleted directories for wc->repos diffs at present.
]]]

Regards,
Malcolm

Re: [PATCH] Note latent bug in libsvn_wc/diff.c

Posted by Malcolm Rowe <ma...@farside.org.uk>.
On Sat, Oct 08, 2005 at 05:47:53PM +0200, Peter N. Lundblad wrote:
> I see. It might be possible to use the binding tests for this, but I no
> nothing about them. Is the reason you're not fixing the bug that you can't
> test it or is it hard to fix in itself?

A bit of both, I think. I'm not going to try to start fixing it until I
can reproduce the problem, and while I could easily hack my local client
to request a diff of -rWORKING:X, that wouldn't help me when it came
to submitting the patch itself, because I'd need a regression test. The
bug's not trivial either, unless I missed something.

> BTW, I think we need real libsvn_client tests for stuff like this and
> stuff like thi diff_summarize APIs that aren't available through the
> cmdline client (yet).

Agreed. I took a look at this briefly today, but I couldn't come up
with any obviously great solution. As a non-commandline client test, I'd
expect to see it in subversion/tests/, with all the other libsvn_* tests.

However, most of what we'd need to test would require the support code
in tests/cmdline/client anyway, because we'd want to intersperse calls
to svn (for 'add', 'ci', etc) with direct calls to libsvn_client. Anyway,
the commandline tests _are_ pretty much the tests for libsvn_client.

So the best I came up with was this: create a stub client in
subversion/tests/, perhaps based on tools/examples/minimal_client.c,
whose only job is to run svn_client_diff3 against -rWORKING:N, for a
specified N and target. Then call that tool from within diff_tests.py,
just as we'd normally call svn or svnadmin.

And similarly for the diff_summarize work. If at a later date, the svn
client actually gains the ability to call any of those APIs, we retire
the stub client and just replace the call to it with one to svn.

It sucks, but I can't think of anything better.

Regards,
Malcolm

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

Re: [PATCH] Note latent bug in libsvn_wc/diff.c

Posted by "Peter N. Lundblad" <pe...@famlundblad.se>.
On Fri, 7 Oct 2005, Malcolm Rowe wrote:

> I've just spotted what I'm almost certain is a bug in libsvn_wc/diff.c,
> and I'd like to make sure it's not forgotten.
>
> So why not add a regression test? The precise bug is that svn_client_diff3()
> will, for wc->repos diffs, report deleted directories as adds. However,
> BASE<->repos diffs don't currently report deleted directories at all
> (that's another bug), and there's no way to get the command-line client
> to request a WORKING->repos diff, only a repos->WORKING diff (which in
> this situation, produces identical - but this time, correct - output).
>
I see. It might be possible to use the binding tests for this, but I no
nothing about them. Is the reason you're not fixing the bug that you can't
test it or is it hard to fix in itself?

BTW, I think we need real libsvn_client tests for stuff like this and
stuff like thi diff_summarize APIs that aren't available through the
cmdline client (yet).

Thanks,
//Peter

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