You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Mark Phippard <ma...@gmail.com> on 2010/05/12 11:48:58 UTC
Re: svn commit: r943445 - in /subversion/trunk/subversion: svn/cl.h
svn/log-cmd.c svn/main.c tests/cmdline/getopt_tests_data/svn_help_log_switch_stdout
On Wed, May 12, 2010 at 7:24 AM, Stefan Sperling <st...@elego.de> wrote:
> On Wed, May 12, 2010 at 10:59:28AM -0000, stsp@apache.org wrote:
>> Author: stsp
>> Date: Wed May 12 10:59:27 2010
>> New Revision: 943445
>>
>> URL: http://svn.apache.org/viewvc?rev=943445&view=rev
>> Log:
>> Add a new --show-diff option to svn log, fixing part of issue #2909.
>
>> + SVN_ERR(svn_client_diff5(diff_options,
>> + lb->target_url,
>> + &start_revision,
>> + lb->target_url,
>> + &end_revision,
>> + NULL,
>> + svn_depth_infinity,
>> + FALSE, /* ignore ancestry */
>> + TRUE, /* no diff deleted */
>> + FALSE, /* show copies as adds */
>> + FALSE, /* ignore content type */
>
> BTW, I deliberately hard-coded the above options in the initial
> implementation. We could decide to leave it like this, given
> that svn log is not the primary tool to produce variants of diffs.
> I set these options with the goal of getting the shortest possible
> diff shown, with the exception of ignore_ancestry.
I don't want to pee in the punch bowl, but why are we doing this?
Especially if we cannot implement it sufficiently?
In terms of the implementation, how are you handling the log output in
XML when the user asks for diffs?
--
Thanks
Mark Phippard
http://markphip.blogspot.com/
Re: svn commit: r943445 - in /subversion/trunk/subversion:
svn/cl.h svn/log-cmd.c svn/main.c
tests/cmdline/getopt_tests_data/svn_help_log_switch_stdout
Posted by Stefan Sperling <st...@elego.de>.
On Wed, May 12, 2010 at 08:13:55AM -0400, Mark Phippard wrote:
> It would also be easy to script and that has always been our position
> on adding options like this. Especially if we cannot implement the
> option internally more efficiently than the script.
Oh, it's not quite the same thing. What I wrote in the log message
has apparently mislead you. Sorry. Let me try to explain better:
To get the same output (log message, diff, log message, diff, etc.)
a script would be even slower because it would have to open an entirely
new connection to the server to get the log of every single
revision. It would work like this:
for rev in [1-1000]:
svn log -r $rev
svn diff -r ($rev - 1):$rev
While the command line client can do this:
client: server, give me all log messages from r1 to r1000 over a single
connection
* server starts spooling log information to the client.
while server is spooling:
* client opens another connection (#2)
client: I just got the log for rN. What would svn diff -r(N-1):N do?
* server starts diff-editor drive
client: thanks!
* client prints diff
* client closes connection #2
So in this example the script needs about 1000 connections more
than the client does.
The script could achieve a similar level of performance by using
three threads:
T1: svn log -r 1-1000
T2: for rev in [1-1000]:
svn diff -r ($rev - 1):$rev
and a third thread that joins the output the other two appropriately.
But I don't think anyone would bother to write such a script.
In fact, adding the feature to the CLI client was fairly easy,
probably easier than a wrapper script that does the above.
Stefan
Re: svn commit: r943445 - in /subversion/trunk/subversion: svn/cl.h
svn/log-cmd.c svn/main.c tests/cmdline/getopt_tests_data/svn_help_log_switch_stdout
Posted by Mark Phippard <ma...@gmail.com>.
On Wed, May 12, 2010 at 8:00 AM, Stefan Sperling <st...@elego.de> wrote:
>> I don't want to pee in the punch bowl, but why are we doing this?
>
> Because it has proven to be a popular feature in other systems.
> It's very convenient if you want to catch up with the latest N
> commits made and skim the diffs. Especially if there aren't any
> commit list archives (many, many shops do not have those).
> And I have missed it in svn sometimes, too.
> Also, Daniel N. and Bert have said they will use it.
It would also be easy to script and that has always been our position
on adding options like this. Especially if we cannot implement the
option internally more efficiently than the script.
>> Especially if we cannot implement it sufficiently?
>
> What do mean by sufficiently? We can certainly implement it
> sufficiently. You ask svn log to show diffs and it shows them.
> What's not sufficient about that?
Sorry, I meant efficiently as you guessed.
--
Thanks
Mark Phippard
http://markphip.blogspot.com/
Re: svn commit: r943445 - in /subversion/trunk/subversion:
svn/cl.h svn/log-cmd.c svn/main.c
tests/cmdline/getopt_tests_data/svn_help_log_switch_stdout
Posted by Stefan Sperling <st...@elego.de>.
On Wed, May 12, 2010 at 07:48:58AM -0400, Mark Phippard wrote:
> On Wed, May 12, 2010 at 7:24 AM, Stefan Sperling <st...@elego.de> wrote:
> > On Wed, May 12, 2010 at 10:59:28AM -0000, stsp@apache.org wrote:
> >> Author: stsp
> >> Date: Wed May 12 10:59:27 2010
> >> New Revision: 943445
> >>
> >> URL: http://svn.apache.org/viewvc?rev=943445&view=rev
> >> Log:
> >> Add a new --show-diff option to svn log, fixing part of issue #2909.
> >
> >> + SVN_ERR(svn_client_diff5(diff_options,
> >> + lb->target_url,
> >> + &start_revision,
> >> + lb->target_url,
> >> + &end_revision,
> >> + NULL,
> >> + svn_depth_infinity,
> >> + FALSE, /* ignore ancestry */
> >> + TRUE, /* no diff deleted */
> >> + FALSE, /* show copies as adds */
> >> + FALSE, /* ignore content type */
> >
> > BTW, I deliberately hard-coded the above options in the initial
> > implementation. We could decide to leave it like this, given
> > that svn log is not the primary tool to produce variants of diffs.
> > I set these options with the goal of getting the shortest possible
> > diff shown, with the exception of ignore_ancestry.
>
> I don't want to pee in the punch bowl, but why are we doing this?
Because it has proven to be a popular feature in other systems.
It's very convenient if you want to catch up with the latest N
commits made and skim the diffs. Especially if there aren't any
commit list archives (many, many shops do not have those).
And I have missed it in svn sometimes, too.
Also, Daniel N. and Bert have said they will use it.
> Especially if we cannot implement it sufficiently?
What do mean by sufficiently? We can certainly implement it
sufficiently. You ask svn log to show diffs and it shows them.
What's not sufficient about that?
If you mean efficient, Bert pointed out that it probably won't be
possible at all to implement it without opening a second RA session.
I guess we could have the server send a diff to the previous revision
along with every log item. But that would require a lot more changes.
How fast it is depends on the connection to the server, as with so
many things in svn. I suggest you try it out and see for yourself.
svn log --diff -l 10 http://svn.apache.org/repos/asf/subversion
And it's completely optional. The extra overhead isn't there if you
don't use the option.
> In terms of the implementation, how are you handling the log output in
> XML when the user asks for diffs?
We error out with an informative error message:
$ svn log --diff --xml
svn: Try 'svn help' for more info
svn: 'show-diff' option is not supported in XML mode
$
Stefan
Re: svn commit: r943445 - in /subversion/trunk/subversion:
svn/cl.h svn/log-cmd.c svn/main.c
tests/cmdline/getopt_tests_data/svn_help_log_switch_stdout
Posted by Stefan Sperling <st...@elego.de>.
On Wed, May 12, 2010 at 07:48:58AM -0400, Mark Phippard wrote:
> On Wed, May 12, 2010 at 7:24 AM, Stefan Sperling <st...@elego.de> wrote:
> > On Wed, May 12, 2010 at 10:59:28AM -0000, stsp@apache.org wrote:
> >> Author: stsp
> >> Date: Wed May 12 10:59:27 2010
> >> New Revision: 943445
> >>
> >> URL: http://svn.apache.org/viewvc?rev=943445&view=rev
> >> Log:
> >> Add a new --show-diff option to svn log, fixing part of issue #2909.
> >
> >> + SVN_ERR(svn_client_diff5(diff_options,
> >> + lb->target_url,
> >> + &start_revision,
> >> + lb->target_url,
> >> + &end_revision,
> >> + NULL,
> >> + svn_depth_infinity,
> >> + FALSE, /* ignore ancestry */
> >> + TRUE, /* no diff deleted */
> >> + FALSE, /* show copies as adds */
> >> + FALSE, /* ignore content type */
> >
> > BTW, I deliberately hard-coded the above options in the initial
> > implementation. We could decide to leave it like this, given
> > that svn log is not the primary tool to produce variants of diffs.
> > I set these options with the goal of getting the shortest possible
> > diff shown, with the exception of ignore_ancestry.
>
> I don't want to pee in the punch bowl, but why are we doing this?
Because it has proven to be a popular feature in other systems.
It's very convenient if you want to catch up with the latest N
commits made and skim the diffs. Especially if there aren't any
commit list archives (many, many shops do not have those).
And I have missed it in svn sometimes, too.
Also, Daniel N. and Bert have said they will use it.
> Especially if we cannot implement it sufficiently?
What do mean by sufficiently? We can certainly implement it
sufficiently. You ask svn log to show diffs and it shows them.
What's not sufficient about that?
If you mean efficient, Bert pointed out that it probably won't be
possible at all to implement it without opening a second RA session.
I guess we could have the server send a diff to the previous revision
along with every log item. But that would require a lot more changes.
How fast it is depends on the connection to the server, as with so
many things in svn. I suggest you try it out and see for yourself.
svn log --diff -l 10 http://svn.apache.org/repos/asf/subversion
And it's completely optional. The extra overhead isn't there if you
don't use the option.
> In terms of the implementation, how are you handling the log output in
> XML when the user asks for diffs?
We error out with an informative error message:
$ svn log --diff --xml
svn: Try 'svn help' for more info
svn: 'show-diff' option is not supported in XML mode
$
Stefan