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.
