common: replace custom xattr implementation with os module's (#24448)

* use os module's xattr function

* fix that

* handle in helper

common/tests/test_xattr.py

@@ -1,47 +0,0 @@
-import os
-import tempfile
-import shutil
-import unittest
-
-from common.xattr import getxattr, setxattr, listxattr, removexattr
-
-class TestParams(unittest.TestCase):
-  USER_TEST='user.test'
-  def setUp(self):
-    self.tmpdir = tempfile.mkdtemp()
-    self.tmpfn = os.path.join(self.tmpdir, 'test.txt')
-    open(self.tmpfn, 'w').close()
-    #print("using", self.tmpfn)
-
-  def tearDown(self):
-    shutil.rmtree(self.tmpdir)
-
-  def test_getxattr_none(self):
-    a = getxattr(self.tmpfn, TestParams.USER_TEST)
-    assert a is None
-
-  def test_listxattr_none(self):
-    l = listxattr(self.tmpfn)
-    assert l == []
-
-  def test_setxattr(self):
-    setxattr(self.tmpfn, TestParams.USER_TEST, b'123')
-    a = getxattr(self.tmpfn, TestParams.USER_TEST)
-    assert a == b'123'
-
-  def test_listxattr(self):
-    setxattr(self.tmpfn, 'user.test1', b'123')
-    setxattr(self.tmpfn, 'user.test2', b'123')
-    l = listxattr(self.tmpfn)
-    assert l == ['user.test1', 'user.test2']
-
-  def test_removexattr(self):
-    setxattr(self.tmpfn, TestParams.USER_TEST, b'123')
-    a = getxattr(self.tmpfn, TestParams.USER_TEST)
-    assert a == b'123'
-    removexattr(self.tmpfn, TestParams.USER_TEST)
-    a = getxattr(self.tmpfn, TestParams.USER_TEST)
-    assert a is None
-
-if __name__ == "__main__":
-  unittest.main()

release/files_common

@@ -28,7 +28,6 @@ common/logging_extra.py
 common/numpy_fast.py
 common/params.py
 common/params_pyx.pyx
-common/xattr.py
 common/profiler.py
 common/basedir.py
 common/dict_helpers.py

selfdrive/loggerd/tests/test_deleter.py

@@ -1,12 +1,12 @@
+#!/usr/bin/env python3
 import os
 import time
 import threading
 import unittest
 from collections import namedtuple
 
-import selfdrive.loggerd.deleter as deleter
 from common.timeout import Timeout, TimeoutException
-
+import selfdrive.loggerd.deleter as deleter
 from selfdrive.loggerd.tests.loggerd_tests_common import UploaderTestCase
 
 Stats = namedtuple("Stats", ['f_bavail', 'f_blocks', 'f_frsize'])

selfdrive/loggerd/tests/test_uploader.py

@@ -9,8 +9,6 @@ import json
 from selfdrive.swaglog import cloudlog
 import selfdrive.loggerd.uploader as uploader
 
-from common.xattr import getxattr
-
 from selfdrive.loggerd.tests.loggerd_tests_common import UploaderTestCase
 
 
@@ -84,7 +82,7 @@ class TestUploader(UploaderTestCase):
     self.assertFalse(len(log_handler.upload_order) < len(exp_order), "Some files failed to upload")
     self.assertFalse(len(log_handler.upload_order) > len(exp_order), "Some files were uploaded twice")
     for f_path in exp_order:
-      self.assertTrue(getxattr(os.path.join(self.root, f_path.replace('.bz2', '')), uploader.UPLOAD_ATTR_NAME), "All files not uploaded")
+      self.assertTrue(os.getxattr(os.path.join(self.root, f_path.replace('.bz2', '')), uploader.UPLOAD_ATTR_NAME), "All files not uploaded")
 
     self.assertTrue(log_handler.upload_order == exp_order, "Files uploaded in wrong order")
 
@@ -103,7 +101,7 @@ class TestUploader(UploaderTestCase):
     self.assertFalse(len(log_handler.upload_ignored) < len(exp_order), "Some files failed to ignore")
     self.assertFalse(len(log_handler.upload_ignored) > len(exp_order), "Some files were ignored twice")
     for f_path in exp_order:
-      self.assertTrue(getxattr(os.path.join(self.root, f_path.replace('.bz2', '')), uploader.UPLOAD_ATTR_NAME), "All files not ignored")
+      self.assertTrue(os.getxattr(os.path.join(self.root, f_path.replace('.bz2', '')), uploader.UPLOAD_ATTR_NAME), "All files not ignored")
 
     self.assertTrue(log_handler.upload_ignored == exp_order, "Files ignored in wrong order")
 
@@ -128,7 +126,7 @@ class TestUploader(UploaderTestCase):
     self.assertFalse(len(log_handler.upload_order) < len(exp_order), "Some files failed to upload")
     self.assertFalse(len(log_handler.upload_order) > len(exp_order), "Some files were uploaded twice")
     for f_path in exp_order:
-      self.assertTrue(getxattr(os.path.join(self.root, f_path.replace('.bz2', '')), uploader.UPLOAD_ATTR_NAME), "All files not uploaded")
+      self.assertTrue(os.getxattr(os.path.join(self.root, f_path.replace('.bz2', '')), uploader.UPLOAD_ATTR_NAME), "All files not uploaded")
 
     self.assertTrue(log_handler.upload_order == exp_order, "Files uploaded in wrong order")
 
@@ -143,7 +141,8 @@ class TestUploader(UploaderTestCase):
     self.join_thread()
 
     for f_path in f_paths:
-      self.assertFalse(getxattr(f_path.replace('.bz2', ''), uploader.UPLOAD_ATTR_NAME), "File upload when locked")
+      uploaded = uploader.UPLOAD_ATTR_NAME in os.listxattr(f_path.replace('.bz2', ''))
+      self.assertFalse(uploaded, "File upload when locked")
 
   def test_clear_locks_on_startup(self):
     f_paths = self.gen_files(lock=True, boot=False)
@@ -156,4 +155,4 @@ class TestUploader(UploaderTestCase):
 
 
 if __name__ == "__main__":
-  unittest.main(failfast=True)
+  unittest.main()

selfdrive/loggerd/tools/mark_all_uploaded.py

@@ -1,9 +1,8 @@
 import os
-from common.xattr import setxattr
 from selfdrive.loggerd.uploader import UPLOAD_ATTR_NAME, UPLOAD_ATTR_VALUE
 
 from selfdrive.loggerd.config import ROOT
 for folder in os.walk(ROOT):
   for file1 in folder[2]:
     full_path = os.path.join(folder[0], file1)
-    setxattr(full_path, UPLOAD_ATTR_NAME, UPLOAD_ATTR_VALUE)
+    os.setxattr(full_path, UPLOAD_ATTR_NAME, UPLOAD_ATTR_VALUE)

selfdrive/loggerd/tools/mark_unuploaded.py

@@ -1,8 +1,8 @@
 #!/usr/bin/env python3
+import os
 import sys
-from common.xattr import removexattr
 from selfdrive.loggerd.uploader import UPLOAD_ATTR_NAME
 
 for fn in sys.argv[1:]:
   print(f"unmarking {fn}")
-  removexattr(fn, UPLOAD_ATTR_NAME)
+  os.removexattr(fn, UPLOAD_ATTR_NAME)

selfdrive/loggerd/xattr_cache.py

@@ -1,15 +1,23 @@
-from typing import Dict, Tuple
+import os
+import errno
+from typing import Dict, Tuple, Optional
 
-from common.xattr import getxattr as getattr1
-from common.xattr import setxattr as setattr1
+_cached_attributes: Dict[Tuple, Optional[bytes]] = {}
 
-cached_attributes: Dict[Tuple, bytes] = {}
-def getxattr(path: str, attr_name: str) -> bytes:
-  if (path, attr_name) not in cached_attributes:
-    response = getattr1(path, attr_name)
-    cached_attributes[(path, attr_name)] = response
-  return cached_attributes[(path, attr_name)]
+def getxattr(path: str, attr_name: str) -> Optional[bytes]:
+  key = (path, attr_name)
+  if key not in _cached_attributes:
+    try:
+      response = os.getxattr(path, attr_name)
+    except OSError as e:
+      # ENODATA means attribute hasn't been set
+      if e.errno == errno.ENODATA:
+        response = None
+      else:
+        raise
+    _cached_attributes[key] = response
+  return _cached_attributes[key]
 
 def setxattr(path: str, attr_name: str, attr_value: bytes) -> None:
-  cached_attributes.pop((path, attr_name), None)
-  return setattr1(path, attr_name, attr_value)
+  _cached_attributes.pop((path, attr_name), None)
+  return os.setxattr(path, attr_name, attr_value)