[xattr] Rework

In particular, explicitly require NT before trying ADS, and do not try to parse process output that may be localized.

1 files changed, 18 insertions, 40 deletions

diff --git a/youtube_dl/postprocessor/xattrpp.py b/youtube_dl/postprocessor/xattrpp.py
index ac2236f35..1cb8ecf1c 100644
--- a/youtube_dl/postprocessor/xattrpp.py
+++ b/youtube_dl/postprocessor/xattrpp.py
@@ -4,6 +4,7 @@ import sys
 
 from .common import PostProcessor
 from ..utils import (
+    check_executable,
     hyphenate_date,
     preferredencoding,
 )
@@ -30,48 +31,35 @@ class XAttrMetadataPP(PostProcessor):
         try:
             # try the pyxattr module...
             import xattr
+
             def write_xattr(path, key, value):
                 return xattr.setxattr(path, key, value)
 
         except ImportError:
+            if os.name == 'nt':
+                # Write xattrs to NTFS Alternate Data Streams:
+                # http://en.wikipedia.org/wiki/NTFS#Alternate_data_streams_.28ADS.29
+                def write_xattr(path, key, value):
+                    assert(key.find(":") < 0)
+                    assert(path.find(":") < 0)
+                    assert(os.path.exists(path))
 
-            if os.name == 'posix':
-                def which(bin):
-                    for dir in os.environ["PATH"].split(":"):
-                        path = os.path.join(dir, bin)
-                        if os.path.exists(path):
-                            return path
-
-                user_has_setfattr = which("setfattr")
-                user_has_xattr    = which("xattr")
+                    ads_fn = path + ":" + key
+                    with open(ads_fn, "w") as f:
+                        f.write(value)
+            else:
+                user_has_setfattr = check_executable("setfattr", ['--version'])
+                user_has_xattr = check_executable("xattr", ['-h'])
 
                 if user_has_setfattr or user_has_xattr:
 
                     def write_xattr(path, key, value):
-                        import errno
-                        potential_errors = {
-                            # setfattr: /tmp/blah: Operation not supported
-                            "Operation not supported": errno.EOPNOTSUPP,
-                            # setfattr: ~/blah: No such file or directory
-                            # xattr: No such file: ~/blah
-                            "No such file": errno.ENOENT,
-                        }
-
                         if user_has_setfattr:
                             cmd = ['setfattr', '-n', key, '-v', value, path]
                         elif user_has_xattr:
                             cmd = ['xattr', '-w', key, value, path]
 
-                        try:
-                            subprocess.check_output(cmd, stderr=subprocess.STDOUT)
-                        except subprocess.CalledProcessError as e:
-                            errorstr = e.output.strip().decode()
-                            for potential_errorstr, potential_errno in potential_errors.items():
-                                if errorstr.find(potential_errorstr) > -1:
-                                    e = OSError(potential_errno, potential_errorstr)
-                                    e.__cause__ = None
-                                    raise e
-                            raise  # Reraise unhandled error
+                        subprocess.check_output(cmd)
 
                 else:
                     # On Unix, and can't find pyxattr, setfattr, or xattr.
@@ -86,19 +74,9 @@ class XAttrMetadataPP(PostProcessor):
                             "Couldn't find a tool to set the xattrs. "
                             "Install either the python 'xattr' module, "
                             "or the 'xattr' binary.")
-            else:
-                # Write xattrs to NTFS Alternate Data Streams: http://en.wikipedia.org/wiki/NTFS#Alternate_data_streams_.28ADS.29
-                def write_xattr(path, key, value):
-                    assert(key.find(":") < 0)
-                    assert(path.find(":") < 0)
-                    assert(os.path.exists(path))
-
-                    ads_fn = path + ":" + key
-                    with open(ads_fn, "w") as f:
-                        f.write(value)
 
         # Write the metadata to the file's xattrs
-        self._downloader.to_screen('[metadata] Writing metadata to file\'s xattrs...')
+        self._downloader.to_screen('[metadata] Writing metadata to file\'s xattrs')
 
         filename = info['filepath']
 
@@ -126,7 +104,7 @@ class XAttrMetadataPP(PostProcessor):
 
             return True, info
 
-        except OSError:
+        except (subprocess.CalledProcessError, OSError):
             self._downloader.report_error("This filesystem doesn't support extended attributes. (You may have to enable them in your /etc/fstab)")
             return False, info