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

Support `python -m invoke` #444

Closed
pekkaklarck opened this Issue May 22, 2017 · 10 comments

Comments

Projects
None yet
2 participants
@pekkaklarck

pekkaklarck commented May 22, 2017

This would make it a lot easier to use Invoke with different Python versions like python2.7 -m invoke or py -3 -m invoke. Same approach works with many other tools like pip and pytest.

@bitprophet

This comment has been minimized.

Member

bitprophet commented May 23, 2017

Starting to suspect @pekkaklarck and @lancelote are co-workers or something 😉

This just requires a package/__main__.py that invokes the CLI stub, I've already just made one and tested it out. (The fact that this trick was only added in 2.7 keeps tripping me up under 2.6...)

Will commit and push in a moment. Thanks!

bitprophet added a commit that referenced this issue May 23, 2017

@bitprophet bitprophet closed this May 23, 2017

@pekkaklarck

This comment has been minimized.

pekkaklarck commented May 24, 2017

Thanks for adding this so fast! I don't even know @lancelote so this must be just a case of great minds thinking a like. =)

Notice that on Python 2.6 you ought to be able to use python -m invoke.__main__. It's somewhat ugly but handy if you need it. Could add separate submodule like to run to allow using python -m invoke.run, but I doubt is adding any new Python 2.6 compatibility code worth it.

@bitprophet

This comment has been minimized.

Member

bitprophet commented May 24, 2017

Yea, I don't want to do extra work for 2.6 if possible, so no worries there. I just wasn't cognizant of the behavior you mention.

Anyway - this change isn't on PyPI yet but will be probably by Monday, FYI.

@pekkaklarck

This comment has been minimized.

pekkaklarck commented May 25, 2017

The current implementation of __main__ makes python -m invoke work but running python path/to/invoke fails with this traceback:

Traceback (most recent call last):
  File "/usr/lib/python2.7/runpy.py", line 162, in _run_module_as_main
    "__main__", fname, loader, pkg_name)
  File "/usr/lib/python2.7/runpy.py", line 72, in _run_code
    exec code in run_globals
  File "/home/peke/Devel/invoke/invoke/__main__.py", line 1, in <module>
    from .main import program
ValueError: Attempted relative import in non-package

This isn't a big deal when using invoke normally, but in some special cases being able to execute invoke directory directly can be useful. One such case is during invoke development itself when you want to test how latest code actually works. Fixing the problem ought to thus be worth the effort, especially when it only requires changing from .main import program to from invoke.main import program.

@bitprophet

This comment has been minimized.

Member

bitprophet commented May 27, 2017

Only downside with that is it would break the feature for anybody who has vendored Invoke, or who is attempting to execute it before it's installed to sys.path (which, ironically, is IIRC one of the main use cases for python -m, yes?).

Former case seems like not a big deal because I can't see someone using python -m on a vendored Invoke, but the latter worries me a bit. Even the old "well, CWD is usually on sys.path" tip doesn't seem like it squares with your path/to/invoke example? Thoughts welcome.

@pekkaklarck

This comment has been minimized.

pekkaklarck commented May 28, 2017

It's true that from invoke.main import program won't work if your program structure is something like myprog.vendor.invoke but I don't think how often people would like to run vendored invoke like that anyway. If the problem is considered real, fixing it is easy:

try:
    from .main import program
except ValueError:
    from invoke.main import program

It's also true that generally running python path/to/invoke won't work but you needed to use PYTHONPATH=path/to python path/to/invoke which doesn't make much sense compared to PYTHONPATH=path/to python -m invoke. When you have the invoke directory in your CWD, like when you've cloned the project and work in its root directory, running python invoke works great with tab-completion as a small bonus.

It would also be possible to make python path/to/invoke work by doing sys.path manipulation in __main__.py. That probably isn't worth the effort, though.

@bitprophet

This comment has been minimized.

Member

bitprophet commented May 31, 2017

Gotcha. Also, on review, I think I was missing something when I last thought about this; the "nice, modern" from .main import program truly is only necessary for vendored modules...it provides zero benefits in the other situations I was thinking of. So I don't really see a downside to making it explicit in this one module.

EXCEPT...I just made the change, and I think we're actually still kind of screwed, unless I'm continuing to be dumb:

  • Moved to another nearby project dir that has a valid tasks.py
  • python ../invoke/invoke --help blows up as noted (ValueError: Attempted relative import in non-package)
  • Made the import in __main__.py absolute
  • python ../invoke/invoke --help now works OK
  • However, anything more involved - like python ../invoke/invoke --list - still yields other package/not-a-package related ValueErrors from the rest of the Invoke source tree (e.g. once it gets to platform.py which tries to from .util import ...)
  • That's definitely unfixable without breaking ability for folks to vendor the bulk of the codebase, which is non-negotiable.

So...we could still make this import absolute, but I'd almost rather keep it relative so the behavior is consistently broken 🙃 either way I don't think it's going to work out terrifically well. thoughts? (tho I think I've now passed the point of diminishing returns for such an off-spec use case!)

@pekkaklarck

This comment has been minimized.

pekkaklarck commented May 31, 2017

This sounds strange. I thought you'd only get that ValueError about relative import when __name__ == '__main__' (see e.g. this SO answer). More importantly, I can run commands like python ../invoke --list and python Devel/invoke/invoke --list just fine in my system. The tasks.py I used was trivial but I can't see how that would matter.

@pekkaklarck

This comment has been minimized.

pekkaklarck commented May 31, 2017

Notice also that PEP-366 linked from the aforementioned SO answer explains how it's possible to explicitly set __package__ attribute in the main modlue to make relative imports work. I don't see much benefits using that trick compared to absolute imports, though.

@bitprophet

This comment has been minimized.

Member

bitprophet commented May 31, 2017

Hm that is odd then. (Also, not worried about __package__, I'm fine having __main__ be absolute, it's the rest of the main source tree that worries me.)

If you can put together concrete details about your setup (how you've got your checkout installed, which exact HEAD commit you're on, what's in your sys.path, what your interpreter is, etc etc etc) maybe a clue will fall out? Cuz right now I don't see why it'll work for you but not me.

Here's a raw gist of my session: https://gist.githubusercontent.com/bitprophet/72237c456a9622d7aa511b80a6b0fc98/raw/fa1a54ff17d68dbddb149665bdbb4986e58451a7/gistfile1.txt

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment