Re: [PATCH][Issue 2287] svn_client_log should take a peg revision

On Wed, 25 Jan 2006, Daniel Rall wrote:

> On Tue, 24 Jan 2006, Malcolm Rowe wrote:
>
> > On Thu, Jan 19, 2006 at 01:35:22PM -0800, Daniel Rall wrote:
> > > I've committed a patch adding support for peg revisions to 'svn log'
> > > (and of course the corresponding underlying APIs), based on work by
> > > myself and Ramaswamy (and all the reviews that work received):
> > >
> > > http://svn.collab.net/viewcvs/svn?rev=18179&view=rev
> > >
> > > This feature should first be available in Subversion 1.4.0.
> >
> > Excellent. What should the behaviour of 'svn log foo@PEG' be, if no
> > revision range is provided? Should it still be -rHEAD:1, as it is now
> > (I think), or should it be -rPEG:1, which might be more in-line with
> > the other commands that operate on peg-revs.
> >
> > See http://svn.haxx.se/dev/archive-2006-01/0352.shtml for an equivalent
> > patch I proposed for 'svn blame', though note here that we have no
> > backward-compatibility concerns.
>
> As Peter Samuelson, I prefer PEG to HEAD on the grounds of both
> consistency and usability. I'll come up with a XFail regression test
> for 'svn log' to get us started.

Here's a patch adding that function to the command-line client (I
assume we want it there, rather than in the library itself?), and some
related tests. If there are no comments, I'll commit it RSN.

[[[
Make 'svn log target@N' consistent with the peg revision operations of
other commands (e.g. 'svn cat target@N'), where the specified peg
revision becomes the virtual HEAD. See
http://svn.haxx.se/dev/archive-2006-01/0352.shtml for related
discussion, or http://svn.collab.net/viewcvs/svn?rev=18179&view=rev
for a related patch (to 'svn blame'). Because peg revision handling
for 'svn log' is new in Subversion 1.4, there are no compatibility
concerns here.

* subversion/tests/cmdline/log_tests.py
  (log_wc_with_peg_revision): New function testing log of a WC target
   with a peg revision.

  (url_missing_in_head): Remove redundant -rN from command (now
   implied by @N peg revision on URI), and improve validation by
   parsing the log output.

  (test_list): Add log_wc_with_peg_revision() to the list.

* subversion/svn/log-cmd.c
  (svn_cl__log): When OPT_STATE->START_REVISION is not specified and
   START_REVISION PEG_REVISION is specified, set
   OPT_STATE->START_REVISION to PEG_REVISION.

Suggested by: malcolm
              Peter Samuelson
]]]

Index: subversion/tests/cmdline/log_tests.py
===================================================================
--- subversion/tests/cmdline/log_tests.py (revision 18242)
+++ subversion/tests/cmdline/log_tests.py (working copy)
@@ -528,15 +528,25 @@
     os.chdir(was_cwd)
 
 #----------------------------------------------------------------------
+def log_wc_with_peg_revision(sbox):
+ "'svn log wc_target@N'"
+ guarantee_repos_and_wc(sbox)
+ my_path = os.path.join(sbox.wc_dir, "A", "B", "E", "beta") + "@8"
+ output, err = svntest.actions.run_and_verify_svn(None, None, [],
+ 'log', my_path)
+ check_log_chain(parse_log_output(output), [1])
+
+#----------------------------------------------------------------------
 def url_missing_in_head(sbox):
- "'svn log -r N URL' when URL is not in HEAD "
+ "'svn log target@N' when target removed from HEAD"
 
   guarantee_repos_and_wc(sbox)
 
   my_url = svntest.main.current_repo_url + "/A/B/E/alpha" + "@8"
   
- svntest.actions.run_and_verify_svn(None, None, [],
- 'log', '-r', '8', my_url)
+ output, err = svntest.actions.run_and_verify_svn(None, None, [],
+ 'log', my_url)
+ check_log_chain(parse_log_output(output), [3, 1])
 
 #----------------------------------------------------------------------
 def log_through_copyfrom_history(sbox):
@@ -781,6 +791,7 @@
               log_to_revision_zero,
               dynamic_revision,
               log_with_path_args,
+ log_wc_with_peg_revision,
               url_missing_in_head,
               log_through_copyfrom_history,
               escape_control_chars,
Index: subversion/svn/log-cmd.c
===================================================================
--- subversion/svn/log-cmd.c (revision 18242)
+++ subversion/svn/log-cmd.c (working copy)
@@ -420,12 +420,18 @@
     }
   else if (opt_state->start_revision.kind == svn_opt_revision_unspecified)
     {
- /* If the first target is a URL, then we default to HEAD:0.
- Otherwise, the default is BASE:0 since WC@HEAD may not exist. */
- if (svn_path_is_url (target))
- opt_state->start_revision.kind = svn_opt_revision_head;
+ /* Default to any specified peg revision. Otherwise, if the
+ first target is an URL, then we default to HEAD:0. Lastly,
+ the default is BASE:0 since WC@HEAD may not exist. */
+ if (peg_revision.kind == svn_opt_revision_unspecified)
+ {
+ if (svn_path_is_url (target))
+ opt_state->start_revision.kind = svn_opt_revision_head;
+ else
+ opt_state->start_revision.kind = svn_opt_revision_base;
+ }
       else
- opt_state->start_revision.kind = svn_opt_revision_base;
+ opt_state->start_revision = peg_revision;
 
       if (opt_state->end_revision.kind == svn_opt_revision_unspecified)
         {