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

Preserve the query portion of URL-type InstallRequirement URLs. #504

Merged
merged 2 commits into from Apr 16, 2012

Conversation

Projects
None yet
2 participants
@wking

wking commented Apr 5, 2012

With this patch, install commands like:

pip install 'http://foo.com/?p=bar.git;a=snapshot;h=v0.3;sf=tgz'

will work as expected.

The naming scheme for Link properties is a bit confused at the moment, and I haven't done anything about that here. Link.url_fragment should probably renamed. Perhaps Link.url_without_query_or_fragment. If you don't mind sacrificing a bit of accuracy, Link.url_path would also work, especially since Link.url_fragment is currently only being used in Link.filename.

W. Trevor King
Preserve the query portion of URL-type InstallRequirement URLs.
With this patch, install commands like:

  pip install 'http://foo.com/?p=bar.git;a=snapshot;h=v0.3;sf=tgz'

will work as expected.
@carljm

This comment has been minimized.

Show comment
Hide comment
@carljm

carljm Apr 6, 2012

Contributor

This looks ok; needs a test (and verification that existing tests still pass).

Contributor

carljm commented Apr 6, 2012

This looks ok; needs a test (and verification that existing tests still pass).

@wking

This comment has been minimized.

Show comment
Hide comment
@wking

wking Apr 7, 2012

The test suite passes, with following exceptions:

======================================================================
ERROR: Test installing current directory ('.') into usersite after installing distribute
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/…/nose/case.py", line 197, in runTest
    self.test(*self.arg)
  File "/…/pip/tests/test_basic.py", line 363, in test_install_subversion_usersite_editable_with_distribute
    (env.lib_path/'no-global-site-packages.txt').rm() # this one reenables user_site
  File "/…/pip/tests/path.py", line 176, in remove
    os.remove(self)
OSError: [Errno 2] No such file or directory: '/tmp/tmp5NqXkR-piptest/.virtualenv/lib/python2.7/no-global-site-packages.txt'

======================================================================
FAIL: Test installing current directory ('.') into usersite
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/…/nose/case.py", line 197, in runTest
    self.test(*self.arg)
  File "/…/pip/tests/test_basic.py", line 348, in test_install_curdir_usersite
    assert fspkg_folder in result.files_created, str(result.stdout)
AssertionError: Unpacking /…/pip/tests/packages/FSPkg
  Running setup.py egg_info for package from file:///…/pip/tests/packages/FSPkg

Installing collected packages: FSPkg
  Running setup.py install for FSPkg

Successfully installed FSPkg
Cleaning up...

which both had the same results before my commit (i.e. as of dfe39ed).

What kind of test did you have in mind for my patch? I think testing would require a package that serves it's download over HTTP. This seems like a lot of infrastructure to set up inside the test suite...

wking commented Apr 7, 2012

The test suite passes, with following exceptions:

======================================================================
ERROR: Test installing current directory ('.') into usersite after installing distribute
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/…/nose/case.py", line 197, in runTest
    self.test(*self.arg)
  File "/…/pip/tests/test_basic.py", line 363, in test_install_subversion_usersite_editable_with_distribute
    (env.lib_path/'no-global-site-packages.txt').rm() # this one reenables user_site
  File "/…/pip/tests/path.py", line 176, in remove
    os.remove(self)
OSError: [Errno 2] No such file or directory: '/tmp/tmp5NqXkR-piptest/.virtualenv/lib/python2.7/no-global-site-packages.txt'

======================================================================
FAIL: Test installing current directory ('.') into usersite
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/…/nose/case.py", line 197, in runTest
    self.test(*self.arg)
  File "/…/pip/tests/test_basic.py", line 348, in test_install_curdir_usersite
    assert fspkg_folder in result.files_created, str(result.stdout)
AssertionError: Unpacking /…/pip/tests/packages/FSPkg
  Running setup.py egg_info for package from file:///…/pip/tests/packages/FSPkg

Installing collected packages: FSPkg
  Running setup.py install for FSPkg

Successfully installed FSPkg
Cleaning up...

which both had the same results before my commit (i.e. as of dfe39ed).

What kind of test did you have in mind for my patch? I think testing would require a package that serves it's download over HTTP. This seems like a lot of infrastructure to set up inside the test suite...

@wking

This comment has been minimized.

Show comment
Hide comment
@wking

wking Apr 7, 2012

Just added tests/test_install_requirement.py testing InstallRequirement.from_line for URLs with query parts.

wking commented Apr 7, 2012

Just added tests/test_install_requirement.py testing InstallRequirement.from_line for URLs with query parts.

carljm added a commit that referenced this pull request Apr 16, 2012

Merge pull request #504 from wking/develop
Preserve the query portion of URL-type InstallRequirement URLs. Thanks W. Trevor King.

@carljm carljm merged commit 0c05413 into pypa:develop Apr 16, 2012

carljm added a commit that referenced this pull request Apr 16, 2012

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