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: Add support for various pip install modes #72097

Open
wants to merge 1 commit into
base: devel
Choose a base branch
from

Conversation

nightowlengineer
Copy link
Contributor

@nightowlengineer nightowlengineer commented Oct 4, 2020

SUMMARY

Introduces support for the various 'site' locations in pip - system (default), user, target, prefix, root. Fixes #56333, potential fix for #71781

ISSUE TYPE
  • Feature Pull Request
COMPONENT NAME

pip

ADDITIONAL INFORMATION

Currently pip can only be used to install to the 'user site' location by including extra_args, which is not a great user experience. The module also (correctly) marks any invocation with extra_args as 'changed' when in check mode. This will allow at least some tasks to be migrated away and have more accurate idempotency.

Before:

{
  "virtualenv": null,
  "changed": true,
  "requirements": null,
  "name": [
    "pyyaml"
  ],
  "stdout": "Collecting pyyaml==5.3.0\nInstalling collected packages: pyyaml\nSuccessfully installed pyyaml-5.3\n",
  "cmd": [
    "/usr/bin/pip3",
    "install",
    "pyyaml==5.3.0"
  ],
  "state": "present",
  "version": "5.3.0",
  "stderr": "",
  "invocation": {
    "module_args": {
      "virtualenv": null,
      "virtualenv_site_packages": false,
      "executable": "pip3",
      "chdir": null,
      "requirements": null,
      "name": [
        "pyyaml"
      ],
      "virtualenv_python": null,
      "editable": false,
      "umask": null,
      "virtualenv_command": "virtualenv",
      "extra_args": null,
      "state": "present",
      "version": "5.3.0"
    }
  }
}

After (user):

{
  "virtualenv": null,
  "changed": true,
  "requirements": null,
  "name": [
    "pyyaml"
  ],
  "stdout": "Collecting pyyaml==5.3.0\nInstalling collected packages: pyyaml\nSuccessfully installed pyyaml-5.3\n",
  "cmd": [
    "/usr/bin/pip3",
    "install",
    "--user",
    "pyyaml==5.3.0"
  ],
  "state": "present",
  "version": "5.3.0",
  "stderr": "",
  "invocation": {
    "module_args": {
      "virtualenv": null,
      "virtualenv_site_packages": false,
      "executable": "pip3",
      "chdir": null,
      "requirements": null,
      "name": [
        "pyyaml"
      ],
      "virtualenv_python": null,
      "editable": false,
      "umask": null,
      "site": "user",
      "virtualenv_command": "virtualenv",
      "extra_args": null,
      "state": "present",
      "version": "5.3.0",
      "path": null
    }
  }
}

After (prefix):

{
  "virtualenv": null,
  "changed": true,
  "requirements": null,
  "name": [
    "pyyaml"
  ],
  "stdout": "Collecting pyyaml==5.3.0\nInstalling collected packages: pyyaml\nSuccessfully installed pyyaml-5.3\n",
  "cmd": [
    "/usr/bin/pip3",
    "install",
    "--root",
    "/tmp/prefixed-path",
    "pyyaml==5.3.0"
  ],
  "state": "present",
  "version": "5.3.0",
  "stderr": "",
  "invocation": {
    "module_args": {
      "virtualenv": null,
      "virtualenv_site_packages": false,
      "executable": "pip3",
      "chdir": null,
      "requirements": null,
      "name": [
        "pyyaml"
      ],
      "virtualenv_python": null,
      "editable": false,
      "umask": null,
      "site": "root",
      "virtualenv_command": "virtualenv",
      "extra_args": null,
      "state": "present",
      "version": "5.3.0",
      "path": "/tmp/prefixed-path"
    }
  }
}

@ansibot ansibot added WIP This issue/PR is a work in progress. Nevertheless it was shared for getting input from peers. affects_2.11 feature This issue/PR relates to a feature request. module This issue/PR relates to a module. needs_triage Needs a first human triage before being processed. packaging Packaging category support:core This issue/PR relates to code supported by the Ansible Engineering Team. labels Oct 4, 2020
@ansibot
Copy link
Contributor

ansibot commented Oct 4, 2020

The test ansible-test sanity --test validate-modules [explain] failed with 1 error:

lib/ansible/modules/pip.py:0:0: doc-choices-do-not-match-spec: Argument 'site' in argument_spec defines choices as (['system', 'user', 'root', 'prefix', 'target']) but documentation defines choices as (['system', 'user', 'home', 'root', 'prefix', 'target'])

The test ansible-test sanity --test pylint [explain] failed with 12 errors:

lib/ansible/modules/pip.py:646:10: bad-whitespace: No space allowed after bracket           [ "site", "prefix", [ "path" ] ],           ^
lib/ansible/modules/pip.py:646:30: bad-whitespace: No space allowed after bracket           [ "site", "prefix", [ "path" ] ],                               ^
lib/ansible/modules/pip.py:646:39: bad-whitespace: No space allowed before bracket           [ "site", "prefix", [ "path" ] ],                                        ^
lib/ansible/modules/pip.py:646:41: bad-whitespace: No space allowed before bracket           [ "site", "prefix", [ "path" ] ],                                          ^
lib/ansible/modules/pip.py:647:10: bad-whitespace: No space allowed after bracket           [ "site", "root", [ "path" ] ],           ^
lib/ansible/modules/pip.py:647:28: bad-whitespace: No space allowed after bracket           [ "site", "root", [ "path" ] ],                             ^
lib/ansible/modules/pip.py:647:37: bad-whitespace: No space allowed before bracket           [ "site", "root", [ "path" ] ],                                      ^
lib/ansible/modules/pip.py:647:39: bad-whitespace: No space allowed before bracket           [ "site", "root", [ "path" ] ],                                        ^
lib/ansible/modules/pip.py:648:10: bad-whitespace: No space allowed after bracket           [ "site", "target", [ "path" ] ]           ^
lib/ansible/modules/pip.py:648:30: bad-whitespace: No space allowed after bracket           [ "site", "target", [ "path" ] ]                               ^
lib/ansible/modules/pip.py:648:39: bad-whitespace: No space allowed before bracket           [ "site", "target", [ "path" ] ]                                        ^
lib/ansible/modules/pip.py:648:41: bad-whitespace: No space allowed before bracket           [ "site", "target", [ "path" ] ]                                          ^

The test ansible-test sanity --test pep8 [explain] failed with 14 errors:

lib/ansible/modules/pip.py:646:11: E121: continuation line under-indented for hanging indent
lib/ansible/modules/pip.py:646:12: E201: whitespace after '['
lib/ansible/modules/pip.py:646:32: E201: whitespace after '['
lib/ansible/modules/pip.py:646:39: E202: whitespace before ']'
lib/ansible/modules/pip.py:646:41: E202: whitespace before ']'
lib/ansible/modules/pip.py:647:12: E201: whitespace after '['
lib/ansible/modules/pip.py:647:30: E201: whitespace after '['
lib/ansible/modules/pip.py:647:37: E202: whitespace before ']'
lib/ansible/modules/pip.py:647:39: E202: whitespace before ']'
lib/ansible/modules/pip.py:648:12: E201: whitespace after '['
lib/ansible/modules/pip.py:648:32: E201: whitespace after '['
lib/ansible/modules/pip.py:648:39: E202: whitespace before ']'
lib/ansible/modules/pip.py:648:41: E202: whitespace before ']'
lib/ansible/modules/pip.py:770:29: E128: continuation line under-indented for visual indent

click here for bot help

@nightowlengineer nightowlengineer force-pushed the fix-71781-add-support-for-pip-install-modes branch 3 times, most recently from 5f91c9d to 6c041e5 Compare Oct 5, 2020
@ansibot ansibot added the support:community This issue/PR relates to code supported by the Ansible community. label Oct 5, 2020
@nightowlengineer nightowlengineer force-pushed the fix-71781-add-support-for-pip-install-modes branch from dd4b6f2 to 4bea9bf Compare Oct 5, 2020
@nightowlengineer nightowlengineer force-pushed the fix-71781-add-support-for-pip-install-modes branch from 4bea9bf to 9b0f96f Compare Oct 5, 2020
@nightowlengineer nightowlengineer changed the title pip: [WIP] Add support for various pip install modes pip: Add support for various pip install modes Oct 5, 2020
@nightowlengineer nightowlengineer marked this pull request as ready for review Oct 5, 2020
@nightowlengineer
Copy link
Contributor Author

ready_for_review

@ansibot ansibot added core_review In order to be merged, this PR must follow the core review workflow. and removed WIP This issue/PR is a work in progress. Nevertheless it was shared for getting input from peers. labels Oct 5, 2020
@bcoca bcoca added P3 Priority 3 - Approved, No Time Limitation and removed needs_triage Needs a first human triage before being processed. labels Oct 6, 2020
@ansibot ansibot added the stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. label Oct 14, 2020
@ansibot ansibot added needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. pre_azp This PR was last tested before migration to Azure Pipelines. and removed core_review In order to be merged, this PR must follow the core review workflow. stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. labels Dec 9, 2020
@ansibot ansibot added the needs_rebase https://docs.ansible.com/ansible/devel/dev_guide/developing_rebasing.html label Jan 10, 2021
@ansibot ansibot removed the support:core This issue/PR relates to code supported by the Ansible Engineering Team. label Jan 18, 2021
@ansibot ansibot added the support:core This issue/PR relates to code supported by the Ansible Engineering Team. label Jan 26, 2021
@ansibot ansibot removed the support:community This issue/PR relates to code supported by the Ansible community. label Mar 7, 2021
@nitzmahone
Copy link
Member

A number of the idempotence checks would be broken by this change, as well as potentially allowing "escape" from a virtualenv. Would also need to look into using module respawn to ensure that the idempotence checks run in the proper context. If you're still interested in moving forward with this, we can do a full review- just let us know. Thanks!

@nightowlengineer
Copy link
Contributor Author

A number of the idempotence checks would be broken by this change, as well as potentially allowing "escape" from a virtualenv. Would also need to look into using module respawn to ensure that the idempotence checks run in the proper context. If you're still interested in moving forward with this, we can do a full review- just let us know. Thanks!

Hey @nitzmahone - certainly happy to consider more changes, though with it being 18 months since I worked on this it will probably require quite a few updates anyway, I see some merge conflicts are showing up. In the meantime, I'd be interested to have more information on the problems you describe 😃

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
affects_2.11 feature This issue/PR relates to a feature request. module This issue/PR relates to a module. needs_rebase https://docs.ansible.com/ansible/devel/dev_guide/developing_rebasing.html needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. P3 Priority 3 - Approved, No Time Limitation packaging Packaging category pre_azp This PR was last tested before migration to Azure Pipelines. support:core This issue/PR relates to code supported by the Ansible Engineering Team.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

pip module aware of user-site
4 participants