Skip to content

Comments

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

Closed
zyv wants to merge 2 commits intopython-poetry:masterfrom
zyv:patch-2
Closed

Fix locking for nested dependencies (fixes #3115)#3125
zyv wants to merge 2 commits intopython-poetry:masterfrom
zyv:patch-2

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 <[email protected]>"]

[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. :)

@github-actions
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