fix bare exception if symlink existed and pointed to invalid path (py…

…pa#215)

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)

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))

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(