Fix dropped markers on dep walk#3512
Fix dropped markers on dep walk#3512johnmacnamararseg wants to merge 17 commits intopython-poetry:masterfrom
Conversation
…ons, similar to what is supported in the 'install' command
abn
left a comment
There was a problem hiding this comment.
@johnmacnamararseg thansk for the fix. The cyclic fix was a bit rushed, appreciate the clean up.
Co-authored-by: Arun Babu Neelicattu <[email protected]>
1b2a3e2 to
3bf48df
Compare
|
@abn just a heads up - went to update my branch (was pretty stale) and it looks like some changes made in PR 3660 (recently merged) is failing the test I added. At first glance it appears an intersection was changed to a union in that PR which is now causing double markers is now this Im gonna do a bit of debugging. hoping there's a small change I can push to satisfy everything |
|
on second thought this Might be easier to open a new issues to handle the consolidation of markers if we really need to ill push up the change to fix the test but let me know what you think @abn |
|
@abn any chance we can get these workflows approved and PR through? |
798eb4a to
9a9315e
Compare
|
@abn ok so my last push was blocked by the python 3.10 ci issue but it appears that stage has been removed. Can I get a ci approval on this? - looks like some more issues are attaching this PR also, I don't quite understand why sonarcloud is failing. It's complaining about duplicate code but its not on any files I have changed |
|
You can ignore the sonar one for now. I think it's because it did that scan without a baseline. I have approved the run for now. Will review later. |
|
@abn thanks for approving the run. any word here on the review? |
|
@abn can I get a review on this? it continues to be a blocker for many teams in my org to move to poetry>1.1.3 |
…-dropped-markers-on-dep-walk
|
SonarCloud Quality Gate failed.
|
|
This seemed to be so close to merging at 8 Jun, but since then @abn seems to have lost interest. (Which is ok! - I understand that I have no claim on volunteer time.) Any chance we can get another maintainer to look at this? @sdispater? |
|
@abn @finswimmer @Dispater any chance I can get a ci approval and review on this PR. It's been open and in good shape for quite a while (~8 months) and continues to be a blocker for my org moving on to newer versions of poetry. |
|
This is closed in light of #4686. |
|
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. |
Pull Request Check List
Resolves: #3511
It looks like the line
if key not in nested_dependencies:was added to solve the issue of circular dependencies but this introduced the issue at hand because thekeyin this routine is not a unique key when a package has a dependency with multiple markers. Under conditions outlined in issue #3511 , this can result in markers not correctly getting assigned to lower depth packages.I'd be happy to describe this issue and implementation choices in more detail if needed