Skip to content

Commit cb732b4

Browse files
sdarwincursoragent
andcommitted
Hand the pre-push hook's terminal to edg-check-policy [GH #164]
git supplies the refs being pushed on a pre-push hook's standard input, so the checker cannot inherit that stream and still read the user's answers to the interactive prompts. Reopening sys.stdin inside the checker rebound only the Python level object and left file descriptor 0 pointing at git's pipe, and it also required a general purpose tool to know how git invokes hooks. Open the terminal in the hook instead and pass it as the child's standard input, which fixes the descriptor for the whole process tree and keeps that knowledge in the one place that needs it. Pushing with no terminal attached previously raised an unhandled OSError from open('/dev/tty'). The hook now falls back to an empty standard input, and an unanswerable prompt leaves the finding standing, so such a push reports its findings instead of failing with a traceback. Co-authored-by: Cursor <cursoragent@cursor.com>
1 parent ce68be2 commit cb732b4

2 files changed

Lines changed: 38 additions & 22 deletions

File tree

‎.git-hooks/pre-push.py‎

Lines changed: 27 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,25 @@
6666

6767
TOOL_PATH_CHECK_POLICY = shutil.which('edg-check-policy')
6868

69+
def terminal_input():
70+
'''Open the terminal to hand to the checker as its standard input.
71+
72+
git supplies the refs being pushed on this hook's stdin, so the checker
73+
cannot simply inherit it and still read the user's answers. On Windows
74+
the relaunch above has already arranged a standalone console, whose stdin
75+
is usable as it is, so that is inherited instead.
76+
'''
77+
if platform.system() == 'Windows':
78+
return None
79+
80+
try:
81+
return open('/dev/tty')
82+
except OSError:
83+
# Pushing with no terminal attached, so there is nobody to prompt. The
84+
# checker reports its findings without asking rather than reading the
85+
# refs above as if they were answers.
86+
return subprocess.DEVNULL
87+
6988
def main():
7089
if TOOL_PATH_CHECK_POLICY is None:
7190
print(
@@ -75,11 +94,14 @@ def main():
7594
)
7695
sys.exit(1)
7796

78-
completed_process = subprocess.run([
79-
TOOL_PATH_CHECK_POLICY,
80-
'--unpushed',
81-
'--interactive'
82-
])
97+
completed_process = subprocess.run(
98+
[
99+
TOOL_PATH_CHECK_POLICY,
100+
'--unpushed',
101+
'--interactive'
102+
],
103+
stdin = terminal_input()
104+
)
83105

84106
sys.exit(completed_process.returncode)
85107

‎dev_tools/bin/edg-check-policy‎

Lines changed: 11 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@ interactive review.
1717
import argparse
1818
import json
1919
import os
20-
import platform
2120
import re
2221
import shutil
2322
import subprocess
@@ -274,15 +273,24 @@ class PolicyReporter:
274273
'''Ask the user whether a policy warning is acceptable.
275274
276275
When stdout is a terminal, prompt on stdin for y/n/q. Otherwise record
277-
an error without prompting (e.g. non-interactive pushes).
276+
an error without prompting (e.g. non-interactive pushes). Supplying a
277+
stdin that can be read is the caller's job: git hands a hook the refs
278+
being pushed on stdin, so the pre-push hook opens the terminal itself
279+
and passes that to this tool instead.
278280
'''
279281
if not sys.stdout.isatty():
280282
self.errors += 1
281283
return
282284

283285
tries = 0
284286
while True:
285-
response = input(' Okay? [ynq] ').lower()
287+
try:
288+
response = input(' Okay? [ynq] ').lower()
289+
except EOFError:
290+
# There is nothing to answer with, so let the finding stand.
291+
print()
292+
self.errors += 1
293+
return
286294

287295
if response == 'y':
288296
return
@@ -719,23 +727,9 @@ def parse_args(argv: Optional[List[str]] = None) -> argparse.Namespace:
719727
)
720728
return parser.parse_args(argv)
721729

722-
def _reopen_terminal_input() -> None:
723-
'''Point stdin at the terminal so findings can be reviewed.
724-
725-
A hook is handed the list of refs being pushed on stdin, so interactive
726-
review has to read the user's answers from the terminal instead. On
727-
Windows the wrapper has already arranged for a console, whose stdin is
728-
usable as it is.
729-
'''
730-
if platform.system() != 'Windows':
731-
sys.stdin = open('/dev/tty')
732-
733730
def main() -> None:
734731
args = parse_args()
735732

736-
if args.interactive:
737-
_reopen_terminal_input()
738-
739733
repo_dir = edgutil.find_mono_repo_or_exit()
740734
repository = GitRepository(repo_dir)
741735
reporter = PolicyReporter(

0 commit comments

Comments
 (0)