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

Private packages not updated unless environment is removed #5791

Open
joaomcarlos opened this issue Jul 13, 2023 · 24 comments
Open

Private packages not updated unless environment is removed #5791

joaomcarlos opened this issue Jul 13, 2023 · 24 comments
Labels
ai-triaged Category: Private PyPIs 😎 Problem relates to private PyPI usage. Contributor Candidate The issue has been identified/triaged and contributions are welcomed/encouraged. Status: Awaiting Update ⏳ This issue requires more information before assistance can be provided. triage Type: Bug 🐛 This issue is a bug.

Comments

@joaomcarlos
Copy link

Issue description

Private packages defined with an object like mylib = {ref = "dev" git = "ssh://git.mycompany.com/my-lib.git"} are not being updated on my virtual env, even after the pipenv update creates a new Pipfile.lock with the correct commit id.

Expected result

After calling pipenv update, I expect the Pipfile.lock ref to change and also the code in my environment to change, such that I have access to the new code within my project.

Actual result

I see the Pipfile.lock ref change, but the actual code for my dependency does not change.

If I do pipenv --rm to remove the environment and then re-create the environment, I can get the actual code change to replicate on disk.

Steps to replicate

Update my-lib with some code change, commit and push. On my project, do pipenv update and wait for the process to finish.

Check that the Pipfile.lock has changed and has the correct ref for the new commit in my-lib.

Check that the code for my-lib hasnt actually changed within my original projects virtual env.

@matteius matteius added Type: Bug 🐛 This issue is a bug. triage Contributor Candidate The issue has been identified/triaged and contributions are welcomed/encouraged. labels Jul 14, 2023
@pypa pypa deleted a comment from edva-lson Aug 1, 2023
@matteius
Copy link
Member

matteius commented Aug 1, 2023

Could you help me verify if this is still an issue on this overhaul branch: #5793

@joaomcarlos
Copy link
Author

joaomcarlos commented Aug 8, 2023

Hi,

I installed this branch of pipenv and my pipenv updates started failing.

If I revert back to normal pipenv it works perfectly.

Error output:

Loading .env environment variables...
Running $ pipenv lock then $ pipenv sync.
Locking [packages] dependencies...
Building requirements...
Resolving dependencies...
✔ Success!
Locking [dev-packages] dependencies...
Building requirements...
Resolving dependencies...
✘ Locking Failed!
⠼ Locking...
[ResolutionFailure]:   File "/usr/local/lib/python3.10/site-packages/pipenv/resolver.py", line 665, in _main
[ResolutionFailure]:       resolve_packages(
[ResolutionFailure]:   File "/usr/local/lib/python3.10/site-packages/pipenv/resolver.py", line 632, in resolve_packages
[ResolutionFailure]:       results, resolver = resolve(
[ResolutionFailure]:       ^^^^^^^^
[ResolutionFailure]:   File "/usr/local/lib/python3.10/site-packages/pipenv/resolver.py", line 612, in resolve
[ResolutionFailure]:       return resolve_deps(
[ResolutionFailure]:       ^^^^^^^^^^^^^
[ResolutionFailure]:   File "/usr/local/lib/python3.10/site-packages/pipenv/utils/resolver.py", line 842, in resolve_deps
[ResolutionFailure]:       results, hashes, internal_resolver = actually_resolve_deps(
[ResolutionFailure]:       ^^^^^^^^^^^^^^^^^^^^^^
[ResolutionFailure]:   File "/usr/local/lib/python3.10/site-packages/pipenv/utils/resolver.py", line 617, in actually_resolve_deps
[ResolutionFailure]:       resolver.resolve()
[ResolutionFailure]:   File "/usr/local/lib/python3.10/site-packages/pipenv/utils/resolver.py", line 441, in resolve
[ResolutionFailure]:       raise ResolutionFailure(message=str(e))
[pipenv.exceptions.ResolutionFailure]: Warning: Your dependencies could not be resolved. You likely have a mismatch in your sub-dependencies.
  You can use $ pipenv run pip install <requirement_name> to bypass this mechanism, then run $ pipenv graph to inspect the versions actually installed in the virtualenv.
  Hint: try $ pipenv lock --pre if it is a pre-release dependency.
ERROR: Constraints cannot have extras

Traceback (most recent call last):
  File "/usr/local/bin/pipenv", line 8, in <module>
    sys.exit(cli())
  File "/usr/local/lib/python3.10/site-packages/pipenv/vendor/click/core.py", line 1130, in __call__
    return self.main(*args, **kwargs)
  File "/usr/local/lib/python3.10/site-packages/pipenv/cli/options.py", line 57, in main
    return super().main(*args, **kwargs, windows_expand_args=False)
  File "/usr/local/lib/python3.10/site-packages/pipenv/vendor/click/core.py", line 1055, in main
    rv = self.invoke(ctx)
  File "/usr/local/lib/python3.10/site-packages/pipenv/vendor/click/core.py", line 1657, in invoke
    return _process_result(sub_ctx.command.invoke(sub_ctx))
  File "/usr/local/lib/python3.10/site-packages/pipenv/vendor/click/core.py", line 1404, in invoke
    return ctx.invoke(self.callback, **ctx.params)
  File "/usr/local/lib/python3.10/site-packages/pipenv/vendor/click/core.py", line 760, in invoke
    return __callback(*args, **kwargs)
  File "/usr/local/lib/python3.10/site-packages/pipenv/vendor/click/decorators.py", line 84, in new_func
    return ctx.invoke(f, obj, *args, **kwargs)
  File "/usr/local/lib/python3.10/site-packages/pipenv/vendor/click/core.py", line 760, in invoke
    return __callback(*args, **kwargs)
  File "/usr/local/lib/python3.10/site-packages/pipenv/vendor/click/decorators.py", line 26, in new_func
    return f(get_current_context(), *args, **kwargs)
  File "/usr/local/lib/python3.10/site-packages/pipenv/cli/command.py", line 570, in update
    do_update(
  File "/usr/local/lib/python3.10/site-packages/pipenv/routines/update.py", line 57, in do_update
    do_lock(
  File "/usr/local/lib/python3.10/site-packages/pipenv/routines/lock.py", line 65, in do_lock
    venv_resolve_deps(
  File "/usr/local/lib/python3.10/site-packages/pipenv/utils/resolver.py", line 783, in venv_resolve_deps
    c = resolve(cmd, st, project=project)
  File "/usr/local/lib/python3.10/site-packages/pipenv/utils/resolver.py", line 654, in resolve
    raise RuntimeError("Failed to lock Pipfile.lock!")
RuntimeError: Failed to lock Pipfile.lock!

My pipfile looks like:

[[source]]
name = "pypi"
url = "https://pypi.org/simple"
verify_ssl = true

[packages]
pydantic = "~=2.0"
mylib = {ref = "dev" git = "ssh://git.mycompany.com/project-name/my-lib.git"}
mylib2 = {ref = "dev" git = "ssh://git.mycompany.com/project-name/my-lib2.git"}
black = "*"
"black[jupyter]" = "*"
bpython = "*"
coverage = "*"
environs = "*"
flake8 = "*"
gelf-formatter = "*"
isort = "*"
minio = "*"
mock = "*"
cryptography = "*"
nameko = "*"
nameko-amqp-retry = "*"
nameko-sqlalchemy = "*"
pendulum = "*"
pytest = "*"
python-dateutil = "*"
pytz = "*"
requests = "*"
rich = "*"
sentry_sdk = "*"
SQLAlchemy = "==1.4.46"
sqlalchemy_utc="*"
pytest-eventlet = "*"
pre-commit = "*"
freezegun = "*"

[requires]
python_version = "3.11"

[dev-packages]
ipykernel = "*"

@matteius
Copy link
Member

matteius commented Aug 9, 2023

@joaomcarlos I just pushed a fix that solves the specific issue you reported of:

ERROR: Constraints cannot have extras

Thanks for catching that -- I think the branch is looking really good now, except there is still an issue with vcs+file:// paths that I am trying to sort out, and also some updates to the documentation around vcs specifiers since the current docs are referencing old pip line formats. Still curious of the branch solves the original issue reported though.

@joaomcarlos
Copy link
Author

We use this style though:

mylib = {ref = "dev" git = "ssh://git.mycompany.com/project-name/my-lib.git"}

Because our libs are in our private gitlab instance. We arent using vcs or file

@matteius
Copy link
Member

@joaomcarlos 2023.8.19 has been released 🤞

@joaomcarlos
Copy link
Author

joaomcarlos commented Aug 21, 2023

Thank you! Will give it a try.

Edit: going on holiday, asked a colleague to check it out

@matteius matteius added the Status: Awaiting Update ⏳ This issue requires more information before assistance can be provided. label Aug 26, 2023
@AndreGraca98
Copy link

AndreGraca98 commented Aug 28, 2023

Changed ref from our package to older one and updated the environment the way we used to before (pipenv lock && pipenv --rm) and installed it back. Then changed the ref to the current we have and did pipenv update and it updated the environment and the piplock, so we can say it's working fine!

@matteius matteius closed this as completed Sep 1, 2023
@joaomcarlos
Copy link
Author

Hi,

This is actually still happening. The refs get updated on the lock file but the code does not change until you --rm it

@matteius
Copy link
Member

@joaomcarlos here was a similar report that was confirmed fixed on the recent version: #5798 (comment)

@joaomcarlos
Copy link
Author

joaomcarlos commented Sep 13, 2023

Not that great at graphics but hopefully this points out the issue.

Basically I have a project that uses health check code from a lib called lib-common. (why doesnt matter, just so i could easily replicate it)

Added print("potatoes") to the lib-commons dependency resolver of that so that i'd instantly see it whenever I health-checked the service.

Pushed that to branch dev and pipenv updated the main project.

Git shows that the .lock file of the main project got updated to the git commit ref that contains the potatoes.

But when I run the project, no potatoes :/ 🗡️ 🗡️ 🗡️ and inspecting the code copy of the lib in the projects virtual environment shows that indeed, there are no potatoes. It will go hungry tonight.

After pipenv --rm and letting it create the environment again, 🪄 , the potatoes finally arrived :D

problem

@joaomcarlos
Copy link
Author

@matteius Tagging you, not sure if you saw my reply :)

@matteius matteius reopened this Sep 14, 2023
@matteius
Copy link
Member

I saw your reply, but I cannot reproduce it, so leaving it open for now in case someone can help you.

@matteius
Copy link
Member

Actually, I think I see why this would be happening, https://github.com/pypa/pipenv/blob/main/pipenv/routines/install.py#L506
Basically the install phase ignores things that are already satisfied, which appears to be determined for vcs if the branch matches (not the resolved hash of the lockfile). 🤔

@matteius
Copy link
Member

I opened a draft PR that I think will fix it if you can help check it out @joaomcarlos

@joaomcarlos
Copy link
Author

Right, we tested it and we saw the same behavior.

Lets re-cap:

We ran, on me and Andres machine, the following two commands:

pip install -U git+https://github.com/pypa/pipenv.git@1afd341a9318f011ac31db3c5367058d65aa4db
cd [into our project]
pipenv --version 
pipenv, version 2023.9.8
pipenv install git+https://github.com/pypa/pipenv.git@1afd341a9318f011ac31db3c5367058d65aa4db
pipenv --version
pipenv, version 2023.9.9.dev0

At this point, we established a baseline for pipenv.

We commit a "potatoes 2" print to our package on the dev branch and run pipenv update, we see that the commit ref has been updated successfully but no change in the files under our virtual env specifically (/Users/joao.carlos/.local/share/virtualenvs/package-5xV1os6e/src/package-name/module/file.py

We tried a bunch of variations and none worked.

We did however manage to get it updating by adding the editable=true to the Pipfile.

With editable=true, we see both the change in ref and the change in the file.

Does this help with debugging the issue?

@joaomcarlos
Copy link
Author

@matteius I am not sure if you got a notification about this, so pinging you.

@matteius
Copy link
Member

I saw, just don't have time at the moment--getting back to work after two weeks off 😅

@joaomcarlos
Copy link
Author

@matteius all is gut :D No rush. I hope your holiday was good!

@matteius
Copy link
Member

matteius commented Dec 6, 2023

@joaomcarlos I think the last release (two releases ago) had an improvement related to this, but would be helpful if you can confirm its fixed.

@myii
Copy link

myii commented Sep 5, 2024

@matteius I've encountered this bug again, so this time I did a little investigating -- I've established a workaround that should help establish a fix.

I must note, that in my case, it also affects GitHub refs, such as:

django-grappelli = {editable = true, ref = "9a818ed4336d190dd9464d1f26668760be077dbf", git = "https://github.com/sehmaschine/django-grappelli.git"}  # Based on "==3.0.9"

The same situation of not actually upgrading the package when running pipenv sync as this issue's author:

... even after the pipenv update creates a new Pipfile.lock with the correct commit id.

So there's quite a bit I have to share, not sure whether to:

  1. Add all the info as a comment in this issue.
  2. Provide it all in a new issue, linking back to this one.
  3. Open a PR with a draft/proposed fix, that can be worked on from there.

@matteius
Copy link
Member

matteius commented Sep 6, 2024

@myii some combination of 1&3 sounds good to me!

@myii
Copy link

myii commented Sep 8, 2024

@myii some combination of 1&3 sounds good to me!

@matteius OK, I'll share my findings here first, then I'll submit a PR depending on your feedback.


TL;DR -- A quick workaround that I've been using

In case @joaomcarlos and/or @AndreGraca98 are still affected by this bug and want to try out a workaround, without trawling through the extended discussion below.

Based on the latest release (v2024.0.1):

if match is not None:
if req.specifier is not None:
return SpecifierSet(str(req.specifier)).contains(
match.version, prereleases=True
)

Add the following two lines:

         if match is not None:
+            if (req.link and req.link.is_vcs):
+                return False
             if req.specifier is not None:
                 return SpecifierSet(str(req.specifier)).contains(
                     match.version, prereleases=True
                 )

Identifying the regression

v2023.9.8 was the last working version for me, so I used that to help narrow down my git bisect range. I found that the regression was introduced in:

Running interactive debugging, I could see that my VCS-based entries were entering this if clause that had been moved to the top and were now returning True:

if req.specifier is not None:
return SpecifierSet(str(req.specifier)).contains(
match.version, prereleases=True
)

I.e. These entries were no longer getting to the specific conditional they were supposed to reach.


Hitting a traceback with the first attempted workaround

The obvious workaround to attempt first was to bring the specific conditional back to the top. So based on the current latest commit in the main branch, I'm referring to this block:

elif match.has_metadata("direct_url.json") or (req.link and req.link.is_vcs):
# Direct URL installs and VCS installs we assume are not satisfied
# since due to skip-lock we may be installing from Pipfile we have insufficient
# information to determine if a branch or ref has actually changed.
return False

However, this led to a traceback due to the match.has_metadata("direct_url.json") clause:

$ pipenv sync
Loading .env environment variables...
Installing dependencies from Pipfile.lock (89ef39)...
Traceback (most recent call last):
  File ".../.local/bin/pipenv", line 8, in <module>
    sys.exit(cli())
  File ".../pipenv/vendor/click/core.py", line 1157, in __call__
    return self.main(*args, **kwargs)
  File ".../pipenv/cli/options.py", line 58, in main
    return super().main(*args, **kwargs, windows_expand_args=False)
  File ".../pipenv/vendor/click/core.py", line 1078, in main
    rv = self.invoke(ctx)
  File ".../pipenv/vendor/click/core.py", line 1688, in invoke
    return _process_result(sub_ctx.command.invoke(sub_ctx))
  File ".../pipenv/vendor/click/core.py", line 1434, in invoke
    return ctx.invoke(self.callback, **ctx.params)
  File ".../pipenv/vendor/click/core.py", line 783, in invoke
    return __callback(*args, **kwargs)
  File ".../pipenv/vendor/click/decorators.py", line 92, in new_func
    return ctx.invoke(f, obj, *args, **kwargs)
  File ".../pipenv/vendor/click/core.py", line 783, in invoke
    return __callback(*args, **kwargs)
  File ".../pipenv/vendor/click/decorators.py", line 33, in new_func
    return f(get_current_context(), *args, **kwargs)
  File ".../pipenv/cli/command.py", line 651, in sync
    retcode = do_sync(
  File ".../pipenv/routines/install.py", line 339, in do_sync
    do_init(
  File ".../pipenv/routines/install.py", line 680, in do_init
    do_install_dependencies(
  File ".../pipenv/routines/install.py", line 438, in do_install_dependencies
    batch_install(
  File ".../pipenv/routines/install.py", line 503, in batch_install
    deps_to_install = [
  File ".../pipenv/routines/install.py", line 506, in <listcomp>
    if not project.environment.is_satisfied(dep)
  File ".../pipenv/environment.py", line 734, in is_satisfied
    if match.has_metadata("direct_url.json") or (req.link and req.link.is_vcs):
AttributeError: 'PathDistribution' object has no attribute 'has_metadata'

With a bit more digging, I found that this was something that slipped through during the pkg_resources => importlib-metadata conversion in b1c14a2.


Finalising the workaround

Not to be presumptuous about the correct fix for this, I expunged the clause mercilessly! This led to the fuller form of the workaround, which may be the basis of a potential fix:

         if match is not None:
+            if (req.link and req.link.is_vcs):
+                # VCS installs we assume are not satisfied since due to
+                # skip-lock we may be installing from Pipfile we have insufficient
+                # information to determine if a branch or ref has actually changed.
+                return False
             if req.specifier is not None:
                 return SpecifierSet(str(req.specifier)).contains(
                     match.version, prereleases=True
                 )
             if req.link is None:
                 return True
             elif req.editable and req.link.is_file:
                 requested_path = req.link.file_path
                 if os.path.exists(requested_path):
                     local_path = requested_path
                 else:
                     parsed_url = urlparse(requested_path)
                     local_path = parsed_url.path
                 return requested_path and os.path.samefile(local_path, match.location)
-            elif match.has_metadata("direct_url.json") or (req.link and req.link.is_vcs):
-                # Direct URL installs and VCS installs we assume are not satisfied
-                # since due to skip-lock we may be installing from Pipfile we have insufficient
-                # information to determine if a branch or ref has actually changed.
-                return False
             return True
         return False

@oz123 oz123 added the Category: Private PyPIs 😎 Problem relates to private PyPI usage. label Oct 4, 2024
@matteius
Copy link
Member

Analysis of Pipenv Issue #5791

1. Problem Summary

The core issue is that pipenv update doesn't always update VCS-based dependencies (e.g., Git) to the latest commit specified in the updated Pipfile.lock. While the lockfile correctly reflects the new commit ID, the actual code in the virtual environment remains unchanged. This forces users to manually delete the environment (pipenv --rm) and recreate it to pull the updated code.

2. Comments Analysis

  • Confirmation and Initial Failure: The maintainer initially suggested testing the issue on a new branch. The reporter tried and encountered a different error related to constraints and extras.
  • Partial Fix and Further Issues: The maintainer addressed the constraint error, but the original problem persisted. The reporter provided a detailed example showcasing the issue with Git dependencies.
  • Regression Identification: The maintainer identified the likely cause: a change in pipenv.environment.is_satisfied that prematurely concluded VCS dependencies were satisfied based on branch name, ignoring the commit hash.
  • Workaround and Further Testing: The reporter offered a workaround by explicitly adding editable=true to the Pipfile for VCS dependencies. This successfully triggered an update. The maintainer acknowledged the workaround but couldn't reproduce the issue consistently.
  • Confirmation of Persistence: The reporter confirmed that even with the latest release, the issue still occurs unless the workaround is used.
  • Workaround Details and Traceback: A different user (@myii) confirmed the bug and provided a more detailed explanation of the workaround. They also identified the specific commit that introduced the regression and highlighted a traceback related to match.has_metadata("direct_url.json") encountered when trying a different workaround.

3. Proposed Resolution

The root cause lies in the is_satisfied function, which incorrectly uses only the branch name for determining if a VCS dependency is satisfied. It needs to be updated to consider the commit hash as well.

Code Changes

File: pipenv/environment.py

Function: is_satisfied(self, req: InstallRequirement)

Current Code (snippet):

	      if match is not None:
	          if (req.link and req.link.is_vcs):
	              # VCS installs we assume are not satisfied since due to
	              # skip-lock we may be installing from Pipfile we have insufficient
	              # information to determine if a branch or ref has actually changed.
	              return False
	          # ... (other checks) ...

Proposed Code (snippet):

	      if match is not None:
	          if (req.link and req.link.is_vcs):
	              # Compare the commit IDs from the requirement and the installed distribution
	              if req.req.vcs_info.commit_id != match.direct_url.info.commit_id:
	                  return False
	          # ... (other checks) ...

This change compares the commit IDs from the InstallRequirement (which represents the desired dependency from the lockfile) and the installed distribution's direct_url.json (which stores information about how the package was installed, including the commit ID for VCS dependencies). If the commit IDs are different, it correctly concludes that the dependency is not satisfied and requires updating.

4. Code Snippet

The proposed code snippet is already included in the resolution section (point 3).

5. Additional Steps

  • Thorough Testing: The proposed change should be rigorously tested with various VCS dependencies (Git, SVN, etc.) and scenarios to ensure it comprehensively addresses the issue.
  • Edge Case Handling: Potential edge cases, like situations where a branch name and tag name point to the same commit, should be considered and addressed.
  • Documentation: Update relevant documentation to clarify the behavior of pipenv update with VCS dependencies, emphasizing the importance of commit hashes.

By implementing these changes, Pipenv can ensure the correct and consistent updating of VCS dependencies based on the latest commit ID specified in the Pipfile.lock, eliminating the need for manual environment recreation.

@matteius
Copy link
Member

Could this be re-checked against #6276 which has some bug fixes to update command?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ai-triaged Category: Private PyPIs 😎 Problem relates to private PyPI usage. Contributor Candidate The issue has been identified/triaged and contributions are welcomed/encouraged. Status: Awaiting Update ⏳ This issue requires more information before assistance can be provided. triage Type: Bug 🐛 This issue is a bug.
Projects
None yet
Development

No branches or pull requests

5 participants