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

Session.run finds incorrect Python #256

Closed
JeremyBYU opened this issue Oct 11, 2019 · 8 comments · Fixed by #310
Closed

Session.run finds incorrect Python #256

JeremyBYU opened this issue Oct 11, 2019 · 8 comments · Fixed by #310

Comments

@JeremyBYU
Copy link

JeremyBYU commented Oct 11, 2019

Describe the bug
session.run('python', ..., ....) can not find the virtual environments python.

How to reproduce
Here is my noxfile.py. The last line is the where the issue occurs. There are no issues in creating a conda environment, installing conda packages, installing pip packages, and performing the pytest. I have verified it all and it completely works as expected (installing and using binaries and scripts in the virtual environment). The issue is only the last command which could only find a default python on the system (external). I have diagnosed the issue and it is explained below.

import nox
import os
VENV = 'conda' if os.name == 'nt' else None
NAME = 'win' if os.name == 'nt' else 'linux'

@nox.session(
    python=['3.6', '3.7'],
    reuse_venv=True,
    venv_backend=VENV,
    name='test_' + NAME)
def test(session):
    if VENV == 'conda':
        session.conda_install('--channel=conda-forge', 'shapely')
    session.install('-e', '.[dev]')
    session.run('pytest')
    session.run('python', 'setup.py', 'sdist', 'bdist_wheel')

Fix

The issue is here: https://github.com/theacodes/nox/blob/ac0992332a266f85152db8a5252b1fdf9818eb75/nox/command.py#L82

where cmd is python and path will be to the virtual environments Script directory. Here is a picture of what path is:
scripts

As you can see there is no python here. The correct python is here:

python

The change I performed to fix this issue is highlighted in the images. I changed this:
https://github.com/theacodes/nox/blob/ac0992332a266f85152db8a5252b1fdf9818eb75/nox/command.py#L36

to this:

if path:
        full_path = py.path.local.sysfind(program, paths=[path, os.path.dirname(path)])

Now everything works as expected. I am not sure if this is the best way to fix but it works well for me. If this is acceptable I can do a pull request if you would like.

Thanks for your great work!

@schuderer
Copy link
Contributor

schuderer commented Nov 14, 2019

Fellow user here. It seems weird to me that which(cmd, path) would lead to this, because its path parameter would be None in your example (you did not provide a path keyword parameter of run(), so path has the default value None). Therefore, the if path: conditional should not even fire, and your fix in command.py should not actually have any effect.

To take a step back: I'm using conda, too and I think I (so far) only saw python.exe in the Scripts subdirectory, not directly in the environment dir. Anyways, if the environment itself has set the system's PATH variable correctly when activating, nox's https://github.com/theacodes/nox/blob/ac0992332a266f85152db8a5252b1fdf9818eb75/nox/command.py#L41 should then be able to get the executable path from the system (i.e. PATH).

I understand that you checked that the environment works correctly, but just to be sure, in order to exclude the possibility that something strange is happening there, and find out more, could you humour me and try out the following commands after activating the environment in question in a command line window:

$ where python
$ set path
$ python -c "import sys; print(sys.executable)"
$ python -c "import py; print(py.path.local.sysfind('python'))"

Depending whether the results are what you expect, it should then become clear whether the environment itself may be contributing to the issue by setting a wonky PATH that nox somehow cannot deal with, or the problem lies somewhere else.

@JeremyBYU
Copy link
Author

I think we should make clear what environment we are taking about. There is my environment that hosts the project. Let's call this env "project". In my nox file I inform nox to create two more envs, one for "python 3.6" and another for "3.7". It inform nox to use conda to manage these. NOX then creates these envs using conda. These are the environments meant to be in use when running the nox session (test function).

So as for your request. Are you asking me to activate these environments that nox created? Im not on my computer now but I do believe they show up when using "conda info --envs". However they don't have any name just a path.

Finally I just want to be clear. Path is not none. It is what I stated above. I checked this through a debugger. I followed the logic before how path was initiialzed and it made sense but don't remember now. I'll try to show the trace to you later.

@JeremyBYU
Copy link
Author

Here is a list of the environments nox created with conda. It is the first two listed:

$ conda info --envs
# conda environments:
#
                         C:\Users\Jeremy\Documents\UMICH\Research\polylidarv2\.nox\test_win-3-6
                         C:\Users\Jeremy\Documents\UMICH\Research\polylidarv2\.nox\test_win-3-7
base                  *  C:\Users\Jeremy\Miniconda3

I activate manually here with this command (as requested):

 conda activate C:/Users/Jeremy/Documents/UMICH/Research/polylidarv2/.nox/test_win-3-6

Output of which python:

$ which python
/c/Users/Jeremy/Miniconda3/python

This does not seem to be the correct python it should find. I was hoping/expecting for C:\Users\Jeremy\Documents\UMICH\Research\polylidarv2\.nox\test_win-3-6\python.exe

However my feeling is that is isn't entirely the issue. I mentioned how path was not None and was actually set to Scripts directory (see first image). I have retraced the code and found that the reason why is because each session holds a member variable called self.bin. https://github.com/theacodes/nox/blob/ac0992332a266f85152db8a5252b1fdf9818eb75/nox/sessions.py#L114

This variable is actually passed as path when calling session.run: https://github.com/theacodes/nox/blob/ac0992332a266f85152db8a5252b1fdf9818eb75/nox/sessions.py#L222

For windows this path (self.bin) is the Scripts directory. In windows this is where additional binaries/scripts are installed for a python installation, but the python executable is the root python environment (see second image, one directory above).

For Linux this is different, self.bin is set to the directory called /bin under the root python folder. This directory contains the additionally installed user binaries as well as the virtual environments python executable. Therefore this single directory is sufficient to search in.

So I think the problem is that for a windows python conda installation, two directories need to be searched. The base python installation and the Scripts directory.

@schuderer
Copy link
Contributor

Thanks a lot for the thorough analysis! 👍

Pretty weird that things seem to be wonky both on the conda level (PATH env var not set correctly, however apparently inconsequential for this problem) as well as the nox level, where I now see that, for Windows, Scripts is the hardcoded environment subdir for the python executable: https://github.com/theacodes/nox/blob/ac0992332a266f85152db8a5252b1fdf9818eb75/nox/virtualenv.py#L173

I just checked on a Windows machine (Anaconda 4.7.11) that I have access to, and I can confirm that the python binary is indeed located in the env's directory directly, and not in the Scripts subdirectory. Unfortunately, I'm not familiar with using nox with conda, personally, but I see that @tswast, who knows more about this, already saw your post.

Side note: I indeed mixed things up a bit in my previous post -- For some reason I thought that reuse_env=True would recycle the outer environment (that I referred to when asking you to run the commands), while in reality it just re-uses the nox-created environments. Glad you got closer to the culprit nevertheless!

@JeremyBYU
Copy link
Author

Yep. As for now things are working because I just manually hardcoded nox to also look at the main python installation. However not sure if that's the correct thing to do.

@tswast
Copy link
Contributor

tswast commented Nov 18, 2019

I used Scripts as the bin directory because that's where most of the binaries are in Windows conda environments.

Nox constructs an absolute path to the binary when running a command. https://sourcegraph.com/github.com/theacodes/nox@ac0992332a266f85152db8a5252b1fdf9818eb75/-/blob/nox/command.py#L87 This logic would need to be changed to support searching multiple directories for binaries.

@smarie
Copy link
Contributor

smarie commented Apr 20, 2020

Is running a command in a conda env with full path, strictly equivalent to activating the conda environment and then using the command without full path ?

  • If so, @tswast I think this could be solved as you suggest to support searching multiple directories for binaries, that is, by adding the root folder of created conda envs to the CondaEnv.bin. That would require a refactoring of ProcessEnv.bin (an optional string) into ProcessEnv.bin_paths (an optional list of strings), though. I'll try this locally and submit a PR if it works.

  • If not, we should in addition to the above, apply exec-wrappers to all commands in the created conda environment, so that they behave exactly like "conda activate " + "". I did not test this though.

smarie pushed a commit to smarie/nox that referenced this issue Apr 20, 2020
…ngle `bin`. `command.run` and `which` both have their `path` argument renamed `paths` and supporting a list of paths, to handle it. Fixes wntrblm#256
@tswast
Copy link
Contributor

tswast commented Apr 21, 2020

Is running a command in a conda env with full path, strictly equivalent to activating the conda environment and then using the command without full path ?

I assumed so, but I don't know the answer for sure.

theacodes pushed a commit that referenced this issue Jun 21, 2020
…ound was not the correct one (#310)

* Virtual environments now have a `bin_paths` attribute instead of a single `bin`. `command.run` and `which` both have their `path` argument renamed `paths` and supporting a list of paths, to handle it. Fixes #256

* Updated tests to handle the `paths` renaming of the `path` argument in `nox.command.run`, and the `bin` to `bin_paths` renaming in `virtualenv`

* First conda test on windows

* Black-ened code

* As per code review: Added a backwards-compatible `session.bin` property

* Fixed lint error

* Added back `bin` to all envs for compatibility with legacy api

Co-authored-by: Sylvain MARIE <sylvain.marie@se.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
4 participants