On Fri, Sep 4, 2026 at 5:58 PM <[email protected]> wrote:
Author: rinrab
Date: Fri Sep 4 15:57:28 2026
New Revision: 1937873
Log:
Add tests on the compat RA API (svn_ra_plugin_t) and fix a
few crashes in
open_session that were making it completely unusable.
* subversion/libsvn_ra/wrapper_template.h
(compat_open): Add scratch_pool and check that session_url
is not NULL.
* subversion/tests/libsvn_ra/ra-test.c
(#includes): Rearrange includes to shut the deprecation
warnings up.
(test_compat_vtable): New test.
(test_funcs): Run the test.
Modified:
subversion/trunk/subversion/libsvn_ra/wrapper_template.h
subversion/trunk/subversion/tests/libsvn_ra/ra-test.c
Modified:
subversion/trunk/subversion/libsvn_ra/wrapper_template.h
==============================================================================
--- subversion/trunk/subversion/libsvn_ra/wrapper_template.h
Fri Sep 4 15:54:51 2026 (r1937872)
+++ subversion/trunk/subversion/libsvn_ra/wrapper_template.h
Fri Sep 4 15:57:28 2026 (r1937873)
@@ -71,7 +71,12 @@ static svn_error_t *compat_open(void **s
* the alternative (creating a new ra_util library) would
be massive
* overkill for the time being. Just be sure to keep the
following
* line and the code of svn_ra_create_callbacks in sync. */
+
+ /* Some RA modules (libsvn_ra_serf) want this exact pool
configuration and
+ * refuse to work with the same pool as both scratch_pool
and result_pool. */
apr_pool_t *sesspool = svn_pool_create(pool);
+ apr_pool_t *scratch_pool = svn_pool_create(sesspool);
+
svn_ra_callbacks2_t *callbacks2 = apr_pcalloc(sesspool,
sizeof(*callbacks2));
@@ -93,9 +98,11 @@ static svn_error_t *compat_open(void **s
SVN_ERR(VTBL.open_session(sess, &session_url, NULL, repos_URL,
callbacks2, callback_baton,
callbacks ?
callbacks->auth_baton : NULL,
- config, sesspool, sesspool));
+ config, sesspool, scratch_pool));
+
+ svn_pool_destroy(scratch_pool);
- if (strcmp(repos_URL, session_url) != 0)
+ if (session_url && strcmp(repos_URL, session_url) != 0)
{
svn_pool_destroy(sesspool);
return
svn_error_createf(SVN_ERR_RA_SESSION_URL_MISMATCH, NULL,
Modified: subversion/trunk/subversion/tests/libsvn_ra/ra-test.c
==============================================================================
--- subversion/trunk/subversion/tests/libsvn_ra/ra-test.c
Fri Sep 4 15:54:51 2026 (r1937872)
+++ subversion/trunk/subversion/tests/libsvn_ra/ra-test.c
Fri Sep 4 15:57:28 2026 (r1937873)
@@ -23,6 +23,9 @@
+#include "../svn_test.h"
+#include "../svn_test_fs.h"
+
#include <apr_general.h>
#include <apr_pools.h>
#include <apr_file_io.h>
@@ -37,8 +40,6 @@
#include "svn_dirent_uri.h"
#include "svn_hash.h"
-#include "../svn_test.h"
-#include "../svn_test_fs.h"
#include "../../libsvn_ra_local/ra_local.h"
/*-------------------------------------------------------------------*/
@@ -1914,6 +1915,49 @@ test_get_deleted_rev_errors(const svn_te
return SVN_NO_ERROR;
}
+static svn_error_t *
+test_compat_vtable(const svn_test_opts_t *opts,
+ apr_pool_t *pool)
+{
+ svn_ra_plugin_t *lib;
+ void *ra_baton, *session_baton, *edit_baton, *root_baton,
*dir_baton;
+ const svn_delta_editor_t *editor;
+ const char *url;
+ svn_ra_callbacks_t cbtable = { 0 };
+
+ SVN_ERR(svn_test__create_repos2(NULL, &url, NULL,
"test_compat_vtable", opts,
+ pool, pool));
+ SVN_ERR(svn_ra_initialize(pool));
+
+ SVN_ERR(svn_ra_init_ra_libs(&ra_baton, pool));
+ SVN_ERR(svn_ra_get_ra_library(&lib, ra_baton, url, pool));
+
+ SVN_ERR(svn_test__init_auth_baton(&cbtable.auth_baton, pool));
+ SVN_ERR(lib->open(&session_baton, url, &cbtable, NULL,
NULL, pool));
+
+ /* mkdir A */
+ SVN_ERR(lib->get_commit_editor(session_baton, &editor,
&edit_baton,
+ "r1", NULL, NULL, pool));
+ SVN_ERR(editor->open_root(edit_baton, SVN_INVALID_REVNUM,
+ pool, &root_baton));
+ SVN_ERR(editor->add_directory("A", 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(lib->get_commit_editor(session_baton, &editor,
&edit_baton,
+ "r2", NULL, NULL, 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;
+}
+
/* The test table. */
@@ -1954,6 +1998,8 @@ static struct svn_test_descriptor_t test
"test get-deleted-rev no delete"),
SVN_TEST_OPTS_PASS(test_get_deleted_rev_errors,
"test get-deleted-rev errors"),
+ SVN_TEST_OPTS_PASS(test_compat_vtable,
+ "test compat svn_ra_plugin_t"),
SVN_TEST_NULL
};
I discovered that the API we provide for backward compatibility
hasn't been working for quite some time (I believe r1873487/1.14
was the revision it was first broken, maybe even earlier). I
don't think anyone should still be using this code. So does
anyone still test it.
The answer is probably we don't. Plus it seems wasteful to
remember to update this wrapper any time something changes. I
think, since it's no longer a compatibility promise, why spend
time maintaining this code at all. This API has been deprecated
since 1.1 and nobody is using it.
I propose we remove it and make svn_ra_get_ra_library & friends
return an error so that we atleast keep it technically binary
compatible. Alternatively, we could remove the symbols alongside
the svn_ra_plugin_t entirely, but it's probably more towards 2.0.
