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

pip module: allow to use both 'name' and 'requirements' parameters #31396

Open
wants to merge 7 commits into
base: devel
Choose a base branch
from

Conversation

pilou-
Copy link
Contributor

@pilou- pilou- commented Oct 6, 2017

SUMMARY

Allow to use both name and requirements parameters. Integration test provided.

ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME

pip

ANSIBLE VERSION
ansible 2.5.0 (devel 7d74c126a9) last updated 2017/10/02 22:32:39 (GMT +200)
ADDITIONAL INFORMATION

Without the change:

fatal: [myapp]: FAILED! => {
    "changed": false,
    "failed": true,
    "invocation": {
        "module_args": {
            "chdir": "/src/myapp/",
            "extra_args": "--process-dependency-links",
            "name": "myapp[webapp]",
            "requirements": "requirements.txt",
            "virtualenv": "/home/pilou/venv",
            "virtualenv_python": "python2.7"
        }
    }
}

MSG:

parameters are mutually exclusive: ['name', 'requirements']

I tested with an old version of Pip (1.5.1), using both name and requirements is supported:

$ python -c "import six"
Traceback (most recent call last):
  File "<string>", line 1, in <module>
ImportError: No module named six
$ python -c "import jinja"
Traceback (most recent call last):
  File "<string>", line 1, in <module>
ImportError: No module named jinja
$ pip --version
pip 1.5.1 from /tmp/testenv2/local/lib/python2.7/site-packages (python 2.7)
$ echo six > /tmp/req.txt
$ pip install -r /tmp/req.txt Jinja
Downloading/unpacking Jinja
  Downloading Jinja-1.2.tar.gz (252kB): 252kB downloaded
  Running setup.py (path:/tmp/testenv2/build/Jinja/setup.py) egg_info for package Jinja
Downloading/unpacking six (from -r /tmp/req.txt (line 1))
  Downloading six-1.11.0-py2.py3-none-any.whl
Installing collected packages: Jinja, six
  Running setup.py install for Jinja
    building 'jinja._debugger' extension
    [...]
    deleting Jinja.egg-info/native_libs.txt
  Could not find .egg-info directory in install record for Jinja
Successfully installed Jinja six
Cleaning up...
$ python -c "import six; import jinja"
$ echo $?
0

@ansibot
Copy link
Contributor

@ansibot ansibot commented Oct 6, 2017

@ansibot ansibot added affects_2.5 bugfix_pull_request core_review module needs_triage python3 support:core labels Oct 6, 2017
@s-hertel s-hertel removed needs_triage python3 labels Oct 6, 2017
@robinro
Copy link
Contributor

@robinro robinro commented Oct 9, 2017

@pilou- Thanks for this PR.
Is there a reason to support this, other than replacing two module calls with one?

It that were the only reason I'm uncertain it's a good idea to enable this. I'm sure bugs in corner cases will be triggered when name and requirements are combined.

@pilou-
Copy link
Contributor Author

@pilou- pilou- commented Oct 9, 2017

pip executable allows to use both parameters, why the module could not handle that ? Moreover this limitation isn't mentioned in the module documentation.

Corner cases could be tested, which ones are you thinking of ?

@robinro
Copy link
Contributor

@robinro robinro commented Oct 11, 2017

Are you sure pip supports this? The help page does not allow a package name and a requirements file together and also the docs don't show the combination. Just because it "works" I wouldn't use it.

>> pip install --help              

Usage:   
  pip install [options] <requirement specifier> [package-index-options] ...
  pip install [options] -r <requirements file> [package-index-options] ...
  pip install [options] [-e] <vcs project url> ...
  pip install [options] [-e] <local project path> ...
  pip install [options] <archive url/path> ...

If we are sure all pip versions supported by ansible can do this and there is some hint that the behavior is officially supported by pip I'd be okay to merge this.

What happens when there are conflicting versions in the 'requirement specifier' and the requirements file?

@sivel
Copy link
Member

@sivel sivel commented Oct 11, 2017

$ echo six > requirements.txt
$ pip install -r requirements.txt iso8601
Collecting iso8601
  Using cached iso8601-0.1.12-py2.py3-none-any.whl
Collecting six (from -r requirements.txt (line 1))
  Using cached six-1.11.0-py2.py3-none-any.whl
Installing collected packages: iso8601, six
Successfully installed iso8601-0.1.12 six-1.11.0
$ echo 'six<1' > requirements.txt
$ pip install -r requirements.txt 'six>1'
Double requirement given: six<1 (from -r requirements.txt (line 1)) (already in six>1, name='six')

@ansibot ansibot added needs_rebase needs_revision stale_ci test and removed core_review labels Oct 18, 2017
@ansibot ansibot added bug and removed bugfix_pull_request labels Mar 2, 2018
@ansibot ansibot added the traceback label May 29, 2018
@ansibot
Copy link
Contributor

@ansibot ansibot commented Nov 10, 2018

@bcoca
Copy link
Member

@bcoca bcoca commented May 11, 2022

@pilou- can you rebase and address the issues?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
affects_2.5 bug collection module needs_rebase needs_revision packaging pre_azp support:core test traceback
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants