refresh display on filesystem change via entr

[rolandwalker]

The "external script" option discussed in #301 turned out to be easy to sketch out, at least in this crude form.

Reasoning

• entr abstracts away the platform APIs for watching files
• ncurses already has the specially-treated signal SIGWINCH, which it catches and synthesizes into a special keystroke KEY_RESIZE. And SIGWINCH already relates to redrawing the screen.

Implementation

• also refresh view content on KEY_RESIZE, with a throttle in case SIGWINCHs arrive very fast during a GUI interaction
• run entr in the background, and forward a SIGWINCH when a change is seen in the filesystem

To test

• set set refresh-mode = entr in ~/.tigrc
• run tig status
• in a separate shell, run sleep 3; for i in $(seq 1 100); do touch "file_${i}.txt"; sleep 1; done

Bugs

• io_run_bg does not actually run processes asynchronously, since io_complete calls io_done, which waits on the pid. So entr is run under control of sh with a backgrounding &, and SIGHUP is arranged to backgrounded children atexit. This is a good principle anyway.
• ~~But hangup_children … killpg does not interact well with the test suite, since make is actually the leader of the process group in that case. Running make test will leave behind many un-HUPped entr watchers. They will self-clean within several seconds, at a maximum of restart_interval which is an aggressive 30 seconds. It should be possible to call setpgid() to make tig the leader of the process group, but that introduces subtle problems with TTYs and also breaks tests. None of this is an issue during ordinary execution.~~

Edit: poll throughout restart_interval for tighter self-cleaning, making the shell command even more hacktackular.

[rolandwalker]

Here is approximately what would guarantee tig its own process group and ownership of the tty:

FILE *tty = fopen("/dev/tty", "r+");
int tty_fd = tty ? fileno(tty) : STDIN_FILENO;
signal(SIGTTOU, SIG_IGN);
setpgid(getpid(), getpid());
tcsetpgrp(tty_fd, getpid());
signal(SIGTTOU, SIG_DFL);

That would make hangup_children() from this PR work reliably, and also let the test suite or something like the tig-pick script run as expected.

It would be natural to approach setpgid after #725, which separates tty init somewhat from ncurses init.

The advantage of hangup_children() (HUPing your own process group) is avoiding bookkeeping on child processes — shifting that burden to the OS.

[rolandwalker]

@jonas did you ever look at this one?

I assume that you like the idea of a separate script per #301, but am not sure you like the idea of piggybacking on SIGWINCH. (It is a lie, though a logical lie: "repaint this content".)

In any case, though this was marked as proof-of-concept for a long time, I have also been using it without issues during that time.

If you'd like to close this, I'd propose to submit a separate PR just for process-group-leader/hangup_children().

[jonas]

@rolandwalker To be honest I have not looked closely at it. It sounds like a great idea to have it as a separate script since the original code to check for changes is quite convoluted.

[rolandwalker]

Did you just beat me to the rebase?

[jonas]

Yes, I tried to handle the conflict via GitHub.

BTW, I want to look at this PR next. If you have time to separate out and submit a separate PR just for process-group-leader/hangup_children() that would be great.

More generally, how feasible do you think it is to avoid having a watcher script at all and instead launching entr or watchman directly by tig or using user supplied arguments (to allow filtering) and simply have it send signals as you mentioned like SIGWINCH or SIGHUP (leaning more towards the latter).

[rolandwalker]

More than happy to spin out the process-group-leader patch. I use tig every day and owe you no end of thanks.

Maybe watchman can work without a complex script. I don't remember evaluating it. entr was very portable, but its action-model was a mismatch for tig's needs, which I believe is well-documented in the shell wrapper.

Mainly I did this because your suggestion in #301 seemed so easy to sketch out, and then was surprised that it worked reliably.

One way to get rid of the watcher script would be to have tig fork itself, and let the logic worked out in bash be implemented in C in the child fork.

SIGUSR1 is also a good choice for novel IPC.

[jonas]

OK, I will have to look closer at entr and how to change the refresh settings to work with the external script solution.

[jonas]

All builds testing the installation fails. Not sure if it's the check for empty directory that fails.

[rolandwalker]

yes definitely rmdir: failed to remove ‘/tmp/conf-prefix/libexec’: Directory not empty https://travis-ci.org/jonas/tig/jobs/381863880

I will play with it some more.