Fix dropped markers on dep walk

[johnmacnamararseg]

Pull Request Check List

Resolves: #3511

• Added tests for changed code.

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 the key in 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

[abn]

@johnmacnamararseg thansk for the fix. The cyclic fix was a bit rushed, appreciate the clean up.

[johnmacnamararseg]

@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
i.e.
what should be this

    bar==7.8.9
    baz==10.11.13; platform_system == "Windows"
    foo==1.2.3

is now this

    bar==7.8.9; platform_system != "Windows" or platform_system == "Windows"
    baz==10.11.13; platform_system == "Windows"
    foo==1.2.3

Im gonna do a bit of debugging. hoping there's a small change I can push to satisfy everything

[johnmacnamararseg]

on second thought this bar==7.8.9; platform_system != "Windows" or platform_system == "Windows" isn't a huge issue since everything will install fine - just pretty confusing if you were to read it.

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

[johnmacnamararseg]

@abn any chance we can get these workflows approved and PR through?

[johnmacnamararseg]

@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

[abn]

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.

[johnmacnamararseg]

@abn thanks for approving the run. any word here on the review?

[johnmacnamararseg]

@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

[sonarcloud]

SonarCloud Quality Gate failed.

0 Bugs
0 Vulnerabilities
0 Security Hotspots
1 Code Smell

No Coverage information
3.4% Duplication

[dimbleby]

@abn is there anything the rest of us can do to help this get merged?

We also have a bunch of projects stuck on poetry 1.1.3 due to #3511, would love to see this longstanding MR at last make its way into a release...

[dimbleby]

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?

[johnmacnamararseg]

@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.

[neersighted]

This is closed in light of #4686.

[github-actions]

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.