From 05bcf678198360c8abc6ca2f37ee67835e690486 Mon Sep 17 00:00:00 2001
From: Iustin Pop <iustin@k1024.org>
Date: Tue, 26 Nov 2019 03:36:40 +0100
Subject: [PATCH] Add support for Path-like objects in Python 3.6+

This is done by switching to PyUnicode_FSConverter (3.1+), which
supports it. The convert_obj() function is now much simpler, which is
a bonus.

Expand tests to check behaviour with path objects, and skip those on
Python 3.6.

Closes #20.
---
 NEWS               |  9 +++++++++
 test/test_xattr.py | 21 +++++++++++++++++++++
 xattr.c            | 37 ++++++++++++++++++-------------------
 3 files changed, 48 insertions(+), 19 deletions(-)

diff --git a/NEWS b/NEWS
index 1e96ebd..7efe344 100644
--- a/NEWS
+++ b/NEWS
@@ -9,6 +9,15 @@ Version 0.7.0
 Major change: drop compatibility with Python 2, which allows significant
 code cleanups.
 
+Other changes:
+
+* Switch internal implementation of argument parsing to a built-in one
+  (`PyUnicode_FSConverter`), which brings automatic support for
+  path-like objects in Python 3.6+, and also a more uniform handling of
+  Unicode path arguments with respect to other Python code.
+* Switch test library to pytest; not that a reasonable recent version is
+  needed.
+
 Version 0.6.1
 -------------
 
diff --git a/test/test_xattr.py b/test/test_xattr.py
index 0a5fbdf..5513e52 100644
--- a/test/test_xattr.py
+++ b/test/test_xattr.py
@@ -6,6 +6,8 @@ import tempfile
 import os
 import errno
 import pytest
+import pathlib
+import platform
 
 import xattr
 from xattr import NS_USER, XATTR_CREATE, XATTR_REPLACE
@@ -105,28 +107,47 @@ def as_bytes(call):
         return call(path).encode()
     return f
 
+def as_fspath(call):
+    def f(path):
+        return pathlib.PurePath(call(path))
+    return f
+
+NOT_BEFORE_36 = pytest.mark.xfail(condition="sys.version_info < (3,6)",
+                                  strict=True)
+NOT_PYPY = pytest.mark.xfail(condition="platform.python_implementation() == 'PyPy'",
+                                  strict=False)
+
 # Note: user attributes are only allowed on files and directories, so
 # we have to skip the symlinks here. See xattr(7).
 ITEMS_P = [
     (get_file_name, False),
     (as_bytes(get_file_name), False),
+    pytest.param((as_fspath(get_file_name), False),
+                 marks=[NOT_BEFORE_36, NOT_PYPY]),
     (get_file_fd, False),
     (get_file_object, False),
     (get_dir, False),
     (as_bytes(get_dir), False),
+    pytest.param((as_fspath(get_dir), False),
+                 marks=[NOT_BEFORE_36, NOT_PYPY]),
     (get_valid_symlink, False),
     (as_bytes(get_valid_symlink), False),
+    pytest.param((as_fspath(get_valid_symlink), False),
+                 marks=[NOT_BEFORE_36, NOT_PYPY]),
 ]
 
 ITEMS_D = [
     "file name",
     "file name (bytes)",
+    "file name (path)",
     "file FD",
     "file object",
     "directory",
     "directory (bytes)",
+    "directory (path)",
     "file via symlink",
     "file via symlink (bytes)",
+    "file via symlink (path)",
 ]
 
 ALL_ITEMS_P = ITEMS_P + [
diff --git a/xattr.c b/xattr.c
index 08d1e25..854beef 100644
--- a/xattr.c
+++ b/xattr.c
@@ -29,9 +29,9 @@
 #include <stdio.h>
 
 #define ITEM_DOC \
-    ":param item: a string representing a file-name, or a file-like\n" \
-    "    object, or a file descriptor; this represents the file on \n" \
-    "    which to act\n"
+    ":param item: a string representing a file-name, a file-like\n" \
+    "    object, a file descriptor, or (in Python 3.6+) a path-like\n" \
+    "    object; this represents the file on which to act\n"
 
 #define NOFOLLOW_DOC \
     ":param nofollow: if true and if\n" \
@@ -130,29 +130,28 @@ static int merge_ns(const char *ns, const char *name,
 static int convert_obj(PyObject *myobj, target_t *tgt, int nofollow) {
     int fd;
     tgt->tmp = NULL;
-    if(PyBytes_Check(myobj)) {
-        tgt->type = nofollow ? T_LINK : T_PATH;
-        tgt->name = PyBytes_AS_STRING(myobj);
-    } else if(PyUnicode_Check(myobj)) {
-        tgt->type = nofollow ? T_LINK : T_PATH;
-        tgt->tmp = \
-            PyUnicode_AsEncodedString(myobj,
-                                      Py_FileSystemDefaultEncoding,
-                                      "surrogateescape"
-                                      );
-        if(tgt->tmp == NULL)
-            return -1;
-        tgt->name = PyBytes_AS_STRING(tgt->tmp);
-    } else if((fd = PyObject_AsFileDescriptor(myobj)) != -1) {
+    if((fd = PyObject_AsFileDescriptor(myobj)) != -1) {
         tgt->type = T_FD;
         tgt->fd = fd;
+        return 0;
+    }
+    // PyObject_AsFileDescriptor sets an error when failing, so clear
+    // it such that further code works; some method lookups fail if an
+    // error already occured when called, which breaks at least
+    // PyOS_FSPath (called by FSConverter).
+    PyErr_Clear();
+
+    if(PyUnicode_FSConverter(myobj, &(tgt->tmp))) {
+        tgt->type = nofollow ? T_LINK : T_PATH;
+        tgt->name = PyBytes_AS_STRING(tgt->tmp);
+        return 0;
     } else {
-        PyErr_SetString(PyExc_TypeError, "argument must be string or int");
+        // Don't set our own exception type, since we'd ignore the
+        // FSConverter-generated one.
         tgt->type = T_PATH;
         tgt->name = NULL;
         return -1;
     }
-    return 0;
 }
 
 /* Combine a namespace string and an attribute name into a
-- 
2.39.5