On Fri, Aug 13, 2010 at 06:54:19PM -0700, Alexey Neyman wrote:
> [Posting here as 'svn log --show-diff' is currently only available in trunk]
>
> Hi All,
>
> I am trying out 1.7.0 (yes, I know it's a bit early) and noticed that svn log
> now supports --show-diff. It's very convenient, thanks.
>
> As the comments in the code explain, in case the path was created in some
> revision, the function will attempt to go one level up until it finds a parent
> that existed in both 'rev' and 'rev-1' revisions. This may result in LOTS of
> extra files - sometimes, the whole revision diff may be dumped. Consider what
> happens if the file being diff'ed originated as a part of 20Mb 'svn import'.
>
> Judging from the commit message, this option should mimic 'git log -p'
> behavior, but git does not dump the whole diff by default when -p is
> specified. It only diffs the requested path and produces a full diff only if
> --full-diff option is also passed.
There is no need for a --full-diff option.
If you have a commit with two paths, A and B, and you run svn log --diff A,
you will only see a diff for A. So you can limit the scope of the diff shown
by limiting the log operation to just the paths you are interested in.
This works fine unless in the situation you describe, unfortunately.
As soon as we step up into the parent, the diff will include everything
beneath the parent.
> From http://www.kernel.org/pub/software/scm/git/docs/git-log.html
>
> [[[
> --full-diff
> Without this flag, "git log -p <path>…" shows commits that touch the
> specified paths, and diffs about the same specified paths. With this, the full
> diff is shown for commits that touch the specified paths; this means that
> "<path>…" limits only commits, and doesn't limit diff for those commits.
> ]]]
>
> Now, I understand this approach was chosen due to svn_client_diff5() returning
> an error if the path does not exist in the specified revision. So, first
> question is why svn_client_diff5() does not consider missing file as if it
> were an empty file? It does that thing when existing parent directory is
> diff'ed...
I don't really know off-hand why it doesn't do this.
Quite possibly this is simply the result of organic growth of the code
base over the years, or a wrong design decision a long time ago, or a case
of "we'll deal with this later...". And nobody has so far been bothered
enough to fix it.
There are quite a few oddities in the diff implementation, e.g. this one
which was fixed recently (but also by using a band-aid approach):
http://subversion.tigris.org/issues/show_bug.cgi?id=2333
The plan to "rewrite diff" has actually been an in-joke between some of the
developers for a while, similar to "rewriting the working copy" (which some
developers assumed would never happen, but is happening now).
It may be possible to show the diff anyway treating one of the sides as
empty as you suggest, and thus showing a diff with only adds/deletes.
I agree that this would make sense.
But making such a change may require substantial work within the Subversion
libraries, so it takes more time than the quick band-aid fix which I applied
to log-cmd.c to make it produce at least some sort of meaningful output.
Sometimes circumstances force people to take short cuts...
See also this issue: http://subversion.tigris.org/issues/show_bug.cgi?id=2873
If that issue was fixed we could remove the log-cmd.c workaround, I think.
> And, if svn_client_diff5() has valid reasons to produce an error in such case,
> why doesn't 'svn log --diff' just stop at that point, perhaps with some
> meaningful message that the specified file was created in that revision?
It should show a diff which shows things as added. We need the output to
be suitable for use with svn patch. A special message that only humans
will understand isn't useful. Showing a diff with too broad a scope is
much better than showing no diff at all.
Stefan
Received on 2010-08-14 13:32:31 CEST