diff --git a/Include/internal/pycore_getopt.h b/Include/internal/pycore_getopt.h index 7f0dd13ae577f78..e767cb1049d0796 100644 --- a/Include/internal/pycore_getopt.h +++ b/Include/internal/pycore_getopt.h @@ -5,18 +5,21 @@ # error "this header requires Py_BUILD_CORE define" #endif -extern int _PyOS_opterr; -extern Py_ssize_t _PyOS_optind; -extern const wchar_t *_PyOS_optarg; +struct _PyOS_GetOpt { + int error; // generate error messages + Py_ssize_t index; // index into argv array + const wchar_t *arg; // optional argument + const wchar_t *ptr; + Py_ssize_t argc; + wchar_t * const *argv; +}; -extern void _PyOS_ResetGetOpt(void); +extern void _PyOS_GetOpt_Init( + struct _PyOS_GetOpt *getopt, + Py_ssize_t argc, + wchar_t * const *argv); -typedef struct { - const wchar_t *name; - int has_arg; - int val; -} _PyOS_LongOption; - -extern int _PyOS_GetOpt(Py_ssize_t argc, wchar_t * const *argv, int *longindex); +extern int _PyOS_GetOpt( + struct _PyOS_GetOpt *getopt); #endif /* !Py_INTERNAL_PYGETOPT_H */ diff --git a/Lib/test/test_cmd_line.py b/Lib/test/test_cmd_line.py index 476a481a0ba54b3..6d07238a9d08abf 100644 --- a/Lib/test/test_cmd_line.py +++ b/Lib/test/test_cmd_line.py @@ -14,6 +14,7 @@ from test import support from test.support import os_helper from test.support import force_not_colorized +from test.support import import_helper from test.support import threading_helper from test.support.script_helper import ( spawn_python, kill_python, assert_python_ok, assert_python_failure, @@ -60,14 +61,17 @@ def verify_valid_flag(self, cmd_line): @support.cpython_only @support.force_not_colorized def test_help(self): - self.verify_valid_flag('-h') - self.verify_valid_flag('-?') - out = self.verify_valid_flag('--help') - lines = out.splitlines() - self.assertIn(b'usage', lines[0]) - self.assertNotIn(b'PYTHONHOME', out) - self.assertNotIn(b'-X dev', out) - self.assertLess(len(lines), 50) + options = ['-h', '-?', '--help'] + if support.MS_WINDOWS: + options.append('/?') + for opt in options: + with self.subTest(opt=opt): + out = self.verify_valid_flag(opt) + lines = out.splitlines() + self.assertIn(b'usage', lines[0]) + self.assertNotIn(b'PYTHONHOME', out) + self.assertNotIn(b'-X dev', out) + self.assertLess(len(lines), 50) @support.cpython_only @support.force_not_colorized @@ -680,16 +684,22 @@ def test_del___main__(self): @support.cpython_only def test_unknown_options(self): - rc, out, err = assert_python_failure('-E', '-z') - self.assertIn(b'Unknown option: -z', err) - self.assertEqual(err.splitlines().count(b'Unknown option: -z'), 1) - self.assertEqual(b'', out) + # Test unknown option -a + for option in ('-z', '--long-option', '---'): + with self.subTest(option=option): + rc, out, err = assert_python_failure('-E', option) + errmsg = f'Unknown option: {option}'.encode() + self.assertIn(errmsg, err) + self.assertEqual(err.splitlines().count(errmsg), 1) + self.assertEqual(b'', out) + # Add "without='-E'" to prevent _assert_python to append -E # to env_vars and change the output of stderr rc, out, err = assert_python_failure('-z', without='-E') self.assertIn(b'Unknown option: -z', err) self.assertEqual(err.splitlines().count(b'Unknown option: -z'), 1) self.assertEqual(b'', out) + rc, out, err = assert_python_failure('-a', '-z', without='-E') self.assertIn(b'Unknown option: -a', err) # only the first unknown option is reported @@ -1403,6 +1413,97 @@ def test_dump_path_config(self): self.assertIn(b'Python path configuration:', proc.err) self.assertIn(f"PYTHONHOME = '{nonexistent}'".encode(), proc.err) + def test_short_options(self): + # Skip the test if _testcapi cannot be imported: + # the test uses _testcapi.config_get(). + import_helper.import_module('_testcapi') + + # Test short command line options + def check(options, config_names, expected): + if isinstance(config_names, str): + config_names = (config_names,) + if isinstance(options, str): + options = (options,) + expr = ', '.join(f'config_get({name!a})' for name in config_names) + code = f'from _testcapi import config_get; print({expr})' + args = options + ("-c", code) + proc = assert_python_ok(*args) + self.assertEqual(proc.out.rstrip(), expected.encode()) + + def check_ignored(option): + # Just test that passing the option doesn't fail + assert_python_ok(option, "-c", "pass") + + check('-b', 'bytes_warning', '1') + check('-bb', 'bytes_warning', '2') + check('-B', 'write_bytecode', 'False') + check('-d', 'parser_debug', 'True') + check('-E', 'use_environment', 'False') + check('-i', ('inspect', 'interactive'), 'True True') + check('-I', 'isolated', 'True') + check('-O', 'optimization_level', '1') + check('-OO', 'optimization_level', '2') + check('-P', 'safe_path', 'True') + check('-q', 'quiet', 'True') + check('-R', 'use_hash_seed', 'False') + check('-s', 'user_site_directory', 'False') + check('-S', 'site_import', 'False') + check_ignored('-t') + check('-u', 'buffered_stdio', 'False') + check('-v', 'verbose', '1') + check('-Wignore', 'warnoptions', "['ignore']") + check('-x', 'skip_source_first_line', 'True') + check(('-X', 'xoption=value'), 'xoptions', + # assert_python_ok() adds -X faulthandler + "{'faulthandler': True, 'xoption': 'value'}") + + # Short options can be combined + check('-bIs', ('bytes_warning', 'isolated', 'user_site_directory'), + '1 True False') + + # -c, -h, -m, -V and -? are tested elsewhere + + def test_missing_argument(self): + def check_missing_arg(option): + proc = assert_python_failure(option) + self.assertEqual(proc.rc, 2) + errmsg = f"Argument expected for the {option} option" + self.assertStartsWith(proc.err.rstrip(), errmsg.encode()) + + check_missing_arg('-c') + check_missing_arg('-m') + check_missing_arg('-W') + check_missing_arg('-X') + check_missing_arg('--check-hash-based-pycs') + + def test_long_options(self): + # Test long command line options + + # Test --check-hash-based-pycs option + code = 'import _imp; print(_imp.check_hash_based_pycs)' + opt = f"--check-hash-based-pycs" + for value in ('always', 'never', 'default'): + with self.subTest(value=value): + proc = assert_python_ok(opt, value, "-c", code) + self.assertEqual(proc.out.rstrip(), value.encode()) + + # "Expected long option" error + proc = assert_python_ok("-b-") + self.assertEqual(proc.out, b'') + self.assertEqual(proc.err.rstrip(), b"Expected long option") + + # Other long options --help-all, --help-env, --help-xoptions + # and --version are tested elsewhere + + def test_dash_option(self): + # Test -- in the command line + code = ( + 'import sys; ' + 'print(sys.flags.isolated, sys.flags.optimize, sys.argv)' + ) + proc = assert_python_ok('-I', '-c', code, '--', '-O') + self.assertEqual(proc.out.rstrip(), b"1 0 ['-c', '--', '-O']") + @unittest.skipIf(interpreter_requires_environment(), 'Cannot run -I tests when PYTHON env vars are required.') diff --git a/Python/getopt.c b/Python/getopt.c index 7e918189c716a9e..45454d39fc67792 100644 --- a/Python/getopt.c +++ b/Python/getopt.c @@ -27,18 +27,19 @@ #include #include #include -#include "pycore_getopt.h" +#include "pycore_getopt.h" // struct _PyOS_GetOpt -int _PyOS_opterr = 1; /* generate error messages */ -Py_ssize_t _PyOS_optind = 1; /* index into argv array */ -const wchar_t *_PyOS_optarg = NULL; /* optional argument */ - -static const wchar_t *opt_ptr = L""; /* Python command line short and long options */ #define SHORT_OPTS L"bBc:dEhiIm:OPqRsStuvVW:xX:?" +typedef struct { + const wchar_t *name; + int has_arg; + int val; +} _PyOS_LongOption; + static const _PyOS_LongOption longopts[] = { /* name, has_arg, val (used in switch in initconfig.c) */ {L"check-hash-based-pycs", 1, 1}, @@ -49,113 +50,136 @@ static const _PyOS_LongOption longopts[] = { }; -void _PyOS_ResetGetOpt(void) +void +_PyOS_GetOpt_Init(struct _PyOS_GetOpt *getopt, + Py_ssize_t argc, wchar_t * const *argv) { - _PyOS_opterr = 1; - _PyOS_optind = 1; - _PyOS_optarg = NULL; - opt_ptr = L""; + getopt->error = 1; + getopt->index = 1; + getopt->arg = NULL; + getopt->ptr = L""; + getopt->argc = argc; + getopt->argv = argv; } -int _PyOS_GetOpt(Py_ssize_t argc, wchar_t * const *argv, int *longindex) +// Parse a command line option. +// +// Return a character for short option (ex: return 'h' for -h). +// Return a number for long options (see 'longopts' array). +// Return '_' on unknown option or missing argument. +// Return -1 when done. +// +// Return 'h' for --help and return 'V' for --version. +// +// If an option has an argument, set getopt->arg to the argument. +// If getopt->error is non-error, write error messages to stderr. +int +_PyOS_GetOpt(struct _PyOS_GetOpt *getopt) { - wchar_t *ptr; - wchar_t option; + // Local copy of read-only members to omit "getopt->" + const Py_ssize_t argc = getopt->argc; + wchar_t * const *argv = getopt->argv; + const int error = getopt->error; - if (*opt_ptr == '\0') { - - if (_PyOS_optind >= argc) + if (*getopt->ptr == '\0') { + if (getopt->index >= argc) { return -1; + } + + const wchar_t *arg = argv[getopt->index]; #ifdef MS_WINDOWS - else if (wcscmp(argv[_PyOS_optind], L"/?") == 0) { - ++_PyOS_optind; + if (wcscmp(arg, L"/?") == 0) { + ++getopt->index; return 'h'; } #endif - else if (argv[_PyOS_optind][0] != L'-' || - argv[_PyOS_optind][1] == L'\0' /* lone dash */ ) + if (arg[0] != L'-' || arg[1] == L'\0' /* lone dash */ ) { return -1; + } - else if (wcscmp(argv[_PyOS_optind], L"--") == 0) { - ++_PyOS_optind; + if (wcscmp(arg, L"--") == 0) { + ++getopt->index; return -1; } - - else if (wcscmp(argv[_PyOS_optind], L"--help") == 0) { - ++_PyOS_optind; + if (wcscmp(arg, L"--help") == 0) { + ++getopt->index; return 'h'; } - - else if (wcscmp(argv[_PyOS_optind], L"--version") == 0) { - ++_PyOS_optind; + if (wcscmp(arg, L"--version") == 0) { + ++getopt->index; return 'V'; } - opt_ptr = &argv[_PyOS_optind++][1]; + getopt->ptr = &argv[getopt->index++][1]; } - if ((option = *opt_ptr++) == L'\0') + wchar_t option = *getopt->ptr++; + if (option == L'\0') { return -1; + } if (option == L'-') { // Parse long option. - if (*opt_ptr == L'\0') { - if (_PyOS_opterr) { + if (*getopt->ptr == L'\0') { + if (error) { fprintf(stderr, "Expected long option\n"); } return -1; } - *longindex = 0; + int longindex = 0; const _PyOS_LongOption *opt; - for (opt = &longopts[*longindex]; opt->name; opt = &longopts[++(*longindex)]) { - if (!wcscmp(opt->name, opt_ptr)) + for (opt = &longopts[longindex]; opt->name; opt = &longopts[++longindex]) { + if (wcscmp(opt->name, getopt->ptr) == 0) { break; + } } + if (!opt->name) { - if (_PyOS_opterr) { - fprintf(stderr, "Unknown option: %ls\n", argv[_PyOS_optind - 1]); + if (error) { + fprintf(stderr, "Unknown option: %ls\n", argv[getopt->index - 1]); } return '_'; } - opt_ptr = L""; + + getopt->ptr = L""; if (!opt->has_arg) { return opt->val; } - if (_PyOS_optind >= argc) { - if (_PyOS_opterr) { + if (getopt->index >= argc) { + if (error) { fprintf(stderr, "Argument expected for the %ls options\n", - argv[_PyOS_optind - 1]); + argv[getopt->index - 1]); } return '_'; } - _PyOS_optarg = argv[_PyOS_optind++]; + getopt->arg = argv[getopt->index++]; return opt->val; } - if ((ptr = wcschr(SHORT_OPTS, option)) == NULL) { - if (_PyOS_opterr) { + wchar_t *ptr = wcschr(SHORT_OPTS, option); + if (ptr == NULL) { + if (error) { fprintf(stderr, "Unknown option: -%c\n", (char)option); } return '_'; } if (*(ptr + 1) == L':') { - if (*opt_ptr != L'\0') { - _PyOS_optarg = opt_ptr; - opt_ptr = L""; + if (*getopt->ptr != L'\0') { + getopt->arg = getopt->ptr; + getopt->ptr = L""; } - else { - if (_PyOS_optind >= argc) { - if (_PyOS_opterr) { + if (getopt->index >= argc) { + if (error) { fprintf(stderr, "Argument expected for the -%c option\n", (char)option); } return '_'; } - _PyOS_optarg = argv[_PyOS_optind++]; + getopt->arg = argv[getopt->index++]; } } diff --git a/Python/initconfig.c b/Python/initconfig.c index 6de12db9d600ecc..178e6f992fa11fc 100644 --- a/Python/initconfig.c +++ b/Python/initconfig.c @@ -3009,11 +3009,11 @@ config_parse_cmdline(PyConfig *config, PyWideStringList *warnoptions, const PyWideStringList *argv = &config->argv; int print_version = 0; - _PyOS_ResetGetOpt(); + struct _PyOS_GetOpt getopt; + _PyOS_GetOpt_Init(&getopt, argv->length, argv->items); do { - int longindex = -1; - int c = _PyOS_GetOpt(argv->length, argv->items, &longindex); - if (c == EOF) { + int c = _PyOS_GetOpt(&getopt); + if (c == -1) { break; } @@ -3022,12 +3022,12 @@ config_parse_cmdline(PyConfig *config, PyWideStringList *warnoptions, /* -c is the last option; following arguments that look like options are left for the command to interpret. */ - size_t len = wcslen(_PyOS_optarg) + 1 + 1; + size_t len = wcslen(getopt.arg) + 1 + 1; wchar_t *command = PyMem_RawMalloc(sizeof(wchar_t) * len); if (command == NULL) { return _PyStatus_NO_MEMORY(); } - memcpy(command, _PyOS_optarg, (len - 2) * sizeof(wchar_t)); + memcpy(command, getopt.arg, (len - 2) * sizeof(wchar_t)); command[len - 2] = '\n'; command[len - 1] = 0; config->run_command = command; @@ -3040,7 +3040,7 @@ config_parse_cmdline(PyConfig *config, PyWideStringList *warnoptions, that look like options are left for the module to interpret. */ if (config->run_module == NULL) { - config->run_module = _PyMem_RawWcsdup(_PyOS_optarg); + config->run_module = _PyMem_RawWcsdup(getopt.arg); if (config->run_module == NULL) { return _PyStatus_NO_MEMORY(); } @@ -3051,13 +3051,13 @@ config_parse_cmdline(PyConfig *config, PyWideStringList *warnoptions, switch (c) { // Integers represent long options, see Python/getopt.c case 1: - // check-hash-based-pycs - if (wcscmp(_PyOS_optarg, L"always") == 0 - || wcscmp(_PyOS_optarg, L"never") == 0 - || wcscmp(_PyOS_optarg, L"default") == 0) + // --check-hash-based-pycs option + if (wcscmp(getopt.arg, L"always") == 0 + || wcscmp(getopt.arg, L"never") == 0 + || wcscmp(getopt.arg, L"default") == 0) { status = PyConfig_SetString(config, &config->check_hash_pycs_mode, - _PyOS_optarg); + getopt.arg); if (_PyStatus_EXCEPTION(status)) { return status; } @@ -3067,17 +3067,17 @@ config_parse_cmdline(PyConfig *config, PyWideStringList *warnoptions, break; case 2: - // help-all + // --help-all option DEFER_OPTION(c); break; case 3: - // help-env + // --help-env option DEFER_OPTION(c); break; case 4: - // help-xoptions + // --help-xoptions option DEFER_OPTION(c); break; @@ -3146,7 +3146,7 @@ config_parse_cmdline(PyConfig *config, PyWideStringList *warnoptions, break; case 'W': - status = PyWideStringList_Append(warnoptions, _PyOS_optarg); + status = PyWideStringList_Append(warnoptions, getopt.arg); if (_PyStatus_EXCEPTION(status)) { return status; } @@ -3177,22 +3177,22 @@ config_parse_cmdline(PyConfig *config, PyWideStringList *warnoptions, } if (config->run_command == NULL && config->run_module == NULL - && _PyOS_optind < argv->length - && wcscmp(argv->items[_PyOS_optind], L"-") != 0 + && getopt.index < argv->length + && wcscmp(argv->items[getopt.index], L"-") != 0 && config->run_filename == NULL) { - config->run_filename = _PyMem_RawWcsdup(argv->items[_PyOS_optind]); + config->run_filename = _PyMem_RawWcsdup(argv->items[getopt.index]); if (config->run_filename == NULL) { return _PyStatus_NO_MEMORY(); } } if (config->run_command != NULL || config->run_module != NULL) { - /* Backup _PyOS_optind */ - _PyOS_optind--; + /* Backup getopt.index */ + getopt.index--; } - *opt_index = _PyOS_optind; + *opt_index = getopt.index; return _PyStatus_OK(); @@ -3257,6 +3257,9 @@ _PyConfig_ProcessDeferredCmdlineOption(PyConfig *config) config_xoptions_usage(); return 0; + case '_': + // Unknown option or missing argument + _Py_FALLTHROUGH; default: config_usage(1, program); return 2; diff --git a/Python/preconfig.c b/Python/preconfig.c index 844ac8e6372fc52..c8005ccdce1a73e 100644 --- a/Python/preconfig.c +++ b/Python/preconfig.c @@ -187,15 +187,14 @@ precmdline_parse_cmdline(_PyPreCmdline *cmdline) { const PyWideStringList *argv = &cmdline->argv; - _PyOS_ResetGetOpt(); + struct _PyOS_GetOpt getopt; + _PyOS_GetOpt_Init(&getopt, argv->length, argv->items); /* Don't log parsing errors into stderr here: PyConfig_Read() is responsible for that */ - _PyOS_opterr = 0; + getopt.error = 0; do { - int longindex = -1; - int c = _PyOS_GetOpt(argv->length, argv->items, &longindex); - - if (c == EOF || c == 'c' || c == 'm') { + int c = _PyOS_GetOpt(&getopt); + if (c == -1 || c == 'c' || c == 'm') { break; } @@ -211,7 +210,7 @@ precmdline_parse_cmdline(_PyPreCmdline *cmdline) case 'X': { PyStatus status = PyWideStringList_Append(&cmdline->xoptions, - _PyOS_optarg); + getopt.arg); if (_PyStatus_EXCEPTION(status)) { return status; }