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

Get packages for pipenv check from the target venv. #5501

Merged
merged 2 commits into from
Nov 24, 2022

Conversation

yeisonvargasf
Copy link
Contributor

The issue

#4819
#5488

The fix

Now, this command gets the packages to be analyzed from the target venv.

@matteius
Copy link
Member

Something not quite right with this change and the Windows CI.

@matteius
Copy link
Member

@yeisonvargasf A variation of these changes work for windows -- the primary issue is that the file gets-re-opened so it needs to be closed first.

PS C:\Users\matte\Projects\pipenv> git diff
diff --git a/pipenv/core.py b/pipenv/core.py
index acebcc20..494191cb 100644
--- a/pipenv/core.py
+++ b/pipenv/core.py
@@ -2959,6 +2959,16 @@ def do_check(
     if safety_project:
         options.append(f"--project={safety_project}")

+    target_venv_packages = run_command(
+        _cmd + ["-m", "pip", "list", "--format=freeze"], is_verbose=project.s.is_verbose()
+    )
+
+    temp_requirements = tempfile.NamedTemporaryFile(mode="w+", prefix=f"{project.virtualenv_name}", suffix="_requirements.txt", delete=False)
+    temp_requirements.write(target_venv_packages.stdout.strip())
+    temp_requirements.close()
+
+    options.extend(["--file", temp_requirements.name])
+
     cmd = _cmd + [safety_path, "check"] + options

     if db:
@@ -3044,6 +3054,8 @@ def do_check(

     cli(prog_name="pipenv")

+    temp_requirements.remove()
+

 def do_graph(project, bare=False, json=False, json_tree=False, reverse=False):
     import json as jsonlib
diff --git a/tests/integration/test_cli.py b/tests/integration/test_cli.py
index f086cdfe..b6009d85 100644
--- a/tests/integration/test_cli.py
+++ b/tests/integration/test_cli.py
@@ -140,7 +140,6 @@ def test_pipenv_graph_reverse(pipenv_instance_private_pypi):

 @pytest.mark.cli
 @pytest.mark.needs_internet(reason='required by check')
-@pytest.mark.skip("Safety 2 ends up scanning the project virtualenv and not the instance created by this test.")
 def test_pipenv_check(pipenv_instance_private_pypi):
     with pipenv_instance_private_pypi() as p:
         c = p.pipenv('install pyyaml')

@yeisonvargasf
Copy link
Contributor Author

@matteius, thank you! I pushed the fix for that case; let me know if there is something more to fix.

@matteius matteius merged commit 2923ccf into pypa:main Nov 24, 2022
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.

2 participants