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

improve parsing of packages with options #2734

Closed
wants to merge 2 commits into from

Conversation

slhck
Copy link
Contributor

@slhck slhck commented Aug 13, 2018

The issue

This fixes somewhat buggy code in parsing the package names during a pipenv install.

The fix

This fixes the problem by going through the arguments to pipenv install in sequence, then checking if one argument is -e or -i/--index/--extra-index-url, and subsequently treating the following argument in the list as necessary.

This makes it possible to specify multiple editable dependencies rather than just one (see #2733). It also supersedes that PR.

The checklist
  • Associated issue
  • A news fragment in the news/ directory to describe this fix with the extension .bugfix, .feature, .behavior, .doc. .vendor. or .trivial (this will appear in the release changelog). Use semantic line breaks and name the file after the issue number or the PR #.

I did not yet create a fragment for this. Happy to get some feedback on this PR first!

@uranusjr
Copy link
Member

The line of logic seems fine to me, just need to figure out what the failing test is

@techalchemy
Copy link
Member

    return _process_result(sub_ctx.command.invoke(sub_ctx))
  File "/var/lib/buildkite-agent/builds/build-6/kennethreitz/pipenv/pipenv/vendor/click/core.py", line 895, in invoke
    return ctx.invoke(self.callback, **ctx.params)
  File "/var/lib/buildkite-agent/builds/build-6/kennethreitz/pipenv/pipenv/vendor/click/core.py", line 535, in invoke
    return callback(*args, **kwargs)
  File "/var/lib/buildkite-agent/builds/build-6/kennethreitz/pipenv/pipenv/cli.py", line 416, in install
    selective_upgrade=selective_upgrade,
  File "/var/lib/buildkite-agent/builds/build-6/kennethreitz/pipenv/pipenv/core.py", line 1861, in do_install
    index=index,
UnboundLocalError: local variable 'index' referenced before assignment
/var/lib/buildkite-agent/builds/build-6/kennethreitz/pipenv/pipenv/_compat.py:113: ResourceWarning: Implicitly cleaning up <TemporaryDirectory '/tmp/pipenv-oG66ij-requirements'>
warnings.warn(warn_message, ResourceWarning)

@slhck slhck force-pushed the fix-package-parsing branch from 64e8777 to 4d1b043 Compare August 21, 2018 07:06
@slhck
Copy link
Contributor Author

slhck commented Aug 21, 2018

Thanks for the pointer. The variables were not defined. Pushed an update with a fix.

(Is there no way to see the build results as a contributor?)

@techalchemy
Copy link
Member

I am actually the only person who can see builds right now unfortunately (because of not very good reasons) -- does this supersede your other pr?

@techalchemy
Copy link
Member

Also here is the log:

____________________________ test_editable_no_args _____________________________
[gw7] linux2 -- Python 2.7.15 /root/.local/share/virtualenvs/pipenv-SHwetwt5-2.7/bin/python2.7
 
PipenvInstance = <class 'tests.integration.conftest._PipenvInstance'>
 
    @pytest.mark.editable
    @pytest.mark.badparameter
    @pytest.mark.install
    def test_editable_no_args(PipenvInstance):
        with PipenvInstance() as p:
            c = p.pipenv("install -e")
            assert c.return_code != 0
>           assert "Please provide path to editable package" in c.err
E           assert 'Please provide path to editable package' in "Creating a virtualenv for this project\xe2\x80\xa6\nPipfile: /tmp/pipenv-vsXgH0-project/Pipfile\nUsing /root/.local/s...y cleaning up <TemporaryDirectory '/tmp/pipenv-CZf41X-requirements'>\n  warnings.warn(warn_message, ResourceWarning)\n"
E            +  where "Creating a virtualenv for this project\xe2\x80\xa6\nPipfile: /tmp/pipenv-vsXgH0-project/Pipfile\nUsing /root/.local/s...y cleaning up <TemporaryDirectory '/tmp/pipenv-CZf41X-requirements'>\n  warnings.warn(warn_message, ResourceWarning)\n" = <Command 'pipenv install -e'>.err
 
tests/integration/test_install_basic.py:347: AssertionError
----------------------------- Captured stdout call -----------------------------
$ pipenv install -e
 
Creating a virtualenv for this project…
Pipfile: /tmp/pipenv-vsXgH0-project/Pipfile
Using /root/.local/share/virtualenvs/pipenv-SHwetwt5-2.7/bin/python2.7 (2.7.15) to create virtualenv…
Already using interpreter /root/.local/share/virtualenvs/pipenv-SHwetwt5-2.7/bin/python2.7
Using real prefix '/usr'
New python executable in /tmp/pipenv-vsXgH0-project/.venv/bin/python2.7
Also creating executable in /tmp/pipenv-vsXgH0-project/.venv/bin/python
Installing setuptools, pip, wheel...done.
 
Virtualenv location: /tmp/pipenv-vsXgH0-project/.venv
Creating a Pipfile for this project…
Traceback (most recent call last):
  File "/root/.local/share/virtualenvs/pipenv-SHwetwt5-2.7/bin/pipenv", line 11, in <module>
    load_entry_point('pipenv', 'console_scripts', 'pipenv')()
  File "/var/lib/buildkite-agent/builds/build-8/kennethreitz/pipenv/pipenv/vendor/click/core.py", line 722, in __call__
    return self.main(*args, **kwargs)
  File "/var/lib/buildkite-agent/builds/build-8/kennethreitz/pipenv/pipenv/vendor/click/core.py", line 697, in main
    rv = self.invoke(ctx)
  File "/var/lib/buildkite-agent/builds/build-8/kennethreitz/pipenv/pipenv/vendor/click/core.py", line 1066, in invoke
    return _process_result(sub_ctx.command.invoke(sub_ctx))
  File "/var/lib/buildkite-agent/builds/build-8/kennethreitz/pipenv/pipenv/vendor/click/core.py", line 895, in invoke
    return ctx.invoke(self.callback, **ctx.params)
  File "/var/lib/buildkite-agent/builds/build-8/kennethreitz/pipenv/pipenv/vendor/click/core.py", line 535, in invoke
    return callback(*args, **kwargs)
  File "/var/lib/buildkite-agent/builds/build-8/kennethreitz/pipenv/pipenv/cli.py", line 416, in install
    selective_upgrade=selective_upgrade,
  File "/var/lib/buildkite-agent/builds/build-8/kennethreitz/pipenv/pipenv/core.py", line 1780, in do_install
    package_name = packages_to_install.pop(0)
IndexError: pop from empty list
/var/lib/buildkite-agent/builds/build-8/kennethreitz/pipenv/pipenv/_compat.py:113: ResourceWarning: Implicitly cleaning up <TemporaryDirectory '/tmp/pipenv-CZf41X-requirements'>
warnings.warn(warn_message, ResourceWarning)

@slhck slhck force-pushed the fix-package-parsing branch from 57c64c2 to b02cd5a Compare August 26, 2018 15:02
@slhck
Copy link
Contributor Author

slhck commented Aug 26, 2018

Hmm, is pipenv install -e without arguments a valid command? I think I've provided the necessary fix. Sorry for the broken build; I haven't been able yet to get the tests running locally.

This does supersede my other PR, yes.

I have provided an update, let's see.

@techalchemy
Copy link
Member

techalchemy commented Aug 26, 2018

No worries :) and no, you would always need a target. -e is just a per-package flag that indicates it’s editable

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants