subversion-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From julianf...@apache.org
Subject svn commit: r1860936 - in /subversion/trunk/subversion: libsvn_ra_svn/client.c svnserve/serve.c tests/libsvn_ra/ra-test.c
Date Mon, 10 Jun 2019 10:52:58 GMT
Author: julianfoad
Date: Mon Jun 10 10:52:58 2019
New Revision: 1860936

URL: http://svn.apache.org/viewvc?rev=1860936&view=rev
Log:
Provide a way for svnserve's 'get-deleted-rev' API to return 'not deleted'.

Previously the answer 'not deleted' was indistinguishable from an error when
using the svnserve protocol.

The standard 'svn' client software does not appear to invoke this case, but
it is meant to be a valid answer and is supported by the other repository
access protocols.

To work around the problem without changing the protocol, we re-purpose the
obsolete error code 'SVN_ERR_ENTRY_MISSING_REVISION' to communicate this
'not deleted' reply to the client.

(Re-purposing an old error code was not essential. An alternative would be
to create a new error code which would become part of the public API in the
next minor release, but would be deemed non-public when backported to older
release lines.)

Behaviour changes:

  - With a new client against a new server, such queries are now handled
    correctly.

  - With an old client against a new server, the client will report a more
    informative error message.

  - With a new client against an old server, there is no improvement.

* subversion/svnserve/serve.c
  (get_deleted_rev): If the answer is SVN_INVALID_REVNUM, return a
    SVN_ERR_ENTRY_MISSING_REVISION error to the client.

* subversion/libsvn_ra_svn/client.c
  (ra_svn_get_deleted_rev): Convert the error SVN_ERR_ENTRY_MISSING_REVISION
    back to a response of SVN_INVALID_REVNUM.

* subversion/tests/libsvn_ra/ra-test.c
  (commit_two_changes): New.
  (test_get_deleted_rev_no_delete,
   test_get_deleted_rev_errors): New tests.
  (test_funcs): Run them.

Modified:
    subversion/trunk/subversion/libsvn_ra_svn/client.c
    subversion/trunk/subversion/svnserve/serve.c
    subversion/trunk/subversion/tests/libsvn_ra/ra-test.c

Modified: subversion/trunk/subversion/libsvn_ra_svn/client.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_ra_svn/client.c?rev=1860936&r1=1860935&r2=1860936&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_ra_svn/client.c (original)
+++ subversion/trunk/subversion/libsvn_ra_svn/client.c Mon Jun 10 10:52:58 2019
@@ -3105,6 +3105,7 @@ ra_svn_get_deleted_rev(svn_ra_session_t
 {
   svn_ra_svn__session_baton_t *sess_baton = session->priv;
   svn_ra_svn_conn_t *conn = sess_baton->conn;
+  svn_error_t *err;
 
   path = reparent_path(session, path, pool);
 
@@ -3116,8 +3117,20 @@ ra_svn_get_deleted_rev(svn_ra_session_t
   SVN_ERR(handle_unsupported_cmd(handle_auth_request(sess_baton, pool),
                                  N_("'get-deleted-rev' not implemented")));
 
-  return svn_error_trace(svn_ra_svn__read_cmd_response(conn, pool, "r",
-                                                       revision_deleted));
+  err = svn_error_trace(svn_ra_svn__read_cmd_response(conn, pool, "r",
+                                                      revision_deleted));
+  /* The protocol does not allow for a reply of SVN_INVALID_REVNUM directly.
+     Instead, a new enough server returns SVN_ERR_ENTRY_MISSING_REVISION to
+     indicate the answer to the query is SVN_INVALID_REVNUM. (An older server
+     closes the connection and returns SVN_ERR_RA_SVN_CONNECTION_CLOSED.) */
+  if (err && err->apr_err == SVN_ERR_ENTRY_MISSING_REVISION)
+    {
+      *revision_deleted = SVN_INVALID_REVNUM;
+      svn_error_clear(err);
+    }
+  else
+    SVN_ERR(err);
+  return SVN_NO_ERROR;
 }
 
 static svn_error_t *

Modified: subversion/trunk/subversion/svnserve/serve.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/svnserve/serve.c?rev=1860936&r1=1860935&r2=1860936&view=diff
==============================================================================
--- subversion/trunk/subversion/svnserve/serve.c (original)
+++ subversion/trunk/subversion/svnserve/serve.c Mon Jun 10 10:52:58 2019
@@ -3516,8 +3516,21 @@ get_deleted_rev(svn_ra_svn_conn_t *conn,
                                svn_relpath_canonicalize(path, pool), pool);
   SVN_ERR(log_command(b, conn, pool, "get-deleted-rev"));
   SVN_ERR(trivial_auth_request(conn, pool, b));
-  SVN_ERR(svn_repos_deleted_rev(b->repository->fs, full_path, peg_revision,
-                                end_revision, &revision_deleted, pool));
+  SVN_CMD_ERR(svn_repos_deleted_rev(b->repository->fs, full_path, peg_revision,
+                                    end_revision, &revision_deleted, pool));
+
+  /* The protocol does not allow for a reply of SVN_INVALID_REVNUM directly.
+     Instead, return SVN_ERR_ENTRY_MISSING_REVISION. A new enough client
+     knows that this means the answer to the query is SVN_INVALID_REVNUM.
+     (An older client reports this as an error.) */
+  if (revision_deleted == SVN_INVALID_REVNUM)
+    SVN_CMD_ERR(svn_error_createf(SVN_ERR_ENTRY_MISSING_REVISION, NULL,
+                                  "svn protocol command 'get-deleted-rev': "
+                                  "path '%s' was not deleted in r%ld-%ld; "
+                                  "NOTE: newer clients handle this case "
+                                  "and do not report it as an error",
+                                  full_path, peg_revision, end_revision));
+
   SVN_ERR(svn_ra_svn__write_cmd_response(conn, pool, "r", revision_deleted));
   return SVN_NO_ERROR;
 }

Modified: subversion/trunk/subversion/tests/libsvn_ra/ra-test.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/libsvn_ra/ra-test.c?rev=1860936&r1=1860935&r2=1860936&view=diff
==============================================================================
--- subversion/trunk/subversion/tests/libsvn_ra/ra-test.c (original)
+++ subversion/trunk/subversion/tests/libsvn_ra/ra-test.c Mon Jun 10 10:52:58 2019
@@ -94,6 +94,41 @@ commit_changes(svn_ra_session_t *session
   return SVN_NO_ERROR;
 }
 
+/* Commit two revisions: add 'B', then delete 'A' */
+static svn_error_t *
+commit_two_changes(svn_ra_session_t *session,
+                   apr_pool_t *pool)
+{
+  apr_hash_t *revprop_table = apr_hash_make(pool);
+  const svn_delta_editor_t *editor;
+  void *edit_baton;
+  void *root_baton, *dir_baton;
+
+  /* mkdir B */
+  SVN_ERR(svn_ra_get_commit_editor3(session, &editor, &edit_baton,
+                                    revprop_table,
+                                    NULL, NULL, NULL, TRUE, pool));
+  SVN_ERR(editor->open_root(edit_baton, SVN_INVALID_REVNUM,
+                            pool, &root_baton));
+  SVN_ERR(editor->add_directory("B", root_baton, NULL, SVN_INVALID_REVNUM,
+                               pool, &dir_baton));
+  SVN_ERR(editor->close_directory(dir_baton, pool));
+  SVN_ERR(editor->close_directory(root_baton, pool));
+  SVN_ERR(editor->close_edit(edit_baton, pool));
+
+  /* delete A */
+  SVN_ERR(svn_ra_get_commit_editor3(session, &editor, &edit_baton,
+                                    revprop_table,
+                                    NULL, NULL, NULL, TRUE, pool));
+  SVN_ERR(editor->open_root(edit_baton, SVN_INVALID_REVNUM,
+                            pool, &root_baton));
+  SVN_ERR(editor->delete_entry("A", SVN_INVALID_REVNUM, root_baton, pool));
+  SVN_ERR(editor->close_directory(root_baton, pool));
+  SVN_ERR(editor->close_edit(edit_baton, pool));
+
+  return SVN_NO_ERROR;
+}
+
 static svn_error_t *
 commit_tree(svn_ra_session_t *session,
             apr_pool_t *pool)
@@ -1784,6 +1819,56 @@ commit_locked_file(const svn_test_opts_t
   return SVN_NO_ERROR;
 }
 
+/* Cases of 'get-deleted-rev' that should return SVN_INVALID_REVNUM. */
+static svn_error_t *
+test_get_deleted_rev_no_delete(const svn_test_opts_t *opts,
+                               apr_pool_t *pool)
+{
+  svn_ra_session_t *ra_session;
+  svn_revnum_t revision_deleted;
+
+  SVN_ERR(make_and_open_repos(&ra_session,
+                              "test-repo-get-deleted-rev-no-delete", opts,
+                              pool));
+  SVN_ERR(commit_changes(ra_session, pool));
+  SVN_ERR(commit_two_changes(ra_session, pool));
+
+  /* expect 'no deletion' in the range up to r2, when it is deleted in r3 */
+  /* This was failing over RA-SVN where the 'get-deleted-rev' wire command's
+     prototype cannot directly represent that result. A new enough client and
+     server collaborate on a work-around implemented using an error code. */
+  SVN_ERR(svn_ra_get_deleted_rev(ra_session, "A", 1, 2,
+                                 &revision_deleted, pool));
+  SVN_TEST_INT_ASSERT(revision_deleted, SVN_INVALID_REVNUM);
+
+  /* this connection should still be open: a simple case should still work */
+  SVN_ERR(svn_ra_get_deleted_rev(ra_session, "A", 1, 3,
+                                 &revision_deleted, pool));
+  SVN_TEST_INT_ASSERT(revision_deleted, 3);
+
+  return SVN_NO_ERROR;
+}
+
+/* Cases of 'get-deleted-rev' that should return an error. */
+static svn_error_t *
+test_get_deleted_rev_errors(const svn_test_opts_t *opts,
+                               apr_pool_t *pool)
+{
+  svn_ra_session_t *ra_session;
+  svn_revnum_t revision_deleted;
+
+  SVN_ERR(make_and_open_repos(&ra_session,
+                              "test-repo-get-deleted-rev-errors", opts, pool));
+  SVN_ERR(commit_changes(ra_session, pool));
+
+  /* expect an error when searching up to r3, when repository head is r1 */
+  SVN_TEST_ASSERT_ERROR(svn_ra_get_deleted_rev(ra_session, "A", 1, 3,
+                                               &revision_deleted, pool),
+                        SVN_ERR_FS_NO_SUCH_REVISION);
+
+  return SVN_NO_ERROR;
+}
+
 
 /* The test table.  */
 
@@ -1820,6 +1905,10 @@ static struct svn_test_descriptor_t test
                        "check how last change applies to empty commit"),
     SVN_TEST_OPTS_PASS(commit_locked_file,
                        "check commit editor for a locked file"),
+    SVN_TEST_OPTS_PASS(test_get_deleted_rev_no_delete,
+                       "test get-deleted-rev no delete"),
+    SVN_TEST_OPTS_PASS(test_get_deleted_rev_errors,
+                       "test get-deleted-rev errors"),
     SVN_TEST_NULL
   };
 



Mime
View raw message