Re-allow including modules with non-identifier names (issue #44)

#79 Merged at 0d781ac
Repository
ptramsey
Branch
default
Repository
anthony_tuininga
Branch
default
Author
  1. Patrick Ramsey
Reviewers
Description

Hi, folks.

I'm new to cx_Freeze, bitbucket, and mercurial, so please let me know if I'm skipping a step here. I'd like to fix #44 if I can, since it's breaking win32com for us.

Since modules can have filenames that aren't valid python identifiers and still be imported, this patch disables that check. To make sure that this doesn't reopen #22, we now exclude module names containing a period.

I've checked and confirmed that the tests still pass on python 2.7.9 and 3.4.2.

Comments (5)

  1. Thomas Kluyver

    Thanks Patrick, it looks like you've done the right things with bitbucket and mercurial.

    I don't think sorting the suffixes makes any difference here; it will check with the Python-specific suffix first and not match the offending module, but it will still check with .so afterwards and find a match, so it's only the valid module name check that's preventing #22 from recurring. Indeed, at least on my system, the Python-version-specific suffixes are already first:

    In [2]: imp.get_suffixes()
    Out[2]: 
    [('.cpython-34m-x86_64-linux-gnu.so', 'rb', 3),
     ('.cpython-34m.so', 'rb', 3),
     ('.abi3.so', 'rb', 3),
     ('.so', 'rb', 3),
     ('.py', 'r', 1),
     ('.pyc', 'rb', 2)]
    

    Worse, I think the order of suffixes returned by imp.get_suffixes() is already meaningful, so we shouldn't be sorting them.

    The 'valid module name' check feels a bit arbitrary. The only meaningful check for a valid module name in Python is that they're valid identifiers. __import__ won't work with a ., but it wouldn't be much harder to write code that would load foo.bar.py as a module if you wanted. And the fact that __import__ seemingly accepts just about anything else seems like an implementation detail - it's not that anything without a dot is valid, it's that __import__ doesn't do any checking of validity.

    I guess this is a case where practicality should beat purity: we know pywin32 relies on 'modules' with dashes in the name, and hopefully not too many other things do that kind of import trick, so it's probably worth relaxing the checks a bit. But I'd like the naming and the docstring to reflect that this is a workaround, not really a validity check.

    Thanks for adding a test for this!

    1. Thomas Kluyver

      Indeed, there's an example of code that loads modules from filenames with dots in in cx_Freeze itself. We don't need to worry about that particular case, because that's for after the code has been frozen. But once you leave identifiers, there really is no good definition of a 'valid' module name.

      1. Patrick Ramsey author

        Fair enough. I'm happy to rename (or just omit and inline) that method. And you're right about sorting the suffix list being both unnecessary and harmful. I'll update this PR with both fixes when I get into work tomorrow morning.