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

Fix locking for nested dependencies (fixes #3115) #3125

Closed
wants to merge 2 commits into from
Closed

Fix locking for nested dependencies (fixes #3115) #3125

wants to merge 2 commits into from

Conversation

zyv
Copy link
Contributor

@zyv zyv commented Oct 7, 2020

Pull Request Check List

Resolves: moneymeets/python-poetry-buildpack#13, #3115

Currently the type of nested dependencies is lost, because they come from pkg.requires as an abstract class, and therefore requirements.txt exporter wrongly writes version out instead of URL for nested git dependencies (see #3115). This is my attempt to fix it...

@zyv
Copy link
Contributor Author

zyv commented Oct 7, 2020

@abn does this make any sense to you? Sorry, have no idea of Poetry internals, but somehow the problem is that extended information about nested dependencies gets lost...

@abn
Copy link
Member

abn commented Oct 7, 2020

I need to check when I'm on a machine. But iirc, you might end up with the wrong candidate. I'll get back to you soon.

@zyv
Copy link
Contributor Author

zyv commented Oct 7, 2020

Thanks for the feedback! Yeah, this is exactly the type of help I need from someone who knows something about Poetry :)

I though of using __get_locked_package function instead of dict-access, but then I saw that it is used elsewhere only if pinning is enabled... and it's not clear to my whether this is the right way to restore type information about the package anyways (and why it gets lost in the first place).

@abn
Copy link
Member

abn commented Oct 7, 2020

@zyv did not get to follow it through fully, but I do thnk the issue is in how the Factory class loads dependencies into poetry.package.requires. So, the issue is actually in core.

Also, feel free to ping me on the discord server. Happy to help walk through the issue.

@zyv
Copy link
Contributor Author

zyv commented Oct 7, 2020

@abn I'd love to be of more help, but I'm really really useless after 12-hour workdays... would really appreciate if you guys could fix it properly or accept my band-aid. I could ask colleagues tomorrow to try to write a proper test based on our repro from #3115, so that it doesn't reoccur and submit a PR if that would be useful for you:

[tool.poetry]
name = "foobar"
version = "0.1.0"
description = ""
authors = ["Your Name <your.name@example.com>"]

[tool.poetry.dependencies]
python = "3.8.3"
sampleproject-mm = { git = "https://github.com/moneymeets/sampleproject.git", rev = "83068b995c2f94b0d51333b932b1a939084e9ef7" }

[build-system]
requires = ["poetry-core>=1.0.0"]
build-backend = "poetry.core.masonry.api"

@abn
Copy link
Member

abn commented Oct 7, 2020

@zyv I managed to dig into it a little bit more, I added a commit to #3119 to fix the issue. The problem was how the lock file stored nested dependencies.

@abn
Copy link
Member

abn commented Oct 7, 2020

I have reused a version of your fix for now, but I suspect the lack of nested metadata might end up being an issue for other edge cases as well eventually.

@zyv
Copy link
Contributor Author

zyv commented Oct 14, 2020

Somehow GitHub didn't close this automatically with #3119, so closing this manually now.

@zyv zyv closed this Oct 14, 2020
@zyv
Copy link
Contributor Author

zyv commented Oct 14, 2020

@abn could you please be so kind as to add "hacktoberfest-accepted" label as it was subsumed in #3119 :) ? Thank you very much for you work on this fix!

@abn
Copy link
Member

abn commented Oct 14, 2020

@zyv GitHub did nto close this because the change is not yet in master. I need to port that up, will do that shortly. :)

Copy link

github-actions bot commented Mar 1, 2024

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 Mar 1, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Error after Poetry v1.1.0 update as default version
2 participants