Skip to content
This repository has been archived by the owner on Feb 14, 2024. It is now read-only.

Use shutil.which() in subprocess.check_call() when executing mamba/micromamba/conda #103

Merged
merged 2 commits into from Jan 2, 2023

Conversation

vasiljevic
Copy link
Contributor

This PR implement a recommendation from the Python documentation for the Popen constructor: "For maximum reliability, use a fully qualified path for the executable. To search for an unqualified name on PATH, use shutil.which()".

Practically, this PR fixes conda/mamba execution on Windows as we can see from the following examples.

Linux:

username:~$ conda --version
conda 22.11.1
username:~$ python -c "import subprocess; subprocess.check_call(['conda','--version'])"
conda 22.11.1
username:~$ python -c "import subprocess; import shutil; subprocess.check_call([shutil.which('conda'),'--version'])"
conda 22.11.1

Windows:

C:\Users\un>conda --version
conda 22.9.0

C:\Users\un>python -c "import subprocess; subprocess.check_call(['conda','--version'])"
Traceback (most recent call last):
  File "<string>", line 1, in <module>
  File "C:\Users\neboj\anaconda3\envs\build-env\Lib\subprocess.py", line 408, in check_call
    retcode = call(*popenargs, **kwargs)
              ^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "C:\Users\neboj\anaconda3\envs\build-env\Lib\subprocess.py", line 389, in call
    with Popen(*popenargs, **kwargs) as p:
         ^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "C:\Users\neboj\anaconda3\envs\build-env\Lib\subprocess.py", line 1022, in __init__
    self._execute_child(args, executable, preexec_fn, close_fds,
  File "C:\Users\neboj\anaconda3\envs\build-env\Lib\subprocess.py", line 1491, in _execute_child
    hp, ht, pid, tid = _winapi.CreateProcess(executable, args,
                       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
FileNotFoundError: [WinError 2] The system cannot find the file specified

C:\Users\un>python -c "import subprocess; import shutil; subprocess.check_call([shutil.which('conda'),'--version'])"
conda 22.9.0

Copy link
Member

@martinRenou martinRenou left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Neat! Thanks!

I just have a minor comment/question

MAMBA_AVAILABLE = True
MAMBA_COMMAND = shutil.which("mamba")
if MAMBA_COMMAND:
check_call([MAMBA_COMMAND, "--version"], **SILENT)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you think we still need these check_call calls? Could shutil.which be enough?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we don't need check_call.

I have not proposed further refactoring just to keep focus on the proposed fix. But if we agree the fix is proper, It makes sense to additionally simplify the code. I have just pushed a commit.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great! Thanks a lot!

@martinRenou martinRenou merged commit b61f799 into jupyterlite:main Jan 2, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants