Don't show fallback prompt if /dev/tty is not available.

[gabyx]

Hooks execution on the server results in:

This small fix, fallsback to stdin if dev/tty is not allocated, which is the case when running the hook on the server.

[gabyx]

Thx for quickly looking into this. Lets see if the tests still work.

[coveralls]

Pull Request Test Coverage Report for Build 530

• 2 of 3 (66.67%) changed or added relevant lines in 1 file are covered.
• No unchanged relevant lines lost coverage.
• Overall coverage decreased (-0.03%) to 87.16%

---

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
base-template.sh	2	3	66.67%
<!--	Total:	2	3	66.67%	-->

Totals
Change from base Build 527:	-0.03%
Covered Lines:	1955
Relevant Lines:	2243

---

💛 - Coveralls

[rycus86]

Thanks for this! Just have one question, otherwise LGTM.

[matthijskooijman]

This actually breaks interactive prompting for me. The prompt is printed, but it does not wait for a reply and continues with accepting the hook implicitly.

When run from inside a pre-push hook on my Ubuntu system, [ -t 0 ] seems to return false here. I suspect that the stdin of the script is not directly connected to the tty, but to a pipe created by git. This is confirmed by putting ls -l /proc/$$/fd/0 in the script, which outputs:

lr-x------ 1 matthijs matthijs 64 apr 16 18:11 /proc/4966/fd/0 -> 'pipe:[8461511]'

(it shows up as a link to /dev/tty in a normal shell).

Also, it seems logic is now somewhat backwards: when FD 0 is a tty, then there is no need to redirect input with </dev/tty at all, read will just read from FD 0, the tty, and work. However, when FD 0 is a pipe, such as with a pre-push hook which takes push infor through stdin, then the </dev/tty redirect is actually needed.

/dev/tty will automatically resolve to "the controlling terminal" of a process, which is apparently the same terminal git runs on, even when the stdin of the process was replaced by a pipe (or file, or whatever). I assume the the "device file" /dev/tty will always exist, but opening it will fail if there is no controlling terminal, which is the original problem this PR was intended to solve.

I looked around a bit to see if there is a way to tell whether you have a controlling terminal, but I could not find anything for that, other than "opening /dev/tty will fail if you have no controllering terminal). So I guess you could maybe try to open /dev/tty (maybe in a subshell) and redirect the error output elsewhere?

Alternatively, you could maybe try checking whether FD1, stdout, is a tty. In the pre-push case where stdin is a terminal, [ -t 1 ] still seems to return success here, so the output is still connected to a terminal. This actually makes some sense: If the output is not a terminal, the question will probably not be seen anyway, so no point in trying to get a tty to read from anyway.

Also, when input is not a terminal, and /dev/tty is not available, it might actually be harmful to try to read from it, since the read might end up reading things sent by get that are intended for the hook. So I guess it might make sense to only ever try to read from /dev/tty, but to prevent the original error, only do that when the output is a terminal.

I also wondered what happens in hooks that do not need input from git, such post-commit but then it seems stdin is connected to /dev/null, so it is indeed never meaningful to read from stdin.

Here's what I tried, which works well for the pre-push and commit hooks:

    # Our stdin is never a tty (either a pipe or /dev/null when called
    # from git), so read from /dev/tty, our controlling terminal.
    # However, only do this when stdout *is* a tty, otherwise it is
    # likely we have no controlling terminal and reading from /dev/tty
    # would fail with an error.
    if [ -t 1 ]; then 
        # shellcheck disable=SC2229
        read -r "$VARIABLE" </dev/tty
    fi

I don't have a quick way to test the "no controlling terminal" case, but maybe @gabyx could test this with his original problematic setup?

[gabyx]

Also, it seems logic is now somewhat backwards: when FD 0 is a tty, then there is no need to redirect input with </dev/tty at all, read will just read from FD 0, the tty, and work. However, when FD 0 is a pipe, such as with a pre-push hook which takes push infor through stdin, then the </dev/tty redirect is actually needed.

This valuable input!

[gabyx]

Your comment disected the problems very good. I try to test that in a PR.