fab -f stat.py uses os.stat instead of local file

[fillest]

f@hvdev:~$ mkdir test
f@hvdev:~$ cd test/
f@hvdev:~/test$ virtualenv venv
New python executable in venv/bin/python
Installing setuptools, pip, wheel...done.
f@hvdev:~/test$ source venv/bin/activate
(venv)f@hvdev:~/test$ pip install -q fabric
(venv)f@hvdev:~/test$ fab --version
Fabric 1.10.2
(venv)f@hvdev:~/test$ touch stat.py
(venv)f@hvdev:~/test$ fab --show=debug -f stat.py -l
Using fabfile '/home/f/test/stat.py'
Constants/functions for interpreting results of os.stat() and os.lstat().

Suggested usage: from stat import *

Available commands:

    S_IFMT
    S_IMODE
    S_ISBLK
    S_ISCHR
    S_ISDIR
    S_ISFIFO
    S_ISLNK
    S_ISREG
    S_ISSOCK

stat.py may have any content or deeper path - same result

[kaaveland]

I was definitely able to reproduce this, but I added some debugging to fabric.main and now I no longer can. What.

[kaaveland]

Can reproduce it again reliably, I have no idea what happened. As far as I know, I changed nothing. My terminal history is making me think I've gone insane. And it's only tuesday. :-(

As far as I can tell, sys.path looks good at the time we run __import__ (current working dir being the first entry). But __import__ still seems to prefer the file located in the standard library.

I'm not sure we should uncritically fix this, by the way. I just had the thought that someone could place socket.py in current working directory (like I just did) and if their fabfile import something that relies on socket (such as paramiko), things will break. It's basically all guarantees of correctness out the window.

That being said, I've previously had to import files that have names that __import__ refuse to allow, you can actually pass a file-handle to https://docs.python.org/2/library/imp.html#imp.load_module which would let you get around this (if __import__ actually is the problem.) I'll see if I can dig up the source of __import__ to check if it explains why this happens.

[kaaveland]

Looking at sys.modules prior to our loading the fabfile, it seems that a lot of standard library modules (including stat, socket etc) are already imported at this time, so I'm thinking that's probably why we're getting the wrong module back from __import__. However, upon consulting import.c, I have to admit that I'm finding it hard to follow this code. Might pick this up at some other time.

[kaaveland]

Definitely think we are hitting a case where we end up using the reload-functionality instead of the import functionality because a module by that name has already been loaded. Poking around in what imp.find_module and imp.load_module gives me it seems pretty clear that we're picking up the correct file on disk, but that the namespace has already been polluted. No clue if this is fixable at all.

[kaaveland]

Obviously, issuing del sys.modules[fabfile] takes care of it (just verified this), but if a module by that name had already been imported, it was probably going to be used for something (likely by fabric, since only our imports have run at this point). Removing it will break stuff.

I suggest we acknowledge that this exists, and maybe in load_fabfile we check whether fabfile in sys.modules and issue a warning or terminate when this happens. I think I am in favor of crashing here...

[bitprophet]

This definitely sounds like a sys.modules problem. I actually ran into one in some Invoke development just the other week - hilariously dumb stuff relating to running the test suites - I too ended up doing del sys.modules[blah] for the time being.

However, +1 for terminating with a warning if the user names their explicit fabfile something that shadows builtins like this. Mucking with sys.modules for actual average-user runtime behavior is a Bad Idea, it's only really acceptable for "support" things - like a test suite only the devs/contributors will be running.