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

Solve upgrade can create incompatible dependency versions in lock file + other bugs #6282

Merged
merged 23 commits into from
Oct 29, 2024

Conversation

matteius
Copy link
Member

@matteius matteius commented Oct 23, 2024

The issue

pipenv upgrade relied on the dependent packages being updated, but sometimes the package being requested for update is a dependency of another package in the tree. This applies a more sophisticated approach to considering both ends of that equation.

Fixes #6281
Fixes #6273
Fixes #6280
Fixes #6217
Fixes #6149
Fixes #6144
Fixes #6165
Fixed #6115

Probably Fixes #6249
Probably Fixes #6154

Possibly fixes #6116
Possibly fixes #6237

The fix

Adds test that fails on main branch but passes here:

=============================================================================================== warnings summary ================================================================================================
..\..\.virtualenvs\pipenv-fJ_HixU3\Lib\site-packages\_pytest\config\__init__.py:1441: 25 warnings
  C:\Users\matte\.virtualenvs\pipenv-fJ_HixU3\Lib\site-packages\_pytest\config\__init__.py:1441: PytestConfigWarning: Unknown config option: plugins

    self._warn_or_fail_if_strict(f"Unknown config option: {key}\n")

tests\integration\conftest.py:47
  C:\Users\matte\projects\pipenv\tests\integration\conftest.py:47: RuntimeWarning: Failed connecting to internet: http://httpbin.org/ip
    warnings.warn(

-- Docs: https://docs.pytest.org/en/stable/how-to/capture-warnings.html
============================================================================================ short test summary info ============================================================================================
FAILED tests/integration/test_upgrade.py::test_pipenv_dependency_incompatibility_resolution - AssertionError: google-api-core was not updated to a compatible version despite the protobuf update
assert '==2.18.0' != '==2.18.0'
=================================================================================== 1 failed, 26 warnings in 64.37s (0:01:04) ===================================================================================

The checklist

  • Associated issue
  • 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 changed the title Issue 6281 Solve pipenv update/upgrade can create incompatible dependency versions in lock file Oct 24, 2024
@matteius matteius marked this pull request as ready for review October 24, 2024 04:23
@matteius matteius requested a review from oz123 October 24, 2024 04:23
@matteius matteius force-pushed the issue-6281 branch 3 times, most recently from e68751e to c34fa08 Compare October 24, 2024 22:47
…ses + pip session requests performance issue patch
categories = ["develop"]
elif not categories or "packages" in categories:
categories = ["default"]
if not categories:
Copy link
Member Author

Choose a reason for hiding this comment

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

If the original authors had just made the package group names identical between Pipfile and lock it would have saved 50% of my headache here.

@@ -223,7 +223,7 @@ def requirement_from_lockfile(
if "extras" in package_info
else ""
)
pip_line = f"{package_name}{extras}=={version}{os_markers}{markers}{hashes}"
Copy link
Member Author

Choose a reason for hiding this comment

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

The cause of our version specifiers being too strict, because this method is actually used in more places than just requirement_from_lockfile 👎

.github/PULL_REQUEST_TEMPLATE.md Outdated Show resolved Hide resolved
k in entry_dict
for k in ["sys_platform", "python_version", "os_name", "platform_machine"]
):
return None, entry_dict
Copy link
Contributor

Choose a reason for hiding this comment

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

You are replicating code from plette here why not just import it?


@property
def original_markers(self):
original_markers, lockfile_dict = self.get_markers_from_dict(self.lockfile_dict)
Copy link
Contributor

Choose a reason for hiding this comment

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

These local methods might be added to plette.

if isinstance(self.extras, list):
self.extras = set(self.extras)
if isinstance(self.hashes, list):
self.hashes = set(self.hashes)

Copy link
Contributor

Choose a reason for hiding this comment

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

Same as comment above.

pipenv/routines/graph.py Show resolved Hide resolved
pipenv/routines/graph.py Show resolved Hide resolved
pyproject.toml Show resolved Hide resolved
tests/integration/test_cli.py Show resolved Hide resolved
@oz123 oz123 merged commit 94b0db6 into main Oct 29, 2024
22 checks passed
@oz123 oz123 deleted the issue-6281 branch October 29, 2024 21:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment