]> arthur.barton.de Git - bup.git/commit
main: always put BUP_FORCE_TTY into the environment
authorJohannes Berg <johannes@sipsolutions.net>
Tue, 30 Nov 2021 20:26:22 +0000 (21:26 +0100)
committerRob Browning <rlb@defaultvalue.org>
Sun, 5 Dec 2021 20:26:53 +0000 (14:26 -0600)
commitba8ed092989c05c30e2b190bb8a7e94853148757
tree2fb487a1ba8a5dc38121aa597aec1c6dcb14cb53
parent842437dd823c955160da5ace911e0a9559d031e8
main: always put BUP_FORCE_TTY into the environment

Nix reports that progress output from midx/bloom while save
is running is no longer printed.

The reason turns out to be somewhat complicated:
 * midx/bloom progress output is printed directly from the C
   code, if stderr is known to be a tty from isatty(2) or the
   environment variable BUP_FORCE_TTY
 * save is a built-in command now, and so are midx/bloom (but
   the latter is less relevant)
 * a process inherits the environment that the parent has,
   unless otherwise specified

So with the _old_ setup (with save not a module but process),
when save is started, main would set both BUP_FORCE_TTY (and
BUP_TTY_WIDTH) in the environment for the subprocess, causing
save to obtain it, and further midx/bloom to inherit it from
save. One of the downsides of this setup is that the size of
the window is now fixed, so resizing doesn't update.

With the _new_ setup, where save is a module, we don't set
BUP_FORCE_TTY or BUP_TTY_WIDTH in the environment since the
redirection and fixing etc. all happens in the main, and the
code is directly accessing stdout/stderr via the filtering.

The problem now is the following:
 1) We create the filter thread, so that stdout/stderr are
    no longer pointing to the real tty fd, so that isatty()
    returns false. This is fine for save, or when we start
    bloom/midx directly, as the _helpers.c istty2 has been
    evaluated already before we redirect the fds.
 2) As described, we don't set the environment variables
    when we run the save code, since it's a module.

However, as a result, when save starts midx/bloom as a new
subprocess, they inherit the redirected fds, so that they're
not writing to the tty themselves, but also don't get the
environment variable BUP_FORCE_TTY since they're started by
save and that was a module.

The solution then is fairly simple: set both BUP_FORCE_TTY
and BUP_TTY_WIDTH in the environment unconditionally. The
latter is necessary so that options._tty_width() works in
the module-based commands as well.

This didn't just affect save -> midx/bloom, but also the ssh
calls that pass BUP_FORCE_TTY/BUP_TTY_WIDTH to the remote
process, so potentially the server's debug output was also
affected. Nix reported that the client debug output was not
shown ("Receiving index from server: %d/%d\r"), but I don't
have an explanation for that related to this commit, as that
code is actually running in the process of the save, thus
should be shown depending on helpers.istty2, which was
always correct in the main process.

Also, I'll note that we now always put BUP_FORCE_TTY into
the environment, which we avoided previously for some calls.
However, prior to the change to have most things be modules,
we were also doing that, since everything we called (git and
other tools like par2) would be started by a subprocess, so
this doesn't really seem to pose any danger.

Reported-by: Nix <nix@esperi.org.uk>
Fixes: 5dd0172dddb9 ("bup: filter stdout/stderr via thread/pipe for internal subcommands")
Reviewed-by: Rob Browning <rlb@defaultvalue.org>
[rlb@defaultvalue.org: remove now unused merge_dict from imports]
Signed-off-by: Rob Browning <rlb@defaultvalue.org>
Tested-by: Rob Browning <rlb@defaultvalue.org>
lib/bup/main.py