Re: [PATCH] Issue #692: Handle "svn log" with empty repos

* Michael W Thelen <thelenm@cs.utah.edu> [2004-07-03 18:52]:
> I think I'll also produce a patch for the changes I was proposing
> before, just so you have the clearest possible idea (i.e. source code)
> of what I'm suggesting.

As promised, here's an alternate patch to demonstrate the change I was
talking about. I think this patch is cleaner than the other one, since
it nicely eliminates the need for special case handling in the client
library. I'm not positive that libsvn_repos is the right place for the
change, but it seems like it.

This patch does change the behavior of the repos library when a range of
revisions is requested that includes zero (e.g. -r5:0 or -r0:5).
Specifically, no log message for revision 0 is produced unless -r0 or
-r0:0 is requested. I don't know if this is an acceptable change of
behavior or not. But I think the change is for the better, and I
couldn't find the current behavior clearly documented anywhere. I
really doubt that anyone depends on the current behavior, but of course
I don't know that for sure.

This patch includes the same new test that was included in the other
patch. All tests pass, and I'm planning on writing some more, but I'll
hold off on that until it's decided which route to go with these two
patches.

----------------------------------------------------------------------------
Log:
Change the behavior of svn_repos_get_logs so that no log message is
produced for revision 0 unless the start and end revisions requested are
both 0. Also change the cmdline client behavior so that "svn log" with
no arguments defaults to HEAD:0 or BASE:0 instead of HEAD:1 or BASE:1.
This eliminates the need for special case code in libsvn_client that
checks for errors occurring due to the default being HEAD:1.

* subversion/include/svn_client.h
  (svn_client_log): Remove documentation for the HEAD:1 special case,
    since it's no longer needed.

* subversion/libsvn_client/log.c
  (svn_client_log): Remove the code that specifically checks for an
    error after requesting HEAD:1. This special-case code is no longer
    needed since HEAD:0 is now the default requested by the client code.

* subversion/libsvn_repos/log.c
  (svn_repos_get_logs): Don't produce a log message for revision 0 if it
    is the endpoint of a range including non-zero revisions. For
    example, if -r5:0 is requested, only produce log messages for r5
    through r1. A log message for revision 0 is only produced if both
    the start and end revisions requested are 0.

* subversion/clients/cmdline/log-cmd.c
  (svn_cl__log): Request 0 as the end revision when the end revision is
    unspecified. This means that "svn log" with no arguments will be
    interpreted as HEAD:0 or BASE:0 instead of HEAD:1 or BASE:1. This
    eliminates the errors that can occur when HEAD or BASE are 0.

* subversion/tests/clients/cmdline/log_tests.py
  (log_with_wc_at_revision_zero): New test case for "svn log" on a
    working copy at BASE revision 0 with a non-empty repository.
  (parse_log_output): Add support for recognizing a revision 0 log
    message, since it is radically different from normal log messages.
  (check_log_chain): Only test log message text for revisions greater
    than 0. Also, fail if there are extra log messages in the list
    after checking all expected messages.

Index: subversion/include/svn_client.h
===================================================================
--- subversion/include/svn_client.h (revision 10131)
+++ subversion/include/svn_client.h (working copy)
@@ -739,20 +739,6 @@
  *
  * Use @a pool for any temporary allocation.
  *
- * Special case for repositories at revision 0:
- *
- * If @a start->kind is @c svn_opt_revision_head, and @a end->kind is
- * @c svn_opt_revision_number && @a end->number is @c 1, then handle an
- * empty (no revisions) repository specially: instead of erroring
- * because requested revision 1 when the highest revision is 0, just
- * invoke @a receiver on revision 0, passing @c NULL for changed paths and
- * empty strings for the author and date. This is because that
- * particular combination of @a start and @a end usually indicates the
- * common case of log invocation -- the user wants to see all log
- * messages from youngest to oldest, where the oldest commit is
- * revision 1. That works fine, except when there are no commits in
- * the repository, hence this special case.
- *
  * If @a ctx->notify_func is non-null, then call @a ctx->notify_func/baton
  * with a 'skip' signal on any unversioned targets.
  *
Index: subversion/libsvn_client/log.c
===================================================================
--- subversion/libsvn_client/log.c (revision 10131)
+++ subversion/libsvn_client/log.c (working copy)
@@ -273,39 +273,6 @@
                                receiver_baton,
                                pool);
       }
-
- /* Special case: If there have been no commits, we'll get an error
- * for requesting log of a revision higher than 0. But the
- * default behavior of "svn log" is to give revisions HEAD through
- * 1, on the assumption that HEAD >= 1.
- *
- * So if we got that error for that reason, and it looks like the
- * user was just depending on the defaults (rather than explicitly
- * requesting the log for revision 1), then we don't error. Instead
- * we just invoke the receiver manually on a hand-constructed log
- * message for revision 0.
- *
- * See also http://subversion.tigris.org/issues/show_bug.cgi?id=692.
- */
- if (err && (err->apr_err == SVN_ERR_FS_NO_SUCH_REVISION)
- && (start->kind == svn_opt_revision_head)
- && ((end->kind == svn_opt_revision_number)
- && (end->value.number == 1)))
- {
- svn_revnum_t youngest_rev;
-
- SVN_ERR (ra_lib->get_latest_revnum (session, &youngest_rev, pool));
- if (youngest_rev == 0)
- {
- err = SVN_NO_ERROR;
-
- /* Log receivers are free to handle revision 0 specially... But
- just in case some don't, we make up a message here. */
- SVN_ERR (receiver (receiver_baton,
- NULL, 0, "", "", _("No commits in repository."),
- pool));
- }
- }
   }
   
   return err;
Index: subversion/clients/cmdline/log-cmd.c
===================================================================
--- subversion/clients/cmdline/log-cmd.c (revision 10131)
+++ subversion/clients/cmdline/log-cmd.c (working copy)
@@ -515,7 +515,7 @@
       if (opt_state->end_revision.kind == svn_opt_revision_unspecified)
         {
           opt_state->end_revision.kind = svn_opt_revision_number;
- opt_state->end_revision.value.number = 1; /* oldest commit */
+ opt_state->end_revision.value.number = 0; /* oldest commit */
         }
     }
 
Index: subversion/tests/clients/cmdline/log_tests.py
===================================================================
--- subversion/tests/clients/cmdline/log_tests.py (revision 10131)
+++ subversion/tests/clients/cmdline/log_tests.py (working copy)
@@ -255,6 +255,22 @@
       if this_item:
         this_item['msg'] = msg
         chain.append (this_item)
+ this_item = None
+ elif this_line == "No commit for revision 0.\n":
+ # Add previous log message
+ if this_item:
+ this_item['msg'] = msg
+ chain.append (this_item)
+ this_item = {}
+ this_item['revision'] = '0'
+ this_item['author'] = '(no author)'
+ this_item['date'] = '1970-01-01 00:00:00 -0000 (Thu, 01 Jan 1970)'
+ this_item['msg'] = this_line
+ chain.append (this_item)
+ this_item = None
+ # Remove the message separator line - it comes after the r0 log message,
+ # not before it.
+ log_lines.pop (0)
     else: # if didn't see separator now, then something's wrong
       raise SVNLogParseError, "trailing garbage after log message"
 
@@ -297,14 +313,17 @@
              or author == '(no author)')): raise svntest.Failure
     # Check that the log message looks right:
     msg_re = re.compile ('Log message for revision ' + `saw_rev`)
- if (not msg_re.search (msg)): raise svntest.Failure
+ if (saw_rev > 0 and not msg_re.search (msg)): raise svntest.Failure
 
   ### todo: need some multi-line log messages mixed in with the
   ### one-liners. Easy enough, just make the prime revisions use REV
   ### lines, and the rest use 1 line, or something, so it's
   ### predictable based on REV.
 
+ # If any log messages are still on the chain, that's a failure
+ if chain: raise svntest.Failure
 
+
 ######################################################################
 # Tests
 #
@@ -515,7 +534,21 @@
                                       'log', '-r', '2', mu2_path)
   svntest.actions.run_and_verify_svn (None, [], SVNAnyOutput,
                                       'log', '-r', '2', mu2_URL)
-
+
+#----------------------------------------------------------------------
+def log_with_wc_at_revision_zero(sbox):
+ "'svn log', no args, top of wc at BASE revision 0"
+
+ guarantee_repos_and_wc(sbox)
+
+ svntest.main.run_svn (None, 'up', '-r', '0', sbox.wc_dir)
+ output, err = svntest.actions.run_and_verify_svn(
+ None, None, [], 'log', sbox.wc_dir)
+
+ log_chain = parse_log_output (output)
+ if check_log_chain (log_chain, [0]):
+ raise svntest.Failure
+
 ########################################################################
 # Run the tests
 
@@ -530,6 +563,7 @@
               log_with_path_args,
               url_missing_in_head,
               log_through_copyfrom_history,
+ log_with_wc_at_revision_zero,
              ]
 
 if __name__ == '__main__':
Index: subversion/libsvn_repos/log.c
===================================================================
--- subversion/libsvn_repos/log.c (revision 10131)
+++ subversion/libsvn_repos/log.c (working copy)
@@ -166,6 +166,13 @@
       (SVN_ERR_FS_NO_SUCH_REVISION, 0,
        _("No such revision %ld"), end);
 
+ /* Don't produce a log message for revision 0 if it's one endpoint
+ * of a range including non-zero revisions */
+ if ((start == 0) && (end > 0))
+ start = 1;
+ else if ((start > 0) && (end == 0))
+ end = 1;
+
   /* If paths were specified, then we only really care about revisions
      in which those paths were changed. So we ask the filesystem for
      all the revisions in which any of the paths was changed.

-- Mike

-- 
Michael W. Thelen
It is a painful thing
To look at your own trouble and know
That you yourself and no one else has made it
                -- Sophocles, "Ajax"