Repository: pypa/pip
Pull request: Don't fail uninstallation if easy-install.pth doesn't exist
Fixes and Closes https://github.com/pypa/pip/issues/7856

I have moved the `raise UninstallationError` exception from the constructor of `UninstallPathEntries` to the start of the `remove` method, and I am logging a warning and returning from the `remove` method if the file does not exist

Problem statement:
Cannot remove entries from nonexistent file
* pip version: 20.0.2
* Python version: 3.7.6
* Operating system: OSX

# Problem

It appears that when attempting to uninstall a package that was installed in editable mode, removing it does not work correctly if the `lib/python3.7/site-packages/easy-install.pth` file does not exist.

# Reproduction

1. In a new folder, create a `setup.py` file:

```python
from setuptools import setup

setup(
    name='repro',
    version='0.1dev',
)
```

1. Create a virtualenv: `python3 -m venv env`
1. Install in editable mode: `env/bin/pip install -e .`
1. Manually remove the file: `rm env/lib/python3.7/site-packages/easy-install.pth`
1. Attempt to uninstall the editable package: `env/bin/pip uninstall repro`
1. Observe error message: `ERROR: Cannot remove entries from nonexistent file /..../env/lib/python3.7/site-packages/easy-install.pth`

# Expected Behavior

The uninstall should complete successfully, removing the `.egg-link`, even if the `easy-install.pth` file is not found.

# Notes

In the wild, there is something that happens with our internal setup that causes the `egg-link`, to be created, but not the `easy-install.pth` file. I'm still trying to track down how this happens. Regardless of this, I believe that `pip` should not fail to uninstall when the file `easy-install.pth` doesn't exist, as the operation it would have done there would have just removed the entry from the file anyway.

I am happy to provide any further detail if needed. Thanks! :)

Hints from issue discussion:
Sounds reasonable to me. The error message can continue to exist, but the uninstallation command does not need to fail. (It would help if pip has a mechanism to fail with a specific error code, but if the choice is between 1 and 0, I would prefer 0.)
I can take this issue up and create the PR, but I have a question about the type of changes we want here.

If I understand correctly, we add `easy-install.pth` in files to remove here: 
https://github.com/pypa/pip/blob/f6b5b40549583b65253b2de5a8ebfc3bf32cb3c1/src/pip/_internal/req/req_uninstall.py#L532

So in order to fix the issue, do we want to stop checking for easy-install.pth here? Or do we check for easy-install.pth but not raise an exception if it isn't found at 

https://github.com/pypa/pip/blob/f6b5b40549583b65253b2de5a8ebfc3bf32cb3c1/src/pip/_internal/req/req_uninstall.py#L588-L592

We would still want to remove it if possible (and to emit an error message if it does not exist), so I’d say the latter. I did not read very deep into the implementation though, so there might be another way.
Hi @uranusjr,

Okay, so do we want to  emit the error message, but not cause the installation to fail by perhaps wrapping https://github.com/pypa/pip/blob/f6b5b40549583b65253b2de5a8ebfc3bf32cb3c1/src/pip/_internal/req/req_uninstall.py#L532
in a try catch?

Or instead of raising an error at https://github.com/pypa/pip/blob/f6b5b40549583b65253b2de5a8ebfc3bf32cb3c1/src/pip/_internal/req/req_uninstall.py#L588-L592
we can raise a warning using the warnings module, or just log the error and continue
Hi @uranusjr ,

If my above suggestions make sense now, I can go ahead and create a PR for the same
I like the second approach more; it’s probably enought to `logger.warning` the message and move on.
Thanks @uranusjr for the response. I will create a PR to do the same.

Commit message: Check for uninstalled package after deleting pth file

Patch to review:
diff --git a/news/7856.bugfix b/news/7856.bugfix
new file mode 100644
index 00000000000..de1c264a46b
--- /dev/null
+++ b/news/7856.bugfix
@@ -0,0 +1 @@
+Uninstall should complete successfully, removing the .egg-link, even if the easy-install.pth file is not found.
diff --git a/src/pip/_internal/req/req_uninstall.py b/src/pip/_internal/req/req_uninstall.py
index ec9da2c3484..559061a6296 100644
--- a/src/pip/_internal/req/req_uninstall.py
+++ b/src/pip/_internal/req/req_uninstall.py
@@ -585,11 +585,6 @@ def from_dist(cls, dist):
 class UninstallPthEntries(object):
     def __init__(self, pth_file):
         # type: (str) -> None
-        if not os.path.isfile(pth_file):
-            raise UninstallationError(
-                "Cannot remove entries from nonexistent file {}".format(
-                    pth_file)
-            )
         self.file = pth_file
         self.entries = set()  # type: Set[str]
         self._saved_lines = None  # type: Optional[List[bytes]]
@@ -613,6 +608,14 @@ def add(self, entry):
     def remove(self):
         # type: () -> None
         logger.debug('Removing pth entries from %s:', self.file)
+
+        # If the file doesn't exist, log a warning and return
+        if not os.path.isfile(self.file):
+            logger.warning(
+                "Cannot remove entries from nonexistent file {}".format(
+                    self.file)
+            )
+            return
         with open(self.file, 'rb') as fh:
             # windows uses '\r\n' with py3k, but uses '\n' with py2.x
             lines = fh.readlines()
diff --git a/tests/functional/test_uninstall.py b/tests/functional/test_uninstall.py
index 04d8550f32a..b049d803703 100644
--- a/tests/functional/test_uninstall.py
+++ b/tests/functional/test_uninstall.py
@@ -553,6 +553,49 @@ def test_uninstall_editable_and_pip_install(script, data):
     assert "FSPkg" not in {p["name"] for p in json.loads(list_result2.stdout)}
 
 
+def test_uninstall_editable_and_pip_install_easy_install_remove(script, data):
+    """Try uninstall after pip install -e after pip install
+    and removing easy-install.pth"""
+    # SETUPTOOLS_SYS_PATH_TECHNIQUE=raw removes the assumption that `-e`
+    # installs are always higher priority than regular installs.
+    # This becomes the default behavior in setuptools 25.
+    script.environ['SETUPTOOLS_SYS_PATH_TECHNIQUE'] = 'raw'
+
+    # Rename easy-install.pth to pip-test.pth
+    easy_install_pth = join(script.site_packages_path, 'easy-install.pth')
+    pip_test_pth = join(script.site_packages_path, 'pip-test.pth')
+    os.rename(easy_install_pth, pip_test_pth)
+
+    # Install FSPkg
+    pkg_path = data.packages.joinpath("FSPkg")
+    script.pip('install', '-e', '.',
+               expect_stderr=True, cwd=pkg_path)
+
+    # Rename easy-install.pth to pip-test-fspkg.pth
+    pip_test_fspkg_pth = join(script.site_packages_path, 'pip-test-fspkg.pth')
+    os.rename(easy_install_pth, pip_test_fspkg_pth)
+
+    # Confirm that FSPkg is installed
+    list_result = script.pip('list', '--format=json')
+    assert {"name": "FSPkg", "version": "0.1.dev0"} \
+        in json.loads(list_result.stdout)
+
+    # Remove pip-test-fspkg.pth
+    os.remove(pip_test_fspkg_pth)
+
+    # Uninstall will fail with given warning
+    uninstall = script.pip('uninstall', 'FSPkg', '-y')
+    assert "Cannot remove entries from nonexistent file" in uninstall.stderr
+
+    # Confirm that FSPkg is uninstalled
+    list_result = script.pip('list', '--format=json')
+    assert {"name": "FSPkg", "version": "0.1.dev0"} \
+        not in json.loads(list_result.stdout)
+
+    # Rename pip-test.pth back to easy-install.pth
+    os.rename(pip_test_pth, easy_install_pth)
+
+
 def test_uninstall_ignores_missing_packages(script, data):
     """Uninstall of a non existent package prints a warning and exits cleanly
     """


Code review comments:
diff_hunk: @@ -0,0 +1 @@
+Uninstall should complete successfully, removing the .egg-link, even if the easy-install.pth file is not found.
path: news/7856.bugfix
text: @user1:
Do we need to add an assertion to make sure `.egg-link` is indeed removed?

Since the fix is more general than just dealing with `easy-install.pth` would probably be less specific in the message. Something like *uninstallation no longer fails on trying to remove non-existent files*.

@author:
That makes sense, Let me fix the message quickly

@author:
Hi @user1 ,

Yes I think we can add that, since `easy-install.pth` and `.egg-link` are the two files we remove while `uninstall`. Can we check for `.egg-link` using the approach as below?

https://github.com/pypa/pip/blob/019637cc67039750ba622aa8e842cf1ea8a65a13/tests/functional/test_uninstall.py#L548-L551

@author:
Okay, I just added that line and the assertion passes successfully, so I can add this assertion to my test

@author:
Hi @user1,

I have made a commit to address the removal of `egg-link` after installation. Please take a look and let me know if that makes sense :)