On Mon, May 25, 2026 at 12:11 PM Branko Čibej <[email protected]> wrote:
> On 25. 5. 26 10:55, Timofei Zhakov wrote: > > On Sun, May 24, 2026 at 2:52 AM <[email protected]> wrote: > >> Author: brane >> Date: Sun May 24 00:52:17 2026 >> New Revision: 1934546 >> >> Log: >> Add svnbrowse to the autotools build. >> >> Building svnbrowse is optional and can be enabled with --enable-svnbrowse. >> When enabled, --with-ncurses can be used to find the ncurses installation. >> > > [...] > > Thank you so much for doing that! > > I've tested the change and can confirm that it works perfectly on my > machine. > > However, I just realised that there is a crash when the program exits and > executes the pool-cleanups (that is also present in cmake build but for > some reason it wasn't printing the error message properly); > > [[[ > > where > #0 __pthread_kill_implementation > (threadid=<optimized out>, signo=signo@entry=6, no_tid=no_tid@entry=0) > at pthread_kill.c:44 > #1 0x00007ffff749a363 in __pthread_kill_internal (threadid=<optimized > out>, signo=6) > at pthread_kill.c:89 > #2 0x00007ffff743e7d0 in __GI_raise (sig=sig@entry=6) at > ../sysdeps/posix/raise.c:26 > #3 0x00007ffff7425681 in __GI_abort () at abort.c:77 > #4 0x00007ffff7b3d4e0 in err_abort (data=0x555555758c50) at > subversion/libsvn_subr/error.c:156 > #5 0x00007ffff77cc7de in run_cleanups (cref=<optimized out>) at > memory/unix/apr_pools.c:2666 > #6 apr_pool_destroy (pool=0x555555758bd8) at memory/unix/apr_pools.c:991 > #7 0x00007ffff77cc7bd in apr_pool_destroy (pool=0x555555560528) at > memory/unix/apr_pools.c:988 > #8 0x00007ffff77ccaa1 in apr_pool_terminate () at > memory/unix/apr_pools.c:719 > #9 apr_pool_terminate () at memory/unix/apr_pools.c:711 > #10 0x00007ffff7441101 in __run_exit_handlers > (status=0, listp=0x7ffff7613680 <__exit_funcs>, > run_list_atexit=run_list_atexit@entry=true, run_dtors=run_dtors@entry=true) > at exit.c:118 > #11 0x00007ffff74411de in __GI_exit (status=<optimized out>) at exit.c:148 > #12 0x00007ffff7427748 in __libc_start_call_main > (main=main@entry=0x55555555a1f0 <main>, argc=argc@entry=1, > argv=argv@entry=0x7fffffffe488) > at ../sysdeps/nptl/libc_start_call_main.h:83 > #13 0x00007ffff7427879 in __libc_start_main_impl > (main=0x55555555a1f0 <main>, argc=1, argv=0x7fffffffe488, > init=<optimized out>, fini=<optimized out>, rtld_fini=<optimized out>, > stack_end=0x7fffffffe478) at ../csu/libc-start.c:360 > #14 0x00005555555575b5 in _start () > ]]] > > Which is probably caused by this line: > > [[[ > static svn_browse__view_t * > view_make(svn_browse__model_t *model, svn_browse__style_t *style, WINDOW > *win, > apr_pool_t *result_pool) > { > svn_browse__view_t *view = apr_pcalloc(result_pool, sizeof(*view)); > view->model = model; > view->style = style; > view->screen = win; > view_layout(view); > * apr_pool_cleanup_register(result_pool, view, view_cleanup, NULL);* > return view; > } > ]]] > > I think apr_pool_cleanup_register() doesn't like NULL for child_cleanup > and in the rest of the codebase we use apr_pool_cleanup_null instead. > > The following change seem to fix this issue: > > [[[ > Index: subversion/svnbrowse/svnbrowse.c > =================================================================== > --- subversion/svnbrowse/svnbrowse.c (revision 1934577) > +++ subversion/svnbrowse/svnbrowse.c (working copy) > @@ -244,7 +244,8 @@ view_make(svn_browse__model_t *model, svn_browse__ > view->style = style; > view->screen = win; > view_layout(view); > - apr_pool_cleanup_register(result_pool, view, view_cleanup, NULL); > + apr_pool_cleanup_register(result_pool, view, view_cleanup, > + apr_pool_cleanup_null); > return view; > } > ]]] > > > Yes. If you take a look at the list of maintainer mode warnings I > attached, one of them is about this very line. APR declares the cleanup > function arguments with __attribute__((nonnull)) but of course only > GCC-like compilers with the right warning flags will warn about that. I > didn't fix that because I only wanted to make compilation work. > > I think it would be enough to yoink the same list of warning flags into cmake build to make it handle them the same way configure build does. > > Also I think it's a common practice with autoconf to enable all possible > features for which there are required dependencies present on the system. > Take swig, javahl, nls, etc. Is it possible to do the same for svnbrowse, > i.e. enable it whenever we have curses? > > > Actually, neither JavaHL nor the Swig bindings nor the very neglected > svnxx are enabled by default in the autotools build. But that's beside the > point. I made svnbrowse disabled by default since it's a work in progress, > but it's trivial to change that to enabled by default. It will be disabled > anyway if ncurses aren't found. > Oh, I'm pretty sure I was getting javahl to build without any extra arguments, but I might be wrong. The only difference is the make target to invoke to build. Anyway I agree with the decision to keep it disabled by default, assuming we always have an easy option to change that. > > > By the way, an offtopic question: how do you use gdb with > binraries produced by a configure build? The ./subversion/svn/svn files > seem to be shell scripts which cannot be run with the debugger directly. > What is the easiest way to do it? I figured you could 'make install' it but > it's annoying to do after every change and I'm sure there is a more > convenient way. > > > libtool --mode=execute gdb -- ./subversion/svnbrowse/svnbrowse args... > > > Those are scripts created by libtool and libtool knows how to interpret > them, heh. It wasn't obvious to me either, at some point. Since svnbrowse > probably doesn't behave all that well with gdb in the same terminal, you > can also attach the debugger to a running process. > Thanks for explaining! > > -- Brane > > -- Timofei Zhakov
