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

Fix for #693 #696

Merged
merged 4 commits into from
Mar 17, 2014
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
64 changes: 62 additions & 2 deletions gunicorn/six.py
Original file line number Diff line number Diff line change
Expand Up @@ -286,6 +286,63 @@ def StringIO(buf=''):
_add_doc(u, """Text literal""")


def _check_if_pyc(fname):
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this might be a lot more readable if we used splitext

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, maybe we can use find_module and this wouldn't even need to check the suffix.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks like imp.load_module is also able to load compiled files so a combination of both should probably work. @jerynmathew did you tried to use them already?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @benoitc,
Sorry for the delay, busy at my day job.
I tested the imp.load_module(); it returns a module-object type.
The exec (or execfile) expects string, file or code-object type. Hence I'm not sure imp.load_module() is the right choice here, because once we obtain the module object from imp.load_module(), we still need to convert it to code object (which is done as shown in my code).

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's why I recommended only find_module.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I made a commit again. Hope this is alright!

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. I think that is easier to read. I particularly like the constants
instead of the file extensions.

It's unlikely to be a big problem in this case, but it would be good style
to bring the read call up above the conditional, wrap it in a try/finally
and close the file in the finally block.

As is, you duplicate the read and close call and don't guarantee to close
if there's an exception.

Could you make that change and then I'll merge this?

In gunicorn/six.py:

@@ -286,6 +286,50 @@ def StringIO(buf=''):
_add_doc(u, """Text literal""")

+def _check_if_pyc(fname):

I made a commit again. Hope this is alright!


Reply to this email directly or view it on
GitHubhttps://github.com//pull/696/files#r10633817
.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch! Sorry about the rookie mistake.
All done now.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not a problem at all! Thanks for the update.

""" Returns True if the extension is .pyc, False if .py and None if otherwise """
from imp import find_module
from os.path import realpath, dirname, basename, splitext

# Normalize the file-path for the find_module()
filepath = realpath(fname)
dirpath = dirname(filepath)
module_name = splitext(basename(filepath))[0]

# Validate and fetch
try:
fileobj, fullpath, (_, _, pytype) = find_module(module_name, [ dirpath ])

except ImportError:
raise IOError("Cannot find config file. Path maybe incorrect! : {0}".format(filepath))

return (pytype, fileobj, fullpath)


def _get_codeobj(pyfile):
""" Returns the code object, given a python file """
from imp import PY_COMPILED, PY_SOURCE

result, fileobj, fullpath = _check_if_pyc(pyfile)

# WARNING:
# fp.read() can blowup if the module is extremely large file.
# Lookout for overflow errors.
try:
data = fileobj.read()
finally:
fileobj.close()

# This is a .pyc file. Treat accordingly.
if result is PY_COMPILED:
# .pyc format is as follows:
# 0 - 4 bytes: Magic number, which changes with each create of .pyc file.
# First 2 bytes change with each marshal of .pyc file. Last 2 bytes is "\r\n".
# 4 - 8 bytes: Datetime value, when the .py was last changed.
# 8 - EOF: Marshalled code object data.
# So to get code object, just read the 8th byte onwards till EOF, and UN-marshal it.
import marshal
code_obj = marshal.loads(data[8:])

elif result is PY_SOURCE:
# This is a .py file.
code_obj = compile(data, fullpath, 'exec')

else:
# Unsupported extension
raise Exception("Input file is unknown format: {0}".format(fullpath))

# Return code object
return code_obj


if PY3:

import builtins
Expand All @@ -300,7 +357,7 @@ def reraise(tp, value, tb=None):
print_ = getattr(builtins, "print")

def execfile_(fname, *args):
return exec_(compile(open(fname, 'rb').read(), fname, 'exec'), *args)
return exec_(_get_codeobj(fname), *args)


del builtins
Expand All @@ -323,7 +380,10 @@ def exec_(code, globs=None, locs=None):
raise tp, value, tb
""")

execfile_ = execfile
def execfile_(fname, *args):
""" Overriding PY2 execfile() implementation to support .pyc files """
return exec_(_get_codeobj(fname), *args)


def print_(*args, **kwargs):
"""The new-style print function."""
Expand Down