diff --git a/menuinst/cli/__main__.py b/menuinst/__main__.py similarity index 100% rename from menuinst/cli/__main__.py rename to menuinst/__main__.py diff --git a/menuinst/api.py b/menuinst/api.py index 930f687d..b711b107 100644 --- a/menuinst/api.py +++ b/menuinst/api.py @@ -11,7 +11,13 @@ from typing import Callable, Union from .platforms import Menu, MenuItem -from .utils import DEFAULT_BASE_PREFIX, DEFAULT_PREFIX, _UserOrSystem, elevate_as_needed +from .utils import ( + DEFAULT_BASE_PREFIX, + DEFAULT_PREFIX, + _UserOrSystem, + elevate_as_needed, + user_is_admin, +) log = getLogger(__name__) @@ -24,8 +30,16 @@ ] +def _maybe_try_user(base_prefix: str, target_prefix: str) -> bool: + if not user_is_admin(): + return False + if Path(target_prefix, ".nonadmin").is_file(): + return True + return Path(base_prefix, ".nonadmin").is_file() + + def _load( - metadata_or_path: Union[os.PathLike, dict], + metadata_or_path: os.PathLike | dict, target_prefix: str | None = None, base_prefix: str | None = None, _mode: _UserOrSystem = "user", @@ -85,6 +99,12 @@ def remove( paths += menu_item.remove() paths += menu.remove() + if not paths and _maybe_try_user(target_prefix, base_prefix): + menu, menu_items = _load(metadata_or_path, target_prefix, base_prefix, "user") + for menu_item in menu_items: + paths += menu_item.remove() + paths += menu.remove() + return paths diff --git a/menuinst/platforms/linux.py b/menuinst/platforms/linux.py index 7d018616..3fcfe43f 100644 --- a/menuinst/platforms/linux.py +++ b/menuinst/platforms/linux.py @@ -66,13 +66,17 @@ def create(self) -> tuple[os.PathLike]: return (path,) def remove(self) -> tuple[os.PathLike]: + if not Path(self.desktop_entries_location).exists(): + return tuple() for fn in os.listdir(self.desktop_entries_location): if fn.startswith(f"{self.render(self.name, slug=True)}_"): # found one shortcut, so don't remove the name from menu - return (self.directory_entry_location,) - unlink(self.directory_entry_location, missing_ok=True) + return tuple() self._remove_this_menu() - return (self.directory_entry_location,) + if self.directory_entry_location.exists(): + unlink(self.directory_entry_location, missing_ok=True) + return (self.directory_entry_location,) + return tuple() @property def placeholders(self) -> dict[str, str]: @@ -112,6 +116,8 @@ def _write_directory_entry(self) -> Path: # def _remove_this_menu(self): + if not Path(self.menu_config_location).exists(): + return log.debug( "Editing %s to remove %s config", self.menu_config_location, self.render(self.name) ) @@ -201,12 +207,13 @@ def create(self) -> Iterable[os.PathLike]: return self._paths() def remove(self) -> Iterable[os.PathLike]: - paths = self._paths() + paths = (path for path in self._paths() if Path(path).is_file()) self._maybe_register_mime_types(register=False) - for path in paths: - log.debug("Removing %s", path) - unlink(path, missing_ok=True) - self._update_desktop_database() + if paths: + for path in paths: + log.debug("Removing %s", path) + unlink(path) + self._update_desktop_database() return paths def _update_desktop_database(self): diff --git a/menuinst/platforms/osx.py b/menuinst/platforms/osx.py index e4deae49..8b24df44 100644 --- a/menuinst/platforms/osx.py +++ b/menuinst/platforms/osx.py @@ -100,8 +100,10 @@ def create(self) -> tuple[Path]: def remove(self) -> tuple[Path]: log.debug("Removing %s", self.location) self._maybe_register_with_launchservices(register=False) - shutil.rmtree(self.location, ignore_errors=True) - return (self.location,) + if self.location.exists(): + shutil.rmtree(self.location, ignore_errors=True) + return (self.location,) + return tuple() def _create_application_tree(self) -> tuple[Path]: paths = [ diff --git a/menuinst/platforms/win.py b/menuinst/platforms/win.py index 3ff23335..b15ab29b 100644 --- a/menuinst/platforms/win.py +++ b/menuinst/platforms/win.py @@ -48,7 +48,8 @@ def remove(self) -> tuple[Path]: except StopIteration: log.debug("Removing %s", self.start_menu_location) shutil.rmtree(self.start_menu_location, ignore_errors=True) - return (self.start_menu_location,) + return (self.start_menu_location,) + return tuple() @property def start_menu_location(self) -> Path: @@ -214,10 +215,10 @@ def remove(self) -> tuple[Path, ...]: for location in self.menu.terminal_profile_locations: self._add_remove_windows_terminal_profile(location, remove=True) - paths = self._paths() + paths = tuple(path for path in self._paths() if Path(path).is_file()) for path in paths: log.debug("Removing %s", path) - unlink(path, missing_ok=True) + unlink(path) return paths diff --git a/menuinst/utils.py b/menuinst/utils.py index 3431138c..cb7826bc 100644 --- a/menuinst/utils.py +++ b/menuinst/utils.py @@ -280,8 +280,7 @@ def needs_admin(target_prefix: os.PathLike, base_prefix: os.PathLike) -> bool: if os.name == "nt": # Absence of $base_prefix/.nonadmin in Windows means we need admin permissions return True - - if os.name == "posix": + elif os.name == "posix": # Absence of $base_prefix/.nonadmin in Linux, macOS and other posix systems # has no meaning for historic reasons, so let's try to see if we can # write to the installation root @@ -292,6 +291,8 @@ def needs_admin(target_prefix: os.PathLike, base_prefix: os.PathLike) -> bool: return True else: return False + else: + raise RuntimeError(f"Unsupported operating system: {os.name}") @lru_cache(maxsize=1) @@ -365,8 +366,8 @@ def elevate_as_needed(func: Callable) -> Callable: @wraps(func) def wrapper_elevate( *args, - target_prefix: os.PathLike = None, - base_prefix: os.PathLike = None, + target_prefix: os.PathLike | None = None, + base_prefix: os.PathLike | None = None, **kwargs, ): kwargs.pop("_mode", None) diff --git a/news/414-fix-user-system-inconsistency b/news/414-fix-user-system-inconsistency new file mode 100644 index 00000000..064cf431 --- /dev/null +++ b/news/414-fix-user-system-inconsistency @@ -0,0 +1,21 @@ +### Enhancements + +* + +### Bug fixes + +* When uninstalling shortcuts as administrator, check user locations if no system shortcuts have + been found and prefixes contain `.nonadmin` files. This allows removing single-user shortcuts + as an administrator. (#341 via #414) + +### Deprecations + +* + +### Docs + +* + +### Other + +* diff --git a/tests/conftest.py b/tests/conftest.py index 0e420cb1..062b94a0 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -63,25 +63,29 @@ def mock_locations(monkeypatch, tmp_path): from menuinst.platforms.osx import MacOSMenuItem if os.name == "nt": + from menuinst.platforms import win as win_platform from menuinst.platforms.win_utils import knownfolders def windows_locations(preferred_mode, check_other_mode, key): - return tmp_path / key + return tmp_path / preferred_mode / key monkeypatch.setattr(knownfolders, "folder_path", windows_locations) + monkeypatch.setattr(win_platform, "windows_folder_path", windows_locations) def osx_base_location(self): - return tmp_path + if self.menu.mode == "user": + return tmp_path / "user" + return tmp_path / "system" if not os.environ.get("CI"): monkeypatch.setattr(MacOSMenuItem, "_base_location", osx_base_location) # For Linux if not os.environ.get("CI"): - monkeypatch.setattr(LinuxMenu, "_system_config_directory", tmp_path / "config") - monkeypatch.setattr(LinuxMenu, "_system_data_directory", tmp_path / "data") - monkeypatch.setenv("XDG_CONFIG_HOME", str(tmp_path / "config")) - monkeypatch.setenv("XDG_DATA_HOME", str(tmp_path / "data")) + monkeypatch.setattr(LinuxMenu, "_system_config_directory", tmp_path / "system" / "config") + monkeypatch.setattr(LinuxMenu, "_system_data_directory", tmp_path / "system" / "data") + monkeypatch.setenv("XDG_CONFIG_HOME", str(tmp_path / "user" / "config")) + monkeypatch.setenv("XDG_DATA_HOME", str(tmp_path / "user" / "data")) @pytest.fixture() diff --git a/tests/test_api.py b/tests/test_api.py index 38a12c7c..3f95a1e0 100644 --- a/tests/test_api.py +++ b/tests/test_api.py @@ -19,7 +19,7 @@ from menuinst.api import install, remove from menuinst.platforms import MenuItem from menuinst.platforms.osx import _lsregister -from menuinst.utils import DEFAULT_PREFIX, logged_run, slugify +from menuinst.utils import DEFAULT_PREFIX, logged_run, slugify, user_is_admin def _poll_for_file_contents(path, timeout=30): @@ -170,6 +170,27 @@ def test_install_remove(tmp_path, delete_files): assert files_found == set() +def test_remove_for_user_as_admin(tmp_path, delete_files, monkeypatch): + from menuinst import api as menuinst_api + from menuinst import utils as menuinst_utils + + metadata = DATA / "jsons" / "sys-prefix.json" + # Ensure that we install as user + monkeypatch.setattr(menuinst_utils, "user_is_admin", lambda: False) + (tmp_path / ".nonadmin").touch() + paths = set(install(metadata, target_prefix=tmp_path, base_prefix=tmp_path)) + delete_files.extend(paths) + files_found = set(filter(lambda x: x.exists(), paths)) + assert files_found == paths + + # Ensure that menuinst thinks we uninstall as admin + monkeypatch.setattr(menuinst_utils, "user_is_admin", lambda: True) + monkeypatch.setattr(menuinst_api, "user_is_admin", lambda: True) + remove(metadata, target_prefix=tmp_path, base_prefix=tmp_path) + files_found = set(filter(lambda x: x.exists(), paths)) + assert files_found == set() + + def test_overwrite_existing_shortcuts(delete_files, caplog): """Test that overwriting shortcuts works without errors by running installation twice.""" check_output_from_shortcut( @@ -195,7 +216,7 @@ def test_overwrite_existing_shortcuts(delete_files, caplog): @pytest.mark.skipif(PLATFORM == "osx", reason="No menu names on MacOS") -def test_placeholders_in_menu_name(delete_files): +def test_placeholders_in_menu_name(tmp_path, delete_files): _, paths, tmp_base_path, _ = check_output_from_shortcut( delete_files, "sys-prefix.json", @@ -204,17 +225,23 @@ def test_placeholders_in_menu_name(delete_files): ) if PLATFORM == "win": for path in paths: - if path.suffix == ".lnk" and "Start Menu" in path.parts: + if path.suffix == ".lnk" and path.parent.parent.name == "start": assert path.parent.name == f"Sys.Prefix {Path(tmp_base_path).name}" break else: raise AssertionError("Didn't find Start Menu") elif PLATFORM == "linux": - config_directory = Path(os.environ.get("XDG_CONFIG_HOME", "~/.config")).expanduser() - desktop_directory = ( - Path(os.environ.get("XDG_DATA_HOME", "~/.local/share")).expanduser() - / "desktop-directories" - ) + if user_is_admin(): + if os.environ.get("CI"): + config_directory = Path("/etc/xdg") + data_directory = Path("/usr/share") + else: + config_directory = tmp_path / "system" / "config" + data_directory = tmp_path / "system" / "data" + else: + config_directory = Path(os.environ.get("XDG_CONFIG_HOME", "~/.config")).expanduser() + data_directory = Path(os.environ.get("XDG_DATA_HOME", "~/.local/share")).expanduser() + desktop_directory = data_directory / "desktop-directories" menu_config_location = ( config_directory / "menus" @@ -392,7 +419,7 @@ def test_url_protocol_association(delete_files): @pytest.mark.skipif(PLATFORM != "win", reason="Windows only") def test_windows_terminal_profiles(tmp_path, run_as_user): settings_file = Path( - tmp_path, "localappdata", "Microsoft", "Windows Terminal", "settings.json" + tmp_path, "user", "localappdata", "Microsoft", "Windows Terminal", "settings.json" ) settings_file.parent.mkdir(parents=True) (tmp_path / ".nonadmin").touch()