Lock markers and groups#9427
Merged
radoering merged 3 commits intopython-poetry:mainfrom Nov 15, 2024
Merged
Conversation
This was referenced May 20, 2024
Merged
Closed
radoering
added a commit
to radoering/poetry
that referenced
this pull request
Jun 2, 2024
radoering
added a commit
to radoering/poetry
that referenced
this pull request
Jun 2, 2024
…ting locked markers) (python-poetry#9427) - introduce "installer.re-resolve" config option (default: True) - if the config option is set to False and the lock file is at least version 2.1, the installer will not re-resolve but evaluate locked markers
ec11430 to
49f69c9
Compare
radoering
added a commit
to radoering/poetry
that referenced
this pull request
Jun 16, 2024
radoering
added a commit
to radoering/poetry
that referenced
this pull request
Jun 16, 2024
…ting locked markers) (python-poetry#9427) - introduce "installer.re-resolve" config option (default: True) - if the config option is set to False and the lock file is at least version 2.1, the installer will not re-resolve but evaluate locked markers
49f69c9 to
4d01327
Compare
radoering
added a commit
to radoering/poetry
that referenced
this pull request
Jun 16, 2024
…ting locked markers) (python-poetry#9427) - introduce "installer.re-resolve" config option (default: True) - if the config option is set to False and the lock file is at least version 2.1, the installer will not re-resolve but evaluate locked markers
4d01327 to
241ecfe
Compare
|
Deploy preview for website ready! Built with commit bf1270b. |
radoering
added a commit
to radoering/poetry
that referenced
this pull request
Jul 14, 2024
radoering
added a commit
to radoering/poetry
that referenced
this pull request
Jul 14, 2024
…ting locked markers) (python-poetry#9427) - introduce "installer.re-resolve" config option (default: True) - if the config option is set to False and the lock file is at least version 2.1, the installer will not re-resolve but evaluate locked markers
241ecfe to
b56b49a
Compare
radoering
added a commit
to radoering/poetry
that referenced
this pull request
Jul 29, 2024
radoering
added a commit
to radoering/poetry
that referenced
this pull request
Jul 29, 2024
…ting locked markers) (python-poetry#9427) - introduce "installer.re-resolve" config option (default: True) - if the config option is set to False and the lock file is at least version 2.1, the installer will not re-resolve but evaluate locked markers
b56b49a to
50e93f7
Compare
radoering
added a commit
to radoering/poetry
that referenced
this pull request
Aug 24, 2024
radoering
added a commit
to radoering/poetry
that referenced
this pull request
Aug 24, 2024
…ting locked markers) (python-poetry#9427) - introduce "installer.re-resolve" config option (default: True) - if the config option is set to False and the lock file is at least version 2.1, the installer will not re-resolve but evaluate locked markers
50e93f7 to
a65c879
Compare
2 tasks
a65c879 to
623602d
Compare
radoering
added a commit
to radoering/poetry
that referenced
this pull request
Oct 4, 2024
…ting locked markers) (python-poetry#9427) - introduce "installer.re-resolve" config option (default: True) - if the config option is set to False and the lock file is at least version 2.1, the installer will not re-resolve but evaluate locked markers
46a8221 to
5db16dd
Compare
radoering
added a commit
to radoering/poetry
that referenced
this pull request
Oct 19, 2024
radoering
added a commit
to radoering/poetry
that referenced
this pull request
Oct 19, 2024
…ting locked markers) (python-poetry#9427) - introduce "installer.re-resolve" config option (default: True) - if the config option is set to False and the lock file is at least version 2.1, the installer will not re-resolve but evaluate locked markers
5db16dd to
517080a
Compare
radoering
added a commit
to radoering/poetry
that referenced
this pull request
Oct 20, 2024
radoering
added a commit
to radoering/poetry
that referenced
this pull request
Oct 20, 2024
…ting locked markers) (python-poetry#9427) - introduce "installer.re-resolve" config option (default: True) - if the config option is set to False and the lock file is at least version 2.1, the installer will not re-resolve but evaluate locked markers
517080a to
bdf5dfb
Compare
radoering
added a commit
to radoering/poetry
that referenced
this pull request
Oct 20, 2024
radoering
added a commit
to radoering/poetry
that referenced
this pull request
Oct 20, 2024
…ting locked markers) (python-poetry#9427) - introduce "installer.re-resolve" config option (default: True) - if the config option is set to False and the lock file is at least version 2.1, the installer will not re-resolve but evaluate locked markers
bdf5dfb to
1e0300b
Compare
radoering
added a commit
to radoering/poetry
that referenced
this pull request
Oct 29, 2024
radoering
added a commit
to radoering/poetry
that referenced
this pull request
Oct 29, 2024
…ting locked markers) (python-poetry#9427) - introduce "installer.re-resolve" config option (default: True) - if the config option is set to False and the lock file is at least version 2.1, the installer will not re-resolve but evaluate locked markers
1e0300b to
1244d21
Compare
Secrus
reviewed
Nov 4, 2024
Member
Secrus
left a comment
There was a problem hiding this comment.
Not my area of expertise, so not 100% sure, but looks pretty good to me, minor comments
…ting locked markers) (python-poetry#9427) - introduce "installer.re-resolve" config option (default: True) - if the config option is set to False and the lock file is at least version 2.1, the installer will not re-resolve but evaluate locked markers
1244d21 to
713e672
Compare
Secrus
approved these changes
Nov 15, 2024
radoering
added a commit
that referenced
this pull request
Nov 15, 2024
reesehyde
added a commit
to reesehyde/poetry
that referenced
this pull request
Nov 18, 2024
* origin/main: Upgrade minimal Cleo to 2.2.1 make `allow-prereleases` a tri-state setting to really forbid pre-releases if the setting is `false` and keep default behavior to allow pre-releases only if necessary feat: add confirmation step feat(cli): add info messages about applied migration docs: add information about --migrate feat(cli): add support for --local for --migrate feat(cli): add --migration option to config command feat(config): add ConfigSourceMigration feat(config): provide method to remove empty config category feat(config): add get_property() to ConfigSource Ignore http credentials with empty usernames Fix regression when using empty username/password fix index error for yanked releases without dependencies ignore installed packages during solving vcs: use peeled ref when retrieving tag revision (python-poetry#9849) chore: update json fixtures and improve generate script so that it produces the same results on Linux and Windows installer: add option to install without re-resolving (just by evaluating locked markers) (python-poetry#9427) - introduce "installer.re-resolve" config option (default: True) - if the config option is set to False and the lock file is at least version 2.1, the installer will not re-resolve but evaluate locked markers locker: lock transitive marker and groups for each package (python-poetry#9427) fix: do not ignore local config for implicit PyPI source (python-poetry#9816) Cleanup, linting, typing (python-poetry#9839) # Conflicts: # src/poetry/installation/installer.py # src/poetry/puzzle/solver.py
reesehyde
added a commit
to reesehyde/poetry
that referenced
this pull request
Nov 18, 2024
* origin/main: Upgrade minimal Cleo to 2.2.1 make `allow-prereleases` a tri-state setting to really forbid pre-releases if the setting is `false` and keep default behavior to allow pre-releases only if necessary feat: add confirmation step feat(cli): add info messages about applied migration docs: add information about --migrate feat(cli): add support for --local for --migrate feat(cli): add --migration option to config command feat(config): add ConfigSourceMigration feat(config): provide method to remove empty config category feat(config): add get_property() to ConfigSource Ignore http credentials with empty usernames Fix regression when using empty username/password fix index error for yanked releases without dependencies ignore installed packages during solving vcs: use peeled ref when retrieving tag revision (python-poetry#9849) chore: update json fixtures and improve generate script so that it produces the same results on Linux and Windows installer: add option to install without re-resolving (just by evaluating locked markers) (python-poetry#9427) - introduce "installer.re-resolve" config option (default: True) - if the config option is set to False and the lock file is at least version 2.1, the installer will not re-resolve but evaluate locked markers locker: lock transitive marker and groups for each package (python-poetry#9427) fix: do not ignore local config for implicit PyPI source (python-poetry#9816) Cleanup, linting, typing (python-poetry#9839) # Conflicts: # src/poetry/installation/installer.py # src/poetry/puzzle/solver.py
|
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. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Pull Request Check List
Requires: #9345
Requires: python-poetry/poetry-core#733
First commit
Write information about the resulting groups that require a locked package and the resulting marker to the lock file. (Information is only written, not used yet.)
Second commit
Introduce a new config option
installer.re-resolve(defaulttrue); when set tofalsethe marker information from the lock file is used, there is no re-resolving anymore when installing from the lock file.Motivation
What triggered me was the discussion at https://discuss.python.org/t/lock-files-again-but-this-time-w-sdists and especially https://github.com/brettcannon/peps/blob/lock-file/peps/pep-9999.rst, which might become a draft for a general lock file. To support this lock file format, we have to lock the resulting markers for packages so that re-resolving is not required for installation.
Further
poetry-plugin-exportand get rid ofdependency walk failedissues.make use of locked markers in lock file version 2.1 and thereby avoid
dependency walk failedpoetry-plugin-export#286PoC: automatically create a
pinnedextra with dependencies pinned from the lock file #9428Performance
We have to distinguish between locking and installing.
Locking
There is an additional step to calculate the resulting markers so this change can only make things slower. However, I could not measure a significant increase in time required for locking for several projects. I suppose that's because what we are doing in the additional step are mostly marker operations, which have probably been cached during dependency resolution before.
Installing
Evaluating markers instead of re-resolving could be faster or slower. However, I also could not measure a significant effect here. I suppose determining the exact packages to install is just not a bottleneck compared to the actual installation. Even when all packages were already installed (and installation is a noop), I could not measure a difference. However, there is a side effect:
Side effect on installation process
With re-resolving we are installing batches of packages in parallel. The batches are determined by the depth in the dependency graph (same depth -> same batch). Since we do not lock the depth, without re-resolving all packages are in the same batch.
On the one hand, this can have a positive effect on performance because we do not have to wait for the last package of a batch.
(I could observe the complete installation of the dependencies of a largish project with all packages in the cache going down from 70s to 60s.)
On the other hand, this could have a negative effect on robustness (due to race conditions). I suppose, creating batches of packages with the same depth results in only installing packages in parallel that are (more or less) independent of each other.
(If a package depends on another package, they will normally have a different depth.) Parallel installation might be an issue if two packages write to the same directory (name space packages) and try to create it at the same time.
Conclusion
Locking the resulting groups and markers opens up some new possibilities, especially for
poetry-plugin-export. Installation without re-resolving is not a game changer on its own. Only the side effect will probably decrease the time for installation in some cases.For now, I decided to make installation without re-resolving opt-in. We can also make it opt-out if we think it is safe enough and want to get more feedback.