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

Internal error py.error.EEXIST: [File exists]: mkdir(...) on case insensitive file systems #3451

Labels
platform: mac mac platform-specific problem type: bug problem that needs to be addressed

Comments

@asottile
Copy link
Member

asottile commented May 4, 2018

real world case

Originally hit for these tests

I typically don't develop on a mac, but noticed the tests failing there.

$ uname -a
Darwin Macbook-Pro 17.5.0 Darwin Kernel Version 17.5.0: Fri Apr 13 19:32:32 PDT 2018; root:xnu-4570.51.2~1/RELEASE_X86_64 x86_64 i386 MacBookPro11,4 Darwin

version

I'm running pytest 3.5.1, but have been able to reproduce this on every version back to 2.5.2 (at which I stopped trying because it can't run python3.6!)

minimal reproduction

import pytest


@pytest.mark.parametrize('x', ('A', 'a'))
def test(x, tmpdir):
    pass

minimal reproduction output

$ .tox/py36/bin/pytest test.py
============================= test session starts ==============================
platform darwin -- Python 3.6.5, pytest-3.4.1, py-1.5.3, pluggy-0.6.0
rootdir: /private/tmp/yesqa, inifile:
collected 2 items                                                              

test.py .E                                                               [100%]

==================================== ERRORS ====================================
__________________________ ERROR at setup of test[a] ___________________________

self = <module 'py.error'>, func = <built-in function mkdir>
args = ('/private/var/folders/7x/97jnmnt13sl46bx2mc9chzpm0000gn/T/pytest-of-asottile/pytest-32/test_a_0',)
kwargs = {}, __tracebackhide__ = False, cls = <class 'py.error.EEXIST'>
value = FileExistsError(17, 'File exists')
tb = <traceback object at 0x1020f57c8>, errno = 17

    def checked_call(self, func, *args, **kwargs):
        """ call a function and raise an errno-exception if applicable. """
        __tracebackhide__ = True
        try:
>           return func(*args, **kwargs)
E           FileExistsError: [Errno 17] File exists: '/private/var/folders/7x/97jnmnt13sl46bx2mc9chzpm0000gn/T/pytest-of-asottile/pytest-32/test_a_0'

.tox/py36/lib/python3.6/site-packages/py/_error.py:66: FileExistsError

During handling of the above exception, another exception occurred:

self = <CallInfo when='setup' exception: [File exists]: mkdir('/private/var/folders/7x/97jnmnt13sl46bx2mc9chzpm0000gn/T/pytest-of-asottile/pytest-32/test_a_0',)>
func = <function call_runtest_hook.<locals>.<lambda> at 0x101f6e620>
when = 'setup'

    def __init__(self, func, when):
        #: context of invocation: one of "setup", "call",
        #: "teardown", "memocollect"
        self.when = when
        self.start = time()
        try:
>           self.result = func()

.tox/py36/lib/python3.6/site-packages/_pytest/runner.py:192: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
.tox/py36/lib/python3.6/site-packages/_pytest/runner.py:178: in <lambda>
    return CallInfo(lambda: ihook(item=item, **kwds), when=when)
.tox/py36/lib/python3.6/site-packages/pluggy/__init__.py:617: in __call__
    return self._hookexec(self, self._nonwrappers + self._wrappers, kwargs)
.tox/py36/lib/python3.6/site-packages/pluggy/__init__.py:222: in _hookexec
    return self._inner_hookexec(hook, methods, kwargs)
.tox/py36/lib/python3.6/site-packages/pluggy/__init__.py:216: in <lambda>
    firstresult=hook.spec_opts.get('firstresult'),
.tox/py36/lib/python3.6/site-packages/_pytest/runner.py:103: in pytest_runtest_setup
    item.session._setupstate.prepare(item)
.tox/py36/lib/python3.6/site-packages/_pytest/runner.py:496: in prepare
    col.setup()
.tox/py36/lib/python3.6/site-packages/_pytest/python.py:1183: in setup
    fixtures.fillfixtures(self)
.tox/py36/lib/python3.6/site-packages/_pytest/fixtures.py:240: in fillfixtures
    request._fillfixtures()
.tox/py36/lib/python3.6/site-packages/_pytest/fixtures.py:382: in _fillfixtures
    item.funcargs[argname] = self.getfixturevalue(argname)
.tox/py36/lib/python3.6/site-packages/_pytest/fixtures.py:424: in getfixturevalue
    return self._get_active_fixturedef(argname).cached_result[0]
.tox/py36/lib/python3.6/site-packages/_pytest/fixtures.py:450: in _get_active_fixturedef
    self._compute_fixture_value(fixturedef)
.tox/py36/lib/python3.6/site-packages/_pytest/fixtures.py:521: in _compute_fixture_value
    fixturedef.execute(request=subrequest)
.tox/py36/lib/python3.6/site-packages/_pytest/fixtures.py:792: in execute
    return hook.pytest_fixture_setup(fixturedef=self, request=request)
.tox/py36/lib/python3.6/site-packages/pluggy/__init__.py:617: in __call__
    return self._hookexec(self, self._nonwrappers + self._wrappers, kwargs)
.tox/py36/lib/python3.6/site-packages/pluggy/__init__.py:222: in _hookexec
    return self._inner_hookexec(hook, methods, kwargs)
.tox/py36/lib/python3.6/site-packages/pluggy/__init__.py:216: in <lambda>
    firstresult=hook.spec_opts.get('firstresult'),
.tox/py36/lib/python3.6/site-packages/_pytest/fixtures.py:823: in pytest_fixture_setup
    result = call_fixture_func(fixturefunc, request, kwargs)
.tox/py36/lib/python3.6/site-packages/_pytest/fixtures.py:715: in call_fixture_func
    res = fixturefunc(**kwargs)
.tox/py36/lib/python3.6/site-packages/_pytest/tmpdir.py:125: in tmpdir
    x = tmpdir_factory.mktemp(name, numbered=True)
.tox/py36/lib/python3.6/site-packages/_pytest/tmpdir.py:41: in mktemp
    keep=0, rootdir=basetemp, lock_timeout=None)
.tox/py36/lib/python3.6/site-packages/py/_path/local.py:863: in make_numbered_dir
    udir = rootdir.mkdir(prefix + str(maxnum+1))
.tox/py36/lib/python3.6/site-packages/py/_path/local.py:465: in mkdir
    py.error.checked_call(os.mkdir, fspath(p))
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

self = <module 'py.error'>, func = <built-in function mkdir>
args = ('/private/var/folders/7x/97jnmnt13sl46bx2mc9chzpm0000gn/T/pytest-of-asottile/pytest-32/test_a_0',)
kwargs = {}, __tracebackhide__ = False, cls = <class 'py.error.EEXIST'>
value = FileExistsError(17, 'File exists')
tb = <traceback object at 0x1020f57c8>, errno = 17

    def checked_call(self, func, *args, **kwargs):
        """ call a function and raise an errno-exception if applicable. """
        __tracebackhide__ = True
        try:
            return func(*args, **kwargs)
        except self.Error:
            raise
        except (OSError, EnvironmentError):
            cls, value, tb = sys.exc_info()
            if not hasattr(value, 'errno'):
                raise
            __tracebackhide__ = False
            errno = value.errno
            try:
                if not isinstance(value, WindowsError):
                    raise NameError
            except NameError:
                # we are not on Windows, or we got a proper OSError
                cls = self._geterrnoclass(errno)
            else:
                try:
                    cls = self._geterrnoclass(_winerrnomap[errno])
                except KeyError:
                    raise value
>           raise cls("%s%r" % (func.__name__, args))
E           py.error.EEXIST: [File exists]: mkdir('/private/var/folders/7x/97jnmnt13sl46bx2mc9chzpm0000gn/T/pytest-of-asottile/pytest-32/test_a_0',)

.tox/py36/lib/python3.6/site-packages/py/_error.py:86: EEXIST
====================== 1 passed, 1 error in 0.31 seconds =======================
@nicoddemus nicoddemus added type: bug problem that needs to be addressed platform: windows windows platform-specific problem platform: mac mac platform-specific problem labels May 4, 2018
@nicoddemus
Copy link
Member

Thanks @asottile!

@nicoddemus
Copy link
Member

Strangely I cannot reproduce this on Windows (which is also case insensitive but preserving).

I see the relevant code and there's a try/except for py.error.EEXIST being properly handled:

https://github.com/pytest-dev/py/blob/5f1f794f5c5aa25802ea61b4430648438c8d4b93/py/_path/local.py#L862-L867

@asottile could you please report which version of py you are using?

@nicoddemus nicoddemus removed the platform: windows windows platform-specific problem label May 4, 2018
@RonnyPfannschmidt
Copy link
Member

this is a path normalization issue in pylib since osx is case-insensitive while pylib is developed based on posix

i beleive a few normcase calls are missing in pylib

@asottile
Copy link
Member Author

asottile commented May 4, 2018

$ pip freeze --all
attrs==18.1.0
more-itertools==4.1.0
pip==10.0.1
pluggy==0.6.0
py==1.5.3
pytest==3.5.1
setuptools==39.1.0
six==1.11.0
wheel==0.31.0

(the py version is also in the original message since pytest spits it out!)

@nicoddemus
Copy link
Member

(Duh sorry, the py version appears on the header 😅)

this is a path normalization issue in pylib

I'm not sure @RonnyPfannschmidt, the code which triggers this error is properly guarded with a try/except which should catch py.error.EEXIST:

            try:
                udir = rootdir.mkdir(prefix + str(maxnum+1))
                if lock_timeout:
                    lockfile = create_lockfile(udir)
                    atexit_remove_lockfile(lockfile)
            except (py.error.EEXIST, py.error.ENOENT, py.error.EBUSY):

(https://github.com/pytest-dev/py/blob/5f1f794f5c5aa25802ea61b4430648438c8d4b93/py/_path/local.py#L862-L867)

Not sure what's going on here...

@RonnyPfannschmidt
Copy link
Member

@asottile please provide the output with --fulltrace --showlocals

@asottile
Copy link
Member Author

asottile commented May 4, 2018

The issue is here:

https://github.com/pytest-dev/py/blob/5f1f794f5c5aa25802ea61b4430648438c8d4b93/py/_path/local.py#L854-L879

(Pdb) repr(parse_num(path))
'None'

so the first time through the loop it sets lastmax (None) = maxnum (-1)

The second pass through the loop, lastmax (-1) == maxnum (-1) so it reraises here

Output is a bit long so here's a paste: https://i.fluffy.cc/rWxqMTZQBzCqZwtpRJWdc384Q5r5G6R1.html

@RonnyPfannschmidt
Copy link
Member

good find - quite an edge-case

@asottile
Copy link
Member Author

asottile commented May 6, 2018

Here's a "simpler" reproduction:

def test(tmpdir):
    tmpdir.make_numbered_dir('a')
    tmpdir.make_numbered_dir('A')

@asottile
Copy link
Member Author

asottile commented May 6, 2018

I guess the further rootcause is this trashfire:

Python 3.6.5 (default, Mar 30 2018, 06:41:53) 
[GCC 4.2.1 Compatible Apple LLVM 9.0.0 (clang-900.0.39.2)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> import os.path
>>> os.path.normcase('A')
'A'

@asottile
Copy link
Member Author

asottile commented May 6, 2018

Reading through a few bpo bugs, (notably this one), it seems that normcase cannot be trusted. It has (~roughly) the following behaviours:

  • posix (including macos): lambda s: s
  • windows: lambda s: s.lower()

which ignores the filesystem type entirely (you could have a case-insensitive mount on a posix platform and you could have a case sensitive mount on a windows platform).

I guess the fix (?) is to write a "more correct" normcase which does some filesystem detection (probably easiest to do this via feature detection?)

@RonnyPfannschmidt
Copy link
Member

since filesystem detection is going to be a expensive mess, i would like to propose to normalize filenames to lowercase and to a certain unicode encoding if necessary, making the normalization function an optional argument defaulting to methodcaller('lower') seems the most sensible to me for now

@asottile
Copy link
Member Author

makes sense, this patch to py "fixes" it:

diff --git a/py/_path/local.py b/py/_path/local.py
index 5a785b0f..ec56cb2e 100644
--- a/py/_path/local.py
+++ b/py/_path/local.py
@@ -810,6 +810,7 @@ class LocalPath(FSBase):
         if rootdir is None:
             rootdir = cls.get_temproot()
 
+        prefix = prefix.lower()
         nprefix = normcase(prefix)
         def parse_num(path):
             """ parse the number out of a path (if it matches the prefix) """

@RonnyPfannschmidt
Copy link
Member

@asottile great job, please submit it together with the proposed testcase

@asottile
Copy link
Member Author

it (unfortunately) breaks this test case:

    def test_make_numbered_dir_case_sensitive(self, tmpdir, monkeypatch):
        # https://github.com/pytest-dev/pytest/issues/708
        monkeypatch.setattr(py._path.local, 'normcase', lambda path: path)
        monkeypatch.setattr(tmpdir, 'listdir',
                            lambda: [tmpdir._fastjoin('case.0')])
        numdir = local.make_numbered_dir(prefix='CAse.', rootdir=tmpdir,
                                         keep=2, lock_timeout=0)
        assert numdir.basename.endswith('.0')

though that's just a consequence of (bad) mocking?

@RonnyPfannschmidt
Copy link
Member

@asottile as far as i can tell that test is bad - it ensures that we break on case-insensitive filesystems

@asottile
Copy link
Member Author

here's that PR: pytest-dev/py#186

@asottile
Copy link
Member Author

This has a fix from the py side, but would need a release over there

joebonrichie pushed a commit to solus-packages/py that referenced this issue Aug 15, 2023
Summary: - Fix pytest-dev/pytest#3451: don't make assumptions about fs case sensitivity in `make_numbered_dir`.

Test Plan: Tested through `pylint` and `python-pytest`.

Reviewers: #triage_team, sunnyflunk

Reviewed By: #triage_team, sunnyflunk

Differential Revision: https://dev.solus-project.com/D3469
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment