Skip to content

Deprecation Notice: Make safety an optional dependency via extras #6365

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

Open
wants to merge 9 commits into
base: main
Choose a base branch
from

Conversation

matteius
Copy link
Member

@matteius matteius commented Mar 21, 2025

Thank you for contributing to Pipenv!

Make safety an optional dependency via extras

Problem

Currently, pipenv vendors the safety package and its dependencies directly in the pipenv/patched/ directory. This approach has several drawbacks:

  1. We recently learned that the latest safety requires typer and its own safety-schemas which requires pydantic
  2. Pydantic was previously removed from pipenv for good reasons, and we don't want to reintroduce it
  3. Vendoring all these dependencies increases the size and complexity of the pipenv codebase
  4. Users who don't use the vulnerability checking functionality still have these dependencies

Solution

This PR makes safety an optional dependency via extras:

  1. Removed the vendored safety directory from pipenv/patched/
  2. Added safety and its dependencies as optional extras in pyproject.toml (pipenv[safety])
  3. Modified the check command to:
    • Check if safety is installed
    • If not, prompt the user to allow installing safety + dependencies
    • Install safety without modifying the user's Pipfile or lockfile
    • Use the installed safety module instead of the vendored one

Implementation Details

  • Added safety>=3.0.0 and typer>=0.9.0 as optional dependencies under optional-dependencies.safety in pyproject.toml
  • Removed the safety package data entry from pyproject.toml
  • Updated the check.py implementation to detect if safety is installed and offer to install it on-demand
  • Converted all click.echo/secho calls to use the project's rich interface (console.print/err.print)

Testing

To test this change:

  1. Run pipenv check without safety installed - it should prompt to install safety
  2. Confirm that installing safety doesn't modify the Pipfile or lockfile
  3. Run pipenv check again to verify it works with the installed safety
  4. Install pipenv with extras: pip install pipenv[safety] and verify check works without prompting

Impact

  • Reduces the dependency footprint of pipenv for users who don't use the vulnerability checking
  • Avoids reintroducing pydantic as a core dependency
  • Provides a cleaner, more maintainable approach to optional functionality
  • Improves user experience by only installing what's needed when it's needed

The checklist

  • Associated issue -- is blocking the pip 25.0.1 vendoring update
  • A news fragment in the news/ directory to describe this fix with the extension .bugfix.rst, .feature.rst, .behavior.rst, .doc.rst. .vendor.rst. or .trivial.rst (this will appear in the release changelog). Use semantic line breaks and name the file after the issue number or the PR #.

@matteius matteius requested a review from Copilot April 21, 2025 10:05
@matteius matteius marked this pull request as ready for review April 21, 2025 10:05
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR removes the vendored safety module and its associated files from Pipenv and makes safety an optional dependency via extras. Key changes include:

  • Deletion of all files under pipenv/patched/safety (formatter, errors, constants, cli, alerts, etc.)
  • Introduction of new flags (--auto-install and --scan) in pipenv/cli/command.py and updated documentation to reflect usage of the new scan command.
  • Removal of safety from the patched dependency list in pipenv/patched/patched.txt.

Reviewed Changes

Copilot reviewed 28 out of 33 changed files in this pull request and generated 1 comment.

File Description
pipenv/patched/safety/* Removal of the vendored safety code to shift to an optional installation via extras.
pipenv/cli/command.py New flags added for auto-installation and scanning; deprecation notice updated for check command.
docs/cli.md & docs/commands.md Documentation updated to reflect the new scan command and auto-install flag.
pipenv/patched/patched.txt Safety dependency removed from the vendor list.
Files not reviewed (5)
  • news/safety-command.bugfix.rst: Language not supported
  • news/safety-extras.feature.rst: Language not supported
  • pipenv/patched/pip/LICENSE-HEADER: Language not supported
  • pipenv/patched/safety/LICENSE: Language not supported
  • pipenv/patched/safety/VERSION: Language not supported
Comments suppressed due to low confidence (1)

pipenv/cli/command.py:560

  • New flags '--auto-install' and '--scan' have been introduced; please add or update unit tests to ensure these options correctly trigger the intended safety installation and scanning behavior.
auto_install=auto_install,

matteius and others added 2 commits April 21, 2025 06:06
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@matteius matteius changed the title Check in work to make safety a module. Deprecation Notice: Make safety an optional dependency via extras Apr 21, 2025
prefix=f"{project.virtualenv_name}",
suffix="_requirements.txt",
delete=False,
"""Create a temporary requirements file that safety can access."""
Copy link
Contributor

Choose a reason for hiding this comment

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

This step is not necessary as safety can directly work with pipfiles.
We can remove it in the future.

@@ -43,6 +43,10 @@ optional-dependencies.dev = [
"sphinx",
"towncrier",
]
optional-dependencies.safety = [
"safety>=3.0.0",
"typer>=0.9.0",
Copy link
Contributor

Choose a reason for hiding this comment

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

isnt' typer pulled automatically with safety?

@pytest.mark.parametrize("category", ["CVE", "packages"])
def test_pipenv_check_check_lockfile_categories(pipenv_instance_pypi, category):
with pipenv_instance_pypi() as p:
c = p.pipenv(f"install wheel==0.37.1 --categories={category}")
Copy link
Contributor

Choose a reason for hiding this comment

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

why remove this test?

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