Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Always re-solve directory dependencies #6843

Merged
merged 19 commits into from
Oct 30, 2022

Conversation

adriangb
Copy link
Contributor

@adriangb adriangb commented Oct 20, 2022

Resolves #3783

This resolves an issue poetry lock --no-update does not recognize updates made to a path dependency because it treats it as immutable

@neersighted this works in practice on a real project, but I'm a bit at a loss as to where to add this. I think it should go in test_solver.py, but I think I'd have to re-create the dependencies manually by adding them to the repo, which seems tedious. Might need a bit of guidance there.

@dimbleby
Copy link
Contributor

cf #3783

@adriangb
Copy link
Contributor Author

@dimbleby thank you for linking to the related issues. Please let me know what you think this PR needs, in particular if we want any tests I would appreciate some guidance along the lines of "please write a test similar to this existing test in this test file" since I'm having a little bit of trouble figuring out what the testing strategy is for the project.

@radoering
Copy link
Member

A test may look similar to

@pytest.mark.parametrize("is_locked", [False, True])
def test_solver_does_not_update_ref_of_locked_vcs_package(

(except that the directory dependency is updated even if a prior version is locked).

@adriangb
Copy link
Contributor Author

@radoering test added, thanks for the pointer!

Copy link
Member

@radoering radoering left a comment

Choose a reason for hiding this comment

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

After taking a closer look at the show command, I came to the conclusion that we don't want to re-solve path dependencies in any case. We only want to re-solve when refreshing the lock file but not when solving for a specific environment for show.

I think we have to try another approach: We shouldn't touch get_locked() but add path dependencies to use_latest when refreshing the lock file. If you want to try that approach, this should be the place to add packages with source type "directory" to use_latest:

ops = solver.solve(use_latest=[]).calculate_operations()

@adriangb
Copy link
Contributor Author

@radoering ok I think I got that approach to work, thanks for the feedback!

radoering
radoering previously approved these changes Oct 30, 2022
@radoering radoering enabled auto-merge (squash) October 30, 2022 14:41
@radoering radoering merged commit 2a70d67 into python-poetry:master Oct 30, 2022
@adriangb adriangb deleted the relock-path branch October 30, 2022 18:29
@adriangb adriangb restored the relock-path branch October 30, 2022 18:29
@adriangb adriangb deleted the relock-path branch October 30, 2022 18:29
@adriangb
Copy link
Contributor Author

@radoering thank you!

@skovhus
Copy link

skovhus commented Oct 30, 2022

Thanks for fixing this @adriangb!

@neersighted neersighted added area/solver Related to the dependency resolver kind/enhancement Not a bug or feature, but improves usability or performance impact/changelog Requires a changelog entry labels Nov 3, 2022
@neersighted neersighted added this to the 1.3 milestone Nov 3, 2022
@radoering radoering changed the title Always re-solve path dependencies Always re-solve directory dependencies Dec 2, 2022
Copy link

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 29, 2024
# for free to subscribe to this conversation on GitHub. Already have an account? #.
Labels
area/solver Related to the dependency resolver impact/changelog Requires a changelog entry kind/enhancement Not a bug or feature, but improves usability or performance
Projects
None yet
Development

Successfully merging this pull request may close these issues.

poetry lock --no-update does not reflect directory packages changes
5 participants