Commit4191f7d

committed

Refactor kill_after_timeout logic so mypy can check it

This changes how Git.execute defines the kill_process callback andhow it performs checks, fixing two mypy errors on Windows about howthe signal module doesn't have SIGKILL. In doing so, it alsoeliminates the need for the assertion added for safety and clarityin2f017ac (#1761), since now kill_process is only defined if it isto be used (which is also guarded by a platform check, needed bymypy).As indc95a76 before this, part of the change here is to replacesome os.named-based checks with sys.platform-based checks, which issafe because, when one is specifically checking only for thedistinction between native Windows and all other systems, one canuse either approach. (Seedc95a76 for more details on that.)

1 parentdc95a76 commit4191f7dCopy full SHA for 4191f7d

File tree

1 file changed

+35

-32

lines changed

git
- cmd.py

1 file changed

+35

-32

lines changed

`‎git/cmd.py`

Lines changed: 35 additions & 32 deletions

Original file line number	Diff line number	Diff line change
`@@ -1134,7 +1134,7 @@ def execute(`
`1134`	`1134`	`ifinline_envisnotNone:`
`1135`	`1135`	`env.update(inline_env)`
`1136`	`1136`
`1137`		`-ifos.name=="nt":`
	`1137`	`+ifsys.platform=="win32":`
`1138`	`1138`	`cmd_not_found_exception=OSError`
`1139`	`1139`	`ifkill_after_timeoutisnotNone:`
`1140`	`1140`	`raiseGitCommandError(`
`@@ -1179,35 +1179,38 @@ def execute(`
`1179`	`1179`	`ifas_process:`
`1180`	`1180`	`returnself.AutoInterrupt(proc,command)`
`1181`	`1181`
`1182`		`-defkill_process(pid:int)->None:`
`1183`		`-"""Callback to kill a process."""`
`1184`		`-ifos.name=="nt":`
`1185`		`-raiseAssertionError("Bug: This callback would be ineffective and unsafe on Windows, stopping.")`
`1186`		`-p=Popen(["ps","--ppid",str(pid)],stdout=PIPE)`
`1187`		`-child_pids= []`
`1188`		`-ifp.stdoutisnotNone:`
`1189`		`-forlineinp.stdout:`
`1190`		`-iflen(line.split())>0:`
`1191`		`-local_pid= (line.split())[0]`
`1192`		`-iflocal_pid.isdigit():`
`1193`		`-child_pids.append(int(local_pid))`
`1194`		`-try:`
`1195`		`-os.kill(pid,signal.SIGKILL)`
`1196`		`-forchild_pidinchild_pids:`
`1197`		`-try:`
`1198`		`-os.kill(child_pid,signal.SIGKILL)`
`1199`		`-exceptOSError:`
`1200`		`-pass`
`1201`		`-kill_check.set()# Tell the main routine that the process was killed.`
`1202`		`-exceptOSError:`
`1203`		`-# It is possible that the process gets completed in the duration after`
`1204`		`-# timeout happens and before we try to kill the process.`
`1205`		`-pass`
`1206`		`-return`
`1207`		`-`
`1208`		`-# END kill_process`
`1209`		`-`
`1210`		`-ifkill_after_timeoutisnotNone:`
	`1182`	`+ifsys.platform!="win32"andkill_after_timeoutisnotNone:`
	`1183`	`+`
	`1184`	`+defkill_process(pid:int)->None:`
	`1185`	`+"""Callback to kill a process.`
	`1186`	`+`
	`1187`	`+ This callback implementation would be ineffective and unsafe on Windows.`
	`1188`	`+ """`
	`1189`	`+p=Popen(["ps","--ppid",str(pid)],stdout=PIPE)`
	`1190`	`+child_pids= []`
	`1191`	`+ifp.stdoutisnotNone:`
	`1192`	`+forlineinp.stdout:`
	`1193`	`+iflen(line.split())>0:`
	`1194`	`+local_pid= (line.split())[0]`
	`1195`	`+iflocal_pid.isdigit():`
	`1196`	`+child_pids.append(int(local_pid))`
	`1197`	`+try:`
	`1198`	`+os.kill(pid,signal.SIGKILL)`
	`1199`	`+forchild_pidinchild_pids:`
	`1200`	`+try:`
	`1201`	`+os.kill(child_pid,signal.SIGKILL)`
	`1202`	`+exceptOSError:`
	`1203`	`+pass`
	`1204`	`+# Tell the main routine that the process was killed.`
	`1205`	`+kill_check.set()`
	`1206`	`+exceptOSError:`
	`1207`	`+# It is possible that the process gets completed in the duration`
	`1208`	`+# after timeout happens and before we try to kill the process.`
	`1209`	`+pass`
	`1210`	`+return`
	`1211`	`+`
	`1212`	`+# END kill_process`
	`1213`	`+`
`1211`	`1214`	`kill_check=threading.Event()`
`1212`	`1215`	`watchdog=threading.Timer(kill_after_timeout,kill_process,args=(proc.pid,))`
`1213`	`1216`
`@@ -1218,10 +1221,10 @@ def kill_process(pid: int) -> None:`
`1218`	`1221`	`newline="\n"ifuniversal_newlineselseb"\n"`
`1219`	`1222`	`try:`
`1220`	`1223`	`ifoutput_streamisNone:`
`1221`		`-ifkill_after_timeoutisnotNone:`
	`1224`	`+ifsys.platform!="win32"andkill_after_timeoutisnotNone:`
`1222`	`1225`	`watchdog.start()`
`1223`	`1226`	`stdout_value,stderr_value=proc.communicate()`
`1224`		`-ifkill_after_timeoutisnotNone:`
	`1227`	`+ifsys.platform!="win32"andkill_after_timeoutisnotNone:`
`1225`	`1228`	`watchdog.cancel()`
`1226`	`1229`	`ifkill_check.is_set():`
`1227`	`1230`	`stderr_value='Timeout: the command "%s" did not complete in %d '"secs."% (`

0 commit comments

Comments

(0)

Movatterモバイル変換

Navigation Menu

Search code, repositories, users, issues, pull requests...

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

Commit4191f7d

File tree

1 file changed

1 file changed

`‎git/cmd.py`

0 commit comments