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

Support running via -m #1474

Closed
brettcannon opened this issue Feb 23, 2018 · 29 comments
Closed

Support running via -m #1474

brettcannon opened this issue Feb 23, 2018 · 29 comments
Labels
Type: Enhancement 💡 This is a feature or enhancement request.

Comments

@brettcannon
Copy link
Member

It would be great if python3 -m pipenv worked in order to support Windows users better as neither pipsi nor --user installs are accessible by default. As it currently stands there's a message about pew needing to be on the PATH, although there's an environment variable called PIPENV_VENV_IN_PROJECT which doesn't seem to have a corresponding command-line flag (I also didn't think to test if this removed the pew warning before leaving work today).

@techalchemy
Copy link
Member

Please see this comment -- you need to call pip via python3 -m in the first place because you haven't configured your %PATH% environment variable, as the error message indicates.

@uranusjr
Copy link
Member

I think making things work with -m is good architecture in general though, whether or not there are alternatives. This would also work around the current workaround we have with the pipsi installation that pipsi install pew is required. [sys.executable, ‘-m’, ‘pew’] would always work as long as pew is installed during Pipenv’s installation (it is with the current setup).

This would require pew to work that way first.

@techalchemy techalchemy reopened this Feb 23, 2018
@techalchemy
Copy link
Member

@uranusjr I am far from an expert with regard to this aspect of our implementation so I will reopen for now

Do you have a clear understanding of what is preventing pew from working this way?

@techalchemy techalchemy added the Type: Enhancement 💡 This is a feature or enhancement request. label Feb 23, 2018
@uranusjr
Copy link
Member

It needs to add a if __name__ == '__main__': block somewhere, and call the program’s entry point in it. This is how Pipenv does it.

pew’s entry point is pew.pew:pew, judging from its configuration in setup.py, so one possible implementation would be to add a __main__.py with

from .pew import pew
if __name__ == '__main__':
    pew()

@techalchemy
Copy link
Member

@uranusjr ah I thought you were saying there was something more significant

You should submit a PR upstream!

@uranusjr
Copy link
Member

uranusjr commented Feb 23, 2018

I have already too many things at hand. This is literally a three-liner but I just don’t want to go through the human part off submitting patches. Somebody please do that please? Pretty please? <insert cat gif here>

@jtratner
Copy link
Collaborator

jtratner commented Feb 23, 2018 via email

@brettcannon
Copy link
Member Author

And if @jtratner can't find the time I can try as well.

@kennethreitz
Copy link
Contributor

Requiring pew to be installed with the same version of python would be a pain — let's fall back to a system-available pew if -m pew doesn't work.

@kennethreitz
Copy link
Contributor

not necessary, just being thorough.

@jtratner
Copy link
Collaborator

PR up - pew-org/pew#177

@kennethreitz
Copy link
Contributor

merged :)

@kennethreitz
Copy link
Contributor

waiting on a release

@techalchemy
Copy link
Member

With the current release:

C:\Users\Dan.Ryan>pipenv --version
pipenv, version 11.0.2

C:\Users\Dan.Ryan>py -3 -m pipenv --version
pipenv, version 11.0.2

/cc @brettcannon

Thanks for the contributions all 🎆

@uranusjr
Copy link
Member

uranusjr commented Mar 7, 2018

@techalchemy Most operations (e.g. install, upgrade, run, shell) wouldn’t work though, unless you also expose the pew executable. That part requires a new Pew release to work.

@kennethreitz
Copy link
Contributor

this is not going to happen.

@kennethreitz
Copy link
Contributor

i experimented heavily with getting us to run pew directly, and it's just not feasible.

@kennethreitz
Copy link
Contributor

so, we're going to have to stick with subprocessing it out and expecting it to be in the PATH.

@kennethreitz
Copy link
Contributor

although, it's very fun to write pew.pew.pew()

@uranusjr
Copy link
Member

uranusjr commented Mar 7, 2018

I don’t mean running it directly, but with subprocess with python -m pew instead. This should be doable after pew-org/pew#177 is released.

@kennethreitz
Copy link
Contributor

ah, i hadn't considered that!

@brettcannon
Copy link
Member Author

So should this issue be re-opened to track calling pew with -m? Or should I open another issue?

@uranusjr uranusjr reopened this Mar 7, 2018
@uranusjr
Copy link
Member

uranusjr commented Mar 7, 2018

Let’s just track it here.

@kennethreitz
Copy link
Contributor

👍

@kennethreitz
Copy link
Contributor

making this work anyway.

kennethreitz added a commit that referenced this issue Mar 7, 2018
@kennethreitz
Copy link
Contributor

done

@kennethreitz
Copy link
Contributor

released

@berdario
Copy link
Contributor

Sorry for holding this up 😞

@techalchemy
Copy link
Member

No worries @berdario we included pew in our vendored packages within a few hours of my PR probably mostly to support the pew.pew.pew() invocation

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Enhancement 💡 This is a feature or enhancement request.
Projects
None yet
Development

No branches or pull requests

6 participants