From 23a311b2543d7ff2bf78c8f39196360cd4853fc1 Mon Sep 17 00:00:00 2001 From: Clark Boylan Date: Mon, 2 Oct 2017 17:54:15 -0700 Subject: [PATCH 1/4] Pip freeze of editable git repos reports repo path Rather than report an error for git repositories with no remotes in pip freeze output report the filesystem location with a file:// url. Fixes issue 4759 --- news/4759.bugfix | 2 ++ src/pip/_internal/vcs/git.py | 29 ++++++++++++++++------------ tests/functional/test_freeze.py | 12 ++++++++++++ tests/functional/test_install_vcs.py | 15 -------------- 4 files changed, 31 insertions(+), 27 deletions(-) create mode 100644 news/4759.bugfix diff --git a/news/4759.bugfix b/news/4759.bugfix new file mode 100644 index 00000000000..0f1885e04d8 --- /dev/null +++ b/news/4759.bugfix @@ -0,0 +1,2 @@ +Rather than report an error for git repositories with no remotes in +pip freeze output report the filesystem location with a file:// url. diff --git a/src/pip/_internal/vcs/git.py b/src/pip/_internal/vcs/git.py index 3528d8fd9b3..9d8ae3eb086 100644 --- a/src/pip/_internal/vcs/git.py +++ b/src/pip/_internal/vcs/git.py @@ -9,7 +9,7 @@ from pip._vendor.six.moves.urllib import request as urllib_request from pip._internal.compat import samefile -from pip._internal.exceptions import BadCommand +from pip._internal.exceptions import BadCommand, InstallationError from pip._internal.utils.misc import display_path from pip._internal.utils.temp_dir import TempDirectory from pip._internal.vcs import VersionControl, vcs @@ -203,17 +203,22 @@ def obtain(self, dest): def get_url(self, location): """Return URL of the first remote encountered.""" - remotes = self.run_command( - ['config', '--get-regexp', r'remote\..*\.url'], - show_stdout=False, cwd=location, - ) - remotes = remotes.splitlines() - found_remote = remotes[0] - for remote in remotes: - if remote.startswith('remote.origin.url '): - found_remote = remote - break - url = found_remote.split(' ')[1] + try: + remotes = self.run_command( + ['config', '--get-regexp', r'remote\..*\.url'], + show_stdout=False, cwd=location, + ) + remotes = remotes.splitlines() + found_remote = remotes[0] + for remote in remotes: + if remote.startswith('remote.origin.url '): + found_remote = remote + break + url = found_remote.split(' ')[1] + except InstallationError: + # If the above command fails we have no git remotes. Just use the + # local repo path in that case instead. + url = 'file://' + location return url.strip() def get_revision(self, location): diff --git a/tests/functional/test_freeze.py b/tests/functional/test_freeze.py index d51029281f6..4252c9db6cf 100644 --- a/tests/functional/test_freeze.py +++ b/tests/functional/test_freeze.py @@ -320,6 +320,18 @@ def test_freeze_git_remote(script, tmpdir): """ ).format(remote=origin_remote).strip() _check_output(result.stdout, expected) + # check when there are no remotes. Should report local file path. + script.run('git', 'remote', 'remove', 'origin', cwd=repo_dir) + script.run('git', 'remote', 'remove', 'other', cwd=repo_dir) + result = script.pip('freeze', expect_stderr=True) + expected = textwrap.dedent( + """ + ...-e git+{remote}@...#egg=version_pkg + ... + """ + ).format(remote='file://' + repo_dir).strip() + _check_output(result.stdout, expected) + # check when there are no remotes. Should report local file path. @need_mercurial diff --git a/tests/functional/test_install_vcs.py b/tests/functional/test_install_vcs.py index 9b3c4a1ddc4..87c5504eab6 100644 --- a/tests/functional/test_install_vcs.py +++ b/tests/functional/test_install_vcs.py @@ -291,21 +291,6 @@ def test_git_with_ambiguous_revs(script): result.assert_installed('version-pkg', with_files=['.git']) -@pytest.mark.network -def test_git_works_with_editable_non_origin_repo(script): - # set up, create a git repo and install it as editable from a local - # directory path - version_pkg_path = _create_test_package(script) - script.pip('install', '-e', version_pkg_path.abspath) - - # 'freeze'ing this should not fall over, but should result in stderr output - # warning - result = script.pip('freeze', expect_stderr=True) - assert "Error when trying to get requirement" in result.stderr - assert "Could not determine repository location" in result.stdout - assert "version-pkg==0.1" in result.stdout - - @pytest.mark.network def test_reinstalling_works_with_editible_non_master_branch(script): """ From feb8d802661064cdad8f7ac15fef84a14ddcc619 Mon Sep 17 00:00:00 2001 From: Clark Boylan Date: Fri, 17 Nov 2017 16:04:34 -0800 Subject: [PATCH 2/4] Cleanup based on PR review comments Reduce scope of try/except when determining git remotes, reword bugfix information, and clean up an unnecessary comment. --- news/4759.bugfix | 4 ++-- src/pip/_internal/vcs/git.py | 10 ++++++---- tests/functional/test_freeze.py | 1 - 3 files changed, 8 insertions(+), 7 deletions(-) diff --git a/news/4759.bugfix b/news/4759.bugfix index 0f1885e04d8..c291984bd4b 100644 --- a/news/4759.bugfix +++ b/news/4759.bugfix @@ -1,2 +1,2 @@ -Rather than report an error for git repositories with no remotes in -pip freeze output report the filesystem location with a file:// url. +Pip freeze now reports URL based on file system location for git +repositories with no remotes. diff --git a/src/pip/_internal/vcs/git.py b/src/pip/_internal/vcs/git.py index 9d8ae3eb086..c780b7b6edc 100644 --- a/src/pip/_internal/vcs/git.py +++ b/src/pip/_internal/vcs/git.py @@ -208,6 +208,12 @@ def get_url(self, location): ['config', '--get-regexp', r'remote\..*\.url'], show_stdout=False, cwd=location, ) + except InstallationError as e: + # If the above command fails we have no git remotes or cannot + # determine what they are. Just use the local repo path in that + # case instead. + url = 'file://' + location + else: remotes = remotes.splitlines() found_remote = remotes[0] for remote in remotes: @@ -215,10 +221,6 @@ def get_url(self, location): found_remote = remote break url = found_remote.split(' ')[1] - except InstallationError: - # If the above command fails we have no git remotes. Just use the - # local repo path in that case instead. - url = 'file://' + location return url.strip() def get_revision(self, location): diff --git a/tests/functional/test_freeze.py b/tests/functional/test_freeze.py index 4252c9db6cf..e697fd3307c 100644 --- a/tests/functional/test_freeze.py +++ b/tests/functional/test_freeze.py @@ -331,7 +331,6 @@ def test_freeze_git_remote(script, tmpdir): """ ).format(remote='file://' + repo_dir).strip() _check_output(result.stdout, expected) - # check when there are no remotes. Should report local file path. @need_mercurial From 1edc10950405e8139ee645cad819483b764e0c39 Mon Sep 17 00:00:00 2001 From: Clark Boylan Date: Mon, 20 Nov 2017 11:27:50 -0800 Subject: [PATCH 3/4] Remove unused local variable This was caught by linting, remove unused exception as e variable in exception handler. --- src/pip/_internal/vcs/git.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/pip/_internal/vcs/git.py b/src/pip/_internal/vcs/git.py index c780b7b6edc..249165e853d 100644 --- a/src/pip/_internal/vcs/git.py +++ b/src/pip/_internal/vcs/git.py @@ -208,7 +208,7 @@ def get_url(self, location): ['config', '--get-regexp', r'remote\..*\.url'], show_stdout=False, cwd=location, ) - except InstallationError as e: + except InstallationError: # If the above command fails we have no git remotes or cannot # determine what they are. Just use the local repo path in that # case instead. From 6e9d961ed049ceecf8d87681892f6324d5ace6b3 Mon Sep 17 00:00:00 2001 From: Clark Boylan Date: Mon, 20 Nov 2017 11:38:51 -0800 Subject: [PATCH 4/4] Address more review comments Handle case insensitive filesystems when checking the local path used when git repo has no remotes during testing. Correct case of 'pip' from 'Pip' in the news update. --- news/4759.bugfix | 2 +- tests/functional/test_freeze.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/news/4759.bugfix b/news/4759.bugfix index c291984bd4b..2155aab41f3 100644 --- a/news/4759.bugfix +++ b/news/4759.bugfix @@ -1,2 +1,2 @@ -Pip freeze now reports URL based on file system location for git +pip freeze now reports URL based on file system location for git repositories with no remotes. diff --git a/tests/functional/test_freeze.py b/tests/functional/test_freeze.py index e697fd3307c..ae0167cce4b 100644 --- a/tests/functional/test_freeze.py +++ b/tests/functional/test_freeze.py @@ -329,7 +329,7 @@ def test_freeze_git_remote(script, tmpdir): ...-e git+{remote}@...#egg=version_pkg ... """ - ).format(remote='file://' + repo_dir).strip() + ).format(remote='file://' + os.path.normcase(repo_dir)).strip() _check_output(result.stdout, expected)