From de9dd7fa5463e84142fde95f112897982f589159 Mon Sep 17 00:00:00 2001 From: Chad Smith Date: Sat, 7 Sep 2019 10:10:11 -0700 Subject: [PATCH] fix bare exception if symlink existed and pointed to invalid path (#215) --- pipx/commands.py | 10 +++++++++- pipx/main.py | 6 +++--- tests/test_pipx.py | 21 ++++++++++++++++++++- 3 files changed, 32 insertions(+), 5 deletions(-) diff --git a/pipx/commands.py b/pipx/commands.py index 47dada9d1c..77c58178e2 100644 --- a/pipx/commands.py +++ b/pipx/commands.py @@ -546,7 +546,9 @@ def _symlink_package_apps( except IsADirectoryError: rmdir(symlink_path) - if symlink_path.exists(): + exists = symlink_path.exists() + is_symlink = symlink_path.is_symlink() + if exists: if symlink_path.samefile(app_path): logging.info(f"Same path {str(symlink_path)} and {str(app_path)}") else: @@ -555,6 +557,12 @@ def _symlink_package_apps( f"to {symlink_path.resolve()}, not {str(app_path)}. Not modifying." ) continue + if is_symlink and not exists: + logging.info( + f"Removing existing symlink {str(symlink_path)} since it " + "pointed non-existent location" + ) + symlink_path.unlink() existing_executable_on_path = which(app_name) symlink_path.symlink_to(app_path) diff --git a/pipx/main.py b/pipx/main.py index 7624648956..129a182ad1 100644 --- a/pipx/main.py +++ b/pipx/main.py @@ -92,7 +92,7 @@ def _split_lines(self, text, width): return textwrap.wrap(text, width) -def get_pip_args(parsed_args: Dict): +def get_pip_args(parsed_args: Dict) -> List[str]: pip_args: List[str] = [] if parsed_args.get("index_url"): pip_args += ["--index-url", parsed_args["index_url"]] @@ -105,14 +105,14 @@ def get_pip_args(parsed_args: Dict): return pip_args -def get_venv_args(parsed_args: Dict): +def get_venv_args(parsed_args: Dict) -> List[str]: venv_args: List[str] = [] if parsed_args.get("system_site_packages"): venv_args += ["--system-site-packages"] return venv_args -def run_pipx_command(args): +def run_pipx_command(args): # noqa: C901 setup(args) verbose = args.verbose if "verbose" in args else False pip_args = get_pip_args(vars(args)) diff --git a/tests/test_pipx.py b/tests/test_pipx.py index 05cd520969..200ff2eba7 100644 --- a/tests/test_pipx.py +++ b/tests/test_pipx.py @@ -259,7 +259,7 @@ def test_run_downloads_from_internet(self): check=True, ) - def test_symlink_points_to_wrong_location_warning(self): + def test_existing_symlink_points_to_existing_wrong_location_warning(self): self.bin_dir.mkdir(exist_ok=True, parents=True) (self.bin_dir / "pycowsay").symlink_to("/dev/null") @@ -279,6 +279,25 @@ def test_symlink_points_to_wrong_location_warning(self): # pointed to the wrong location) self.assertTrue("is not on your PATH environment variable" not in stderr) + def test_existing_symlink_points_to_nothing(self): + self.bin_dir.mkdir(exist_ok=True, parents=True) + (self.bin_dir / "pycowsay").symlink_to("/asdf/jkl") + + env = os.environ.copy() + env["PATH"] = f"{str(self.bin_dir)}:{env.get('PATH')}" + ret = subprocess.run( + [self.pipx_bin, "install", "pycowsay"], + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + env=env, + check=True, + ) + stdout = ret.stdout.decode() + self.assertTrue("These apps are now globally available" in stdout) + self.assertTrue( + "symlink missing or pointing to unexpected location" not in stdout + ) + def test_path_warning(self): # warning should appear since temp directory is not on PATH self.assertTrue(