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.

Sign up for GitHub

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

feat(pip/_internal): Skip current directory when performing freeze #7810

Closed
wants to merge 5 commits into from
Closed

feat(pip/_internal): Skip current directory when performing freeze #7810

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
a898196
feat(pip/_internal): Skip current directory when performing freeze
gutsytechster Mar 1, 2020
f908a25
fix(_vendors): Exclude '' when searching for distributions
gutsytechster Mar 7, 2020
3c4346b
add(*.patch): diff for pkg_resource
gutsytechster Mar 15, 2020
d41c15a
fix(freeze.py): Skip current directory through freeze and not vendor
gutsytechster Mar 21, 2020
85c1568
docs(news): Update news entry
gutsytechster Apr 2, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions news/2926.bugfix
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Avoid listing packages from current directory, when invoked as 'python -m pip freeze'
9 changes: 8 additions & 1 deletion src/pip/_internal/commands/freeze.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

from __future__ import absolute_import

import os
import sys

from pip._internal.cache import WheelCache
Expand Down Expand Up @@ -83,12 +84,18 @@ def run(self, options, args):

cmdoptions.check_list_path_option(options)

paths = options.path
# Filter sys.path, to avoid listing distributions from
# current directory
if paths is None:
paths = [item for item in sys.path if item and item != os.getcwd()]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this actually work? sys.path[0] is '' (empty string) for me, while os.getcwd() returns an absolute path. This would probably be

paths = [item for item in sys.path if item and item not in (os.curdir, '')]

Copy link
Contributor Author

@gutsytechster gutsytechster Mar 28, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A '' entry would not be considered as, through the first part of if statement i.e. if '' would be False. Since sys.path contains an absolute path of cwd, apart from '', I have used that specific if statement.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh sorry I missed the if item part. I do not think sys.path contains an absolute version of cwd by default though (it does not for me).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It does not actually. However, when you'll print sys.path within the freeze operation or in get_installed_distribution() method, it somehow does.

On the contrary, when I print sys.path within the WorkingSet defined in the pkg_resource it does not.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That’s weird. We’ll probably need to figure out why it’s the case before we can come up with a proper fix.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi there, how about

paths = [item for item in sys.path if os.path.abspath(item) != os.path.abspath(os.getcwd())]

since neither of sys.path nor os.getcwd() is warrantied to always give absolute paths.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@McSinyx, the problem is that somehow sys.path contains absolute path of CWD apart from '', which is added by default when you use python -m. We have to show the package during freeze if the full path is present in sys.path but not when just '' is present.


freeze_kwargs = dict(
requirement=options.requirements,
find_links=options.find_links,
local_only=options.local,
user_only=options.user,
paths=options.path,
paths=paths,
skip_regex=options.skip_requirements_regex,
isolated=options.isolated_mode,
wheel_cache=wheel_cache,
Expand Down
48 changes: 31 additions & 17 deletions tests/functional/test_freeze.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,34 +79,36 @@ def test_freeze_with_pip(script):
assert 'pip==' in result.stdout


def _fake_install(pkgname, dest):
egg_info_path = os.path.join(
dest, '{}-1.0-py{}.{}.egg-info'.format(
pkgname.replace('-', '_'),
sys.version_info[0],
sys.version_info[1]
)
)
with open(egg_info_path, 'w') as egg_info_file:
egg_info_file.write(textwrap.dedent("""\
Metadata-Version: 1.0
Name: {}
Version: 1.0
""".format(pkgname)
))
return egg_info_path


def test_freeze_with_invalid_names(script):
"""
Test that invalid names produce warnings and are passed over gracefully.
"""

def fake_install(pkgname, dest):
egg_info_path = os.path.join(
dest, '{}-1.0-py{}.{}.egg-info'.format(
pkgname.replace('-', '_'),
sys.version_info[0],
sys.version_info[1]
)
)
with open(egg_info_path, 'w') as egg_info_file:
egg_info_file.write(textwrap.dedent("""\
Metadata-Version: 1.0
Name: {}
Version: 1.0
""".format(pkgname)
))

valid_pkgnames = ('middle-dash', 'middle_underscore', 'middle.dot')
invalid_pkgnames = (
'-leadingdash', '_leadingunderscore', '.leadingdot',
'trailingdash-', 'trailingunderscore_', 'trailingdot.'
)
for pkgname in valid_pkgnames + invalid_pkgnames:
fake_install(pkgname, script.site_packages_path)
_fake_install(pkgname, script.site_packages_path)
result = script.pip('freeze', expect_stderr=True)
for pkgname in valid_pkgnames:
_check_output(
Expand All @@ -129,6 +131,18 @@ def fake_install(pkgname, dest):
_check_output(result.stderr, expected)


def test_freeze_skip_curr_dir(script):
"""
Test that an .egginfo is skipped when present in current directory
"""

curr_dir = os.getcwd()
egg_info_path = _fake_install("foo-package", curr_dir)
result = script.pip("freeze")
os.remove(egg_info_path)
assert "foo-package==" not in result.stdout


@pytest.mark.git
def test_freeze_editable_not_vcs(script, tmpdir):
"""
Expand Down