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

reloader uses "-m" instead of file if possible #1416

Merged
merged 2 commits into from Dec 8, 2018
Merged

Conversation

davidism
Copy link
Member

@davidism davidism commented Dec 8, 2018

Unfortunately, Python doesn't make the fact that it was run as python -m module available, it replaces -m module with the path to the module in sys.argv. However, Python also has different behavior for python -m module vs python file.py: running a file adds its directory to sys.path. When the reloader was run with python -m werkzeug.serving, it would end up calling python /path/to/werkzeug/serving.py on reload, which added /path/to/werkzeug to sys.path. This caused import conflicts with module names in Werkzeug shadowing builtins.

To determine if -m was used, I looked at __main__.__package__. For all the cases I could think of, it seems that if that's not None, then it was run as -m, otherwise it was run as a file. If we're running as -m, then the module name can be reconstructed by appending the name of the file in sys.argv[0] to __main__.__package__.

This includes a workaround for Flask's workaround of the previous behavior. Flask currently replaces sys.argv[0:0] with ["-m", "flask"]. If Werkzeug sees "-m" as the first arg it assumes the command line is already correct. This can be removed once Flask depends on the next version of Werkzeug.

I tested every combination I could think of on Python 3 and 2, Linux and Windows:

  • python app.py
  • python -m app
    • As a module and as a package with __main__.py.
  • python -m app.cli
    • As a module and as a subpackage with __main__.py.
  • python -m werkzeug.serving
    • Also with Werkzeug installed as a zip file (egg). (Yes, it's not supported, but still.)
  • python -m flask run
  • flask run
  • python2 -m /path/to/app (without ".py" extension, Python 2 only)

Also removed a comment saying not to use relative imports in werkzeug.serving (8b2b621), now that we reload with -m relative imports would work correctly.

Closes #461
Closes #531
Closes #593
Closes pallets/flask#2777

Python replaces "-m module" with the path to the module file. But this
causes the reloader to be called with the path instead of "-m". Calling
Python with a file adds its directory to the path. Adding "werkzeug" to
the path broke imports.
@davidism davidism added this to the 0.15 milestone Dec 8, 2018
@davidism davidism merged commit 8c61acb into master Dec 8, 2018
@davidism davidism deleted the reload-module branch December 8, 2018 22:28
@waynew
Copy link

waynew commented Dec 8, 2018

I don't remember if I ran into this problem when I was launching my app via an entrypoint as defined in setup.py... I do know I had problems with the python -m approach in the past, so this is a super welcome improvement!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

import collision between http.py and Python 3's http modules Reloader, python -m, and sys.path
2 participants