conda_install and install args are now automatically double-quoted when needed.

[smarie]

Changelog (let me know if I should commit it somewhere):

• conda_install and install args are now automatically double-quoted when they contain a < or a >. Fixes conda_install should automatically escape args when they contain version constraints #311

[smarie]

Note: although the tests pass, it seems that this fix is only needed for conda and makes pip crash with InvalidRequirement. I'll update the PR.

Invalid requirement: '"setuptools"'
Traceback (most recent call last):
  File "C:\...\.nox\tests-2-7\lib\site-packages\pip\req\req_install.py", line 82, in __init__
    req = Requirement(req)
  File "C:\...\.nox\tests-2-7\lib\site-packages\pip\_vendor\packaging\requirements.py", line 96, in __init__
    requirement_string[e.loc:e.loc + 8]))
InvalidRequirement: Invalid requirement, parse error at "'"setupto'"

[theacodes]

This looks fine, but can you show me some invocations that cause this behavior?

[smarie]

For example this:

session.conda_install("pytest>7")

On windows, it runs with success ! Indeed the >7 is understood by cmd as "pipe the output to a new file named 7".

And here is the content of the file 7:

Collecting package metadata (current_repodata.json): ...working... done
Solving environment: ...working... done

## Package Plan ##

  environment location: C:\Miniconda3\envs\tools_py37

  added / updated specs:
    - pytest


The following packages will be downloaded:

    package                    |            build
    ---------------------------|-----------------
    certifi-2020.4.5.2         |           py37_0         157 KB
    pytest-5.4.3               |           py37_0         404 KB
    ------------------------------------------------------------
                                           Total:         561 KB

The following packages will be UPDATED:

  certifi                                 2020.4.5.1-py37_0 --> 2020.4.5.2-py37_0
  pytest                                       5.4.1-py37_0 --> 5.4.3-py37_0



Downloading and Extracting Packages

pytest-5.4.3         | 404 KB    |            |   0% 
pytest-5.4.3         | 404 KB    | 3          |   4% 
pytest-5.4.3         | 404 KB    | ##3        |  24% 
pytest-5.4.3         | 404 KB    | #####5     |  55% 
pytest-5.4.3         | 404 KB    | ########## | 100% 

certifi-2020.4.5.2   | 157 KB    |            |   0% 
certifi-2020.4.5.2   | 157 KB    | ###        |  31% 
certifi-2020.4.5.2   | 157 KB    | ########## | 100% 
Preparing transaction: ...working... done
Verifying transaction: ...working... done
Executing transaction: ...working... done

As can be seen above, the version constraint was not even seen by conda.

[smarie]

Note that there is a workaround today: the user has to quote the dependency, e.g. session.conda_install('"pytest>7"')

[theacodes]

You can remove this instead of leaving it commented out.

[smarie]

Done. This was for maintainability - but instead of writing this here, I edited the comment in the conda part so that future readers are not tempted to apply the same escaping procedure to the pip section.

[theacodes]

This looks good, but I'd want one of our maintainers that actually uses conda to review. Maybe @tswast?

[tswast]

I'm a bit confused as to why this is necessary. When I run conda install "urllib3<1.25", the extra quotes around the package shouldn't actually be sent to conda itself. Adding extra quotes doesn't seem to hurt (conda install "'urllib3<1.25'" does work), but why are they necessary?

[tswast]

It appears that this is a windows-only issue (https://docs.python.org/3/library/subprocess.html#subprocess.Popen)

On Windows, if args is a sequence, it will be converted to a string in a manner described in Converting an argument sequence to a string on Windows. This is because the underlying CreateProcess() operates on strings.

[tswast]

LGTM, since conda is able to handle the extra quotes on all platforms.

[smarie]

Great, thanks for the review @tswast ! I updated the branch, it should be ready for merging

[theacodes]

Thanks all!