Skip to content

Commit 1f39519

Browse files
committed
gh-152132: Address review feedback — rename helpers, add tests
- Rename _PyRun_SimpleFileObjectEx → _PyRun_SimpleFileObjectNoPrint to avoid the deprecated 'Ex' suffix. Make _PyRun_SimpleFileObject a thin wrapper around it to avoid code duplication. - Rename _PyRun_SimpleStringFlagsEx → _PyRun_SimpleStringFlagsNoPrint and make _PyRun_SimpleStringFlagsWithName a thin wrapper similarly. - Rename single-letter variable 'v' and 'res' to 'result' throughout. - Add test cases for sys.exit() handling via -c, file, -m and message.
1 parent e88db2f commit 1f39519

5 files changed

Lines changed: 101 additions & 139 deletions

File tree

Include/internal/pycore_pylifecycle.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -114,7 +114,7 @@ int _PyRun_SimpleStringFlagsWithName(const char *command, const char* name, PyCo
114114
// Like _PyRun_SimpleStringFlagsWithName but returns the result object
115115
// instead of calling PyErr_Print() on failure. The caller should handle
116116
// the error with _Py_HandleSystemExitAndKeyboardInterrupt or PyErr_Print.
117-
PyObject *_PyRun_SimpleStringFlagsEx(const char *command, const char* name, PyCompilerFlags *flags);
117+
PyObject *_PyRun_SimpleStringFlagsNoPrint(const char *command, const char* name, PyCompilerFlags *flags);
118118

119119

120120
/* interpreter config */

Include/internal/pycore_pythonrun.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ extern int _PyRun_AnyFileObject(
2020
int closeit,
2121
PyCompilerFlags *flags);
2222

23-
extern PyObject * _PyRun_SimpleFileObjectEx(
23+
extern PyObject * _PyRun_SimpleFileObjectNoPrint(
2424
FILE *fp,
2525
PyObject *filename,
2626
int closeit,

Lib/test/test_runpy.py

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -916,6 +916,45 @@ def test_pymain_run_module(self):
916916
ham = self.ham
917917
self.assertSigInt(["-m", ham.stem], cwd=ham.parent)
918918

919+
# Tests for sys.exit() handling (gh-152132)
920+
@requires_subprocess()
921+
def test_sys_exit_run_command(self):
922+
cmd = [sys.executable, '-c', 'import sys; sys.exit(42)']
923+
proc = subprocess.run(cmd, text=True, stderr=subprocess.PIPE)
924+
self.assertEqual(proc.returncode, 42)
925+
926+
@requires_subprocess()
927+
def test_sys_exit_run_command_default(self):
928+
cmd = [sys.executable, '-c', 'import sys; sys.exit()']
929+
proc = subprocess.run(cmd, text=True, stderr=subprocess.PIPE)
930+
self.assertEqual(proc.returncode, 0)
931+
932+
@requires_subprocess()
933+
def test_sys_exit_run_command_message(self):
934+
cmd = [sys.executable, '-c', "import sys; sys.exit('error message')"]
935+
proc = subprocess.run(cmd, text=True, stdout=subprocess.PIPE,
936+
stderr=subprocess.PIPE)
937+
self.assertEqual(proc.returncode, 1)
938+
self.assertIn('error message', proc.stderr)
939+
940+
@requires_subprocess()
941+
def test_sys_exit_run_file(self):
942+
with self.tmp_path() as tmp:
943+
script = tmp / "exit_script.py"
944+
script.write_text("import sys; sys.exit(42)")
945+
cmd = [sys.executable, str(script)]
946+
proc = subprocess.run(cmd, text=True, stderr=subprocess.PIPE)
947+
self.assertEqual(proc.returncode, 42)
948+
949+
@requires_subprocess()
950+
def test_sys_exit_run_module(self):
951+
with self.tmp_path() as tmp:
952+
script = tmp / "exit_mod.py"
953+
script.write_text("import sys; sys.exit(42)")
954+
cmd = [sys.executable, '-m', 'exit_mod']
955+
proc = subprocess.run(cmd, cwd=tmp, text=True, stderr=subprocess.PIPE)
956+
self.assertEqual(proc.returncode, 42)
957+
919958

920959
if __name__ == "__main__":
921960
unittest.main()

Modules/main.c

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@
1010
#include "pycore_pathconfig.h" // _PyPathConfig_ComputeSysPath0()
1111
#include "pycore_pylifecycle.h" // _Py_PreInitializeFromPyArgv()
1212
#include "pycore_pystate.h" // _PyInterpreterState_GET()
13-
#include "pycore_pythonrun.h" // _PyRun_SimpleFileObjectEx()
13+
#include "pycore_pythonrun.h" // _PyRun_SimpleFileObjectNoPrint()
1414
#include "pycore_tuple.h" // _PyTuple_FromPair
1515
#include "pycore_unicodeobject.h" // _PyUnicode_Dedent()
1616

@@ -258,12 +258,12 @@ pymain_run_command(wchar_t *command)
258258

259259
PyCompilerFlags cf = _PyCompilerFlags_INIT;
260260
cf.cf_flags |= PyCF_IGNORE_COOKIE;
261-
PyObject *res = _PyRun_SimpleStringFlagsEx(PyBytes_AsString(bytes), "<string>", &cf);
261+
PyObject *result = _PyRun_SimpleStringFlagsNoPrint(PyBytes_AsString(bytes), "<string>", &cf);
262262
Py_DECREF(bytes);
263-
if (res == NULL) {
263+
if (result == NULL) {
264264
return pymain_exit_err_print();
265265
}
266-
Py_DECREF(res);
266+
Py_DECREF(result);
267267
return 0;
268268

269269
error:
@@ -409,14 +409,14 @@ pymain_run_file_obj(PyObject *program_name, PyObject *filename,
409409
return pymain_exit_err_print();
410410
}
411411

412-
/* Use _PyRun_SimpleFileObjectEx which returns PyObject* without calling
412+
/* Use _PyRun_SimpleFileObjectNoPrint which returns PyObject* without calling
413413
PyErr_Print(), so we can handle SystemExit properly via pymain_exit_err_print. */
414414
PyCompilerFlags cf = _PyCompilerFlags_INIT;
415-
PyObject *v = _PyRun_SimpleFileObjectEx(fp, filename, 1, &cf);
416-
if (v == NULL) {
415+
PyObject *result = _PyRun_SimpleFileObjectNoPrint(fp, filename, 1, &cf);
416+
if (result == NULL) {
417417
return pymain_exit_err_print();
418418
}
419-
Py_DECREF(v);
419+
Py_DECREF(result);
420420
return 0;
421421
}
422422

Python/pythonrun.c

Lines changed: 52 additions & 129 deletions
Original file line numberDiff line numberDiff line change
@@ -458,95 +458,15 @@ set_main_loader(PyObject *d, PyObject *filename, const char *loader_name)
458458
}
459459

460460

461-
int
462-
_PyRun_SimpleFileObject(FILE *fp, PyObject *filename, int closeit,
463-
PyCompilerFlags *flags)
464-
{
465-
int ret = -1;
466-
467-
PyObject *main_module = PyImport_AddModuleRef("__main__");
468-
if (main_module == NULL)
469-
return -1;
470-
PyObject *dict = PyModule_GetDict(main_module); // borrowed ref
471-
472-
int set_file_name = 0;
473-
int has_file = PyDict_ContainsString(dict, "__file__");
474-
if (has_file < 0) {
475-
goto done;
476-
}
477-
if (!has_file) {
478-
if (PyDict_SetItemString(dict, "__file__", filename) < 0) {
479-
goto done;
480-
}
481-
set_file_name = 1;
482-
}
483-
484-
int pyc = maybe_pyc_file(fp, filename, closeit);
485-
if (pyc < 0) {
486-
goto done;
487-
}
488-
489-
PyObject *v;
490-
if (pyc) {
491-
FILE *pyc_fp;
492-
/* Try to run a pyc file. First, re-open in binary */
493-
if (closeit) {
494-
fclose(fp);
495-
}
496-
497-
pyc_fp = Py_fopen(filename, "rb");
498-
if (pyc_fp == NULL) {
499-
fprintf(stderr, "python: Can't reopen .pyc file\n");
500-
goto done;
501-
}
502-
503-
if (set_main_loader(dict, filename, "SourcelessFileLoader") < 0) {
504-
fprintf(stderr, "python: failed to set __main__.__loader__\n");
505-
ret = -1;
506-
fclose(pyc_fp);
507-
goto done;
508-
}
509-
v = run_pyc_file(pyc_fp, dict, dict, flags);
510-
} else {
511-
/* When running from stdin, leave __main__.__loader__ alone */
512-
if ((!PyUnicode_Check(filename) || !PyUnicode_EqualToUTF8(filename, "<stdin>")) &&
513-
set_main_loader(dict, filename, "SourceFileLoader") < 0) {
514-
fprintf(stderr, "python: failed to set __main__.__loader__\n");
515-
ret = -1;
516-
goto done;
517-
}
518-
v = pyrun_file(fp, filename, Py_file_input, dict, dict,
519-
closeit, flags);
520-
}
521-
flush_io();
522-
if (v == NULL) {
523-
Py_CLEAR(main_module);
524-
PyErr_Print();
525-
goto done;
526-
}
527-
Py_DECREF(v);
528-
ret = 0;
529-
530-
done:
531-
if (set_file_name) {
532-
if (PyDict_PopString(dict, "__file__", NULL) < 0) {
533-
PyErr_Print();
534-
}
535-
}
536-
Py_XDECREF(main_module);
537-
return ret;
538-
}
539-
540-
541-
/* Variant of _PyRun_SimpleFileObject that returns the result object
542-
instead of calling PyErr_Print() on failure. The caller (typically
543-
pymain_run_file_obj in Modules/main.c) should handle the error
544-
with _Py_HandleSystemExitAndKeyboardInterrupt or pymain_exit_err_print. */
461+
/* Run a simple file object. Returns the result PyObject* on success,
462+
or NULL on failure. Does NOT call PyErr_Print(); the caller must
463+
handle the error (e.g. with PyErr_Print() or
464+
_Py_HandleSystemExitAndKeyboardInterrupt()). */
545465
PyObject *
546-
_PyRun_SimpleFileObjectEx(FILE *fp, PyObject *filename, int closeit,
547-
PyCompilerFlags *flags)
466+
_PyRun_SimpleFileObjectNoPrint(FILE *fp, PyObject *filename, int closeit,
467+
PyCompilerFlags *flags)
548468
{
549-
PyObject *v = NULL;
469+
PyObject *result = NULL;
550470

551471
PyObject *main_module = PyImport_AddModuleRef("__main__");
552472
if (main_module == NULL)
@@ -589,22 +509,22 @@ _PyRun_SimpleFileObjectEx(FILE *fp, PyObject *filename, int closeit,
589509
fclose(pyc_fp);
590510
goto done;
591511
}
592-
v = run_pyc_file(pyc_fp, dict, dict, flags);
512+
result = run_pyc_file(pyc_fp, dict, dict, flags);
593513
} else {
594514
/* When running from stdin, leave __main__.__loader__ alone */
595515
if ((!PyUnicode_Check(filename) || !PyUnicode_EqualToUTF8(filename, "<stdin>")) &&
596516
set_main_loader(dict, filename, "SourceFileLoader") < 0) {
597517
fprintf(stderr, "python: failed to set __main__.__loader__\n");
598518
goto done;
599519
}
600-
v = pyrun_file(fp, filename, Py_file_input, dict, dict,
601-
closeit, flags);
520+
result = pyrun_file(fp, filename, Py_file_input, dict, dict,
521+
closeit, flags);
602522
}
603523
flush_io();
604524

605525
done:
606526
if (set_file_name) {
607-
if (v == NULL) {
527+
if (result == NULL) {
608528
// Main code failed: save the exception before cleanup
609529
// so PyDict_PopString doesn't overwrite it
610530
PyObject *saved_exc = PyErr_GetRaisedException();
@@ -622,7 +542,22 @@ _PyRun_SimpleFileObjectEx(FILE *fp, PyObject *filename, int closeit,
622542
}
623543
}
624544
Py_XDECREF(main_module);
625-
return v;
545+
return result;
546+
}
547+
548+
549+
int
550+
_PyRun_SimpleFileObject(FILE *fp, PyObject *filename, int closeit,
551+
PyCompilerFlags *flags)
552+
{
553+
PyObject *result = _PyRun_SimpleFileObjectNoPrint(fp, filename, closeit,
554+
flags);
555+
if (result == NULL) {
556+
PyErr_Print();
557+
return -1;
558+
}
559+
Py_DECREF(result);
560+
return 0;
626561
}
627562

628563

@@ -640,61 +575,49 @@ PyRun_SimpleFileExFlags(FILE *fp, const char *filename, int closeit,
640575
}
641576

642577

643-
int
644-
_PyRun_SimpleStringFlagsWithName(const char *command, const char* name, PyCompilerFlags *flags) {
645-
PyObject *main_module = PyImport_AddModuleRef("__main__");
646-
if (main_module == NULL) {
647-
return -1;
648-
}
649-
PyObject *dict = PyModule_GetDict(main_module); // borrowed ref
650-
651-
PyObject *res = NULL;
652-
if (name == NULL) {
653-
res = PyRun_StringFlags(command, Py_file_input, dict, dict, flags);
654-
} else {
655-
PyObject* the_name = PyUnicode_FromString(name);
656-
if (!the_name) {
657-
PyErr_Print();
658-
Py_DECREF(main_module);
659-
return -1;
660-
}
661-
res = _PyRun_StringFlagsWithName(command, the_name, Py_file_input, dict, dict, flags, 0);
662-
Py_DECREF(the_name);
663-
}
664-
Py_DECREF(main_module);
665-
if (res == NULL) {
666-
PyErr_Print();
667-
return -1;
668-
}
669-
670-
Py_DECREF(res);
671-
return 0;
672-
}
673-
578+
/* Run a simple string. Returns the result PyObject* on success,
579+
or NULL on failure. Does NOT call PyErr_Print(); the caller must
580+
handle the error (e.g. with PyErr_Print() or
581+
_Py_HandleSystemExitAndKeyboardInterrupt()). */
674582
PyObject *
675-
_PyRun_SimpleStringFlagsEx(const char *command, const char* name, PyCompilerFlags *flags)
583+
_PyRun_SimpleStringFlagsNoPrint(const char *command, const char* name,
584+
PyCompilerFlags *flags)
676585
{
677586
PyObject *main_module = PyImport_AddModuleRef("__main__");
678587
if (main_module == NULL) {
679588
return NULL;
680589
}
681590
PyObject *dict = PyModule_GetDict(main_module); // borrowed ref
682591

683-
PyObject *res = NULL;
592+
PyObject *result = NULL;
684593
if (name == NULL) {
685-
res = PyRun_StringFlags(command, Py_file_input, dict, dict, flags);
594+
result = PyRun_StringFlags(command, Py_file_input, dict, dict, flags);
686595
} else {
687596
PyObject* the_name = PyUnicode_FromString(name);
688597
if (!the_name) {
689598
Py_DECREF(main_module);
690599
return NULL;
691600
}
692-
res = _PyRun_StringFlagsWithName(command, the_name, Py_file_input,
693-
dict, dict, flags, 0);
601+
result = _PyRun_StringFlagsWithName(command, the_name, Py_file_input,
602+
dict, dict, flags, 0);
694603
Py_DECREF(the_name);
695604
}
696605
Py_DECREF(main_module);
697-
return res;
606+
return result;
607+
}
608+
609+
610+
int
611+
_PyRun_SimpleStringFlagsWithName(const char *command, const char* name,
612+
PyCompilerFlags *flags)
613+
{
614+
PyObject *result = _PyRun_SimpleStringFlagsNoPrint(command, name, flags);
615+
if (result == NULL) {
616+
PyErr_Print();
617+
return -1;
618+
}
619+
Py_DECREF(result);
620+
return 0;
698621
}
699622

700623

0 commit comments

Comments
 (0)