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

Add ~/.virtualenvs to python.venvFolders #4642

Closed
ace510 opened this issue Mar 5, 2019 · 16 comments
Closed

Add ~/.virtualenvs to python.venvFolders #4642

ace510 opened this issue Mar 5, 2019 · 16 comments
Assignees
Labels
area-linting bug Issue identified by VS Code Team member as probable bug good first issue

Comments

@ace510
Copy link

ace510 commented Mar 5, 2019

Issue Type: Bug

by default, the command ran when installing Pylint defaults to 'python.exe -m pip install -U pylint --user' and gives the error 'Can not perform a '--user' install. User site-packages are not visible in this virtualenv.'

Extension version: 2019.2.5433
VS Code version: Code - Insiders 1.32.0-insider (c23e8995f6e31b3540e0a2c3d4fdc58e6d7cba53, 2019-03-04T17:23:10.903Z)
OS version: Windows_NT x64 10.0.16299

System Info
Item Value
CPUs Intel(R) Core(TM) i7-2600 CPU @ 3.40GHz (8 x 3392)
GPU Status 2d_canvas: enabled
checker_imaging: disabled_off
flash_3d: enabled
flash_stage3d: enabled
flash_stage3d_baseline: enabled
gpu_compositing: enabled
multiple_raster_threads: enabled_on
native_gpu_memory_buffers: disabled_software
rasterization: enabled
surface_synchronization: enabled_on
video_decode: enabled
webgl: enabled
webgl2: enabled
Memory (System) 15.96GB (4.30GB free)
Process Argv
Screen Reader no
VM 0%
@ghost ghost added the triage-needed Needs assignment to the proper sub-team label Mar 5, 2019
@DonJayamanne DonJayamanne self-assigned this Mar 5, 2019
@ghost ghost removed the triage-needed Needs assignment to the proper sub-team label Mar 5, 2019
@cebor
Copy link

cebor commented Mar 11, 2019

Have the same issue with virtualenvwrapper.

Steps to reproduce this issue:

# install virtualenvwrapper
pip install virtualenvwrapper
source /usr/local/bin/virtualenvwrapper.sh

# make venv
mkvirtualenv foo

# create dirs
mkdir -p foo/.vscode
cd foo

# create vscode config with venv set
cat <<EOF > .vscode/settings.json
{
    "python.pythonPath": "~/.virtualenvs/foo/bin/python"
}
EOF

# create python file
echo "print('foo')" > foo.py

# open vscode 
code .

After vscode has opened, click on foo.py and wait for the Linter pylint is not installed message. Press Install and see the error.

Weirdly enough, the terminal recognizes the virtualenv correctly.

@karlcow
Copy link

karlcow commented Mar 14, 2019

Stumbled on this today too using virtualenvwrapper.
I wonder what changed.

#808 seems to be a similar issue.

@karlcow
Copy link

karlcow commented Mar 14, 2019

So it seems that setting globalModuleInstallation to true fixes it.

Capture d’écran 2019-03-14 à 09 50 59

but that seems not what we want. We just want the module to be installed in the current environment. So that is slightly weird. Let's try.

I have set globalModuleInstallation to true in my workspace settings and it worked.
The linter was installed in the virtualenv.

ls -al ~/.virtualenvs/webcompatcom/lib/python2.7/site-packages/flake8
total 456
drwxr-xr-x   25 karl  staff    800 14 mar 09:53 .
drwxr-xr-x  110 karl  staff   3520 14 mar 09:53 ..
-rw-r--r--    1 karl  staff   2118 14 mar 09:53 __init__.py
…

and not globally.

@brettcannon
Copy link
Member

It seems we are not detecting that the virtual environments created by virtualenvwrapper are not virtual environments. I'm not sure if it's because it's in a global location or some other reason.

@cebor @karlcow if you set "python.venvPath": "~/.virtualenvs" does that solve the problem?

@karlcow
Copy link

karlcow commented Apr 24, 2019

@brettcannon yes that worked for me.

@brettcannon brettcannon changed the title --user flag with virtualenv Add .virtualenvs to python.venvFolders Apr 24, 2019
@DonJayamanne DonJayamanne changed the title Add .virtualenvs to python.venvFolders Add ~.virtualenvs to python.venvFolders May 9, 2019
@DonJayamanne DonJayamanne changed the title Add ~.virtualenvs to python.venvFolders Add ~/.virtualenvs to python.venvFolders May 9, 2019
@DonJayamanne
Copy link

DonJayamanne commented Jun 12, 2019

Prescribed Solutions:

  • Remove defaults from pythonPath.venvFolders defined in package.json
  • Leave python.venvFolders as is but with a default value of []
  • Hardcode the defaults currently defined package.json for python.venvFolders in code.
    • This way, the defaults are hardcoded and users cannot remove them (as we don't see the need for that - not yet)
  • Hardcode defaults in src/client/interpreter/locators/services/globalVirtualEnvService.ts in the getSearchPaths method.
    • Combine the value provided by user in settings along with the hardcoded values.
  • Add .virtualenvs as one of the hardcoded values.

Note:

  • Remove duplicates when combining the two lists

@brettcannon
Copy link
Member

Do note that ~/.virtualenvs is the wrong value as venvFolders is explicitly documented as being directories in your home directory (hence why there has been talk of dropping the setting and coming up with a new one that let users specify any directory; maybe we could just modify a user's settings to prepend ~/ and then add support for user expansion?).

@brettcannon
Copy link
Member

Also add whatever the following tools use:

  • pipenv
  • virtualenvwrapper

@brettcannon
Copy link
Member

Can probably also do #118 at the same time since it mostly likely should be done in the same place in the code. It also will let us say in the docs in package.json for this setting to say we support virtualenvwrapper and pipenv instead of listing specific directories.

@kimadeline kimadeline self-assigned this Jun 14, 2019
@kimadeline
Copy link

kimadeline commented Jun 14, 2019

Todo (summary of comments above):

@DonJayamanne
Copy link

DonJayamanne commented Jun 14, 2019

Aren't these out of scope (i.e. Separate issues)

Add folders used by pipenv and virtualenvwrapper to defaults (#5899) (OS-specific folders?)
 Add WORKON_HOME to defaults if set (#118)

@kimadeline
Copy link

#5899 was closed as a duplicate of this one, i believe it's just about adding .local/share/virtualenvs along with .virtualenvs?
for #118 you're right I'll remove the last 2 boxes since they belong to it

@brettcannon
Copy link
Member

@DonJayamanne they aren't out of scope if those are the projects the reasons we are adding these defaults. 😁 The key point is to know whether we know we need to add more or whether this covers everything.

And the WORKON_HOME is necessary to not have a half-baked solution for those tools, otherwise we are only covering the hard-coded defaults and not what the tools actually use for users to specify their preferences. They can definitely be done as separate PRs, but I think not doing it now leaves a rough edge in our support and we don't have a good history of going back and shaved those rough edges down quickly.

@brettcannon
Copy link
Member

And #5899 was closed because Luciana's specific ask in that issue was "support pipenv and virtualenvwrapper" which this might implicitly cover, otherwise we can do separate issues if necessary.

@kimadeline
Copy link

kimadeline commented Jun 17, 2019

After discussion with @brettcannon and @luabud we won't add pipenv's .local/share/.virtualenvs to python.venvFolders (so #5899), but we're still gonna add support for virtualenvwrapper.

@DonJayamanne
Copy link

@kimadeline Can we document the reason (or work around) in #5899

@kimadeline kimadeline added this to the 2019 - June Sprint 12 milestone Jun 17, 2019
@DonJayamanne DonJayamanne self-assigned this Jun 18, 2019
@ghost ghost removed the needs PR label Jun 19, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Jun 26, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-linting bug Issue identified by VS Code Team member as probable bug good first issue
Projects
None yet
Development

No branches or pull requests

7 participants