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

ensure we follow PEP-235 on case-insensitive filesystems #2761

Closed
merriam opened this issue Sep 7, 2017 · 17 comments

Comments

Projects
None yet
4 participants
@merriam
Copy link

commented Sep 7, 2017

Running PyTest with files including ZODB gives an odd problem: ZODB can be imported, but its component FileSystem cannot.

import ZODB   # That works fine.
import ZODB.FileStorage
ImportError: No module named FileStorage

On the other hand, running the same code from the Python interpreter runs fine.

This is version PyTest version 3.2.2, Python 2.7.13, and ZODB 5.2.4
There exists ZODB-5.2.4-py2.7.egg/ZODB/FileStorage/init.py, a ZODB.pth pointing to the egg, and no other modules have issues; just any submodule of ZODB. No odd permission errors. After an hour of experiments and exercising Google-Fu, I got nothing.

Any workaround would be appreciated.

@nicoddemus

This comment has been minimized.

Copy link
Member

commented Sep 7, 2017

Hi, can you post a reproducible example?

@RonnyPfannschmidt

This comment has been minimized.

Copy link
Member

commented Sep 8, 2017

does the error also happen when you install zodb as normal isntall instead of an egg
just to see if we mistreat eggs

@merriam

This comment has been minimized.

Copy link
Author

commented Sep 11, 2017

Hi. Sorry, I got slammed and didn't make a trimmed down example. This might help:

  1. The error I saw was from importing ZODB and ZODB.FileStorage in the 2.7 site-packages packages directory, from another package in the 2.7 site-packages directory.

  2. Changing the import error did not help; any subpackage of ZODB causes issues, while ZODB does not.

@RonnyPfannschmidt

This comment has been minimized.

Copy link
Member

commented Sep 11, 2017

again - does it work if ZODB is installed as a normal python package instead of a egg-package

@merriam

This comment has been minimized.

Copy link
Author

commented Sep 17, 2017

No.

@RonnyPfannschmidt

This comment has been minimized.

Copy link
Member

commented Sep 17, 2017

i see, thanks, i will try to experiment soon

@merriam

This comment has been minimized.

Copy link
Author

commented Oct 9, 2017

Well, I dropped this because it 'went away'. Now its back. This I know so far:

  • This shows up sometimes with PyTest, usually running a profiler. I haven't found a way around it. I don't
    know what causes it. I have some issues filed but haven't tracked it down.

    File "/Users/cmerriam/p/platform/cbmx/data/ZODB.py", line 41, in <module>
    import ZODB.FileStorage                                 # flake8: noqa
    ImportError: No module named FileStorage
    
    • Things I see:
      • Same issue from PyCharm or command line.

      • No issue running normal code from PyCharm or command line.

      • Once it starts to be an issue, it continues. Still don't know why.

      • Once it goes away it stays away.

      • My edit configuration has a warning "No Py.Test runner found for current configuration", though
        /opt/zzz/bin/py.test is in path

      • Seems to have occurred after a reboot.

      • No reason, at all, to suspect FileStorage.

      • This is in my ...../site-packages directory:

        $ ls -l ZODB*
        -rw-rw-r--  1 cmerriam  zzz  23 Oct  6 18:08 ZODB.pth
        
        ZODB-5.2.4-py2.7.egg:
        total 0
        drwxr-xr-x   9 cmerriam  zzz   306 Oct  6 18:08 EGG-INFO/
        drwxr-xr-x  74 cmerriam  zzz  2516 Oct  9 15:58 ZODB/
        
        
        $ cd ZODB-5.2.4-py2.7.egg
        $ tree
           |-EGG-INFO
           |-ZODB
           |---FileStorage
           |---__pycache__
           |---scripts
           |-----manual_tests
           |-----tests
           |---tests
        

.

  • Things that haven't worked:
    • Uninstall and reinstall PyTest
    • Restart pycharm checking environment
    • Right-click in project "clean compiled python files"
    • Running regular python test first.
    • Running pytest from command line in different directories
    • Reboot again
    • Pip Install pytest-runner
    • Install new zzz version
    • Run 'python setup.py install'
    • Moving ZODB-5.2.4-py2.7.egg/EGG-INFO/ to ZODB-5.2.4-py2.7.egg-info
@merriam

This comment has been minimized.

Copy link
Author

commented Oct 10, 2017

Updated more on the StackOverflow. There was a module hiding in the hierarchy named zodb.py and I'm using OS/X (case preserving, not case sensitive).

@merriam merriam closed this Oct 10, 2017

@RonnyPfannschmidt

This comment has been minimized.

Copy link
Member

commented Oct 10, 2017

@merriam oh my goodness, those little traps are so devious, we could have searched for months and months without ever seeing the light

glad you discovered it in the end

@merriam

This comment has been minimized.

Copy link
Author

commented Oct 10, 2017

As an FYI, there exists PEP 235 to address case sensitivity, sort of.

Would you still think of this as a problem in that Python and Py.Test created different behaviors? The offending lines were, inside of my data/zodb.py:

import ZODB  # flake8: noqa
import ZODB.FileStorage  # flake8: noqa

and in another file in the same data/ directory:

import ZODB # wanting the package ZODB in site-packages.

@RonnyPfannschmidt

This comment has been minimized.

Copy link
Member

commented Oct 11, 2017

thanks for the note, i was not aware of that one,

i think now we actually have a pytest bug

@RonnyPfannschmidt RonnyPfannschmidt changed the title Import errors on subsystems for ZODB. ensure we follow PEP-235 on case-insensitive filesystems Oct 11, 2017

@merriam

This comment has been minimized.

Copy link
Author

commented Oct 19, 2017

To be more clear, when doing an import ZODB from cbmx/data/zodb.py:

  • Pytest is picking up cbmx/data/zodb.py.
  • Python is picking up '/opt/CBMX/lib/python2.7/site-packages/ZODB-5.2.4-py2.7.egg/ZODB/init.pyc'

These assertions pass under pytest, but not under python:

mods to site-packages/cbmx/data/zodb.py

before = sys.modules.copy()
import ZODB                                             # flake8: noqa
after = sys.modules.copy()
difference = sorted([name for name in set(after.keys()).difference(set(before.keys()))])
assert len(difference) == 0
assert 'cbmx/data' in ZODB.__file__
assert ZODB.__file__ == __file__

We made one or two 'toy' file structures but cannot make pytest fail on the toys, yet.

@merriam

This comment has been minimized.

Copy link
Author

commented Oct 20, 2017

As a bit more information on this bug, I'm looking at the offending 'import ZODB'.

First, we call _pytest/assertion/rewrite.py:AssertionRewritingHood:find_module(), line 97ish. The name parameter is cbmx.data.ZODB and path=..../cbmx/data. These don't seem like the right parameters: the name should probably be 'ZODB' and the path should probably be None.

The execution continues through AssertionRewrite's load_module() is called, can't find cbmx.data.ZODB in sys.modules, even though cbmx.data.zodb is there. Eventually,
it works its way down to py/_builtin.py/exec_() and calls

exec2( obj -- which has looks like the (wrong) data/zodb.py, locals, globals)

The TL;DR, some import hook is getting the wrong name, like a resolver function, and deciding to tack the 'cbmx.data' on the front of a non-relative import.

@merriam

This comment has been minimized.

Copy link
Author

commented Nov 3, 2017

OK. To reframe this:

  • when AssertionRewrite hits an input statement like import ZODB from with a module named thing.zodb that was imported using a relative import like from .zodb import stuff, then itsfind_module is called with the name thing.ZODB instead of ZODB.
  • This error can be confirmed by dastardly hacking to convert thing.ZODB to ZODB at the beginning of find_module.
  • I do not understand what in pytest, or py, is calling find_module with this parameter.
  • If someone can give a hint where this happens, I can kill the bug. Without it, I cannot.

This only happens with case-insensitive volumes, such as the default OS/X file systems and default Windows filesystem. It only happens with PyTest, not Python. It appears to only happen after a relative import.

Any hint?

@nicoddemus

This comment has been minimized.

Copy link
Member

commented Nov 3, 2017

@merriam thanks for diving into this; those kind of bugs are the worst to track down.

find_module is called by Python's own import system, is not called by pytest or py.

I can reproduce the case you describe by (actually I can't, see note at the end) putting a debugger at the start of find_module which is triggered if the expression "zodb" in name.lower() is True. I also see thing.ZODB from the import ZODB statement. But following through that execution in the debugger, I see we return None from find_module at this point:

fn_pypath = py.path.local(fn)
if not self._should_rewrite(name, fn_pypath, state):
return None

Continuing the execution, I hit that same breakpoint again but now name is "ZODB" as we would expect. In that case, we hit the return in this line:

elif tp != imp.PY_SOURCE:
# Don't know what this is.
return None

I can see that in that case, tp is 5, which according to imp.py:

https://github.com/python/cpython/blob/dcfb0e3c04f1b29a0d09bb0a81dcd5ee5a5fef1a/Lib/imp.py#L36-L45

Here it makes sense to return None for the rewriter hook: we don't to rewrite package folders.

But then, continuing the debugging session (I was typing this while debugging), I noticed that actually everything works for me and my test passes. 😛

I decided to leave my notes here anyway because I think it could shine an understanding on how things work with the rewrite hook.

@merriam I created my project like this:

case_sensitive/
  thing/
    __init__.py
    test_foo.py
    zodb.py
pytest.ini 
# file: zodb.py
import ZODB.FileStorage
stuff = 1

# file: test_foo.py
from .zodb import stuff
def test(): pass

# file: __init__.py is empty

I'm using Python 2.7, and tried this experiment on master and 3.2.3. I'm probably missing some detail on my attempt to reproduce this, can you describe your example in detail for me to try debugging it again?

Thanks again for your efforts on this, we appreciate it.

@merriam

This comment has been minimized.

Copy link
Author

commented Nov 6, 2017

I'm a bit confused by the layout, and I'll take a crack this week at the consistency failing minimal ayout. My walkthough (OS/X, Python 2.7) did have a line in zodb.py of 'import ZODB' call find_module('thing.ZODB') instead find_module('ZODB'). The FileStorage error was simply that on the recursive find_module call (thing.zodb calling for thing.ZODB), it ran across trying to find the FileStorage feature.

In other words, it fails at the arguments to find_module(), and the rest is just the particular trainwreck that follows.

Could you do a quick recheck, changing import ZODB.FileStorage to from ZODB import FileStorage. I know it seems small; I just feel like its a blind check for a dot. Probably should be the same call. Passing would eliminate that guess.

Thank you for looking into this.

@asottile

This comment has been minimized.

Copy link
Member

commented Jun 23, 2019

Here's a minimal reproduction:

$ touch x/Y.py
$ touch y.py
$ PYTHONPATH=.:x python3
Python 3.7.3 (default, Mar 27 2019, 09:23:15) 
[Clang 10.0.1 (clang-1001.0.46.3)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> import imp
__main__:1: DeprecationWarning: the imp module is deprecated in favour of importlib; see the module's documentation for alternative uses
>>> imp.find_module('Y')
(<_io.TextIOWrapper name='Y.py' mode='r' encoding='utf-8'>, 'Y.py', ('.py', 'r', 1))
>>> import importlib.machinery
>>> importlib.machinery.PathFinder.find_spec('y')
ModuleSpec(name='y', loader=<_frozen_importlib_external.SourceFileLoader object at 0x101e42860>, origin='/private/tmp/y.py')
>>> importlib.machinery.PathFinder.find_spec('Y')
ModuleSpec(name='Y', loader=<_frozen_importlib_external.SourceFileLoader object at 0x101e13748>, origin='/private/tmp/x/Y.py')

This'll be solved by #5468

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.