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

gutsytechster · 2020-03-01T17:28:06Z

This skips any *.egg_info present in the current directory when
performing pip freeze.

This fixes #2926

gutsytechster · 2020-03-01T18:27:47Z

My changes are failing some tests, but I am unable to find any relation between the failed tests and my changes.

uranusjr · 2020-03-02T05:57:17Z

This probably has the unintended effect of not showing installed packages if the current working directory is the site-packages directory. So those pip list tests starts failing because they are not printing anything.

The logic needs to be changed to don’t show a package if it’s in the current working directory, unless the current working directory is one of the entries of sys.path (except '' and '.').

gutsytechster · 2020-03-02T17:39:02Z

Thanks for the Pointer. I am just looking into it.

gutsytechster · 2020-03-03T18:38:07Z

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

This test doesn't fail even when I undo all the changes, but it should. As foo-package should be present in the pip freeze result. And I found that script.pip actually invokes pip as python -m pip. So, it should definitely be present. But that doesn't seem so. Is there something wrong with the tests?

gutsytechster · 2020-03-07T07:28:43Z

On Wed, Mar 4, 2020 at 9:35 AM Tzu-ping Chung ***@***.***> wrote: > I *think* (without actually running the code) the problem is you’re running freeze in the wrong directory. The test code roughly translates to the following commands: > cd /path/to/pip/tests > pushd /path/to/cwd > _fake_install foo-package > popd > python -m pip freeze > The last command run is run in the test directory, not where the .egg-info is.

Thanks. Well the piece of code, uses `os.getcwd()` to find the current directory within the tests ie where the tests run. So, shouldn't it be already present in the same directory where the tests run? (I couldn't find this comment on GitHub, therefore replying here.) Regards Prashant Sharma

gutsytechster · 2020-03-08T18:00:14Z

On a bit of experimentation. I wrote a simple test as

def test_pip_script(script):
    assert script.pip("freeze").stdout == script.pip("freeze", use_module=False).stdout

But the test fails. Shouldn't it actually be passed? Its failure results in the report

___________________________________________________ test_pip_script ___________________________________________________
[gw4] linux -- Python 3.6.9 /home/gutsytechster/Documents/Projects/Open-Source-Projects/pip/.tox/py36/bin/python

script = <tests.lib.PipTestEnvironment object at 0x7f696adfc8d0>

    def test_pip_script(script):
>       assert script.pip("freeze").stdout == script.pip("freeze", use_module=False).stdout
E       AssertionError: assert '' == 'apipkg==1.5\natomicwrites==...zeug==0.16.0\nzipp==3.0.0\n'
E         + apipkg==1.5
E         + atomicwrites==1.3.0
E         + attrs==19.3.0
E         + cffi==1.14.0
E         + coverage==5.0.3
E         + cryptography==2.8
E         + csv23==0.1.6
E         + execnet==1.7.1
E         + freezegun==0.3.15
E         + importlib-metadata==1.5.0
E         + mock==4.0.1
E         + more-itertools==8.2.0
E         + pluggy==0.13.1
E         + pretend==1.0.9
E         + py==1.8.1
E         + pycparser==2.19
E         + pytest==3.8.2
E         + pytest-cov==2.8.1
E         + pytest-forked==1.1.3
E         + pytest-rerunfailures==6.0
E         + pytest-timeout==1.3.4
E         + pytest-xdist==1.27.0
E         + python-dateutil==2.8.1
E         + PyYAML==5.3
E         + scripttest==1.3
E         + six==1.14.0
E         + virtualenv==16.7.10
E         + Werkzeug==0.16.0
E         + zipp==3.0.0

@pradyunsg @uranusjr, can you have a look at it? Is this the intended behaviour or is there something that I am missing? What I can understand is that there is a difference in invoking pip. But with python -m pip freeze, it doesn't show any packages, while with normal pip freeze, it does.

gutsytechster · 2020-03-11T18:43:46Z

ping @pradyunsg ^^ 😅

pradyunsg · 2020-03-15T10:22:00Z

I don't think modifying pkg_resources is the way to go here... We instead want to more directly specify where pip freeze searches for packages. That'd be something like:

diff --git a/src/pip/_internal/commands/freeze.py b/src/pip/_internal/commands/freeze.py
index 4758e303..55d475e4 100644
--- a/src/pip/_internal/commands/freeze.py
+++ b/src/pip/_internal/commands/freeze.py
@@ -83,12 +83,17 @@ class FreezeCommand(Command):
 
         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]
+
         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,

pradyunsg · 2020-03-15T10:26:36Z

can you have a look at it?

Hmm... Looks like pip on your setup, is not the same as what gets invoked with python -m pip. This might be system specific or a broader problem with the test tooling.

I suggest trying the following, and seeing if/how it fails.

def test_pip_script(script):
    r1 = script.pip("--version")
    r2 = script.pip("--version", use_module=False)
    assert r1.stdout == r2.stdout

gutsytechster · 2020-03-15T11:28:51Z

I don't think modifying pkg_resources is the way to go here... We instead want to more directly specify where pip freeze searches for packages.

I performed it this way because the behaviour is not only for freeze but also, as you referenced above, for list command. We'd then need to implement similar logic in both places or for any other command, that fetches the distributions from WorkingSet. I just wanted to avoid repetition by filtering it at the common point. 😬

pradyunsg · 2020-03-17T09:13:03Z

@gutsytechster For a whole host of reasons, we avoid modifying vendored packages. Think of them as dependencies; we don't want patches on them except those needed to make them work.

https://pip.pypa.io/en/latest/development/vendoring-policy/

gutsytechster · 2020-03-21T17:29:56Z

I have updated the PR based on @pradyunsg suggestions and some of my findings. I found that an absolute path of the CWD is also added to sys.path within freeze operation and within get_installed_distribution. Hence, I also excluded the absolute path to CWD

What I thought is if current directory is to be added, then it could be using --path option.

gutsytechster · 2020-03-28T08:24:25Z

@uranusjr^^ ping 😅

uranusjr · 2020-03-28T15:13:52Z

src/pip/_internal/commands/freeze.py

+        # 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()]


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, '')]

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.

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).

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.

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

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.

@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.

pradyunsg

LGTM, though the NEWS fragment could use a bit of rephrasing. :)

pradyunsg · 2020-04-01T12:56:41Z

news/2926.bugfix

@@ -0,0 +1 @@
+Skip local directory by default for 'pip freeze' or when invoked as 'python -m pip freeze'


Suggested change

Skip local directory by default for 'pip freeze' or when invoked as 'python -m pip freeze'

Avoid listing packages from current directory, when invoked as 'python -m pip freeze'

@pradyunsg, I am unsure about this comment https://github.com/pypa/pip/pull/7810#discussion_r399684766https://github.com/pypa/pip/pull/7810#discussion_r399684766

Also currently the test passes irrespective of changes due to #7864, so it just provides a pseudo confidence.

This skips any *.egg_info present in the current directory when performing pip freeze. This fixes pypa#2926

sbidoul

Please don't merge as is. See #7955 (comment) and following comments.

gutsytechster · 2020-04-05T17:05:32Z

Closing this since the related issue has been resolved via #7955. :)

gutsytechster force-pushed the skip_curr_dir branch from 1c7ada5 to eb876cf Compare March 1, 2020 17:40

gutsytechster mentioned this pull request Mar 15, 2020

script.pip() does not work with use_module=True #7864

Open

pradyunsg mentioned this pull request Mar 15, 2020

Different results between pip list and python -m pip list #7731

Closed

gutsytechster force-pushed the skip_curr_dir branch from ef95a74 to 69bf2d3 Compare March 28, 2020 08:04

uranusjr reviewed Mar 28, 2020

View reviewed changes

pradyunsg approved these changes Apr 1, 2020

View reviewed changes

deveshks mentioned this pull request Apr 1, 2020

Don't list packages in current directory #7955

Merged

gutsytechster added 5 commits April 2, 2020 22:33

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

a898196

This skips any *.egg_info present in the current directory when performing pip freeze. This fixes pypa#2926

fix(_vendors): Exclude '' when searching for distributions

f908a25

add(*.patch): diff for pkg_resource

3c4346b

fix(freeze.py): Skip current directory through freeze and not vendor

d41c15a

docs(news): Update news entry

85c1568

gutsytechster force-pushed the skip_curr_dir branch from 69bf2d3 to 85c1568 Compare April 2, 2020 17:04

sbidoul requested changes Apr 4, 2020

View reviewed changes

gutsytechster closed this Apr 5, 2020

lock bot added the auto-locked Outdated issues that have been locked by automation label May 5, 2020

lock bot locked as resolved and limited conversation to collaborators May 5, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

gutsytechster commented Mar 1, 2020

gutsytechster commented Mar 1, 2020

uranusjr commented Mar 2, 2020 •

edited

Loading

gutsytechster commented Mar 2, 2020

gutsytechster commented Mar 3, 2020

gutsytechster commented Mar 7, 2020 via email

gutsytechster commented Mar 8, 2020 •

edited

Loading

gutsytechster commented Mar 11, 2020

pradyunsg commented Mar 15, 2020 •

edited

Loading

pradyunsg commented Mar 15, 2020

gutsytechster commented Mar 15, 2020 •

edited

Loading

pradyunsg commented Mar 17, 2020

gutsytechster commented Mar 21, 2020 •

edited

Loading

gutsytechster commented Mar 28, 2020

uranusjr Mar 28, 2020

gutsytechster Mar 28, 2020 •

edited

Loading

uranusjr Mar 28, 2020

gutsytechster Mar 28, 2020

uranusjr Mar 28, 2020

McSinyx Mar 29, 2020

gutsytechster Mar 29, 2020

pradyunsg left a comment

pradyunsg Apr 1, 2020

gutsytechster Apr 2, 2020

sbidoul left a comment

gutsytechster commented Apr 5, 2020

		@@ -0,0 +1 @@
		Skip local directory by default for 'pip freeze' or when invoked as 'python -m pip freeze'

	Skip local directory by default for 'pip freeze' or when invoked as 'python -m pip freeze'
	Avoid listing packages from current directory, when invoked as 'python -m pip freeze'

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

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

Conversation

gutsytechster commented Mar 1, 2020

gutsytechster commented Mar 1, 2020

uranusjr commented Mar 2, 2020 • edited Loading

gutsytechster commented Mar 2, 2020

gutsytechster commented Mar 3, 2020

gutsytechster commented Mar 7, 2020 via email

gutsytechster commented Mar 8, 2020 • edited Loading

gutsytechster commented Mar 11, 2020

pradyunsg commented Mar 15, 2020 • edited Loading

pradyunsg commented Mar 15, 2020

gutsytechster commented Mar 15, 2020 • edited Loading

pradyunsg commented Mar 17, 2020

gutsytechster commented Mar 21, 2020 • edited Loading

gutsytechster commented Mar 28, 2020

Choose a reason for hiding this comment

gutsytechster Mar 28, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pradyunsg left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sbidoul left a comment

Choose a reason for hiding this comment

gutsytechster commented Apr 5, 2020

uranusjr commented Mar 2, 2020 •

edited

Loading

gutsytechster commented Mar 8, 2020 •

edited

Loading

pradyunsg commented Mar 15, 2020 •

edited

Loading

gutsytechster commented Mar 15, 2020 •

edited

Loading

gutsytechster commented Mar 21, 2020 •

edited

Loading

gutsytechster Mar 28, 2020 •

edited

Loading