On Wed, Apr 21, 2021 at 9:58 PM Daniel Shahaf <d...@daniel.shahaf.name> wrote:
>
> svn_test_main.c:1073 has a condition of the form:
>
> /* just run all tests */
> ⋮
> if (max_threads == 1 || !parallel)
> {
> ⋮
> }
> #if APR_HAS_THREADS
> else
> {
> ⋮
> }
> #endif
>
> max_threads is a value hard-coded in each individual foo-test.c file.
>
> «parallel» should always be FALSE in unthreaded builds, but if for some
> reason it were TRUE, the foo-test executable would silently exit 0
> without running any tests, so how about this? —
Agreed that the way it's written now leaves open that possibility if
something (else) should go wrong.
(snip inline patch)
> Seems safe enough, but I don't have a threadless APR build handy to test
> this with.
LGTM.
I couldn't apply the patch as copy-pasted from your email so I
recreated the change manually (patch of my manual change attached as
.txt for reference).
I just finished building and running the test suite against trunk @
r1889114 with this patch and threadless (specifically with
/tools/dev/unix-build with 'make THREADING=no') and there were no
failures or any problems I could see.
I verified APR's configure's output contained:
Checking for Threads...
APR will be non-threaded
Should be safe to commit.
Cheers,
Nathan
Index: subversion/tests/svn_test_main.c
===================================================================
--- subversion/tests/svn_test_main.c (revision 1889130)
+++ subversion/tests/svn_test_main.c (working copy)
@@ -1089,9 +1089,9 @@
svn_pool_clear(cleanup_pool);
}
}
-#if APR_HAS_THREADS
else
{
+#if APR_HAS_THREADS
got_error = do_tests_concurrently(opts.prog_name, test_funcs,
array_size, max_threads,
&opts, test_pool);
@@ -1099,8 +1099,11 @@
/* Execute all cleanups */
svn_pool_clear(test_pool);
svn_pool_clear(cleanup_pool);
+#else
+ /* Can't happen */
+ SVN_ERR_MALFUNCTION();
+#endif
}
-#endif
}
/* Clean up APR */
