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

PyB plugins use wrong external tool dependencies #272

Closed
arcivanov opened this Issue Nov 24, 2015 · 12 comments

Comments

Projects
None yet
2 participants
@arcivanov
Contributor

arcivanov commented Nov 24, 2015

Throughout the PyB internal plugins use project.build_depends_on:

Firstly, PyB build does not really depend on those. It's the runtime use of the plugin that requires these external tools.

Therefore, I propose the following:

  • Create another method project.plugin_depends_on(package_name, version_spec=None, external_tool=None)
    • package_name is package name 😄
    • version_spec, if specified is a required PEP0440 SpecifierSet (see #271)
    • external_tool if specified is then name of the executable to check with assert_can_execute

project.plugin_depends_on will:

  • via pip show <package_name> check the version of the package installed
  • if nothing is installed, pip install <package_name>[version_spec] will be executed
  • if package is installed but version_spec is specified and version is not contained in SpecifierSet pip install --upgrade <package_name>[version_spec] will be run
  • once package versions are settled, if external_tool is specified the assert_can_execute will be performed.

@mriehl @esc review please

@arcivanov arcivanov added the bug label Nov 24, 2015

@arcivanov arcivanov self-assigned this Nov 24, 2015

@arcivanov arcivanov added this to the v0.11.3 milestone Nov 24, 2015

@arcivanov

This comment has been minimized.

Show comment
Hide comment
@arcivanov

arcivanov Nov 30, 2015

Contributor

@mriehl @esc thoughts? I'm going to start implementing as stated, please weigh in if you have objections

Contributor

arcivanov commented Nov 30, 2015

@mriehl @esc thoughts? I'm going to start implementing as stated, please weigh in if you have objections

arcivanov added a commit to arcivanov/pybuilder that referenced this issue Dec 2, 2015

PyB plugins use wrong external tool dependencies
Also: Env is not inherited properly by integrationtest plugin for PyB

fixes #272, connected to #272
fixes #284, connected to #284

arcivanov added a commit to arcivanov/pybuilder that referenced this issue Dec 2, 2015

PyB plugins use wrong external tool dependencies
Also: Env is not inherited properly by integrationtest plugin for PyB

fixes #272, connected to #272
fixes #284, connected to #284

arcivanov added a commit to arcivanov/pybuilder that referenced this issue Dec 3, 2015

PyB plugins use wrong external tool dependencies
Also: Env is not inherited properly by integrationtest plugin for PyB

fixes #272, connected to #272
fixes #284, connected to #284

arcivanov added a commit to arcivanov/pybuilder that referenced this issue Dec 3, 2015

PyB plugins use wrong external tool dependencies
Also: Env is not inherited properly by integrationtest plugin for PyB

fixes #272, connected to #272
fixes #284, connected to #284

arcivanov added a commit to arcivanov/pybuilder that referenced this issue Dec 3, 2015

PyB plugins use wrong external tool dependencies
Also: Env is not inherited properly by integrationtest plugin for PyB

fixes #272, connected to #272
fixes #284, connected to #284

arcivanov added a commit to arcivanov/pybuilder that referenced this issue Dec 3, 2015

PyB plugins use wrong external tool dependencies
Also: Env is not inherited properly by integrationtest plugin for PyB

fixes #272, connected to #272
fixes #284, connected to #284

arcivanov added a commit to arcivanov/pybuilder that referenced this issue Dec 3, 2015

PyB plugins use wrong external tool dependencies
Also: Env is not inherited properly by integrationtest plugin for PyB

fixes #272, connected to #272
fixes #284, connected to #284

arcivanov added a commit to arcivanov/pybuilder that referenced this issue Dec 3, 2015

PyB plugins use wrong external tool dependencies
Also: Env is not inherited properly by integrationtest plugin for PyB

fixes #272, connected to #272
fixes #284, connected to #284

arcivanov added a commit to arcivanov/pybuilder that referenced this issue Dec 3, 2015

PyB plugins use wrong external tool dependencies
Also: Env is not inherited properly by integrationtest plugin for PyB

fixes #272, connected to #272
fixes #284, connected to #284

@arcivanov arcivanov closed this in #285 Dec 3, 2015

@arcivanov arcivanov removed the in progress label Dec 3, 2015

@mriehl

This comment has been minimized.

Show comment
Hide comment
@mriehl

mriehl Dec 4, 2015

Member

get_package_version doesn't seem to always work, on my machine (using a virtualenv) for instance it always reports None even when the packages are installed, leading to long build times since dependencies are installed all the time.

I'm investigating why that is.

Member

mriehl commented Dec 4, 2015

get_package_version doesn't seem to always work, on my machine (using a virtualenv) for instance it always reports None even when the packages are installed, leading to long build times since dependencies are installed all the time.

I'm investigating why that is.

@mriehl

This comment has been minimized.

Show comment
Hide comment
@mriehl

mriehl Dec 4, 2015

Member

For example

» pip list | grep cram
cram (0.6)

but

>>> list(search_packages_info("cram"))
[]
Member

mriehl commented Dec 4, 2015

For example

» pip list | grep cram
cram (0.6)

but

>>> list(search_packages_info("cram"))
[]
@mriehl

This comment has been minimized.

Show comment
Hide comment
@mriehl

mriehl Dec 4, 2015

Member

Ahhh. In the pip version I have, search_packages_info expects a Traversable:

https://github.com/pypa/pip/blob/13d43e3af8b5c8e8c7c66dde8a9f76b340697c3e/pip/commands/show.py#L54

So this works:

>>> list(search_packages_info(["cram"]))
[{'files': ['../../../bin/cram', 'cram-0.6.dist-info/DESCRIPTION.rst', 'cram-0.6.dist-info/METADATA', 'cram-0.6.dist-info/RECORD', 'cram-0.6.dist-info/WHEEL', 'cram-0.6.dist-info/pydist.json', 'cram-0.6.dist-info/top_level.txt', 'cram.py', 'cram.pyc'], 'name': 'cram', 'license': 'GNU GPL', 'author': 'Brodie Rao', 'metadata-version': '2.0', 'home-page': 'https://bitheap.org/cram/', 'summary': 'A simple testing framework for command line applications', 'version': '0.6', 'location': '/data/home/mriehl/workspace/pybuilder/venv/lib/python2.7/site-packages', 'requires': [], 'author-email': 'brodie@bitheap.org'}]

Isn't that the case on your pip version @arcivanov ?

Member

mriehl commented Dec 4, 2015

Ahhh. In the pip version I have, search_packages_info expects a Traversable:

https://github.com/pypa/pip/blob/13d43e3af8b5c8e8c7c66dde8a9f76b340697c3e/pip/commands/show.py#L54

So this works:

>>> list(search_packages_info(["cram"]))
[{'files': ['../../../bin/cram', 'cram-0.6.dist-info/DESCRIPTION.rst', 'cram-0.6.dist-info/METADATA', 'cram-0.6.dist-info/RECORD', 'cram-0.6.dist-info/WHEEL', 'cram-0.6.dist-info/pydist.json', 'cram-0.6.dist-info/top_level.txt', 'cram.py', 'cram.pyc'], 'name': 'cram', 'license': 'GNU GPL', 'author': 'Brodie Rao', 'metadata-version': '2.0', 'home-page': 'https://bitheap.org/cram/', 'summary': 'A simple testing framework for command line applications', 'version': '0.6', 'location': '/data/home/mriehl/workspace/pybuilder/venv/lib/python2.7/site-packages', 'requires': [], 'author-email': 'brodie@bitheap.org'}]

Isn't that the case on your pip version @arcivanov ?

@mriehl mriehl referenced this issue Dec 4, 2015

Closed

Release 0.11.3 #286

2 of 2 tasks complete
@arcivanov

This comment has been minimized.

Show comment
Hide comment
@arcivanov

arcivanov Dec 5, 2015

Contributor

@mriehl search_packages_info always works for me with an iterable query. I'm going to create a separate issue to increase plugin dependency check performance.

PS: I see what you mean! Good catch!

Contributor

arcivanov commented Dec 5, 2015

@mriehl search_packages_info always works for me with an iterable query. I'm going to create a separate issue to increase plugin dependency check performance.

PS: I see what you mean! Good catch!

arcivanov added a commit to arcivanov/pybuilder that referenced this issue Dec 5, 2015

arcivanov added a commit that referenced this issue Dec 5, 2015

Merge pull request #294 from arcivanov/issue_293
Harden and optimize use of `search_packages_info` in #272
@mriehl

This comment has been minimized.

Show comment
Hide comment
@mriehl

mriehl Dec 6, 2015

Member

@arcivanov I think there's still an issue :/ the latest build from master installed unittest-xml-reporting twice, for example (but I'm not sure why that is).

https://travis-ci.org/pybuilder/pybuilder/jobs/95025723#L749

and

https://travis-ci.org/pybuilder/pybuilder/jobs/95025723#L1443

Member

mriehl commented Dec 6, 2015

@arcivanov I think there's still an issue :/ the latest build from master installed unittest-xml-reporting twice, for example (but I'm not sure why that is).

https://travis-ci.org/pybuilder/pybuilder/jobs/95025723#L749

and

https://travis-ci.org/pybuilder/pybuilder/jobs/95025723#L1443

@mriehl

This comment has been minimized.

Show comment
Hide comment
@mriehl

mriehl Dec 6, 2015

Member

Ah, looking at https://github.com/arcivanov/pybuilder/blob/da5f51009fd0df2889d9939de39e276049e386ca/src/main/python/pybuilder/pip_utils.py#L137 it seems the pip query is using the package name string as an iterable, e.G. ["u", "n", "i", "t", "t", "e", "s", "t".....] so this doesn't match anything. We should return [mixed.name] when normalizing instead.

Member

mriehl commented Dec 6, 2015

Ah, looking at https://github.com/arcivanov/pybuilder/blob/da5f51009fd0df2889d9939de39e276049e386ca/src/main/python/pybuilder/pip_utils.py#L137 it seems the pip query is using the package name string as an iterable, e.G. ["u", "n", "i", "t", "t", "e", "s", "t".....] so this doesn't match anything. We should return [mixed.name] when normalizing instead.

@arcivanov

This comment has been minimized.

Show comment
Hide comment
@mriehl

This comment has been minimized.

Show comment
Hide comment
@mriehl

mriehl Dec 6, 2015

Member

Yes, that's right.. I'm not sure why travis installs plugin runtime dependencies twice then. Will investigate tomorrow morning

Member

mriehl commented Dec 6, 2015

Yes, that's right.. I'm not sure why travis installs plugin runtime dependencies twice then. Will investigate tomorrow morning

@arcivanov

This comment has been minimized.

Show comment
Hide comment
@arcivanov

arcivanov Dec 6, 2015

Contributor

I'm investigating already, don't worry about it. I suspect this is related to how pip is implemented and that it doesn't actually refresh internal package cache every time running in-process.

Contributor

arcivanov commented Dec 6, 2015

I'm investigating already, don't worry about it. I suspect this is related to how pip is implemented and that it doesn't actually refresh internal package cache every time running in-process.

@arcivanov

This comment has been minimized.

Show comment
Hide comment
@arcivanov

arcivanov Dec 6, 2015

Contributor

@mriehl yep, it's internal pip implementation

/Users/arcivanov/pyhome/current/bin/python "/Applications/PyCharm CE.app/Contents/helpers/pycharm/utrunner.py" /Users/arcivanov/Documents/src/arcivanov/pybuilder/src/unittest/python/pip_utils_tests.py::PipVersionTests::test_test_install_test true
Testing started at 13:09 ...
{}
Collecting unittest-xml-reporting
  Using cached unittest_xml_reporting-1.13.0-py2.py3-none-any.whl
Requirement already satisfied (use --upgrade to upgrade): six>=1.4.0 in /Users/arcivanov/pyhome/master/lib/python2.7/site-packages (from unittest-xml-reporting)
Installing collected packages: unittest-xml-reporting
Successfully installed unittest-xml-reporting-1.13.0
{}

Process finished with exit code 0

for code

        print(pip_utils.get_package_version("unittest-xml-reporting"))
        pip_utils.pip_install("unittest-xml-reporting")
        print(pip_utils.get_package_version("unittest-xml-reporting"))
Contributor

arcivanov commented Dec 6, 2015

@mriehl yep, it's internal pip implementation

/Users/arcivanov/pyhome/current/bin/python "/Applications/PyCharm CE.app/Contents/helpers/pycharm/utrunner.py" /Users/arcivanov/Documents/src/arcivanov/pybuilder/src/unittest/python/pip_utils_tests.py::PipVersionTests::test_test_install_test true
Testing started at 13:09 ...
{}
Collecting unittest-xml-reporting
  Using cached unittest_xml_reporting-1.13.0-py2.py3-none-any.whl
Requirement already satisfied (use --upgrade to upgrade): six>=1.4.0 in /Users/arcivanov/pyhome/master/lib/python2.7/site-packages (from unittest-xml-reporting)
Installing collected packages: unittest-xml-reporting
Successfully installed unittest-xml-reporting-1.13.0
{}

Process finished with exit code 0

for code

        print(pip_utils.get_package_version("unittest-xml-reporting"))
        pip_utils.pip_install("unittest-xml-reporting")
        print(pip_utils.get_package_version("unittest-xml-reporting"))

arcivanov added a commit to arcivanov/pybuilder that referenced this issue Dec 6, 2015

Need to reinitialize Pip workset in pip_utils
fixes #295, connected to #295, connected to #272, connected to #293

arcivanov added a commit to arcivanov/pybuilder that referenced this issue Dec 6, 2015

Need to reinitialize Pip workset in pip_utils
fixes #295, connected to #295, connected to #272, connected to #293

arcivanov added a commit to arcivanov/pybuilder that referenced this issue Dec 6, 2015

Need to reinitialize Pip workset in pip_utils
fixes #295, connected to #295, connected to #272, connected to #293

arcivanov added a commit to arcivanov/pybuilder that referenced this issue Dec 6, 2015

Need to reinitialize Pip workset in pip_utils
fixes #295, connected to #295, connected to #272, connected to #293

arcivanov added a commit to arcivanov/pybuilder that referenced this issue Dec 6, 2015

Need to reinitialize Pip workset in pip_utils
fixes #295, connected to #295, connected to #272, connected to #293

arcivanov added a commit to arcivanov/pybuilder that referenced this issue Dec 6, 2015

Need to reinitialize PIP workset in pip_utils
fixes #295, connected to #295, connected to #272, connected to #293
@mriehl

This comment has been minimized.

Show comment
Hide comment
@mriehl

mriehl Dec 7, 2015

Member

Yup works great now! Thanks 👍

Member

mriehl commented Dec 7, 2015

Yup works great now! Thanks 👍

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