From 2f381217b3142a464912864d9aa4f19106540e18 Mon Sep 17 00:00:00 2001 From: Steve Dower Date: Tue, 13 May 2025 00:54:24 +0100 Subject: [PATCH 01/13] [WIP] Add 'first run' configuration experience to prompt users to modify system settings. Fixes #93 --- _msbuild.py | 1 + _msbuild_test.py | 1 + src/_native/misc.cpp | 35 ++++++ src/manage/commands.py | 68 +++++++++-- src/manage/firstrun.py | 264 +++++++++++++++++++++++++++++++++++++++++ src/pymanager/main.cpp | 29 ++++- 6 files changed, 386 insertions(+), 12 deletions(-) create mode 100644 src/manage/firstrun.py diff --git a/_msbuild.py b/_msbuild.py index ee87f7c..87b2c6d 100644 --- a/_msbuild.py +++ b/_msbuild.py @@ -84,6 +84,7 @@ class ResourceFile(CSourceFile): CFunction('date_as_str'), CFunction('datetime_as_str'), CFunction('reg_rename_key'), + CFunction('read_alias_package'), source='src/_native', RootNamespace='_native', ) diff --git a/_msbuild_test.py b/_msbuild_test.py index b5d5c26..66e7512 100644 --- a/_msbuild_test.py +++ b/_msbuild_test.py @@ -50,6 +50,7 @@ CFunction('date_as_str'), CFunction('datetime_as_str'), CFunction('reg_rename_key'), + CFunction('read_alias_package'), source='src/_native', ), DllPackage('_shellext_test', diff --git a/src/_native/misc.cpp b/src/_native/misc.cpp index f3499e7..ad7552b 100644 --- a/src/_native/misc.cpp +++ b/src/_native/misc.cpp @@ -119,4 +119,39 @@ PyObject *reg_rename_key(PyObject *, PyObject *args, PyObject *kwargs) { return r; } + +PyObject *read_alias_package(PyObject *, PyObject *args, PyObject *kwargs) { + static const char * keywords[] = {"path", NULL}; + wchar_t *path = NULL; + if (!PyArg_ParseTupleAndKeywords(args, kwargs, "O&:read_alias_package", keywords, + as_utf16, &path)) { + return NULL; + } + + HANDLE h = CreateFileW(path, GENERIC_READ, FILE_SHARE_READ, NULL, OPEN_EXISTING, + FILE_FLAG_OPEN_REPARSE_POINT | FILE_FLAG_BACKUP_SEMANTICS, NULL); + PyMem_Free(path); + if (h == INVALID_HANDLE_VALUE) { + PyErr_SetFromWindowsErr(0); + return NULL; + } + + wchar_t buffer[32768]; + DWORD nread; + + if (!DeviceIoControl(h, FSCTL_GET_REPARSE_POINT, NULL, 0, + buffer, sizeof(buffer), &nread, NULL)) { + PyErr_SetFromWindowsErr(0); + CloseHandle(h); + return NULL; + } + CloseHandle(h); + + if (*(DWORD*)buffer != IO_REPARSE_TAG_APPEXECLINK) { + return Py_GetConstant(Py_CONSTANT_NONE); + } + + return PyUnicode_FromWideChar(&buffer[4], nread / sizeof(wchar_t) - 5); +} + } diff --git a/src/manage/commands.py b/src/manage/commands.py index ae3b7df..90de58e 100644 --- a/src/manage/commands.py +++ b/src/manage/commands.py @@ -25,11 +25,19 @@ DEFAULT_TAG = "3.14" +# TODO: Remove the /dev/ for stable release +HELP_URL = "https://docs.python.org/dev/using/windows" + + COPYRIGHT = f"""Python installation manager {__version__} Copyright (c) Python Software Foundation. All Rights Reserved. """ +if EXE_NAME.casefold() == "py-manager".casefold(): + EXE_NAME = "py" + + WELCOME = f"""!B!Python install manager was successfully updated to {__version__}.!W! """ @@ -188,6 +196,7 @@ def execute(self): "enable-shortcut-kinds": ("enable_shortcut_kinds", _NEXT, config_split), "disable-shortcut-kinds": ("disable_shortcut_kinds", _NEXT, config_split), "help": ("show_help", True), # nested to avoid conflict with command + "configure": ("configure", True), # Set when the manager is doing an automatic install. # Generally won't be set by manual invocation "automatic": ("automatic", True), @@ -202,6 +211,10 @@ def execute(self): "force": ("confirm", False), "help": ("show_help", True), # nested to avoid conflict with command }, + + "**first_run": { + "explicit": ("explicit", True), + }, } @@ -240,6 +253,17 @@ def execute(self): "disable_shortcut_kinds": (str, config_split_append), }, + "first_run": { + "enabled": (config_bool, None, "env"), + "explicit": (config_bool, None), + "check_app_alias": (config_bool, None, "env"), + "check_long_paths": (config_bool, None, "env"), + "check_py_on_path": (config_bool, None, "env"), + "check_any_install": (config_bool, None, "env"), + "check_default_tag": (config_bool, None, "env"), + "check_global_dir": (config_bool, None, "env"), + }, + # These configuration settings are intended for administrative override only # For example, if you are managing deployments that will use your own index # and/or your own builds. @@ -419,11 +443,11 @@ def __init__(self, args, root=None): # If our command has any config, load them to override anything that # wasn't set on the command line. try: - cmd_config = config[self.CMD] + cmd_config = config[self.CMD.lstrip("*")] except (AttributeError, LookupError): pass else: - arg_names = frozenset(CONFIG_SCHEMA[self.CMD]) + arg_names = frozenset(CONFIG_SCHEMA[self.CMD.lstrip("*")]) for k, v in cmd_config.items(): if k in arg_names and k not in _set_args: LOGGER.debug("Overriding command option %s with %r", k, v) @@ -511,7 +535,7 @@ def get_log_file(self): logs_dir = Path(os.getenv("TMP") or os.getenv("TEMP") or os.getcwd()) from _native import datetime_as_str self._log_file = logs_dir / "python_{}_{}_{}.log".format( - self.CMD, datetime_as_str(), os.getpid() + self.CMD.strip("*"), datetime_as_str(), os.getpid() ) return self._log_file @@ -564,8 +588,7 @@ def show_usage(cls): LOGGER.print(r) LOGGER.print() - # TODO: Remove the /dev/ for stable release - LOGGER.print("Find additional information at !B!https://docs.python.org/dev/using/windows!W!.") + LOGGER.print("Find additional information at !B!%s!W!.", HELP_URL) LOGGER.print() @classmethod @@ -746,6 +769,7 @@ class InstallCommand(BaseCommand): -u, --update Overwrite existing install if a newer version is available. --dry-run Choose runtime but do not install --refresh Update shortcuts and aliases for all installed versions. + --configure Re-run the system configuration helper. --by-id Require TAG to exactly match the install ID. (For advanced use.) !B! !W! ... One or more tags to install (Company\Tag format) @@ -775,6 +799,7 @@ class InstallCommand(BaseCommand): dry_run = False refresh = False by_id = False + configure = False automatic = False from_script = None enable_shortcut_kinds = None @@ -801,9 +826,13 @@ def __init__(self, args, root=None): self.download = Path(self.download).absolute() def execute(self): - from .install_command import execute self.show_welcome() - execute(self) + if self.configure: + cmd = FirstRun(["**first_run", "--explicit"], self.root) + cmd.execute() + else: + from .install_command import execute + execute(self) class UninstallCommand(BaseCommand): @@ -891,7 +920,7 @@ def execute(self): class HelpWithErrorCommand(HelpCommand): - CMD = "__help_with_error" + CMD = "**help_with_error" def __init__(self, args, root=None): # Essentially disable argument processing for this command @@ -945,6 +974,29 @@ def __init__(self, root): super().__init__([], root) +class FirstRun(BaseCommand): + CMD = "**first_run" + enabled = True + explicit = False + check_app_alias = True + check_long_paths = True + check_py_on_path = True + check_any_install = True + check_default_tag = True + check_global_dir = True + + def execute(self): + if not self.enabled: + return + from .firstrun import first_run + first_run(self) + if not self.explicit: + show_help([]) + if self.confirm and not self.ask_ny(f"View more help online? (!B!{HELP_URL}!W!)"): + import os + os.startfile(HELP_URL) + + def load_default_config(root): return DefaultConfig(root) diff --git a/src/manage/firstrun.py b/src/manage/firstrun.py new file mode 100644 index 0000000..9b32a37 --- /dev/null +++ b/src/manage/firstrun.py @@ -0,0 +1,264 @@ +import os +import sys + +if __name__ == "__main__": + __package__ = "manage" + sys.path.append(os.path.dirname(os.path.dirname(__file__))) + + import _native + if not hasattr(_native, "coinitialize"): + import _native_test + for k in dir(_native_test): + if k[:1] not in ("", "_"): + setattr(_native, k, getattr(_native_test, k)) + + +from .logging import LOGGER +from .pathutils import Path + + +def check_app_alias(cmd): + LOGGER.debug("Checking app execution aliases") + from _native import read_alias_package + root = Path(os.environ["LocalAppData"]) / "Microsoft/WindowsApps" + for name in ["py.exe", "pyw.exe", "python.exe", "pythonw.exe", "python3.exe", "pymanager.exe"]: + exe = root / name + try: + LOGGER.debug("Reading from %s", exe) + package = (read_alias_package(exe) or "").split("\0") + LOGGER.debug("Data: %r", package) + if package[1] not in ( + # Side-loaded MSIX + "PythonSoftwareFoundation.PythonManager_3847v3x7pw1km", + # Store packaged + "PythonSoftwareFoundation.PythonManager_qbz5n2kfra8p0", + # Development build + "PythonSoftwareFoundation.PythonManager_m8z88z54g2w36", + ): + LOGGER.debug("Check failed: package did not match identity") + except FileNotFoundError: + LOGGER.debug("Check failed: did not find %s", exe) + return False + LOGGER.debug("Check passed: aliases are correct") + return True + + +def check_long_paths(cmd): + LOGGER.debug("Checking long paths setting") + import winreg + try: + with winreg.OpenKeyEx(winreg.HKEY_LOCAL_MACHINE, r"System\CurrentControlSet\Control\FileSystem") as key: + if winreg.QueryValueEx(key, "LongPathsEnabled") == (1, winreg.REG_DWORD): + LOGGER.debug("Check passed: registry key is OK") + return True + except FileNotFoundError: + pass + LOGGER.debug("Check failed: registry key was missing or incorrect") + return False + + +def check_py_on_path(cmd): + LOGGER.debug("Checking for legacy py.exe on PATH") + from _native import read_alias_package + for p in os.environ["PATH"].split(";"): + if not p: + continue + py = Path(p) / "py.exe" + try: + read_alias_package(py) + LOGGER.debug("Check passed: found alias at %s", py) + # We found the alias, so we're good + return True + except FileNotFoundError: + pass + except OSError: + # Probably not an alias, so we're not good + LOGGER.debug("Check failed: found %s on PATH", py) + return False + + +def check_global_dir(cmd): + LOGGER.debug("Checking for global dir on PATH") + if not cmd.global_dir: + LOGGER.debug("Check passed: global dir is not configured") + return True + for p in os.environ["PATH"].split(";"): + if not p: + continue + if Path(p).absolute().match(cmd.global_dir): + LOGGER.debug("Check passed: %s is on PATH", p) + return True + LOGGER.debug("Check failed: %s not found in PATH", cmd.global_dir) + return False + + +def do_global_dir_on_path(cmd): + import winreg + LOGGER.debug("Adding %s to PATH", cmd.global_dir) + # TODO: Add to PATH (correctly!) + # TODO: Send notification + + +def check_any_install(cmd): + LOGGER.debug("Checking for any Python runtime install") + if not cmd.get_installs(include_unmanaged=True, set_default=False): + LOGGER.debug("Check failed: no installs found") + return False + LOGGER.debug("Check passed: installs found") + return True + + +def do_install(cmd): + from .commands import find_command + try: + inst_cmd = find_command(["install", "default", "--automatic"], cmd.root) + except Exception as ex: + LOGGER.debug("Failed to find 'install' command.", exc_info=True) + LOGGER.warn("We couldn't install right now.") + LOGGER.info("Use !B!py install default!W! later to install.") + sys.exit(1) + else: + try: + inst_cmd.execute() + except Exception as ex: + LOGGER.debug("Failed to run 'install' command.", exc_info=True) + raise + + +class _Welcome: + _shown = False + def __call__(self): + if not self._shown: + self._shown = True + LOGGER.info("!G!Welcome to the Python installation manager " + "configuration helper.!W!") + LOGGER.info("") + + +def first_run(cmd): + if not cmd.enabled: + return + + welcome = _Welcome() + if cmd.explicit: + welcome() + + if cmd.check_app_alias: + if not check_app_alias(cmd): + welcome() + LOGGER.warn("Your app execution alias settings are configured to launch " + "other commands besides 'py' and 'python'.") + LOGGER.info("This can be fixed by opening the '!B!Manage app execution " + "aliases!W!' settings page and enabling each item labelled " + "'!B!Python (default)!W!' and '!B!Python install manager!W!'.") + if ( + cmd.confirm and + not cmd.ask_ny("Open Settings now? (Select !B!App execution aliases!W! after opening)") + ): + os.startfile("ms-settings:advanced-apps") + elif cmd.explicit: + LOGGER.info("Checked app execution aliases") + + if cmd.check_long_paths: + if not check_long_paths(cmd): + welcome() + LOGGER.warn("Windows is not configured to allow paths longer than " + "260 characters.") + LOGGER.info("Python and some other apps can bypass this setting, but it " + "requires changing a system-wide setting and a reboot. " + "Some packages may fail to install without long path " + "support enabled.") + if ( + cmd.confirm and + not cmd.ask_ny("Update setting now? You may be prompted for " + "administrator credentials.") + ): + os.startfile(sys.executable, "runas", "**configure-long-paths", show_cmd=0) + elif cmd.explicit: + LOGGER.info("Checked system long paths setting") + + if cmd.check_py_on_path: + if not check_py_on_path(cmd): + welcome() + LOGGER.warn("The legacy 'py' command is still installed.") + LOGGER.info("This may interfere with launching the new 'py' command, " + "and may be resolved by uninstalling '!B!Python launcher!W!'.") + if ( + cmd.confirm and + not cmd.ask_ny("Open Installed apps now?") + ): + os.startfile("ms-settings:appsfeatures") + elif cmd.explicit: + LOGGER.info("Checked PATH for legacy 'py' command") + + if cmd.check_global_dir: + if not check_global_dir(cmd): + welcome() + LOGGER.warn("The directory for versioned Python commands is not configured.") + LOGGER.info("This will prevent commands like !B!python3.14.exe!W! " + "working, but will not affect the !B!python!W! or " + "!B!py!W! commands (for example, !B!py -V:3.14!W!).") + LOGGER.info("We can add the directory to PATH now, but you will need " + "to restart your terminal to see the change, and may need " + "to manually edit your environment variables if you later " + "decide to remove the entry.") + if ( + cmd.confirm and + not cmd.ask_ny("Add commands directory to your PATH now?") + ): + do_global_dir_on_path(cmd) + elif cmd.explicit: + LOGGER.info("Checked PATH for versioned commands directory") + + # This check must be last, because 'do_install' will exit the program. + if cmd.check_any_install: + if not check_any_install(cmd): + welcome() + LOGGER.warn("You do not have any Python runtimes installed.") + LOGGER.info("Install the current latest version of CPython? If not, " + "you can use !B!py install default!W! later to install, or " + "one will be installed automatically when needed.") + if cmd.ask_yn("Install CPython now?"): + do_install(cmd) + elif cmd.explicit: + LOGGER.info("Checked for any Python installs") + + if cmd.explicit: + LOGGER.info("!G!All checks passed.!W!") + + +if __name__ == "__main__": + class TestCommand: + enabled = True + global_dir = r".\test-bin" + explicit = False + confirm = True + check_app_alias = True + check_long_paths = True + check_py_on_path = True + check_any_install = True + check_global_dir = True + check_default_tag = True + + def get_installs(self, *args, **kwargs): + return [] + + def _ask(self, fmt, *args, yn_text="Y/n", expect_char="y"): + if not self.confirm: + return True + LOGGER.print(f"{fmt} [{yn_text}] ", *args, end="") + try: + resp = input().casefold() + except Exception: + return False + return not resp or resp.startswith(expect_char.casefold()) + + def ask_yn(self, fmt, *args): + "Returns True if the user selects 'yes' or confirmations are skipped." + return self._ask(fmt, *args) + + def ask_ny(self, fmt, *args): + "Returns True if the user selects 'no' or confirmations are skipped." + return self._ask(fmt, *args, yn_text="y/N", expect_char="n") + + first_run(TestCommand()) diff --git a/src/pymanager/main.cpp b/src/pymanager/main.cpp index 0277d0c..701e32a 100644 --- a/src/pymanager/main.cpp +++ b/src/pymanager/main.cpp @@ -77,6 +77,23 @@ is_env_var_set(const wchar_t *name) } +static int +configure_long_path() +{ + HKEY key; + LRESULT r; + r = RegCreateKeyExW(HKEY_LOCAL_MACHINE, L"System\\CurrentControlSet\\Control\\FileSystem", + 0, NULL, 0, KEY_WRITE, NULL, &key, NULL); + if (r) { + return r; + } + DWORD value = 1; + r = RegSetValueExW(key, L"LongPathsEnabled", 0, REG_DWORD, (BYTE *)&value, sizeof(value)); + RegCloseKey(key); + return r; +} + + static void per_exe_settings( int argc, @@ -131,16 +148,16 @@ per_exe_settings( return; } if (CompareStringOrdinal(name, cch, L"pymanager", -1, TRUE) == CSTR_EQUAL) { - *default_command = argc >= 2 ? L"__help_with_error" : L"help"; + *default_command = argc >= 2 ? L"**help_with_error" : L"help"; *commands = argc >= 2; *cli_tag = false; *shebangs = false; *autoinstall = argc >= 2 && !wcscmp(argv[1], L"exec"); return; } - // This case is for direct launches (including first run), Start menu - // launch, or via file associations. - *default_command = NULL; + // This case is for direct launches (including first run), or Start menu + // launch. + *default_command = L"**first_run"; *commands = argc >= 2; *cli_tag = true; *shebangs = true; @@ -474,6 +491,10 @@ wmain(int argc, wchar_t **argv) std::wstring executable, args, tag, script; int skip_argc = 0; + if (argc == 2 && 0 == wcscmp(argv[1], L"**configure-long-paths")) { + return configure_long_path(); + } + err = init_python(); if (err) { return err; From c827c1fdb550d47a8aa5b9fcc56a0f1760c896dd Mon Sep 17 00:00:00 2001 From: Steve Dower Date: Tue, 13 May 2025 14:44:00 +0100 Subject: [PATCH 02/13] Improved aliases check --- _msbuild.py | 1 + _msbuild_test.py | 1 + src/_native/misc.cpp | 35 ++++++++++++++++++++++++---- src/manage/firstrun.py | 53 +++++++++++++++++++++++++++++++----------- 4 files changed, 72 insertions(+), 18 deletions(-) diff --git a/_msbuild.py b/_msbuild.py index 87b2c6d..cde8ae1 100644 --- a/_msbuild.py +++ b/_msbuild.py @@ -84,6 +84,7 @@ class ResourceFile(CSourceFile): CFunction('date_as_str'), CFunction('datetime_as_str'), CFunction('reg_rename_key'), + CFunction('get_current_package'), CFunction('read_alias_package'), source='src/_native', RootNamespace='_native', diff --git a/_msbuild_test.py b/_msbuild_test.py index 66e7512..4008273 100644 --- a/_msbuild_test.py +++ b/_msbuild_test.py @@ -50,6 +50,7 @@ CFunction('date_as_str'), CFunction('datetime_as_str'), CFunction('reg_rename_key'), + CFunction('get_current_package'), CFunction('read_alias_package'), source='src/_native', ), diff --git a/src/_native/misc.cpp b/src/_native/misc.cpp index ad7552b..f5e060a 100644 --- a/src/_native/misc.cpp +++ b/src/_native/misc.cpp @@ -1,5 +1,6 @@ #include #include +#include #include "helpers.h" @@ -120,6 +121,22 @@ PyObject *reg_rename_key(PyObject *, PyObject *args, PyObject *kwargs) { } +PyObject *get_current_package(PyObject *, PyObject *, PyObject *) { + wchar_t package_name[256]; + UINT32 cch = sizeof(package_name) / sizeof(package_name[0]); + int err = GetCurrentPackageFamilyName(&cch, package_name); + switch (err) { + case ERROR_SUCCESS: + return PyUnicode_FromWideChar(package_name, cch); + case APPMODEL_ERROR_NO_PACKAGE: + return Py_GetConstant(Py_CONSTANT_NONE); + default: + PyErr_SetFromWindowsErr(err); + return NULL; + } +} + + PyObject *read_alias_package(PyObject *, PyObject *args, PyObject *kwargs) { static const char * keywords[] = {"path", NULL}; wchar_t *path = NULL; @@ -136,22 +153,32 @@ PyObject *read_alias_package(PyObject *, PyObject *args, PyObject *kwargs) { return NULL; } - wchar_t buffer[32768]; + struct { + DWORD tag; + DWORD _reserved1; + DWORD _reserved2; + wchar_t package_name[256]; + wchar_t nul; + } buffer; DWORD nread; if (!DeviceIoControl(h, FSCTL_GET_REPARSE_POINT, NULL, 0, - buffer, sizeof(buffer), &nread, NULL)) { + &buffer, sizeof(buffer), &nread, NULL) + // we expect our buffer to be too small, but we only want the package + && GetLastError() != ERROR_MORE_DATA) { PyErr_SetFromWindowsErr(0); CloseHandle(h); return NULL; } + CloseHandle(h); - if (*(DWORD*)buffer != IO_REPARSE_TAG_APPEXECLINK) { + if (buffer.tag != IO_REPARSE_TAG_APPEXECLINK) { return Py_GetConstant(Py_CONSTANT_NONE); } - return PyUnicode_FromWideChar(&buffer[4], nread / sizeof(wchar_t) - 5); + buffer.nul = 0; + return PyUnicode_FromWideChar(buffer.package_name, -1); } } diff --git a/src/manage/firstrun.py b/src/manage/firstrun.py index 9b32a37..5c81d2e 100644 --- a/src/manage/firstrun.py +++ b/src/manage/firstrun.py @@ -19,23 +19,36 @@ def check_app_alias(cmd): LOGGER.debug("Checking app execution aliases") - from _native import read_alias_package + from _native import get_current_package, read_alias_package + # Expected identities: + # Side-loaded MSIX + # * "PythonSoftwareFoundation.PythonManager_3847v3x7pw1km", + # Store package + # * "PythonSoftwareFoundation.PythonManager_qbz5n2kfra8p0", + # Development build + # * "PythonSoftwareFoundation.PythonManager_m8z88z54g2w36", + # MSI/dev install + # * None + try: + pkg = get_current_package() + except OSError: + LOGGER.debug("Failed to get current package name.", exc_info=True) + pkg = None + if not pkg: + pkg = "" + #LOGGER.debug("Check skipped: MSI install can't do this check") + #return True + LOGGER.debug("Checking for %s", pkg) root = Path(os.environ["LocalAppData"]) / "Microsoft/WindowsApps" for name in ["py.exe", "pyw.exe", "python.exe", "pythonw.exe", "python3.exe", "pymanager.exe"]: exe = root / name try: LOGGER.debug("Reading from %s", exe) - package = (read_alias_package(exe) or "").split("\0") - LOGGER.debug("Data: %r", package) - if package[1] not in ( - # Side-loaded MSIX - "PythonSoftwareFoundation.PythonManager_3847v3x7pw1km", - # Store packaged - "PythonSoftwareFoundation.PythonManager_qbz5n2kfra8p0", - # Development build - "PythonSoftwareFoundation.PythonManager_m8z88z54g2w36", - ): + package = read_alias_package(exe) + LOGGER.debug("Package: %r", package) + if package not in pkg: LOGGER.debug("Check failed: package did not match identity") + return False except FileNotFoundError: LOGGER.debug("Check failed: did not find %s", exe) return False @@ -59,7 +72,10 @@ def check_long_paths(cmd): def check_py_on_path(cmd): LOGGER.debug("Checking for legacy py.exe on PATH") - from _native import read_alias_package + from _native import get_current_package, read_alias_package + if not get_current_package(): + LOGGER.debug("Check skipped: MSI install can't do this check") + return True for p in os.environ["PATH"].split(";"): if not p: continue @@ -230,7 +246,7 @@ def first_run(cmd): if __name__ == "__main__": class TestCommand: enabled = True - global_dir = r".\test-bin" + global_dir = Path(os.path.expandvars(r"%LocalAppData%\Python\bin")) explicit = False confirm = True check_app_alias = True @@ -241,7 +257,16 @@ class TestCommand: check_default_tag = True def get_installs(self, *args, **kwargs): - return [] + import json + root = Path(os.path.expandvars(r"%LocalAppData%\Python")) + result = [] + for d in root.iterdir(): + inst = d / "__install__.json" + try: + result.append(json.loads(inst.read_text())) + except FileNotFoundError: + pass + return result def _ask(self, fmt, *args, yn_text="Y/n", expect_char="y"): if not self.confirm: From 9dc31040474c20d00331e50103e26fafe257d2a2 Mon Sep 17 00:00:00 2001 From: Steve Dower Date: Tue, 13 May 2025 15:25:54 +0100 Subject: [PATCH 03/13] Add PATH registry update --- _msbuild.py | 1 + _msbuild_test.py | 1 + src/_native/misc.cpp | 52 +++++++++++++++++++ src/manage/firstrun.py | 113 ++++++++++++++++++++++++++++++++++------- 4 files changed, 149 insertions(+), 18 deletions(-) diff --git a/_msbuild.py b/_msbuild.py index cde8ae1..387a6cd 100644 --- a/_msbuild.py +++ b/_msbuild.py @@ -86,6 +86,7 @@ class ResourceFile(CSourceFile): CFunction('reg_rename_key'), CFunction('get_current_package'), CFunction('read_alias_package'), + CFunction('broadcast_settings_change'), source='src/_native', RootNamespace='_native', ) diff --git a/_msbuild_test.py b/_msbuild_test.py index 4008273..5a3e71a 100644 --- a/_msbuild_test.py +++ b/_msbuild_test.py @@ -52,6 +52,7 @@ CFunction('reg_rename_key'), CFunction('get_current_package'), CFunction('read_alias_package'), + CFunction('broadcast_settings_change'), source='src/_native', ), DllPackage('_shellext_test', diff --git a/src/_native/misc.cpp b/src/_native/misc.cpp index f5e060a..5791977 100644 --- a/src/_native/misc.cpp +++ b/src/_native/misc.cpp @@ -181,4 +181,56 @@ PyObject *read_alias_package(PyObject *, PyObject *args, PyObject *kwargs) { return PyUnicode_FromWideChar(buffer.package_name, -1); } + +typedef LRESULT (*PSendMessageTimeoutW)( + HWND hWnd, + UINT Msg, + WPARAM wParam, + LPARAM lParam, + UINT fuFlags, + UINT uTimeout, + PDWORD_PTR lpdwResult +); + +PyObject *broadcast_settings_change(PyObject *, PyObject *, PyObject *) { + // Avoid depending on user32 because it's so slow + HMODULE user32 = LoadLibraryExW(L"user32.dll", NULL, LOAD_LIBRARY_SEARCH_SYSTEM32); + if (!user32) { + PyErr_SetFromWindowsErr(0); + return NULL; + } + PSendMessageTimeoutW sm = (PSendMessageTimeoutW)GetProcAddress(user32, "SendMessageTimeoutW"); + if (!sm) { + PyErr_SetFromWindowsErr(0); + FreeLibrary(user32); + return NULL; + } + + // SendMessageTimeout needs special error handling + SetLastError(0); + LPARAM lParam = (LPARAM)L"Environment"; + + if (!(*sm)( + HWND_BROADCAST, + WM_SETTINGCHANGE, + NULL, + lParam, + SMTO_ABORTIFHUNG, + 50, + NULL + )) { + int err = GetLastError(); + if (!err) { + PyErr_SetString(PyExc_OSError, "Unspecified error"); + } else { + PyErr_SetFromWindowsErr(err); + } + FreeLibrary(user32); + return NULL; + } + + FreeLibrary(user32); + return Py_GetConstant(Py_CONSTANT_NONE); +} + } diff --git a/src/manage/firstrun.py b/src/manage/firstrun.py index 5c81d2e..ff8472e 100644 --- a/src/manage/firstrun.py +++ b/src/manage/firstrun.py @@ -35,9 +35,8 @@ def check_app_alias(cmd): LOGGER.debug("Failed to get current package name.", exc_info=True) pkg = None if not pkg: - pkg = "" - #LOGGER.debug("Check skipped: MSI install can't do this check") - #return True + LOGGER.debug("Check skipped: MSI install can't do this check") + return "skip" LOGGER.debug("Checking for %s", pkg) root = Path(os.environ["LocalAppData"]) / "Microsoft/WindowsApps" for name in ["py.exe", "pyw.exe", "python.exe", "pythonw.exe", "python3.exe", "pymanager.exe"]: @@ -75,7 +74,7 @@ def check_py_on_path(cmd): from _native import get_current_package, read_alias_package if not get_current_package(): LOGGER.debug("Check skipped: MSI install can't do this check") - return True + return "skip" for p in os.environ["PATH"].split(";"): if not p: continue @@ -96,23 +95,89 @@ def check_py_on_path(cmd): def check_global_dir(cmd): LOGGER.debug("Checking for global dir on PATH") if not cmd.global_dir: - LOGGER.debug("Check passed: global dir is not configured") - return True + LOGGER.debug("Check skipped: global dir is not configured") + return "skip" for p in os.environ["PATH"].split(";"): if not p: continue if Path(p).absolute().match(cmd.global_dir): LOGGER.debug("Check passed: %s is on PATH", p) return True + # In case user has updated their registry but not the terminal + import winreg + try: + with winreg.OpenKeyEx(winreg.HKEY_CURRENT_USER, "Environment") as key: + path, kind = winreg.QueryValueEx(key, "Path") + LOGGER.debug("Current registry path: %s", path) + if kind == winreg.REG_EXPAND_SZ: + path = os.path.expandvars(path) + elif kind != winreg.REG_SZ: + LOGGER.debug("Check skipped: PATH registry key is not a string.") + return "skip" + for p in path.split(";"): + if not p: + continue + if Path(p).absolute().match(cmd.global_dir): + LOGGER.debug("Check skipped: %s will be on PATH after restart", p) + return True + except Exception: + LOGGER.debug("Failed to read PATH setting from registry", exc_info=True) LOGGER.debug("Check failed: %s not found in PATH", cmd.global_dir) return False def do_global_dir_on_path(cmd): import winreg - LOGGER.debug("Adding %s to PATH", cmd.global_dir) - # TODO: Add to PATH (correctly!) - # TODO: Send notification + added = notified = False + try: + LOGGER.debug("Adding %s to PATH", cmd.global_dir) + with winreg.OpenKeyEx(winreg.HKEY_CURRENT_USER, "Environment") as key: + initial, kind = winreg.QueryValueEx(key, "Path") + LOGGER.debug("Initial path: %s", initial) + if kind not in (winreg.REG_SZ, winreg.REG_EXPAND_SZ) or not isinstance(initial, str): + LOGGER.debug("Value kind is %s and not REG_[EXPAND_]SZ. Aborting.") + return + for p in initial.split(";"): + if not p: + continue + if p.casefold() == str(cmd.global_dir).casefold(): + LOGGER.debug("Path is already found.") + return + newpath = initial + (";" if initial else "") + str(Path(cmd.global_dir).absolute()) + LOGGER.debug("New path: %s", newpath) + # Expand the value and ensure we are found + for p in os.path.expandvars(newpath).split(";"): + if not p: + continue + if p.casefold() == str(cmd.global_dir).casefold(): + LOGGER.debug("Path is added successfully") + break + else: + return + + with winreg.CreateKeyEx(winreg.HKEY_CURRENT_USER, "Environment", + access=winreg.KEY_READ|winreg.KEY_WRITE) as key: + initial2, kind2 = winreg.QueryValueEx(key, "Path") + if initial2 != initial or kind2 != kind: + LOGGER.debug("PATH has changed while we were working. Aborting.") + return + winreg.SetValueEx(key, "Path", 0, kind, newpath) + added = True + + from _native import broadcast_settings_change + broadcast_settings_change() + notified = True + except Exception: + LOGGER.debug("Failed to update PATH environment variable", exc_info=True) + finally: + if added and not notified: + LOGGER.warn("Failed to notify of PATH environment variable change.") + LOGGER.info("You may need to sign out or restart to see the changes.") + elif not added: + LOGGER.warn("Failed to update PATH environment variable successfully.") + LOGGER.info("You may add it yourself by opening 'Edit environment " + "variables' and adding this directory to 'PATH': !B!%s!W!", + cmd.global_dir) def check_any_install(cmd): @@ -160,7 +225,8 @@ def first_run(cmd): welcome() if cmd.check_app_alias: - if not check_app_alias(cmd): + r = check_app_alias(cmd) + if not r: welcome() LOGGER.warn("Your app execution alias settings are configured to launch " "other commands besides 'py' and 'python'.") @@ -173,7 +239,10 @@ def first_run(cmd): ): os.startfile("ms-settings:advanced-apps") elif cmd.explicit: - LOGGER.info("Checked app execution aliases") + if r == "skip": + LOGGER.info("Skipped app execution aliases check") + else: + LOGGER.info("Checked app execution aliases") if cmd.check_long_paths: if not check_long_paths(cmd): @@ -194,7 +263,8 @@ def first_run(cmd): LOGGER.info("Checked system long paths setting") if cmd.check_py_on_path: - if not check_py_on_path(cmd): + r = check_py_on_path(cmd) + if not r: welcome() LOGGER.warn("The legacy 'py' command is still installed.") LOGGER.info("This may interfere with launching the new 'py' command, " @@ -205,26 +275,33 @@ def first_run(cmd): ): os.startfile("ms-settings:appsfeatures") elif cmd.explicit: - LOGGER.info("Checked PATH for legacy 'py' command") + if r == "skip": + LOGGER.info("Skipped check for legacy 'py' command") + else: + LOGGER.info("Checked PATH for legacy 'py' command") if cmd.check_global_dir: - if not check_global_dir(cmd): + r = check_global_dir(cmd) + if not r: welcome() LOGGER.warn("The directory for versioned Python commands is not configured.") LOGGER.info("This will prevent commands like !B!python3.14.exe!W! " "working, but will not affect the !B!python!W! or " "!B!py!W! commands (for example, !B!py -V:3.14!W!).") LOGGER.info("We can add the directory to PATH now, but you will need " - "to restart your terminal to see the change, and may need " - "to manually edit your environment variables if you later " - "decide to remove the entry.") + "to restart your terminal to see the change, and must " + "manually edit environment variables to later remove the " + "entry.") if ( cmd.confirm and not cmd.ask_ny("Add commands directory to your PATH now?") ): do_global_dir_on_path(cmd) elif cmd.explicit: - LOGGER.info("Checked PATH for versioned commands directory") + if r == "skip": + LOGGER.info("Skipped check for commands directory on PATH") + else: + LOGGER.info("Checked PATH for versioned commands directory") # This check must be last, because 'do_install' will exit the program. if cmd.check_any_install: From ab1eec749c554b08e5818cb82c4a880a3626c890 Mon Sep 17 00:00:00 2001 From: Steve Dower Date: Tue, 13 May 2025 15:54:04 +0100 Subject: [PATCH 04/13] Add tests --- src/manage/firstrun.py | 63 ++++++++++++++++++--------- tests/conftest.py | 15 ++++++- tests/test_firstrun.py | 99 ++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 154 insertions(+), 23 deletions(-) create mode 100644 tests/test_firstrun.py diff --git a/src/manage/firstrun.py b/src/manage/firstrun.py index ff8472e..99602db 100644 --- a/src/manage/firstrun.py +++ b/src/manage/firstrun.py @@ -17,9 +17,13 @@ from .pathutils import Path +def _package_name(): + from _native import get_current_package + return get_current_package() + + def check_app_alias(cmd): LOGGER.debug("Checking app execution aliases") - from _native import get_current_package, read_alias_package # Expected identities: # Side-loaded MSIX # * "PythonSoftwareFoundation.PythonManager_3847v3x7pw1km", @@ -30,13 +34,15 @@ def check_app_alias(cmd): # MSI/dev install # * None try: - pkg = get_current_package() + pkg = _package_name() except OSError: LOGGER.debug("Failed to get current package name.", exc_info=True) pkg = None if not pkg: LOGGER.debug("Check skipped: MSI install can't do this check") return "skip" + + from _native import read_alias_package LOGGER.debug("Checking for %s", pkg) root = Path(os.environ["LocalAppData"]) / "Microsoft/WindowsApps" for name in ["py.exe", "pyw.exe", "python.exe", "pythonw.exe", "python3.exe", "pymanager.exe"]: @@ -59,7 +65,8 @@ def check_long_paths(cmd): LOGGER.debug("Checking long paths setting") import winreg try: - with winreg.OpenKeyEx(winreg.HKEY_LOCAL_MACHINE, r"System\CurrentControlSet\Control\FileSystem") as key: + with winreg.OpenKeyEx(winreg.HKEY_LOCAL_MACHINE, + r"System\CurrentControlSet\Control\FileSystem") as key: if winreg.QueryValueEx(key, "LongPathsEnabled") == (1, winreg.REG_DWORD): LOGGER.debug("Check passed: registry key is OK") return True @@ -71,9 +78,14 @@ def check_long_paths(cmd): def check_py_on_path(cmd): LOGGER.debug("Checking for legacy py.exe on PATH") - from _native import get_current_package, read_alias_package - if not get_current_package(): - LOGGER.debug("Check skipped: MSI install can't do this check") + from _native import read_alias_package + try: + if not _package_name(): + LOGGER.debug("Check skipped: MSI install can't do this check") + return "skip" + except OSError: + LOGGER.debug("Failed to get current package name.", exc_info=True) + LOGGER.debug("Check skipped: can't do this check") return "skip" for p in os.environ["PATH"].split(";"): if not p: @@ -90,6 +102,8 @@ def check_py_on_path(cmd): # Probably not an alias, so we're not good LOGGER.debug("Check failed: found %s on PATH", py) return False + LOGGER.debug("Check passed: no py.exe on PATH at all") + return True def check_global_dir(cmd): @@ -104,28 +118,35 @@ def check_global_dir(cmd): LOGGER.debug("Check passed: %s is on PATH", p) return True # In case user has updated their registry but not the terminal - import winreg try: - with winreg.OpenKeyEx(winreg.HKEY_CURRENT_USER, "Environment") as key: - path, kind = winreg.QueryValueEx(key, "Path") - LOGGER.debug("Current registry path: %s", path) - if kind == winreg.REG_EXPAND_SZ: - path = os.path.expandvars(path) - elif kind != winreg.REG_SZ: - LOGGER.debug("Check skipped: PATH registry key is not a string.") - return "skip" - for p in path.split(";"): - if not p: - continue - if Path(p).absolute().match(cmd.global_dir): - LOGGER.debug("Check skipped: %s will be on PATH after restart", p) - return True + r = _check_global_dir_registry(cmd) + if r: + return r except Exception: LOGGER.debug("Failed to read PATH setting from registry", exc_info=True) LOGGER.debug("Check failed: %s not found in PATH", cmd.global_dir) return False +def _check_global_dir_registry(cmd): + import winreg + with winreg.OpenKeyEx(winreg.HKEY_CURRENT_USER, "Environment") as key: + path, kind = winreg.QueryValueEx(key, "Path") + LOGGER.debug("Current registry path: %s", path) + if kind == winreg.REG_EXPAND_SZ: + path = os.path.expandvars(path) + elif kind != winreg.REG_SZ: + LOGGER.debug("Check skipped: PATH registry key is not a string.") + return "skip" + for p in path.split(";"): + if not p: + continue + if Path(p).absolute().match(cmd.global_dir): + LOGGER.debug("Check skipped: %s will be on PATH after restart", p) + return True + return False + + def do_global_dir_on_path(cmd): import winreg added = notified = False diff --git a/tests/conftest.py b/tests/conftest.py index eaf3b56..5858a4a 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -66,6 +66,9 @@ def skip_until(self, pattern, args=()): def not_logged(self, pattern, args=()): return ('not', pattern, args) + def end_of_log(self): + return ('eol', None, None) + def __call__(self, *cmp): i = 0 for y in cmp: @@ -84,6 +87,11 @@ def __call__(self, *cmp): return continue + if op == 'eol': + if i < len(self): + pytest.fail(f"Expected end of log; found {self[i]}") + return + while True: try: x = self[i] @@ -92,7 +100,10 @@ def __call__(self, *cmp): pytest.fail(f"Not enough elements were logged looking for {pat}") if op == 'until' and not re.match(pat, x[0], flags=re.S): continue - assert re.match(pat, x[0], flags=re.S) + if not pat: + assert not x[0] + else: + assert re.match(pat, x[0], flags=re.S) if args is not None: assert tuple(x[1]) == tuple(args) break @@ -140,7 +151,7 @@ def __init__(self, installs=[]): self.shebang_can_run_anything = True self.shebang_can_run_anything_silently = False - def get_installs(self): + def get_installs(self, *, include_unmanaged=True, set_default=True): return self.installs def get_install_to_run(self, tag): diff --git a/tests/test_firstrun.py b/tests/test_firstrun.py new file mode 100644 index 0000000..133688c --- /dev/null +++ b/tests/test_firstrun.py @@ -0,0 +1,99 @@ +import os +import pytest + +from pathlib import Path + +from manage import firstrun +from _native import get_current_package, read_alias_package + +def test_get_current_package(): + # The only circumstance where this may be different is if we're running + # tests in a Store install of Python without a virtualenv. That should never + # happen, because we run with 3.14 and there are no more Store installs. + assert get_current_package() is None + + +def test_read_alias_package(): + # Hopefully there's at least one package add with an alias on this machine. + root = Path(os.environ["LocalAppData"]) / "Microsoft/WindowsApps" + if not root.is_dir() or not any(root.rglob("*.exe")): + pytest.skip("Requires an installed app") + for f in root.rglob("*.exe"): + print("Reading package name from", f) + p = read_alias_package(f) + print("Read:", p) + # One is sufficient + return + pytest.skip("Requires an installed app") + + +def fake_package_name(): + return "PythonSoftwareFoundation.PythonManager_m8z88z54g2w36" + + +def fake_package_name_error(): + raise OSError("injected failure") + + +def fake_package_name_none(): + return None + + +def test_check_app_alias(fake_config, monkeypatch): + monkeypatch.setattr(firstrun, "_package_name", fake_package_name) + assert firstrun.check_app_alias(fake_config) in (True, False) + + monkeypatch.setattr(firstrun, "_package_name", fake_package_name_error) + assert firstrun.check_app_alias(fake_config) == "skip" + + monkeypatch.setattr(firstrun, "_package_name", fake_package_name_none) + assert firstrun.check_app_alias(fake_config) == "skip" + + +def test_check_long_paths(fake_config): + assert firstrun.check_long_paths(fake_config) in (True, False) + + +def test_check_py_on_path(fake_config, monkeypatch): + monkeypatch.setattr(firstrun, "_package_name", fake_package_name) + assert firstrun.check_py_on_path(fake_config) in (True, False) + + monkeypatch.setattr(firstrun, "_package_name", fake_package_name_error) + assert firstrun.check_py_on_path(fake_config) == "skip" + + monkeypatch.setattr(firstrun, "_package_name", fake_package_name_none) + assert firstrun.check_py_on_path(fake_config) == "skip" + + +def test_check_global_dir(fake_config, monkeypatch, tmp_path): + fake_config.global_dir = str(tmp_path) + assert firstrun.check_global_dir(fake_config) == False + + monkeypatch.setattr(firstrun, "_check_global_dir_registry", lambda *a: "called") + assert firstrun.check_global_dir(fake_config) == "called" + + monkeypatch.setitem(os.environ, "PATH", f"{os.environ['PATH']};{tmp_path}") + assert firstrun.check_global_dir(fake_config) == True + + +def test_check_global_dir_registry(fake_config, monkeypatch, tmp_path): + fake_config.global_dir = str(tmp_path) + assert firstrun._check_global_dir_registry(fake_config) == False + # Deliberately not going to modify the registry for this test. + # Integration testing will verify that it reads correctly. + + +def test_check_any_install(fake_config): + assert firstrun.check_any_install(fake_config) == False + + fake_config.installs.append("an install") + assert firstrun.check_any_install(fake_config) == True + + +def test_welcome(assert_log): + welcome = firstrun._Welcome() + assert_log(assert_log.end_of_log()) + welcome() + assert_log(".*Welcome.*", "", assert_log.end_of_log()) + welcome() + assert_log(".*Welcome.*", "", assert_log.end_of_log()) From 869461588b1d9bf24b099eb003b5e534bd82768e Mon Sep 17 00:00:00 2001 From: Steve Dower Date: Tue, 13 May 2025 16:04:49 +0100 Subject: [PATCH 05/13] Update src/manage/firstrun.py Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- src/manage/firstrun.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/manage/firstrun.py b/src/manage/firstrun.py index 99602db..9d36b40 100644 --- a/src/manage/firstrun.py +++ b/src/manage/firstrun.py @@ -51,7 +51,7 @@ def check_app_alias(cmd): LOGGER.debug("Reading from %s", exe) package = read_alias_package(exe) LOGGER.debug("Package: %r", package) - if package not in pkg: + if package != pkg: LOGGER.debug("Check failed: package did not match identity") return False except FileNotFoundError: From 930872375aeae24205f45e439732664eb23dc51f Mon Sep 17 00:00:00 2001 From: Steve Dower Date: Tue, 13 May 2025 20:51:32 +0100 Subject: [PATCH 06/13] Increase test coverage --- tests/test_firstrun.py | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/tests/test_firstrun.py b/tests/test_firstrun.py index 133688c..1b1f905 100644 --- a/tests/test_firstrun.py +++ b/tests/test_firstrun.py @@ -54,10 +54,14 @@ def test_check_long_paths(fake_config): assert firstrun.check_long_paths(fake_config) in (True, False) -def test_check_py_on_path(fake_config, monkeypatch): +def test_check_py_on_path(fake_config, monkeypatch, tmp_path): monkeypatch.setattr(firstrun, "_package_name", fake_package_name) + mp = monkeypatch.setitem(os.environ, "PATH", f";{tmp_path};") assert firstrun.check_py_on_path(fake_config) in (True, False) + mp = monkeypatch.setitem(os.environ, "PATH", "") + assert firstrun.check_py_on_path(fake_config) == True + monkeypatch.setattr(firstrun, "_package_name", fake_package_name_error) assert firstrun.check_py_on_path(fake_config) == "skip" @@ -66,13 +70,17 @@ def test_check_py_on_path(fake_config, monkeypatch): def test_check_global_dir(fake_config, monkeypatch, tmp_path): + fake_config.global_dir = None + assert firstrun.check_global_dir(fake_config) == "skip" + fake_config.global_dir = str(tmp_path) assert firstrun.check_global_dir(fake_config) == False monkeypatch.setattr(firstrun, "_check_global_dir_registry", lambda *a: "called") assert firstrun.check_global_dir(fake_config) == "called" - monkeypatch.setitem(os.environ, "PATH", f"{os.environ['PATH']};{tmp_path}") + # Some empty elements, as well as our "real" one + monkeypatch.setitem(os.environ, "PATH", f";;{os.environ['PATH']};{tmp_path}") assert firstrun.check_global_dir(fake_config) == True From e286ec733dc2094ee0f46b181cabc1685367ffdc Mon Sep 17 00:00:00 2001 From: Steve Dower Date: Wed, 14 May 2025 19:53:47 +0100 Subject: [PATCH 07/13] Improved first run and logging --- src/_native/misc.cpp | 2 +- src/manage/firstrun.py | 133 ++++++++++++++++++++++++---------- src/manage/install_command.py | 3 +- src/manage/logging.py | 12 ++- 4 files changed, 108 insertions(+), 42 deletions(-) diff --git a/src/_native/misc.cpp b/src/_native/misc.cpp index 5791977..37b7320 100644 --- a/src/_native/misc.cpp +++ b/src/_native/misc.cpp @@ -127,7 +127,7 @@ PyObject *get_current_package(PyObject *, PyObject *, PyObject *) { int err = GetCurrentPackageFamilyName(&cch, package_name); switch (err) { case ERROR_SUCCESS: - return PyUnicode_FromWideChar(package_name, cch); + return PyUnicode_FromWideChar(package_name, cch ? cch - 1 : 0); case APPMODEL_ERROR_NO_PACKAGE: return Py_GetConstant(Py_CONSTANT_NONE); default: diff --git a/src/manage/firstrun.py b/src/manage/firstrun.py index 9d36b40..8e8e6bb 100644 --- a/src/manage/firstrun.py +++ b/src/manage/firstrun.py @@ -1,5 +1,7 @@ import os import sys +import time + if __name__ == "__main__": __package__ = "manage" @@ -13,9 +15,11 @@ setattr(_native, k, getattr(_native_test, k)) -from .logging import LOGGER +from . import logging from .pathutils import Path +LOGGER = logging.LOGGER + def _package_name(): from _native import get_current_package @@ -50,7 +54,7 @@ def check_app_alias(cmd): try: LOGGER.debug("Reading from %s", exe) package = read_alias_package(exe) - LOGGER.debug("Package: %r", package) + LOGGER.debug("Package: %s", package) if package != pkg: LOGGER.debug("Check failed: package did not match identity") return False @@ -164,7 +168,10 @@ def do_global_dir_on_path(cmd): if p.casefold() == str(cmd.global_dir).casefold(): LOGGER.debug("Path is already found.") return - newpath = initial + (";" if initial else "") + str(Path(cmd.global_dir).absolute()) + newpath = initial.rstrip(";") + if newpath: + newpath += ";" + newpath += str(Path(cmd.global_dir).absolute()) LOGGER.debug("New path: %s", newpath) # Expand the value and ensure we are found for p in os.path.expandvars(newpath).split(";"): @@ -195,10 +202,13 @@ def do_global_dir_on_path(cmd): LOGGER.warn("Failed to notify of PATH environment variable change.") LOGGER.info("You may need to sign out or restart to see the changes.") elif not added: - LOGGER.warn("Failed to update PATH environment variable successfully.") + LOGGER.error("Failed to update PATH environment variable successfully.") LOGGER.info("You may add it yourself by opening 'Edit environment " "variables' and adding this directory to 'PATH': !B!%s!W!", cmd.global_dir) + else: + LOGGER.info("PATH has been updated, and will take effect after " + "opening a new terminal.") def check_any_install(cmd): @@ -214,7 +224,7 @@ def do_install(cmd): from .commands import find_command try: inst_cmd = find_command(["install", "default", "--automatic"], cmd.root) - except Exception as ex: + except Exception: LOGGER.debug("Failed to find 'install' command.", exc_info=True) LOGGER.warn("We couldn't install right now.") LOGGER.info("Use !B!py install default!W! later to install.") @@ -222,7 +232,7 @@ def do_install(cmd): else: try: inst_cmd.execute() - except Exception as ex: + except Exception: LOGGER.debug("Failed to run 'install' command.", exc_info=True) raise @@ -232,9 +242,14 @@ class _Welcome: def __call__(self): if not self._shown: self._shown = True - LOGGER.info("!G!Welcome to the Python installation manager " - "configuration helper.!W!") - LOGGER.info("") + LOGGER.print("!G!Welcome to the Python installation manager " + "configuration helper.!W!") + + +def line_break(): + LOGGER.print() + LOGGER.print("!B!" + "*" * logging.CONSOLE_MAX_WIDTH + "!W!") + LOGGER.print() def first_run(cmd): @@ -245,20 +260,32 @@ def first_run(cmd): if cmd.explicit: welcome() + shown_any = False + if cmd.check_app_alias: r = check_app_alias(cmd) if not r: welcome() - LOGGER.warn("Your app execution alias settings are configured to launch " - "other commands besides 'py' and 'python'.") - LOGGER.info("This can be fixed by opening the '!B!Manage app execution " - "aliases!W!' settings page and enabling each item labelled " - "'!B!Python (default)!W!' and '!B!Python install manager!W!'.") + line_break() + shown_any = True + LOGGER.print("!Y!Your app execution alias settings are configured to launch " + "other commands besides 'py' and 'python'.!W!", + level=logging.WARN) + LOGGER.print("\nThis can be fixed by opening the '!B!Manage app " + "execution aliases!W!' settings page and enabling each " + "item labelled '!B!Python (default)!W!' and '!B!Python " + "install manager!W!'.\n", wrap=True) if ( cmd.confirm and - not cmd.ask_ny("Open Settings now? (Select !B!App execution aliases!W! after opening)") + not cmd.ask_ny("Open Settings now? Select !B!App execution " + "aliases!W! after opening and scroll to the " + "'!B!Python!W!' entries.") ): os.startfile("ms-settings:advanced-apps") + LOGGER.print("\nThe Settings app should be open. Navigate to the " + "!B!App execution aliases!W! page and scroll to the " + "'!B!Python!W!' entries to enable the new commands.", + wrap=True) elif cmd.explicit: if r == "skip": LOGGER.info("Skipped app execution aliases check") @@ -268,18 +295,30 @@ def first_run(cmd): if cmd.check_long_paths: if not check_long_paths(cmd): welcome() - LOGGER.warn("Windows is not configured to allow paths longer than " - "260 characters.") - LOGGER.info("Python and some other apps can bypass this setting, but it " - "requires changing a system-wide setting and a reboot. " - "Some packages may fail to install without long path " - "support enabled.") + line_break() + shown_any = True + LOGGER.print("!Y!Windows is not configured to allow paths longer than " + "260 characters.!W!", level=logging.WARN) + LOGGER.print("\nPython and some other apps can exceed this limit, " + "but it requires changing a system-wide setting and a " + "reboot. Some packages may fail to install without long " + "path support enabled.\n", wrap=True) if ( cmd.confirm and not cmd.ask_ny("Update setting now? You may be prompted for " - "administrator credentials.") + "administrator credentials, and must reboot for " + "the change to take effect.") ): os.startfile(sys.executable, "runas", "**configure-long-paths", show_cmd=0) + for _ in range(5): + time.sleep(0.25) + if check_long_paths(cmd): + LOGGER.info("The setting has been successfully updated.") + break + else: + LOGGER.warn("The setting may not have been updated. Please " + "visit the additional help link at the end for " + "more assistance.") elif cmd.explicit: LOGGER.info("Checked system long paths setting") @@ -287,9 +326,12 @@ def first_run(cmd): r = check_py_on_path(cmd) if not r: welcome() - LOGGER.warn("The legacy 'py' command is still installed.") - LOGGER.info("This may interfere with launching the new 'py' command, " - "and may be resolved by uninstalling '!B!Python launcher!W!'.") + line_break() + shown_any = True + LOGGER.print("!Y!The legacy 'py' command is still installed.!W!", level=logging.WARN) + LOGGER.print("\nThis may interfere with launching the new 'py' " + "command, and may be resolved by uninstalling " + "'!B!Python launcher!W!'.\n", wrap=True) if ( cmd.confirm and not cmd.ask_ny("Open Installed apps now?") @@ -305,14 +347,18 @@ def first_run(cmd): r = check_global_dir(cmd) if not r: welcome() - LOGGER.warn("The directory for versioned Python commands is not configured.") - LOGGER.info("This will prevent commands like !B!python3.14.exe!W! " - "working, but will not affect the !B!python!W! or " - "!B!py!W! commands (for example, !B!py -V:3.14!W!).") - LOGGER.info("We can add the directory to PATH now, but you will need " - "to restart your terminal to see the change, and must " - "manually edit environment variables to later remove the " - "entry.") + line_break() + shown_any = True + LOGGER.print("!Y!The directory for versioned Python commands is not " + "configured.!W!", level=logging.WARN) + LOGGER.print("\nThis will prevent commands like !B!python3.14.exe!W! " + "working, but will not affect the !B!python!W! or " + "!B!py!W! commands (for example, !B!py -V:3.14!W!).", + wrap=True) + LOGGER.print("\nWe can add the directory to PATH now, but you will " + "need to restart your terminal to see the change, and " + "must manually edit environment variables to later " + "remove the entry.\n", wrap=True) if ( cmd.confirm and not cmd.ask_ny("Add commands directory to your PATH now?") @@ -328,17 +374,26 @@ def first_run(cmd): if cmd.check_any_install: if not check_any_install(cmd): welcome() - LOGGER.warn("You do not have any Python runtimes installed.") - LOGGER.info("Install the current latest version of CPython? If not, " - "you can use !B!py install default!W! later to install, or " - "one will be installed automatically when needed.") + line_break() + shown_any = True + LOGGER.print("!Y!You do not have any Python runtimes installed.!W!", + level=logging.WARN) + LOGGER.print("\nInstall the current latest version of CPython? If " + "not, you can use !B!py install default!W! later to " + "install, or one will be installed automatically when " + "needed.\n", wrap=True) + LOGGER.info("") if cmd.ask_yn("Install CPython now?"): do_install(cmd) elif cmd.explicit: LOGGER.info("Checked for any Python installs") - if cmd.explicit: - LOGGER.info("!G!All checks passed.!W!") + if shown_any or cmd.explicit: + line_break() + LOGGER.print("!G!Configuration checks completed.!W!", level=logging.WARN) + LOGGER.print("To run these checks again, launch !B!Python install " + "manager!W! from your Start menu, or !B!py install " + "--configure!W! from the terminal.", wrap=True) if __name__ == "__main__": diff --git a/src/manage/install_command.py b/src/manage/install_command.py index bda3c1e..2fd3fcb 100644 --- a/src/manage/install_command.py +++ b/src/manage/install_command.py @@ -761,7 +761,8 @@ def execute(cmd): LOGGER.info("Skipping shortcut refresh due to --dry-run") else: update_all_shortcuts(cmd) - print_cli_shortcuts(cmd) + if not cmd.automatic: + print_cli_shortcuts(cmd) finally: if cmd.automatic: diff --git a/src/manage/logging.py b/src/manage/logging.py index d7aca56..eeac8bc 100644 --- a/src/manage/logging.py +++ b/src/manage/logging.py @@ -162,7 +162,7 @@ def would_print(self, *args, always=False, level=INFO, **kwargs): return False return True - def print(self, msg=None, *args, always=False, level=INFO, colours=True, **kwargs): + def print(self, msg=None, *args, always=False, level=INFO, colours=True, wrap=False, **kwargs): if self._list is not None: if args: self._list.append(((msg or "") % args, ())) @@ -186,6 +186,16 @@ def print(self, msg=None, *args, always=False, level=INFO, colours=True, **kwarg msg = str(args[0]) else: msg = "" + if wrap: + while len(msg) > CONSOLE_MAX_WIDTH: + part = msg[:CONSOLE_MAX_WIDTH] + n = 0 + while len(part) > n: + n = len(part) + part = msg[:CONSOLE_MAX_WIDTH + 5 * part.count("\033")] + pre, sep, rest = part.rpartition(" ") + print(pre, **kwargs, file=self.print_console) + msg = rest + msg[len(part):] print(msg, **kwargs, file=self.print_console) def print_raw(self, *msg, **kwargs): From 13a7a6171ac67aad9791ff88d9e4d491f76ab8f8 Mon Sep 17 00:00:00 2001 From: Steve Dower Date: Wed, 14 May 2025 19:57:27 +0100 Subject: [PATCH 08/13] Fix test --- tests/test_firstrun.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/test_firstrun.py b/tests/test_firstrun.py index 1b1f905..2195733 100644 --- a/tests/test_firstrun.py +++ b/tests/test_firstrun.py @@ -102,6 +102,6 @@ def test_welcome(assert_log): welcome = firstrun._Welcome() assert_log(assert_log.end_of_log()) welcome() - assert_log(".*Welcome.*", "", assert_log.end_of_log()) + assert_log(".*Welcome.*", assert_log.end_of_log()) welcome() - assert_log(".*Welcome.*", "", assert_log.end_of_log()) + assert_log(".*Welcome.*", assert_log.end_of_log()) From fb98909a6d5cd65689779ee5432e2508ba1852ee Mon Sep 17 00:00:00 2001 From: Steve Dower Date: Wed, 14 May 2025 20:21:33 +0100 Subject: [PATCH 09/13] Clarified text --- src/manage/commands.py | 7 ++++--- src/manage/firstrun.py | 18 +++++++----------- 2 files changed, 11 insertions(+), 14 deletions(-) diff --git a/src/manage/commands.py b/src/manage/commands.py index 90de58e..e49a41a 100644 --- a/src/manage/commands.py +++ b/src/manage/commands.py @@ -896,6 +896,7 @@ class HelpCommand(BaseCommand): """ _create_log_file = False + commands_only = False def __init__(self, args, root=None): super().__init__([self.CMD], root) @@ -906,7 +907,7 @@ def execute(self): self.show_welcome(copyright=False) if not self.args: self.show_usage() - LOGGER.print(BaseCommand.help_text()) + LOGGER.print(BaseCommand.help_text(commands_only)) for a in self.args: try: cls = COMMANDS[a.lower()] @@ -991,8 +992,8 @@ def execute(self): from .firstrun import first_run first_run(self) if not self.explicit: - show_help([]) - if self.confirm and not self.ask_ny(f"View more help online? (!B!{HELP_URL}!W!)"): + self.show_usage() + if self.confirm and not self.ask_ny(f"View online help?"): import os os.startfile(HELP_URL) diff --git a/src/manage/firstrun.py b/src/manage/firstrun.py index 8e8e6bb..942a4cf 100644 --- a/src/manage/firstrun.py +++ b/src/manage/firstrun.py @@ -277,9 +277,8 @@ def first_run(cmd): "install manager!W!'.\n", wrap=True) if ( cmd.confirm and - not cmd.ask_ny("Open Settings now? Select !B!App execution " - "aliases!W! after opening and scroll to the " - "'!B!Python!W!' entries.") + not cmd.ask_ny("Open Settings now, so you can modify !B!App " + "execution aliases!W!?") ): os.startfile("ms-settings:advanced-apps") LOGGER.print("\nThe Settings app should be open. Navigate to the " @@ -300,15 +299,11 @@ def first_run(cmd): LOGGER.print("!Y!Windows is not configured to allow paths longer than " "260 characters.!W!", level=logging.WARN) LOGGER.print("\nPython and some other apps can exceed this limit, " - "but it requires changing a system-wide setting and a " + "but it requires changing a system-wide setting, which " + "may need an administrator to approve, and will require a " "reboot. Some packages may fail to install without long " "path support enabled.\n", wrap=True) - if ( - cmd.confirm and - not cmd.ask_ny("Update setting now? You may be prompted for " - "administrator credentials, and must reboot for " - "the change to take effect.") - ): + if cmd.confirm and not cmd.ask_ny("Update setting now?"): os.startfile(sys.executable, "runas", "**configure-long-paths", show_cmd=0) for _ in range(5): time.sleep(0.25) @@ -391,9 +386,10 @@ def first_run(cmd): if shown_any or cmd.explicit: line_break() LOGGER.print("!G!Configuration checks completed.!W!", level=logging.WARN) - LOGGER.print("To run these checks again, launch !B!Python install " + LOGGER.print("\nTo run these checks again, launch !B!Python install " "manager!W! from your Start menu, or !B!py install " "--configure!W! from the terminal.", wrap=True) + line_break() if __name__ == "__main__": From eea541fa96cdaffae69a7b2a4bb0278c742029dc Mon Sep 17 00:00:00 2001 From: Steve Dower Date: Wed, 14 May 2025 21:46:00 +0100 Subject: [PATCH 10/13] Refactor wrap and indent functionality --- src/manage/commands.py | 26 +++++-------------- src/manage/logging.py | 50 +++++++++++++++++++++++++++-------- tests/test_logging.py | 59 ++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 104 insertions(+), 31 deletions(-) create mode 100644 tests/test_logging.py diff --git a/src/manage/commands.py b/src/manage/commands.py index e49a41a..d53587a 100644 --- a/src/manage/commands.py +++ b/src/manage/commands.py @@ -569,27 +569,13 @@ def show_usage(cls): if usage_ljust % 4: usage_ljust += 4 - (usage_ljust % 4) usage_ljust = max(usage_ljust, 16) + 1 - sp = " " * usage_ljust LOGGER.print("!G!Usage:!W!") for k, d in usage_docs: - if k.endswith("\n") and len(logging.strip_colour(k)) >= usage_ljust: - LOGGER.print(k.rstrip()) - r = sp - else: - k = k.rstrip() - r = k.ljust(usage_ljust + len(k) - len(logging.strip_colour(k))) - for b in d.split(" "): - if len(r) >= logging.CONSOLE_MAX_WIDTH: - LOGGER.print(r.rstrip()) - r = sp - r += b + " " - if r.rstrip(): - LOGGER.print(r) - - LOGGER.print() - LOGGER.print("Find additional information at !B!%s!W!.", HELP_URL) - LOGGER.print() + for s in logging.wrap_and_indent(d, indent=usage_ljust, hang=k.rstrip()): + LOGGER.print(s) + + LOGGER.print("\nFind additional information at !B!%s!W!.\n", HELP_URL) @classmethod def help_text(cls): @@ -907,7 +893,7 @@ def execute(self): self.show_welcome(copyright=False) if not self.args: self.show_usage() - LOGGER.print(BaseCommand.help_text(commands_only)) + LOGGER.print(BaseCommand.help_text()) for a in self.args: try: cls = COMMANDS[a.lower()] @@ -993,7 +979,7 @@ def execute(self): first_run(self) if not self.explicit: self.show_usage() - if self.confirm and not self.ask_ny(f"View online help?"): + if self.confirm and not self.ask_ny("View online help?"): import os os.startfile(HELP_URL) diff --git a/src/manage/logging.py b/src/manage/logging.py index eeac8bc..89648f1 100644 --- a/src/manage/logging.py +++ b/src/manage/logging.py @@ -48,6 +48,40 @@ def strip_colour(msg): return msg +def _len_without_codes(s, codes_subbed=False): + n = len(s) + for k, v in COLOURS.items(): + if not codes_subbed: + n -= len(k) * s.count(k) + n -= len(v) * s.count(v) + return n + + +def wrap_and_indent(s, indent=0, width=None, hang="", codes_subbed=False): + if width is None: + width = CONSOLE_MAX_WIDTH + + bits = [" " * indent] + if hang: + cchw = _len_without_codes(hang, codes_subbed=codes_subbed) + if cchw <= indent - 1: + bits = [hang + " " * (indent - cchw)] + else: + yield hang + cch = indent + for w in s.split(" "): + cchw = _len_without_codes(w, codes_subbed=codes_subbed) + if len(bits) > 1 and cch + cchw > width: + yield "".join(bits).rstrip() + bits = [" " * indent] + cch = indent + bits.append(w) + bits.append(" ") + cch += cchw + 1 + if bits: + yield "".join(bits).rstrip() + + def supports_colour(stream): if os.getenv("PYTHON_COLORS", "").lower() in ("0", "no", "false"): return False @@ -62,7 +96,7 @@ def supports_colour(stream): if type(stream).__name__ != "_WindowsConsoleIO": return False try: - # Allows us to import logging on its own + # Lazy import to allow us to import logging on its own from _native import fd_supports_vt100 return fd_supports_vt100(stream.fileno()) except Exception: @@ -187,16 +221,10 @@ def print(self, msg=None, *args, always=False, level=INFO, colours=True, wrap=Fa else: msg = "" if wrap: - while len(msg) > CONSOLE_MAX_WIDTH: - part = msg[:CONSOLE_MAX_WIDTH] - n = 0 - while len(part) > n: - n = len(part) - part = msg[:CONSOLE_MAX_WIDTH + 5 * part.count("\033")] - pre, sep, rest = part.rpartition(" ") - print(pre, **kwargs, file=self.print_console) - msg = rest + msg[len(part):] - print(msg, **kwargs, file=self.print_console) + for s in wrap_and_indent(msg, codes_subbed=True): + print(s, **kwargs, file=self.print_console) + else: + print(msg, **kwargs, file=self.print_console) def print_raw(self, *msg, **kwargs): kwargs["always"] = True diff --git a/tests/test_logging.py b/tests/test_logging.py new file mode 100644 index 0000000..6dc59ea --- /dev/null +++ b/tests/test_logging.py @@ -0,0 +1,59 @@ +import pytest + +from manage import logging + + +def test_wrap_and_indent(): + r = list(logging.wrap_and_indent("12345678 12345 123 1 123456 1234567890", + width=8)) + assert r == [ + "12345678", + "12345", + "123 1", + "123456", + "1234567890", + ] + + r = list(logging.wrap_and_indent("12345678 12345 123 1 123456 1234567890", + indent=4, width=8)) + assert r == [ + " 12345678", + " 12345", + " 123", + " 1", + " 123456", + " 1234567890", + ] + + r = list(logging.wrap_and_indent("12345678 12345 123 1 123456 1234567890", + indent=4, width=8, hang="AB")) + assert r == [ + "AB 12345678", + " 12345", + " 123", + " 1", + " 123456", + " 1234567890", + ] + + r = list(logging.wrap_and_indent("12345678 12345 123 1 123456 1234567890", + indent=4, width=8, hang="ABC")) + assert r == [ + "ABC 12345678", + " 12345", + " 123", + " 1", + " 123456", + " 1234567890", + ] + + r = list(logging.wrap_and_indent("12345678 12345 123 1 123456 1234567890", + indent=3, width=8, hang="ABCD")) + assert r == [ + "ABC", + " 12345678", + " 12345", + " 123 1", + " 123456", + " 1234567890", + ] From 823a321ef3fd76766769f866e5cf14b09428262f Mon Sep 17 00:00:00 2001 From: Steve Dower Date: Wed, 14 May 2025 22:08:12 +0100 Subject: [PATCH 11/13] Remove extra D --- tests/test_logging.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_logging.py b/tests/test_logging.py index 6dc59ea..c20c805 100644 --- a/tests/test_logging.py +++ b/tests/test_logging.py @@ -48,7 +48,7 @@ def test_wrap_and_indent(): ] r = list(logging.wrap_and_indent("12345678 12345 123 1 123456 1234567890", - indent=3, width=8, hang="ABCD")) + indent=3, width=8, hang="ABC")) assert r == [ "ABC", " 12345678", From 3a68fd30349d90c5a4f9c2fea2ca0c96b300d46e Mon Sep 17 00:00:00 2001 From: Steve Dower Date: Wed, 14 May 2025 23:08:39 +0100 Subject: [PATCH 12/13] Improve test coverage --- scripts/test-firstrun.py | 25 ++++++++ src/manage/firstrun.py | 61 +----------------- tests/conftest.py | 7 ++- tests/test_firstrun.py | 132 +++++++++++++++++++++++++++++++++++++++ 4 files changed, 162 insertions(+), 63 deletions(-) create mode 100644 scripts/test-firstrun.py diff --git a/scripts/test-firstrun.py b/scripts/test-firstrun.py new file mode 100644 index 0000000..968bb0d --- /dev/null +++ b/scripts/test-firstrun.py @@ -0,0 +1,25 @@ +"""Simple script to allow running manage/firstrun.py without rebuilding. + +You'll need to build the test module (_msbuild_test.py). +""" + +import os +import pathlib +import sys + + +ROOT = pathlib.Path(__file__).absolute().parent.parent / "src" +sys.path.append(str(ROOT)) + + +import _native +if not hasattr(_native, "coinitialize"): + import _native_test + for k in dir(_native_test): + if k[:1] not in ("", "_"): + setattr(_native, k, getattr(_native_test, k)) + + +import manage.commands +cmd = manage.commands.FirstRun([], ROOT) +sys.exit(cmd.execute() or 0) diff --git a/src/manage/firstrun.py b/src/manage/firstrun.py index 942a4cf..59846a6 100644 --- a/src/manage/firstrun.py +++ b/src/manage/firstrun.py @@ -2,19 +2,6 @@ import sys import time - -if __name__ == "__main__": - __package__ = "manage" - sys.path.append(os.path.dirname(os.path.dirname(__file__))) - - import _native - if not hasattr(_native, "coinitialize"): - import _native_test - for k in dir(_native_test): - if k[:1] not in ("", "_"): - setattr(_native, k, getattr(_native_test, k)) - - from . import logging from .pathutils import Path @@ -160,7 +147,7 @@ def do_global_dir_on_path(cmd): initial, kind = winreg.QueryValueEx(key, "Path") LOGGER.debug("Initial path: %s", initial) if kind not in (winreg.REG_SZ, winreg.REG_EXPAND_SZ) or not isinstance(initial, str): - LOGGER.debug("Value kind is %s and not REG_[EXPAND_]SZ. Aborting.") + LOGGER.debug("Value kind is %s and not REG_[EXPAND_]SZ. Aborting.", kind) return for p in initial.split(";"): if not p: @@ -390,49 +377,3 @@ def first_run(cmd): "manager!W! from your Start menu, or !B!py install " "--configure!W! from the terminal.", wrap=True) line_break() - - -if __name__ == "__main__": - class TestCommand: - enabled = True - global_dir = Path(os.path.expandvars(r"%LocalAppData%\Python\bin")) - explicit = False - confirm = True - check_app_alias = True - check_long_paths = True - check_py_on_path = True - check_any_install = True - check_global_dir = True - check_default_tag = True - - def get_installs(self, *args, **kwargs): - import json - root = Path(os.path.expandvars(r"%LocalAppData%\Python")) - result = [] - for d in root.iterdir(): - inst = d / "__install__.json" - try: - result.append(json.loads(inst.read_text())) - except FileNotFoundError: - pass - return result - - def _ask(self, fmt, *args, yn_text="Y/n", expect_char="y"): - if not self.confirm: - return True - LOGGER.print(f"{fmt} [{yn_text}] ", *args, end="") - try: - resp = input().casefold() - except Exception: - return False - return not resp or resp.startswith(expect_char.casefold()) - - def ask_yn(self, fmt, *args): - "Returns True if the user selects 'yes' or confirmations are skipped." - return self._ask(fmt, *args) - - def ask_ny(self, fmt, *args): - "Returns True if the user selects 'no' or confirmations are skipped." - return self._ask(fmt, *args, yn_text="y/N", expect_char="n") - - first_run(TestCommand()) diff --git a/tests/conftest.py b/tests/conftest.py index 5858a4a..ecfa53a 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -146,7 +146,8 @@ def localserver(): class FakeConfig: - def __init__(self, installs=[]): + def __init__(self, global_dir, installs=[]): + self.global_dir = global_dir self.installs = list(installs) self.shebang_can_run_anything = True self.shebang_can_run_anything_silently = False @@ -161,8 +162,8 @@ def get_install_to_run(self, tag): @pytest.fixture -def fake_config(): - return FakeConfig() +def fake_config(tmp_path): + return FakeConfig(tmp_path / "bin") REG_TEST_ROOT = r"Software\Python\PyManagerTesting" diff --git a/tests/test_firstrun.py b/tests/test_firstrun.py index 2195733..d98121a 100644 --- a/tests/test_firstrun.py +++ b/tests/test_firstrun.py @@ -1,5 +1,6 @@ import os import pytest +import winreg from pathlib import Path @@ -105,3 +106,134 @@ def test_welcome(assert_log): assert_log(".*Welcome.*", assert_log.end_of_log()) welcome() assert_log(".*Welcome.*", assert_log.end_of_log()) + + + +def test_firstrun_command(monkeypatch): + from manage import commands + + called_first_run = False + called_show_usage = False + + def fake_first_run(*args): + nonlocal called_first_run + called_first_run = True + + def fake_show_usage(*args): + nonlocal called_show_usage + called_show_usage = True + + monkeypatch.setattr(firstrun, "first_run", fake_first_run) + monkeypatch.setattr(commands.FirstRun, "confirm", False) + monkeypatch.setattr(commands.FirstRun, "show_usage", fake_show_usage) + cmd = commands.find_command(["**first_run"], None) + cmd.execute() + assert called_first_run + assert called_show_usage + + +def test_install_configure_command(monkeypatch): + from manage import commands + + called_first_run = False + called_show_usage = False + + def fake_first_run(*args): + nonlocal called_first_run + called_first_run = True + + def fake_show_usage(*args): + nonlocal called_show_usage + called_show_usage = True + + monkeypatch.setattr(firstrun, "first_run", fake_first_run) + monkeypatch.setattr(commands.FirstRun, "confirm", False) + monkeypatch.setattr(commands.FirstRun, "show_usage", fake_show_usage) + cmd = commands.find_command(["install", "--configure"], None) + cmd.execute() + assert called_first_run + assert not called_show_usage + + +def _create_key_read_only(key, subkey, *args, **kwargs): + return winreg.OpenKeyEx(key, subkey) + + +def _raise_oserror(*args, **kwargs): + raise OSError("injected error") + + +@pytest.fixture +def protect_reg(monkeypatch): + import _native + monkeypatch.setattr(winreg, "CreateKeyEx", _create_key_read_only) + monkeypatch.setattr(winreg, "SetValueEx", _raise_oserror) + monkeypatch.setattr(_native, "broadcast_settings_change", lambda *a: None) + + +def test_do_global_dir_open_fail(protect_reg, fake_config, assert_log, monkeypatch): + monkeypatch.setattr(winreg, "OpenKeyEx", _raise_oserror) + firstrun.do_global_dir_on_path(fake_config) + assert_log(assert_log.skip_until("Failed to update PATH.+")) + + +def test_do_global_dir_read_fail(protect_reg, fake_config, assert_log, monkeypatch): + monkeypatch.setattr(winreg, "QueryValueEx", _raise_oserror) + firstrun.do_global_dir_on_path(fake_config) + assert_log(assert_log.skip_until("Failed to update PATH.+")) + + +def test_do_global_dir_read_kind_fail(protect_reg, fake_config, assert_log, monkeypatch): + monkeypatch.setattr(winreg, "QueryValueEx", lambda *a: (100, winreg.REG_DWORD)) + firstrun.do_global_dir_on_path(fake_config) + assert_log( + assert_log.skip_until("Initial path: %s", (100,)), + ("Value kind is %s.+", (winreg.REG_DWORD,)), + ) + + +def test_do_global_dir_path_already_set(protect_reg, fake_config, assert_log, monkeypatch): + monkeypatch.setattr(winreg, "QueryValueEx", lambda *a: (f"{fake_config.global_dir};b;c", winreg.REG_SZ)) + firstrun.do_global_dir_on_path(fake_config) + assert_log(assert_log.skip_until("Path is already found")) + + monkeypatch.setattr(winreg, "QueryValueEx", lambda *a: (f"a;{fake_config.global_dir};c", winreg.REG_SZ)) + firstrun.do_global_dir_on_path(fake_config) + assert_log(assert_log.skip_until("Path is already found")) + + monkeypatch.setattr(winreg, "QueryValueEx", lambda *a: (f"a;b;{fake_config.global_dir}", winreg.REG_SZ)) + firstrun.do_global_dir_on_path(fake_config) + assert_log(assert_log.skip_until("Path is already found")) + + +def test_do_global_dir_path_lost_race(protect_reg, fake_config, assert_log, monkeypatch): + paths = ["a;b", "a;b;c"] + monkeypatch.setattr(winreg, "QueryValueEx", lambda *a: (paths.pop(), winreg.REG_SZ)) + firstrun.do_global_dir_on_path(fake_config) + assert_log( + assert_log.skip_until("New path: %s", None), + "Path is added successfully", + "PATH has changed.+", + ) + + +def test_do_global_dir_write_same_kind(protect_reg, fake_config, monkeypatch): + saved = [] + monkeypatch.setattr(winreg, "SetValueEx", lambda *a: saved.append(a)) + + monkeypatch.setattr(winreg, "QueryValueEx", lambda *a: ("a;", winreg.REG_SZ)) + firstrun.do_global_dir_on_path(fake_config) + assert saved[-1][1:] == ("Path", 0, winreg.REG_SZ, f"a;{fake_config.global_dir}") + + monkeypatch.setattr(winreg, "QueryValueEx", lambda *a: ("a", winreg.REG_EXPAND_SZ)) + firstrun.do_global_dir_on_path(fake_config) + assert saved[-1][1:] == ("Path", 0, winreg.REG_EXPAND_SZ, f"a;{fake_config.global_dir}") + + +def test_do_global_dir_path_fail_broadcast(protect_reg, fake_config, assert_log, monkeypatch): + import _native + monkeypatch.setattr(_native, "broadcast_settings_change", _raise_oserror) + monkeypatch.setattr(winreg, "QueryValueEx", lambda *a: ("a;", winreg.REG_SZ)) + monkeypatch.setattr(winreg, "SetValueEx", lambda *a: None) + firstrun.do_global_dir_on_path(fake_config) + assert_log(assert_log.skip_until("Failed to notify of PATH environment.+")) From 1071d0274bbd7834c9470d59ae56d25df1b088fe Mon Sep 17 00:00:00 2001 From: Steve Dower Date: Wed, 14 May 2025 23:35:40 +0100 Subject: [PATCH 13/13] Remove unused option --- src/manage/commands.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/manage/commands.py b/src/manage/commands.py index d53587a..7fdd101 100644 --- a/src/manage/commands.py +++ b/src/manage/commands.py @@ -260,7 +260,6 @@ def execute(self): "check_long_paths": (config_bool, None, "env"), "check_py_on_path": (config_bool, None, "env"), "check_any_install": (config_bool, None, "env"), - "check_default_tag": (config_bool, None, "env"), "check_global_dir": (config_bool, None, "env"), }, @@ -969,7 +968,6 @@ class FirstRun(BaseCommand): check_long_paths = True check_py_on_path = True check_any_install = True - check_default_tag = True check_global_dir = True def execute(self):