Repository: jaraco/keyrings.alt (language: python)

Issue:
pyfs backend seems to be incompatible with fs-2.4.16, even though requirements declare <3
I have fs-2.4.16 installed, and all pyfs tests are skipped. If I remove the "skipif", I get lots of test failures, such as:

```pytb
_______________________________ TestUnencryptedMemoryPyfilesystemKeyringNoSubDir.test_password_set_get ________________________________

self = <keyrings.alt.pyfs.PlaintextKeyring object at 0x7f14bb08f350>, mode = 'r'

    def _open(self, mode='r'):
        """Open the password file in the specified mode"""
        open_file = None
        writeable = 'w' in mode or 'a' in mode or '+' in mode
        try:
>           open_file = self._get_opener(mode, writeable)

keyrings/alt/pyfs.py:86: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _

self = <keyrings.alt.pyfs.PlaintextKeyring object at 0x7f14bb08f350>, mode = 'r', writeable = False

    def _get_opener(self, mode, writeable):
        # NOTE: currently the MemOpener does not split off any filename
        #       which causes errors on close()
        #       so we add a dummy name and open it separately
        if self.filename.startswith('mem://') or self.filename.startswith('ram://'):
>           open_file = fs.opener.fsopendir(self.filename).open('kr.cfg', mode)
E           AttributeError: module 'fs.opener' has no attribute 'fsopendir'

keyrings/alt/pyfs.py:146: AttributeError

During handling of the above exception, another exception occurred:

self = <tests.test_pyfs.TestUnencryptedMemoryPyfilesystemKeyringNoSubDir object at 0x7f14b9fd0650>

    def test_password_set_get(self):
        password = random_string(20)
        username = random_string(20)
        service = random_string(20)
>       self.check_set_get(service, username, password)

/usr/lib/python3.11/site-packages/keyring/testing/backend.py:72: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
/usr/lib/python3.11/site-packages/keyring/testing/backend.py:58: in check_set_get
    assert keyring.get_password(service, username) is None
keyrings/alt/pyfs.py:177: in get_password
    password_base64 = self.config.get(service, username).encode()
keyrings/alt/pyfs.py:166: in config
    raw_config.read_file(self._open() or [])
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _

self = <keyrings.alt.pyfs.PlaintextKeyring object at 0x7f14bb08f350>, mode = 'r'

    def _open(self, mode='r'):
        """Open the password file in the specified mode"""
        open_file = None
        writeable = 'w' in mode or 'a' in mode or '+' in mode
        try:
            open_file = self._get_opener(mode, writeable)
>       except fs.errors.ResourceNotFoundError:
E       AttributeError: module 'fs.errors' has no attribute 'ResourceNotFoundError'

keyrings/alt/pyfs.py:87: AttributeError
```

So I guess the requirement `fs>=0.5,<3` is incorrect since 2.4.x doesn't work. FWICS the tests are also skipped on CI.


Discussion / hints:
I have not done the archeology yet, so I don't know exactly which version of 'fs' is no longer compatible with keyrings.alt, but you're right that the <3 test is not correct right now as fs 2.4.16 will not work with keyrings.alt 4.2.0.

As more packages grow dependency on the latest 'fs' module, this will become more of a problem for keyrings.alt.  For example 'fonttools' has this requirement:

            "fs >= 2.2.0, < 3",

There should probably be some work to support newer 'fs' in keyrings.alt.  I took a quick look - it doesn't seem trivial at a glance.
> I have not done the archeology yet, so I don't know exactly which version of 'fs' is no longer compatible with keyrings.alt, but you're right that the <3 test is not correct right now as fs 2.4.16 will not work with keyrings.alt 4.2.0.

It looks like [fs >= 2](https://github.com/PyFilesystem/pyfilesystem2) has not been compatible with keyrings.alt's pyfs since the inception of the newer 'fs v2' in 2016.  So without an update to the keryings.alt implementation of support for the 'fs' module (in `alt/pyfs.py`), the requirement should probably be changed to `fs>=0.5,<2`.

For more history, see:
 - issue #11
 - issue #20
 - pull request #29
 - pull request #19
 - pull request #44 (committed incorrectly it seems)
> For more history, see:

Thanks for the investigation, especially for the link to #44, which indicates how this dependency was incorrectly bumped.

As noted in #50, there are no viable versions of `fs` that work with this library, and since no one seems to have been bothered by the broken dependency (lack of support in pyfs), I'm inclined to just drop the functionality.

Fix patch:
diff --git a/docs/index.rst b/docs/index.rst
index 6782a44..19e67ae 100644
--- a/docs/index.rst
+++ b/docs/index.rst
@@ -38,11 +38,6 @@ Welcome to |project| documentation!
     :undoc-members:
     :show-inheritance:
 
-.. automodule:: keyrings.alt.pyfs
-    :members:
-    :undoc-members:
-    :show-inheritance:
-
 .. automodule:: keyrings.alt.Windows
     :members:
     :undoc-members:
diff --git a/keyrings/alt/pyfs.py b/keyrings/alt/pyfs.py
deleted file mode 100644
index 452ad92..0000000
--- a/keyrings/alt/pyfs.py
+++ /dev/null
@@ -1,256 +0,0 @@
-import os
-import base64
-import configparser
-
-from jaraco.classes import properties
-
-from keyring import errors
-from .escape import escape as escape_for_ini
-from keyring.util import platform_
-from keyring.backend import KeyringBackend, NullCrypter
-from . import keyczar
-
-try:
-    import fs.opener
-    import fs.osfs
-    import fs.errors
-    import fs.path
-    import fs.remote
-except ImportError:
-    pass
-
-
-def has_pyfs():
-    """
-    Does this environment have pyfs 1.x installed?
-    Should return False even when Mercurial's Demand Import allowed import of
-    fs.*.
-    """
-    with errors.ExceptionRaisedContext() as exc:
-        fs.opener.opener
-    return not bool(exc)
-
-
-class BasicKeyring(KeyringBackend):
-    """BasicKeyring is a Pyfilesystem-based implementation of
-    keyring.
-
-    It stores the password directly in the file, and supports
-    encryption and decryption. The encrypted password is stored in base64
-    format.
-    Being based on Pyfilesystem the file can be local or network-based and
-    served by any of the filesystems supported by Pyfilesystem including Amazon
-    S3, FTP, WebDAV, memory and more.
-    """
-
-    _filename = 'keyring_pyf_pass.cfg'
-
-    def __init__(self, crypter, filename=None, can_create=True, cache_timeout=None):
-        super(BasicKeyring, self).__init__()
-        self._crypter = crypter
-        def_fn = os.path.join(platform_.data_root(), self.__class__._filename)
-        self._filename = filename or def_fn
-        self._can_create = can_create
-        self._cache_timeout = cache_timeout
-
-    @properties.NonDataProperty
-    def file_path(self):
-        """
-        The path to the file where passwords are stored. This property
-        may be overridden by the subclass or at the instance level.
-        """
-        return os.path.join(platform_.data_root(), self.filename)
-
-    @property
-    def filename(self):
-        """The filename used to store the passwords."""
-        return self._filename
-
-    def encrypt(self, password):
-        """Encrypt the password."""
-        if not password or not self._crypter:
-            return password or b''
-        return self._crypter.encrypt(password)
-
-    def decrypt(self, password_encrypted):
-        """Decrypt the password."""
-        if not password_encrypted or not self._crypter:
-            return password_encrypted or b''
-        return self._crypter.decrypt(password_encrypted)
-
-    def _open(self, mode='r'):
-        """Open the password file in the specified mode"""
-        open_file = None
-        writeable = 'w' in mode or 'a' in mode or '+' in mode
-        try:
-            open_file = self._get_opener(mode, writeable)
-        except fs.errors.ResourceNotFoundError:
-            if self._can_create:
-                pyfs, url2 = self._create_pyfs()
-
-                try:
-                    # reuse the pyfilesystem and path
-                    self._pyfs = pyfs
-                    self._path = url2
-                    return pyfs.open(url2, mode)
-                except fs.errors.ResourceNotFoundError:
-                    if writeable:
-                        raise
-                    else:
-                        pass
-            # NOTE: ignore read errors as the underlying caller can fail safely
-            if writeable:
-                raise
-            else:
-                pass
-        return open_file
-
-    def _create_pyfs(self):
-        segments = fs.opener.opener.split_segments(self.filename)
-        if segments:
-            # this seems broken, but pyfilesystem uses it, so we must
-            fs_name, credentials, url1, url2, path = segments.groups()
-            assert fs_name, 'Should be a remote filesystem'
-            host = ''
-            # allow for domain:port
-            if ':' in url2:
-                split_url2 = url2.split('/', 1)
-                if len(split_url2) > 1:
-                    url2 = split_url2[1]
-                else:
-                    url2 = ''
-                host = split_url2[0]
-            pyfs = fs.opener.opener.opendir('%s://%s' % (fs_name, host))
-            # cache if permitted
-            if self._cache_timeout is not None:
-                pyfs = fs.remote.CacheFS(pyfs, cache_timeout=self._cache_timeout)
-            # NOTE: fs.path.split does not function in the same
-            # way os os.path.split... at least under windows
-            url2_path, url2_filename = os.path.split(url2)
-            if url2_path and not pyfs.exists(url2_path):
-                pyfs.makedir(url2_path, recursive=True)
-        else:
-            # assume local filesystem
-            full_url = fs.opener._expand_syspath(self.filename)
-            # NOTE: fs.path.split does not function in the same
-            # way os os.path.split... at least under windows
-            url2_path, url2 = os.path.split(full_url)
-            pyfs = fs.osfs.OSFS(url2_path)
-        return pyfs, url2
-
-    def _get_opener(self, mode, writeable):
-        # NOTE: currently the MemOpener does not split off any filename
-        #       which causes errors on close()
-        #       so we add a dummy name and open it separately
-        if self.filename.startswith('mem://') or self.filename.startswith('ram://'):
-            open_file = fs.opener.fsopendir(self.filename).open('kr.cfg', mode)
-        else:
-            if not hasattr(self, '_pyfs'):
-                # reuse the pyfilesystem and path
-                self._pyfs, self._path = fs.opener.opener.parse(
-                    self.filename, writeable=writeable
-                )
-                # cache if permitted
-                if self._cache_timeout is not None:
-                    self._pyfs = fs.remote.CacheFS(
-                        self._pyfs, cache_timeout=self._cache_timeout
-                    )
-            open_file = self._pyfs.open(self._path, mode)
-        return open_file
-
-    @property
-    def config(self):
-        """load the passwords from the config file"""
-        if not hasattr(self, '_config'):
-            raw_config = configparser.RawConfigParser()
-            raw_config.read_file(self._open() or [])
-            self._config = raw_config
-        return self._config
-
-    def get_password(self, service, username):
-        """Read the password from the file."""
-        service = escape_for_ini(service)
-        username = escape_for_ini(username)
-
-        # fetch the password
-        try:
-            password_base64 = self.config.get(service, username).encode()
-            # decode with base64
-            password_encrypted = base64.decodebytes(password_base64)
-            # decrypted the password
-            password = self.decrypt(password_encrypted).decode('utf-8')
-        except (configparser.NoOptionError, configparser.NoSectionError):
-            password = None
-        return password
-
-    def set_password(self, service, username, password):
-        """Write the password in the file."""
-        service = escape_for_ini(service)
-        username = escape_for_ini(username)
-
-        # encrypt the password
-        password = password or ''
-        password_encrypted = self.encrypt(password.encode('utf-8'))
-
-        # encode with base64
-        password_base64 = base64.encodebytes(password_encrypted).decode()
-        # write the modification
-        if not self.config.has_section(service):
-            self.config.add_section(service)
-        self.config.set(service, username, password_base64)
-        config_file = self._open('w')
-        self.config.write(config_file)
-        config_file.close()
-
-    def delete_password(self, service, username):
-        service = escape_for_ini(service)
-        username = escape_for_ini(username)
-
-        try:
-            self.config.remove_option(service, username)
-        except configparser.NoSectionError:
-            raise errors.PasswordDeleteError('Password not found')
-        config_file = self._open('w')
-        self.config.write(config_file)
-        config_file.close()
-
-    @properties.classproperty
-    def priority(cls):
-        if not has_pyfs():
-            raise RuntimeError("pyfs required")
-        return 2
-
-
-class PlaintextKeyring(BasicKeyring):
-    """Unencrypted Pyfilesystem Keyring"""
-
-    def __init__(self, filename=None, can_create=True, cache_timeout=None):
-        super(PlaintextKeyring, self).__init__(
-            NullCrypter(),
-            filename=filename,
-            can_create=can_create,
-            cache_timeout=cache_timeout,
-        )
-
-
-class EncryptedKeyring(BasicKeyring):
-    """Encrypted Pyfilesystem Keyring"""
-
-    _filename = 'crypted_pyf_pass.cfg'
-
-    def __init__(self, crypter, filename=None, can_create=True, cache_timeout=None):
-        super(EncryptedKeyring, self).__init__(
-            crypter,
-            filename=filename,
-            can_create=can_create,
-            cache_timeout=cache_timeout,
-        )
-
-
-class KeyczarKeyring(EncryptedKeyring):
-    """Encrypted Pyfilesystem Keyring using Keyczar keysets specified in
-    environment vars
-    """
-
-    def __init__(self):
-        super(KeyczarKeyring, self).__init__(keyczar.EnvironCrypter())
diff --git a/newsfragments/49.removal.rst b/newsfragments/49.removal.rst
new file mode 100644
index 0000000..fc187bb
--- /dev/null
+++ b/newsfragments/49.removal.rst
@@ -0,0 +1,1 @@
+Removed the pyfs backend, as it has eroded beyond repair.
\ No newline at end of file
diff --git a/setup.cfg b/setup.cfg
index 9b3299b..78d0f92 100644
--- a/setup.cfg
+++ b/setup.cfg
@@ -45,7 +45,6 @@ testing =
 	backports.unittest_mock
 	keyring >= 20
 
-	fs>=0.5,<3
 	pycryptodomex
 	pycryptodome
 
@@ -77,5 +76,4 @@ keyring.backends =
 	Google = keyrings.alt.Google
 	keyczar = keyrings.alt.keyczar
 	multi = keyrings.alt.multi
-	pyfs = keyrings.alt.pyfs
 	Windows (alt) = keyrings.alt.Windows


Test patch:
diff --git a/tests/test_pyfs.py b/tests/test_pyfs.py
deleted file mode 100644
index 2e1fa25..0000000
--- a/tests/test_pyfs.py
+++ /dev/null
@@ -1,112 +0,0 @@
-from __future__ import unicode_literals
-
-import os
-import tempfile
-import textwrap
-
-import pytest
-
-import keyring.backend
-from keyrings.alt import pyfs
-from keyring.testing.backend import BackendBasicTests
-from keyring.testing.util import random_string
-
-
-class ReverseCrypter(keyring.backend.Crypter):
-    """Very silly crypter class"""
-
-    def encrypt(self, value):
-        return value[::-1]
-
-    def decrypt(self, value):
-        return value[::-1]
-
-
-@pytest.mark.skipif(not pyfs.BasicKeyring.viable, reason="Need Pyfilesystem")
-class PyFSBackend(BackendBasicTests):
-    """Base class for Pyfilesystem tests"""
-
-    def init_keyring(self):
-        return pyfs.PlaintextKeyring(filename=self.keyring_filename)
-
-    def test_encrypt_decrypt(self):
-        password = random_string(20)
-        encrypted = self.keyring.encrypt(password)
-
-        assert password == self.keyring.decrypt(encrypted)
-
-
-class TestUnencryptedMemoryPyfilesystemKeyringNoSubDir(PyFSBackend):
-    """Test in memory with no encryption"""
-
-    keyring_filename = 'mem://unencrypted'
-
-
-class TestUnencryptedMemoryPyfilesystemKeyringSubDir(PyFSBackend):
-    """Test in memory with no encryption"""
-
-    keyring_filename = 'mem://some/sub/dir/unencrypted'
-
-
-class TestUnencryptedLocalPyfilesystemKeyringNoSubDir(PyFSBackend):
-    """Test using local temp files with no encryption"""
-
-    keyring_filename = '%s/keyring.cfg' % tempfile.mkdtemp()
-
-    def test_handles_preexisting_keyring(self):
-        from fs.opener import opener
-
-        fs, path = opener.parse(self.keyring_filename, writeable=True)
-        keyring_file = fs.open(path, 'w')
-        file_data = textwrap.dedent(
-            """
-            [svc1]
-            user1 = cHdkMQ==
-            """
-        ).lstrip()
-        keyring_file.write(file_data)
-        keyring_file.close()
-        pyf_keyring = pyfs.PlaintextKeyring(filename=self.keyring_filename)
-        assert 'pwd1' == pyf_keyring.get_password('svc1', 'user1')
-
-    @pytest.fixture(autouse=True)
-    def remove_keyring_filename(self):
-        if os.path.exists(self.keyring_filename):
-            os.remove(self.keyring_filename)
-
-
-class TestUnencryptedLocalPyfilesystemKeyringSubDir(PyFSBackend):
-    """Test using local temp files with no encryption"""
-
-    keyring_dir = os.path.join(tempfile.mkdtemp(), 'more', 'sub', 'dirs')
-    keyring_filename = os.path.join(keyring_dir, 'keyring.cfg')
-
-    def init_keyring(self):
-        if not os.path.exists(self.keyring_dir):
-            os.makedirs(self.keyring_dir)
-        return pyfs.PlaintextKeyring(filename=self.keyring_filename)
-
-
-class TestEncryptedMemoryPyfilesystemKeyring(PyFSBackend):
-    """Test in memory with encryption"""
-
-    def init_keyring(self):
-        return pyfs.EncryptedKeyring(
-            ReverseCrypter(), filename='mem://encrypted/keyring.cfg'
-        )
-
-
-class TestEncryptedLocalPyfilesystemKeyringNoSubDir(PyFSBackend):
-    """Test using local temp files with encryption"""
-
-    def init_keyring(self):
-        return pyfs.EncryptedKeyring(ReverseCrypter(), filename='temp://keyring.cfg')
-
-
-class TestEncryptedLocalPyfilesystemKeyringSubDir(PyFSBackend):
-    """Test using local temp files with encryption"""
-
-    def init_keyring(self):
-        return pyfs.EncryptedKeyring(
-            ReverseCrypter(), filename='temp://a/sub/dir/hierarchy/keyring.cfg'
-        )