use pathlib to resolve symlinks

4 files changed, 52 insertions, 66 deletions

diff --git a/CHANGES.rst b/CHANGES.rst
index 55578c6..c52c160 100644
--- a/CHANGES.rst
+++ b/CHANGES.rst
@@ -1,5 +1,14 @@
 .. currentmodule:: click
 
+Version 8.0.3
+-------------
+
+Unreleased
+
+-   Fix issue with ``Path(resolve_path=True)`` type creating invalid
+    paths. :issue:`2088`
+
+
 Version 8.0.2
 -------------
 
diff --git a/src/click/types.py b/src/click/types.py
index a7de43b..103d218 100644
--- a/src/click/types.py
+++ b/src/click/types.py
@@ -836,20 +836,11 @@ class Path(ParamType):
 
         if not is_dash:
             if self.resolve_path:
-                # Get the absolute directory containing the path.
-                dir_ = os.path.dirname(os.path.abspath(rv))
-
-                # Resolve a symlink. realpath on Windows Python < 3.9
-                # doesn't resolve symlinks. This might return a relative
-                # path even if the path to the link is absolute.
-                if os.path.islink(rv):
-                    rv = os.readlink(rv)
-
-                # Join dir_ with the resolved symlink if the resolved
-                # path is relative. This will make it relative to the
-                # original containing directory.
-                if not os.path.isabs(rv):
-                    rv = os.path.join(dir_, rv)
+                # os.path.realpath doesn't resolve symlinks on Windows
+                # until Python 3.8. Use pathlib for now.
+                import pathlib
+
+                rv = os.fsdecode(pathlib.Path(rv).resolve())
 
             try:
                 st = os.stat(rv)
diff --git a/tests/conftest.py b/tests/conftest.py
index d33df11..a71db7d 100644
--- a/tests/conftest.py
+++ b/tests/conftest.py
@@ -1,5 +1,4 @@
 import os
-import shutil
 import tempfile
 
 import pytest
@@ -12,20 +11,17 @@ def runner(request):
     return CliRunner()
 
 
-def check_symlink_impl():
-    """This function checks if using symlinks is allowed
-    on the host machine"""
-    tempdir = tempfile.mkdtemp(prefix="click-")
-    test_pth = os.path.join(tempdir, "check_sym_impl")
-    sym_pth = os.path.join(tempdir, "link")
-    open(test_pth, "w").close()
-    rv = True
-    try:
-        os.symlink(test_pth, sym_pth)
-    except (NotImplementedError, OSError):
-        # Creating symlinks on Windows require elevated access.
-        # OSError is thrown if the function is called without it.
-        rv = False
-    finally:
-        shutil.rmtree(tempdir, ignore_errors=True)
-    return rv
+def _check_symlinks_supported():
+    with tempfile.TemporaryDirectory(prefix="click-pytest-") as tempdir:
+        target = os.path.join(tempdir, "target")
+        open(target, "w").close()
+        link = os.path.join(tempdir, "link")
+
+        try:
+            os.symlink(target, link)
+            return True
+        except OSError:
+            return False
+
+
+symlinks_supported = _check_symlinks_supported()
diff --git a/tests/test_types.py b/tests/test_types.py
index dff8172..df44d8a 100644
--- a/tests/test_types.py
+++ b/tests/test_types.py
@@ -2,7 +2,7 @@ import os.path
 import pathlib
 
 import pytest
-from conftest import check_symlink_impl
+from conftest import symlinks_supported
 
 import click
 
@@ -104,37 +104,27 @@ def test_path_type(runner, cls, expect):
     assert result.return_value == expect
 
 
-@pytest.mark.skipif(not check_symlink_impl(), reason="symlink not allowed on device")
-@pytest.mark.parametrize(
-    ("sym_file", "abs_fun"),
-    [
-        (("relative_symlink",), os.path.basename),
-        (("test", "absolute_symlink"), lambda x: x),
-    ],
+@pytest.mark.skipif(
+    not symlinks_supported, reason="The current OS or FS doesn't support symlinks."
 )
-def test_symlink_resolution(tmpdir, sym_file, abs_fun):
-    """This test ensures symlinks are properly resolved by click"""
-    tempdir = str(tmpdir)
-    real_path = os.path.join(tempdir, "test_file")
-    sym_path = os.path.join(tempdir, *sym_file)
-
-    # create dirs and files
-    os.makedirs(os.path.join(tempdir, "test"), exist_ok=True)
-    open(real_path, "w").close()
-    os.symlink(abs_fun(real_path), sym_path)
-
-    # test
-    ctx = click.Context(click.Command("do_stuff"))
-    rv = click.Path(resolve_path=True).convert(sym_path, None, ctx)
-
-    # os.readlink prepends path prefixes to absolute
-    # links in windows.
-    # https://docs.microsoft.com/en-us/windows/win32/
-    # ... fileio/naming-a-file#win32-file-namespaces
-    #
-    # Here we strip win32 path prefix from the resolved path
-    rv_drive, rv_path = os.path.splitdrive(rv)
-    stripped_rv_drive = rv_drive.split(os.path.sep)[-1]
-    rv = os.path.join(stripped_rv_drive, rv_path)
-
-    assert rv == real_path
+def test_path_resolve_symlink(tmp_path, runner):
+    test_file = tmp_path / "file"
+    test_file_str = os.fsdecode(test_file)
+    test_file.write_text("")
+
+    path_type = click.Path(resolve_path=True)
+    param = click.Argument(["a"], type=path_type)
+    ctx = click.Context(click.Command("cli", params=[param]))
+
+    test_dir = tmp_path / "dir"
+    test_dir.mkdir()
+
+    abs_link = test_dir / "abs"
+    abs_link.symlink_to(test_file)
+    abs_rv = path_type.convert(os.fsdecode(abs_link), param, ctx)
+    assert abs_rv == test_file_str
+
+    rel_link = test_dir / "rel"
+    rel_link.symlink_to(pathlib.Path("..") / "file")
+    rel_rv = path_type.convert(os.fsdecode(rel_link), param, ctx)
+    assert rel_rv == test_file_str