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

Make core aware of poetry lock files #83

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

Conversation

abn
Copy link
Member

@abn abn commented Sep 22, 2020

Lock file parsing logic and should be implemented by poetry-core. In many cases downstream consumers need to depend on poetry in order parse information, get packages etc from the lock file. This change proposes to move basic implementation to core such that poetry only needs to implements specifics such as Locker.locked_packages() which now will become return Repository(packages=self.get_packages(categories=["main"]).

This change also now ensures poetry.lock is included the package sdist.

@abn abn requested a review from a team September 22, 2020 15:10
@abn
Copy link
Member Author

abn commented Sep 22, 2020

The build failures are because of the last poetry prerelease not working on windows/2.7.

poetry/core/poetry.py Outdated Show resolved Hide resolved
@johnthagen
Copy link
Contributor

Thank you for working on this. The missing lock file support is currently a blocker to better tox integration (enpaul/tox-poetry-installer#2, sinoroc/tox-poetry-dev-dependencies#45), so getting this in would be a big step forward for many users who want to use tox and poetry together.

@abn
Copy link
Member Author

abn commented Oct 23, 2020

Appreciate the input. I would really appreciate wider testing of this to ensure it covers required scenarios. So, if anyone want to define various complex scenarios to test even just manually, that would help the confidence in the implementation.

Note that the lock parsing logic has undergone significant bug fixes in [email protected] some real interesting scenarios came up.

I am also considering a re-write of how lock data is loaded, so it's more a graph in memory.

Additionally working on the bug fixes, I realised that in order for the lock file to be truly independent the 508 information from pyproject for top level dependencies will also need to be preserved in the lock file.

@johnthagen
Copy link
Contributor

johnthagen commented Oct 23, 2020

@abn A couple of edge cases I hit when trying some of the tox plugins. Also, note that Poetry 1.1 changed how the lock file works, so in some cases a plugin worked on <1.1, but doesn't work on >1.1. I will say that in my opinion making it compatible with newer poetry is most important as that is what most users will be using.

Markers in sub-dependencies (one current plugin has issues dealing with this, so we need them parsed out properly in poetry-core):

[[package]]
name = "ipython"
version = "7.18.1"
description = "IPython: Productive Interactive Computing"
category = "main"
optional = false
python-versions = ">=3.7"

[package.dependencies]
appnope = {version = "*", markers = "sys_platform == \"darwin\""}
backcall = "*"
colorama = {version = "*", markers = "sys_platform == \"win32\""}
decorator = "*"
jedi = ">=0.10"
pexpect = {version = ">4.3", markers = "sys_platform != \"win32\""}
pickleshare = "*"
prompt-toolkit = ">=2.0.0,<3.0.0 || >3.0.0,<3.0.1 || >3.0.1,<3.1.0"
pygments = "*"
traitlets = ">=4.2"

URL dependencies:

[tool.poetry.dependencies]
en-core-web-sm = {url = "https://github.com/explosion/spacy-models/releases/download/en_core_web_sm-2.3.0/en_core_web_sm-2.3.0.tar.gz"}

And Git dependencies, for example:

$ poetry add git+https://github.com/sdispater/pendulum.git#develop

@cognifloyd
Copy link

Pants (pantsbuild/pants#10655 (comment)) recently merged a custom way to read poetry's metadata. The lack of lock file support in poetry-core was cited as a reason to not reuse poetry-core. I would have preferred they reuse poetry-core, but oh well.

What's the next step to get this merged?

@abn
Copy link
Member Author

abn commented Oct 24, 2021

Sorry for letting this become stale, next step is for me to get this rebased and reworked with the recent changes. Additionally, once it is ready, I would definitely appreciate some addtional eyes on this.

@abn abn force-pushed the add-locker branch 2 times, most recently from 48ea8ac to 95da74c Compare March 9, 2022 23:48
@abn
Copy link
Member Author

abn commented Mar 9, 2022

Rebased this. While the code works, and is an exact port of current poetry@master, the functionality introduced via groups is a bit ambiguous at present.

@sonarcloud
Copy link

sonarcloud bot commented Mar 9, 2022

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 5 Code Smells

No Coverage information No Coverage information
1.5% 1.5% Duplication

@abn abn mentioned this pull request Mar 23, 2022
2 tasks
@riconnon
Copy link

riconnon commented Apr 9, 2022

@abn this currently represents the primary reason I'm stuck installing poetry rather than poetry-core for my container builds, because I want to install a consistent set of things based on the lock file.
I'm very keen to see this functionality included in poetry-core if possible so let me know if there's anything that can be done to help move this along.

@riconnon
Copy link

In case it's beneficial, I have rebased this PR on my branch here: https://github.com/riconnon/poetry-core/tree/add-locker

@abn
Copy link
Member Author

abn commented May 25, 2022

Thanks @riconnon I will see what the other's on the core team think about this in general.

@riconnon
Copy link

@abn did you get a chance to talk to the rest of the core team? This remains my biggest poetry bugbear so I'm keen to work to a solution if possible.
Let me know if there's anything I can do to help.

@rbuckland
Copy link

@abn - do you have an update for this ticket ?

@noamraph
Copy link

@abn @sdispater can you say what's missing here, if anything? I just heard of pipx run, and I want to write utilities that would allow me, and others in my company, to just pipx run them. For this, I obviously want users to use the dependencies in the lock file, so my utility will run in the most repeatable way. However, I understand that this is waiting for this PR, from 3 years ago, and specifically, to your feedback. Can you help in saying what needs to be done?

Thanks!
Noam

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.

7 participants