Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Five tests error/fail in a testing chroot on Linux with bizarre error messages #67

Open
juliangilbey opened this issue Jan 25, 2022 · 8 comments

Comments

@juliangilbey
Copy link
Contributor

Hi!
As part of the Debian build, the package tests are run in a minimal chroot environment. I've set the HOME environment variable first:

(sid-sbuild)jdg@euler:/build/send2trash-IKQvlF/send2trash-1.8.1~b0$ export HOME=$(mktemp -p)
(sid-sbuild)jdg@euler:/build/send2trash-IKQvlF/send2trash-1.8.1~b0$ echo $HOME
/tmp/tmp.CSc9dofzh7/
(sid-sbuild)jdg@euler:/build/send2trash-IKQvlF/send2trash-1.8.1~b0$ mkdir -p $HOME/.local/share/Trash
(sid-sbuild)jdg@euler:/build/send2trash-IKQvlF/send2trash-1.8.1~b0$ dpkg-buildpackage -b -us -uc
dpkg-buildpackage: info: source package send2trash
dpkg-buildpackage: info: source version 1.8.1~b0-1
dpkg-buildpackage: info: source distribution unstable
dpkg-buildpackage: info: source changed by Julian Gilbey <jdg@debian.org>
dpkg-buildpackage: info: host architecture amd64
[... build the python package, snipped ...]
Successfully built Send2Trash-1.8.1b0-py3-none-any.whl
I: pybuild plugin_pyproject:123: Unpacking wheel built for python3.10 with "installer" module
[... do the same for Python 3.9, snipped ...]
Successfully built Send2Trash-1.8.1b0-py3-none-any.whl
I: pybuild plugin_pyproject:123: Unpacking wheel built for python3.9 with "installer" module
   dh_auto_test -O--buildsystem=pybuild
I: pybuild base:237: cd '/build/send2trash-IKQvlF/send2trash-1.8.1~b0/.pybuild/cpython3_3.10/build'; python3.10 -m pytest tests
============================= test session starts ==============================
platform linux -- Python 3.10.2, pytest-6.2.5, py-1.10.0, pluggy-0.13.0
rootdir: /build/send2trash-IKQvlF/send2trash-1.8.1~b0/.pybuild/cpython3_3.10/build
collected 10 items / 1 skipped / 9 selected                                    

tests/test_plat_other.py ....FFFEF                                       [ 80%]
tests/test_script_main.py ..                                             [100%]

==================================== ERRORS ====================================
________________ ERROR at teardown of test_trash_topdir_failure ________________

    @pytest.fixture
    def testExtVol():
        trashTopdir = mkdtemp(prefix="s2t")
        volume = ExtVol(trashTopdir)
        fileName = "test.txt"
        filePath = op.join(volume.trashTopdir, fileName)
        touch(filePath)
        assert op.exists(filePath) is True
        yield volume, fileName, filePath
>       volume.cleanup()

tests/test_plat_other.py:156: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
tests/test_plat_other.py:144: in cleanup
    shutil.rmtree(self.trashTopdir)
/usr/lib/python3.10/shutil.py:717: in rmtree
    _rmtree_safe_fd(fd, path, onerror)
/usr/lib/python3.10/shutil.py:674: in _rmtree_safe_fd
    onerror(os.unlink, fullname, sys.exc_info())
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

topfd = 11, path = '/tmp/s2tw916afk4'
onerror = <function rmtree.<locals>.onerror at 0x7fa2915feef0>

    def _rmtree_safe_fd(topfd, path, onerror):
        try:
            with os.scandir(topfd) as scandir_it:
                entries = list(scandir_it)
        except OSError as err:
            err.filename = path
            onerror(os.scandir, path, sys.exc_info())
            return
        for entry in entries:
            fullname = os.path.join(path, entry.name)
            try:
                is_dir = entry.is_dir(follow_symlinks=False)
            except OSError:
                is_dir = False
            else:
                if is_dir:
                    try:
                        orig_st = entry.stat(follow_symlinks=False)
                        is_dir = stat.S_ISDIR(orig_st.st_mode)
                    except OSError:
                        onerror(os.lstat, fullname, sys.exc_info())
                        continue
            if is_dir:
                try:
                    dirfd = os.open(entry.name, os.O_RDONLY, dir_fd=topfd)
                except OSError:
                    onerror(os.open, fullname, sys.exc_info())
                else:
                    try:
                        if os.path.samestat(orig_st, os.fstat(dirfd)):
                            _rmtree_safe_fd(dirfd, fullname, onerror)
                            try:
                                os.rmdir(entry.name, dir_fd=topfd)
                            except OSError:
                                onerror(os.rmdir, fullname, sys.exc_info())
                        else:
                            try:
                                # This can only happen if someone replaces
                                # a directory with a symlink after the call to
                                # os.scandir or stat.S_ISDIR above.
                                raise OSError("Cannot call rmtree on a symbolic "
                                              "link")
                            except OSError:
                                onerror(os.path.islink, fullname, sys.exc_info())
                    finally:
                        os.close(dirfd)
            else:
                try:
>                   os.unlink(entry.name, dir_fd=topfd)
E                   PermissionError: [Errno 13] Permission denied: 'test.txt'

/usr/lib/python3.10/shutil.py:672: PermissionError
=================================== FAILURES ===================================
______________________________ test_trash_topdir _______________________________

testExtVol = (<tests.test_plat_other.ExtVol object at 0x7fa2915df430>, 'test.txt', '/tmp/s2t1_l_qvbt/test.txt')

    def test_trash_topdir(testExtVol):
        trashDir = op.join(testExtVol[0].trashTopdir, ".Trash")
        os.mkdir(trashDir, 0o777 | stat.S_ISVTX)
    
>       s2t(testExtVol[2])

tests/test_plat_other.py:163: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
send2trash/plat_other.py:213: in send2trash
    dest_trash = find_ext_volume_trash(topdir)
send2trash/plat_other.py:169: in find_ext_volume_trash
    trash_dir = find_ext_volume_fallback_trash(volume_root)
send2trash/plat_other.py:158: in find_ext_volume_fallback_trash
    check_create(trash_dir)
send2trash/plat_other.py:96: in check_create
    os.makedirs(dir, 0o700)
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

name = b'/tmp/s2t1_l_qvbt/test.txt/.Trash-1000', mode = 448, exist_ok = False

    def makedirs(name, mode=0o777, exist_ok=False):
        """makedirs(name [, mode=0o777][, exist_ok=False])
    
        Super-mkdir; create a leaf directory and all intermediate ones.  Works like
        mkdir, except that any intermediate path segment (not just the rightmost)
        will be created if it does not exist. If the target directory already
        exists, raise an OSError if exist_ok is False. Otherwise no exception is
        raised.  This is recursive.
    
        """
        head, tail = path.split(name)
        if not tail:
            head, tail = path.split(head)
        if head and tail and not path.exists(head):
            try:
                makedirs(head, exist_ok=exist_ok)
            except FileExistsError:
                # Defeats race condition when another thread created the path
                pass
            cdir = curdir
            if isinstance(tail, bytes):
                cdir = bytes(curdir, 'ASCII')
            if tail == cdir:           # xxx/newdir/. exists if xxx/newdir exists
                return
        try:
>           mkdir(name, mode)
E           NotADirectoryError: [Errno 20] Not a directory: b'/tmp/s2t1_l_qvbt/test.txt/.Trash-1000'

/usr/lib/python3.10/os.py:225: NotADirectoryError
__________________________ test_trash_topdir_fallback __________________________

testExtVol = (<tests.test_plat_other.ExtVol object at 0x7fa291497a60>, 'test.txt', '/tmp/s2tmx3ups_b/test.txt')

    def test_trash_topdir_fallback(testExtVol):
>       s2t(testExtVol[2])

tests/test_plat_other.py:175: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
send2trash/plat_other.py:213: in send2trash
    dest_trash = find_ext_volume_trash(topdir)
send2trash/plat_other.py:169: in find_ext_volume_trash
    trash_dir = find_ext_volume_fallback_trash(volume_root)
send2trash/plat_other.py:158: in find_ext_volume_fallback_trash
    check_create(trash_dir)
send2trash/plat_other.py:96: in check_create
    os.makedirs(dir, 0o700)
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

name = b'/tmp/s2tmx3ups_b/test.txt/.Trash-1000', mode = 448, exist_ok = False

    def makedirs(name, mode=0o777, exist_ok=False):
        """makedirs(name [, mode=0o777][, exist_ok=False])
    
        Super-mkdir; create a leaf directory and all intermediate ones.  Works like
        mkdir, except that any intermediate path segment (not just the rightmost)
        will be created if it does not exist. If the target directory already
        exists, raise an OSError if exist_ok is False. Otherwise no exception is
        raised.  This is recursive.
    
        """
        head, tail = path.split(name)
        if not tail:
            head, tail = path.split(head)
        if head and tail and not path.exists(head):
            try:
                makedirs(head, exist_ok=exist_ok)
            except FileExistsError:
                # Defeats race condition when another thread created the path
                pass
            cdir = curdir
            if isinstance(tail, bytes):
                cdir = bytes(curdir, 'ASCII')
            if tail == cdir:           # xxx/newdir/. exists if xxx/newdir exists
                return
        try:
>           mkdir(name, mode)
E           NotADirectoryError: [Errno 20] Not a directory: b'/tmp/s2tmx3ups_b/test.txt/.Trash-1000'

/usr/lib/python3.10/os.py:225: NotADirectoryError
__________________________ test_trash_topdir_failure ___________________________

testExtVol = (<tests.test_plat_other.ExtVol object at 0x7fa2914756c0>, 'test.txt', '/tmp/s2tw916afk4/test.txt')

    def test_trash_topdir_failure(testExtVol):
        os.chmod(testExtVol[0].trashTopdir, 0o500)  # not writable to induce the exception
>       pytest.raises(TrashPermissionError, s2t, [testExtVol[2]])

tests/test_plat_other.py:182: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
send2trash/plat_other.py:213: in send2trash
    dest_trash = find_ext_volume_trash(topdir)
send2trash/plat_other.py:169: in find_ext_volume_trash
    trash_dir = find_ext_volume_fallback_trash(volume_root)
send2trash/plat_other.py:158: in find_ext_volume_fallback_trash
    check_create(trash_dir)
send2trash/plat_other.py:96: in check_create
    os.makedirs(dir, 0o700)
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

name = b'/tmp/s2tw916afk4/test.txt/.Trash-1000', mode = 448, exist_ok = False

    def makedirs(name, mode=0o777, exist_ok=False):
        """makedirs(name [, mode=0o777][, exist_ok=False])
    
        Super-mkdir; create a leaf directory and all intermediate ones.  Works like
        mkdir, except that any intermediate path segment (not just the rightmost)
        will be created if it does not exist. If the target directory already
        exists, raise an OSError if exist_ok is False. Otherwise no exception is
        raised.  This is recursive.
    
        """
        head, tail = path.split(name)
        if not tail:
            head, tail = path.split(head)
        if head and tail and not path.exists(head):
            try:
                makedirs(head, exist_ok=exist_ok)
            except FileExistsError:
                # Defeats race condition when another thread created the path
                pass
            cdir = curdir
            if isinstance(tail, bytes):
                cdir = bytes(curdir, 'ASCII')
            if tail == cdir:           # xxx/newdir/. exists if xxx/newdir exists
                return
        try:
>           mkdir(name, mode)
E           NotADirectoryError: [Errno 20] Not a directory: b'/tmp/s2tw916afk4/test.txt/.Trash-1000'

/usr/lib/python3.10/os.py:225: NotADirectoryError
______________________________ test_trash_symlink ______________________________

testExtVol = (<tests.test_plat_other.ExtVol object at 0x7fa2915da8c0>, 'test.txt', '/tmp/s2t9qns8us6/test.txt')

    def test_trash_symlink(testExtVol):
        # Use mktemp (race conditioney but no symlink equivalent)
        # Since is_parent uses realpath(), and our getdev uses is_parent,
        # this should work
        slDir = mktemp(prefix="s2t", dir=op.expanduser("~"))
        os.mkdir(op.join(testExtVol[0].trashTopdir, "subdir"), 0o700)
        filePath = op.join(testExtVol[0].trashTopdir, "subdir", testExtVol[1])
        touch(filePath)
        os.symlink(op.join(testExtVol[0].trashTopdir, "subdir"), slDir)
>       s2t(op.join(slDir, testExtVol[1]))

tests/test_plat_other.py:195: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
send2trash/plat_other.py:213: in send2trash
    dest_trash = find_ext_volume_trash(topdir)
send2trash/plat_other.py:169: in find_ext_volume_trash
    trash_dir = find_ext_volume_fallback_trash(volume_root)
send2trash/plat_other.py:158: in find_ext_volume_fallback_trash
    check_create(trash_dir)
send2trash/plat_other.py:96: in check_create
    os.makedirs(dir, 0o700)
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

name = b'/tmp/s2t9qns8us6/subdir/test.txt/.Trash-1000', mode = 448
exist_ok = False

    def makedirs(name, mode=0o777, exist_ok=False):
        """makedirs(name [, mode=0o777][, exist_ok=False])
    
        Super-mkdir; create a leaf directory and all intermediate ones.  Works like
        mkdir, except that any intermediate path segment (not just the rightmost)
        will be created if it does not exist. If the target directory already
        exists, raise an OSError if exist_ok is False. Otherwise no exception is
        raised.  This is recursive.
    
        """
        head, tail = path.split(name)
        if not tail:
            head, tail = path.split(head)
        if head and tail and not path.exists(head):
            try:
                makedirs(head, exist_ok=exist_ok)
            except FileExistsError:
                # Defeats race condition when another thread created the path
                pass
            cdir = curdir
            if isinstance(tail, bytes):
                cdir = bytes(curdir, 'ASCII')
            if tail == cdir:           # xxx/newdir/. exists if xxx/newdir exists
                return
        try:
>           mkdir(name, mode)
E           NotADirectoryError: [Errno 20] Not a directory: b'/tmp/s2t9qns8us6/subdir/test.txt/.Trash-1000'

/usr/lib/python3.10/os.py:225: NotADirectoryError
=========================== short test summary info ============================
FAILED tests/test_plat_other.py::test_trash_topdir - NotADirectoryError: [Err...
FAILED tests/test_plat_other.py::test_trash_topdir_fallback - NotADirectoryEr...
FAILED tests/test_plat_other.py::test_trash_topdir_failure - NotADirectoryErr...
FAILED tests/test_plat_other.py::test_trash_symlink - NotADirectoryError: [Er...
ERROR tests/test_plat_other.py::test_trash_topdir_failure - PermissionError: ...
=============== 4 failed, 6 passed, 1 skipped, 1 error in 0.20s ================
E: pybuild pybuild:367: test: plugin pyproject failed with: exit code=1: cd '/build/send2trash-IKQvlF/send2trash-1.8.1~b0/.pybuild/cpython3_3.10/build'; python3.10 -m pytest tests
[... similar errors with Python 3.9 snipped ...]

The thing that seems so weird to me is the strange filenames/directories it's trying to work with, such as b'/tmp/s2t9qns8us6/subdir/test.txt/.Trash-1000'. It strikes me that something has gone wrong in the calculation of the trash directory name. (And there's no sign of the HOME directory I so carefully set up!)

@juliangilbey
Copy link
Contributor Author

OK, I think I've figured this one out: because my chroot is mounted as an overlay filesystem, /tmp/s2t9qns8us6 is on the original device but /tmp/s2t9qns8us6/test.txt, which is in the overlay, actually lies on a different device, so os.path.ismount() regards this as the mount point of the filesystem. And of course a mount point is a directory, right?! So send2text gets hopelessly confused, for very good reason. I don't think there's anything I'll be able to easily do about this (admittedly quite bizarre) setup, and I don't think it makes sense for send2trash to handle such a case, so I'll just skip these tests. If send2trash were to, it would have to do something like the following in plat_other.py in the find_mount_point function:

def find_mount_point(path):
    # Even if something's wrong, "/" is a mount point, so the loop will exit.
    # Use realpath in case it's a symlink
    path = op.realpath(path)  # Required to avoid infinite loop
    while not op.ismount(path) or op.isfile(path):  # Note ismount() does not always detect mounts
        path = op.split(path)[0]
    return path

(noting the extra or op.isfile(path) in the loop condition), but I'm not sure I particularly like this.

@juliangilbey
Copy link
Contributor Author

BTW, this patch fixes the issue and shouldn't change the behaviour of the function in "normal" situations, so it may be worth applying.

@juliangilbey
Copy link
Contributor Author

No, I spoke too soon; the test:

               topdir = find_mount_point(path_b)
               trash_dev = get_dev(topdir)
               if trash_dev != path_dev:
                   raise OSError("Couldn't find mount point for %s" % path)

causes an OSError :-( So I guess this problem can't be fixed and should just be left; I'll just skip these tests for the Debian setup.

@arsenetar
Copy link
Owner

arsenetar commented Jan 26, 2022

Yeah, I have found a workaround to actually get mounts better than what ismount() does but it relies on other utilities being available on the system and running them essentially from the script. I really did not like that, even though it utilized utilities which were almost always present on systems. I can take a bit of a look at this case, as a similar case was encountered (partially the reason for the 1.8.1b0 version), and was at least able to be handled better.

As for your bit about the home environment, send2trash looks for XDG_DATA_HOME and defaults to ~/.local/share if not found. This is essentially following the freedesktop spec at https://specifications.freedesktop.org/trash-spec/trashspec-latest.html, although the directory should fall back to $HOME/.local/share but I think in most instances ~ and $HOME end up being the same, although if not the case that certainly could be changed. Granted that will not fix/affect these topdir tests.

@juliangilbey
Copy link
Contributor Author

Indeed! os.expandpath(b"~") expands to the value of HOME if it is set (https://docs.python.org/3/library/os.path.html#os.path.expanduser). In my (admittedly weird) use case, though, HOME was being inherited by the schroot call, but the home partition is not mounted in the chroot environment, so $HOME is set but points to a non-existent directcory, hence the need for creating a temporary home directory.

I'm not sure, though, that "fixing" the topdir tests will help even then, as the chances are that the user won't have write permission to create the .Trash directory in the volume root directory.

I have tried finding a workaround, but sadly to no avail; in the chroot overlay setup, creating a directory in /tmp is created on the same device as /tmp, and creating a subdirectory of that is still created in the same device. But any regular file created is created in a different device (as far as lstat() is concerned). Oh well, I'll just have to skip these tests in this weird environment.

@juliangilbey
Copy link
Contributor Author

I guess the only other way of resolving this is to make the modification to find_mount_point() that I suggested above and then remove the check on line 211: if trash_dev != path_dev. But I think just my locally skipping the tests is the safer way to go.

@juliangilbey
Copy link
Contributor Author

I guess the only other way of resolving this is to make the modification to find_mount_point() that I suggested above and then remove the check on line 211: if trash_dev != path_dev. But I think just my locally skipping the tests is the safer way to go.

Oh, that didn't work either: it just ended up trying to create /.Trash-100 and failing because it didn't have permission to do so.

@biggestsonicfan
Copy link

biggestsonicfan commented Jul 22, 2023

Coming back to this with the same issue I previously posted here.

Traceback (most recent call last):
  File "/home/rob/.local/lib/python3.11/site-packages/send2trash/plat_gio.py", line 17, in send2trash
    f.trash(cancellable=None)
gi.repository.GLib.GError: g-io-error-quark: Unable to find or create trash directory [path redacted]/.Trash-1000 to trash [file redacted] (15)

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/home/rob/gallery-dl/verify-fantia.py", line 33, in <module>
    send2trash(oldoldfile)          
    ^^^^^^^^^^^^^^^^^^^^^^
  File "/home/rob/.local/lib/python3.11/site-packages/send2trash/plat_gio.py", line 22, in send2trash
    raise TrashPermissionError("")
send2trash.exceptions.TrashPermissionError: [Errno 13] Permission denied: ''

Drive is ntfs. Is there a reason it can't do this. Logged in user can trash files just fine without a .Trash-1000 existing on the root directory. A .Trash-1000 directory does exist on the drive.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants