On 23-04-13 12:01AM, Mark Jamsek wrote:
> On 23-04-12 03:50PM, Stefan Sperling wrote:
> > On Wed, Apr 12, 2023 at 11:28:10PM +1000, Mark Jamsek wrote:
> > > One caveat I want to highlight: testing requires a regress build
> > > which is basically a profile build (i.e., *unpledged*):
> > 
> > Is this just because tog is opening the screendump file late?
> > Or because of some other problem?
> 
> You might be right! Geez, that will be easy enough to change. I thought
> it was something else, but hopefully this does it.
> 
> > Since the screendump file path is constant throughout the lifetime of
> > the program it should be possible to open this file before pledging.
> > The screendump function should probably truncate the open file before
> > writing, such that the test suite would not have to worry about the
> > state of this file when a new test run is started.
> > 
> > I am ok with this diff going in as a starting point in any case.
> 
> Yes, we can definitely open it before calling pledge(). I was about to
> zzz so will send this now but will cook a diff tomorrow to see if that
> fixes the pledge issue. Thanks :)

Unfortunately, my initial suspicions were valid: opening the screendump
file before pledging still doesn't work. I thought you might be onto
something because earlier I was calling init_mock_term() from main
before pledge() and it was aborting--but I wasn't opening the screendump
file then.

I checked-out that revision and moved the screendump file opening to
init_mock_term() before pledge, but no dice.

It seems every curses ioctl() request SIGABRTs. When we init_curses(),
we uncook the terminal with cbreak(), which calls ioctl():

---
(gdb) bt
#0  ioctl () at /tmp/-:3
#1  0xcc7f5c4cb251f6e2 in ?? ()
#2  0x000005490c58d8fd in _libc_tcsetattr (fd=5, opt=<optimized out>, t=0x7f1da3cf6570)
    at /usr/src/lib/libc/termios/tcsetattr.c:49
#3  0x00000549f1d887b6 in _nc_set_tty_mode (buf=0x7f1da3cf6570)
    at /usr/src/lib/libcurses/tinfo/lib_ttyflags.c:88
#4  0x00000549f1d8b91f in cbreak () at /usr/src/lib/libcurses/tinfo/lib_raw.c:136
#5  0x00000546f5419ebf in init_curses () at tog.c:4144
#6  0x00000546f5418356 in cmd_log (argc=0, argv=0x7f1da3cf67b8) at tog.c:4262
#7  0x00000546f54171d1 in main (argc=1, argv=0x7f1da3cf67b0) at tog.c:9797
---

So I moved init_curses() before pledge too, but the halfdelay() call in
trigger_log_thread() also calls ioctl():

---
(gdb) bt
#0  ioctl () at /tmp/-:3
#1  0xab94fb234c36192d in ?? ()
#2  0x00000eff5ce748fd in _libc_tcsetattr (fd=5, opt=<optimized out>, t=0x7476efa1cdf0)
    at /usr/src/lib/libc/termios/tcsetattr.c:49
#3  0x00000eff356f07b6 in _nc_set_tty_mode (buf=0x7476efa1cdf0)
    at /usr/src/lib/libcurses/tinfo/lib_ttyflags.c:88
#4  0x00000eff356f391f in cbreak () at /usr/src/lib/libcurses/tinfo/lib_raw.c:136
#5  0x00000eff356de0bc in halfdelay (t=1) at /usr/src/lib/libcurses/tinfo/lib_options.c:84
#6  0x00000efc7e9cb1d8 in trigger_log_thread (view=0xeff5ab96520, wait=1) at tog.c:2858
#7  0x00000efc7e9c992a in show_log_view (view=0xeff5ab96520) at tog.c:3678
#8  0x00000efc7e9c8a2a in view_loop (view=0xeff5ab96520) at tog.c:1928
#9  0x00000efc7e9c6619 in cmd_log (argc=0, argv=0x7476efa1d168) at tog.c:4306
#10 0x00000efc7e9c5146 in main (argc=1, argv=0x7476efa1d160) at tog.c:9779
---

And we many such calls in tog (e.g., view_input, view_loop,
view_search_start, draw_commits, show_blame_view), so it will require
invasive surgery which may not be viable.

I still want to tweak the script handling a bit and open the screendump
file earlier, instead calling ftruncate() in screendump(), so we can do
multiple screendumps per test. Initially, I didn't want to allow that to
enforce granular tests with the goal of making debugging test failures
easier. But I've changed my mind. We should have the ability to diff
multiple screen dumps per test. Even if we don't want to do it often, it
should be available.


-- 
Mark Jamsek <fnc.bsdbox.org|got.bsdbox.org>
GPG: F2FF 13DE 6A06 C471 CA80  E6E2 2930 DC66 86EE CF68