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

pipenv --py raises exception outside of a Pipenv project #2305

Closed
pwalsh opened this issue Jun 5, 2018 · 11 comments
Closed

pipenv --py raises exception outside of a Pipenv project #2305

pwalsh opened this issue Jun 5, 2018 · 11 comments
Labels
good first issue Issues suitable as a newcomer to get familiar with Pipenv!

Comments

@pwalsh
Copy link
Contributor

pwalsh commented Jun 5, 2018

I'm running latest macOS, latest Homebrew-installed Python (2 and 3), and latest Homebrew installed Pipenv.

$ brew --version
Homebrew 1.6.6-88-gb088302
Homebrew/homebrew-core (git revision 70def; last commit 2018-06-04)

$ pipenv --version
pipenv, version 2018.05.18

$ python --version
Python 3.6.5

Compare

$ pipenv --where
No Pipfile present at project home. Consider running `pipenv install` first to automatically generate a Pipfile for you.
$ pipenv --venv
No virtualenv has been created for this project yet!

with

pipenv --py
Traceback (most recent call last):
  File "/usr/local/Cellar/pipenv/2018.5.18/libexec/bin/pipenv", line 11, in <module>
    load_entry_point('pipenv==2018.5.18', 'console_scripts', 'pipenv')()
  File "/usr/local/Cellar/pipenv/2018.5.18/libexec/lib/python3.6/site-packages/pipenv/vendor/click/core.py", line 722, in __call__
    return self.main(*args, **kwargs)
  File "/usr/local/Cellar/pipenv/2018.5.18/libexec/lib/python3.6/site-packages/pipenv/vendor/click/core.py", line 697, in main
    rv = self.invoke(ctx)
  File "/usr/local/Cellar/pipenv/2018.5.18/libexec/lib/python3.6/site-packages/pipenv/vendor/click/core.py", line 1043, in invoke
    return Command.invoke(self, ctx)
  File "/usr/local/Cellar/pipenv/2018.5.18/libexec/lib/python3.6/site-packages/pipenv/vendor/click/core.py", line 895, in invoke
    return ctx.invoke(self.callback, **ctx.params)
  File "/usr/local/Cellar/pipenv/2018.5.18/libexec/lib/python3.6/site-packages/pipenv/vendor/click/core.py", line 535, in invoke
    return callback(*args, **kwargs)
  File "/usr/local/Cellar/pipenv/2018.5.18/libexec/lib/python3.6/site-packages/pipenv/vendor/click/decorators.py", line 17, in new_func
    return f(get_current_context(), *args, **kwargs)
  File "/usr/local/Cellar/pipenv/2018.5.18/libexec/lib/python3.6/site-packages/pipenv/cli.py", line 217, in cli
    do_py()
  File "/usr/local/Cellar/pipenv/2018.5.18/libexec/lib/python3.6/site-packages/pipenv/core.py", line 1703, in do_py
    click.echo(which('python', allow_global=system))
  File "/usr/local/Cellar/pipenv/2018.5.18/libexec/lib/python3.6/site-packages/pipenv/core.py", line 121, in which
    location = project.virtualenv_location or os.environ.get('VIRTUAL_ENV')
  File "/usr/local/Cellar/pipenv/2018.5.18/libexec/lib/python3.6/site-packages/pipenv/project.py", line 314, in virtualenv_location
    if not self.is_venv_in_project():
  File "/usr/local/Cellar/pipenv/2018.5.18/libexec/lib/python3.6/site-packages/pipenv/project.py", line 219, in is_venv_in_project
    os.path.exists(os.path.join(self.project_directory, '.venv'))
  File "/usr/local/Cellar/pipenv/2018.5.18/libexec/bin/../lib/python3.6/posixpath.py", line 80, in join
    a = os.fspath(a)
TypeError: expected str, bytes or os.PathLike object, not NoneType
@pwalsh
Copy link
Contributor Author

pwalsh commented Jun 5, 2018

I can also reproduce on Alpine Linux. See this Dockerfile and the same exception raised in the test suite for my Emacs library that wraps Pipenv. I have not been locking my dependency on Pipenv to a version number, but this behaviour started appearing on automated test runs around 4 months ago.

@techalchemy
Copy link
Member

Yeah we’ve seen this error quite a lot. I completely reimplemented our python version lookup tooling internally to tackle this and wound up with a cross platform finder that just hasn’t landed yet.

Can you run against master and see if we’ve caught this one already? Or have you already tried?

@pwalsh
Copy link
Contributor Author

pwalsh commented Jun 5, 2018

Yeah we’ve seen this error quite a lot. I completely reimplemented our python version lookup tooling internally to tackle this and wound up with a cross platform finder that just hasn’t landed yet.

Oh, it is great to know that this is a known issue and a fix is on the way.

Can you run against master and see if we’ve caught this one already? Or have you already tried?

I haven't tried that as I have limited time hacking on pipenv.el at present. I tried just now with brew reinstall pipenv --HEAD (ref.) but it looks like the Pipenv brew formula for Pipenv doesn't support --HEAD. I'll check properly when I have a bit more time, and report back.

@techalchemy
Copy link
Member

I don't have access to a mac of any kind so I can't really say anything about brew other than that people seem to have very mixed experiences with the pipenv formula. I have historically been more of a vim person myself so I'm not really sure how to test your code offhand, otherwise I would just clone it and give it a try :p

@uranusjr uranusjr added the good first issue Issues suitable as a newcomer to get familiar with Pipenv! label Jun 6, 2018
@uranusjr
Copy link
Member

uranusjr commented Jun 6, 2018

Regarding this particular issue, the fix is essentially adding a check in do_py to ensure the project exists before the which call if system is False, and emit an error message if the check fails.

I’m marking this as a good first issue for people interested to work on. Bonus points if you

  1. Unify the checks and error messages in do_py and do_where.
  2. Refactor do_py to clean up the unneeded system argument.

@pwalsh
Copy link
Contributor Author

pwalsh commented Jun 6, 2018

@uranusjr you seem to be suggesting something quite different to @techalchemy who suggests the fix is already on master. If it is, what new issue is there to work on?

@uranusjr
Copy link
Member

uranusjr commented Jun 6, 2018

Oops, I should have actually tested this before commenting. There are only the bonus points left then. Basically there are a lot of if project_location is None checks in the code base at the moment, which is a little annoying, and is not very future-proof—this error will pop up somewhere else if we’re not careful. It would be wonderful if someone could refactor the code a little to make the check automatic, e.g. when you access the project location property, or call some functions, etc.

The error messages are also all standalone at the moment. You get different error messages for pipenv --where, pipenv --venv, and pipenv --py. It would also be nice if some refactor can be done to unify them (probably related to the first refactor; if you unify the checks, the error messages can probably be unified as well).

I’m going to close this since the title issue is fixed. The refactoring part would be welcomed still!

@uranusjr uranusjr closed this as completed Jun 6, 2018
@pwalsh
Copy link
Contributor Author

pwalsh commented Jun 6, 2018

@uranusjr the title issue is not confirmed as fixed, and is definitely not fixed in any released version of Pipenv. Seems quite premature to close this issue.

@uranusjr
Copy link
Member

uranusjr commented Jun 6, 2018

It is fixed in master. (BTW the Homebrew formula should probably support --HEAD… Someone please work on that). GitHub issues do not have a fixed-but-not-released state, so we close issues when they are fixed in master. Otherwise we won’t be able to know what fixes are already done and what are not, which is more problematic. Please do reopen if you find this to be unfixed in the next version though!

@pwalsh
Copy link
Contributor Author

pwalsh commented Jun 6, 2018

@techalchemy

Can you run against master and see if we’ve caught this one already? Or have you already tried?

Confirming that master fixes the issue.

@uranusjr

GitHub issues do not have a fixed-but-not-released state, so we close issues when they are fixed in master

Sure, no problem, and I do the same. For the future, it might be good practice to link in to either or both the commit that fixed the issue, or, tests that show the issue is fixed.

@techalchemy
Copy link
Member

@pwalsh - see 1e0882c

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Issues suitable as a newcomer to get familiar with Pipenv!
Projects
None yet
Development

No branches or pull requests

3 participants