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

conda bin_paths are missing Library\... subdirectories on windows #514

Closed
thatcr opened this issue Dec 16, 2021 · 6 comments · Fixed by #535
Closed

conda bin_paths are missing Library\... subdirectories on windows #514

thatcr opened this issue Dec 16, 2021 · 6 comments · Fixed by #535
Labels

Comments

@thatcr
Copy link

thatcr commented Dec 16, 2021

Describe the bug

When running python code that requires external executables from conda packages - i.e. ipopt, they cannot be found on the PATH.

How to reproduce

This noxfile fails - it should find 'ipopt.exe' on the PATH:

import nox

@nox.session(python="3.7", venv_backend="conda")
def conda_path_bug(session):
    session.conda_install("conda-forge::ipopt==3.11.1")  
    session.run("python", "-c", "import shutil; assert shutil.which('ipopt')")

Expected behavior

The PATH should be set to include the same entries as Anaconda's cwp.py provides:

new_paths = pathsep.join([prefix,
                         join(prefix, "Library", "mingw-w64", "bin"),
                         join(prefix, "Library", "usr", "bin"),
                         join(prefix, "Library", "bin"),
                         join(prefix, "Scripts")])
env = os.environ.copy()
env['PATH'] = new_paths + pathsep + env['PATH']
env['CONDA_PREFIX'] = prefix
@cjolowicz cjolowicz added the bug label Dec 22, 2021
@cjolowicz
Copy link
Collaborator

Thanks for reporting this, makes sense.

IIUC we should emulate the way conda activate changes the environment variables. What would be a portable and maintainable way to do this?

@cjolowicz
Copy link
Collaborator

If you'd like to have a go at this, take a look at CondaEnv.bin_paths:

https:/theacodes/nox/blob/441b2bc666e209757bdccafd647083493e14c1cc/nox/virtualenv.py#L218-L225

The bin_paths are used to locate the commands passed to session.run:

https:/theacodes/nox/blob/441b2bc666e209757bdccafd647083493e14c1cc/nox/sessions.py#L360

@Tolker-KU
Copy link
Contributor

Is a viable solution simply to hardcode the correct paths into the .bin_paths property in CondaEnv? The advantage of this approach is that it is easy to implement and very transparent.

Another approach is to hook into Conda in some way to retrieve the correct paths but this would be a lot more cumbersome to implement.

Both approaches are probably equally exposed to Conda changing something in its internals, so the simple approach seems more attractive to me.

Any thoughts on this?

@thatcr
Copy link
Author

thatcr commented Jan 20, 2022

Thank you for this - I have just confirmed that 2022.1.7 fixes the original issue.

More generally - conda activate can run arbitrary scripts from etc/conda/activate.d so the PATH modification isn't a complete solution, but probably works for 99.9% of packages.

Wrapping any calls to session.run or session.install with conda run --prefix <prefix> <cmd> would probably be the right (cross platform) way, since that deals with the mechanics of shells and activation before running the command.

The behaviour of conda activate is dependent on which packages are currently installed, and some do modify the PATH in a bespoke way, so it would be wrong to run once to extract the environment variables and then cache them for later.

@Tolker-KU
Copy link
Contributor

Happy to help 🙂

As far as I know the conda run feature is still experimental, so using it in Nox is maybe questionable?

I hope this fix works for (almost) everything as you mentioned. If that is not the case we can reconsider how to solve this.

@smarie
Copy link
Contributor

smarie commented Jan 25, 2022

Great job guys ! Indeed, I had only added the Scripts folder originally, it was definitely not complete :(

I like the conda run --prefix idea, this is definitely something to follow if the feature becomes official.

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

Successfully merging a pull request may close this issue.

4 participants