Sort extras and extras dependencies in lockfile

[mkniewallner]

Pull Request Check List

Resolves: #6153

• Added tests for changed code.
• Updated documentation for changed code.

This PR sorts extras under [package.extras] in the lockfile, as well as their dependencies.

Note:

poetry/src/poetry/packages/locker.py

Lines 591 to 592 in ed88e34

	# TODO: This should use dep.to_pep_508() once this is fixed
	# https://github.com/python-poetry/poetry-core/pull/102

suggests to use to_pep_508 once python-poetry/poetry-core#102 is merged, but doing so, even with with_extras=False, would add environment markers, which I'm not sure is the desired behaviour, so I preferred to leave that untouched.
If it is the expected behaviour, I'd be happy to do the change in a follow-up PR.

[Kurt-von-Laven]

@radoering, would you consider accepting a pull request to cherry pick this fix to Poetry 1.1? The issue it fixes is presently breaking Dependabot for all users (in addition to all of our CI pipelines).

[radoering]

Not sure if that's enough for another 1.1 release. We will probably only do another 1.1 release if poetry breaks for the majority of users like the breaking warehouse change, which was the reason for 1.1.14.

Is it really breaking Dependabot or is it just adding noise to the PRs? Actually, I think some shuffling in addition to the real change is annoying but not a blocker.

[Kurt-von-Laven]

I get what you are saying about Dependabot continuing to function. The extra pull request noise isn't a major concern in and of itself. There is presently no single correct lock file though as the output of poetry lock --no-update is inconsistent, which complicates automatically checking the lock file in CI and will lead to a lot of time wasting merge conflicts on high velocity projects. As a practical consequence of there being no single correct lock file, folks will disable or refrain from implementing automated checking of lock files in CI. Without such checking, they will be relying on human reviewers to catch issues in a machine-generated file that is collapsed in pull requests by default on GitHub at least. This is a serious security concern for the Python ecosystem if left unaddressed for a protracted period of time as it will facilitate the creation of malicious pull requests that modify the lock file in ways that are not consistent with the pyproject.toml reviewed (and comprehended) by the human. I am saying all of this on the potentially erroneous assumption that a stable 1.2 release is at least a few months away.

[radoering]

@finswimmer Since #5834 proposes to do another 1.1 release: Do you think we should include this one as well if we really do another release? It seems like a quite riskless change and #6153 could be considered as an (externally triggered) regression. 🤷

[mkniewallner]

@finswimmer Since #5834 proposes to do another 1.1 release: Do you think we should include this one as well if we really do another release? It seems like a quite riskless change and #6153 could be considered as an (externally triggered) regression. shrug

+1 on backporting, if we are certain to do another 1.1 release, since this is pretty much harmless. I can prepare a backport PR just in case.

[dimbleby]

there are zillions of bug fixes that could be backported, this one isn't special, just release 1.2 already! Then no-one will need the backport.

[Kurt-von-Laven]

This is the only Poetry 1.1 bug I'm aware of that knocks over many CI pipelines in a way where the path-of-least-resistance workaround is to disable (or not implement) an important security check (namely that poetry lock --no-update doesn't modify poetry.lock). It also creates friction with Dependabot in a way that's cumbersome to deal with in an automated fashion. (#6201 is similar but at least has the simpler workaround of removing indentation from pyproject.toml that is more likely to be more widely adopted if necessary.) Even once 1.2 is stable, there will probably be teams that remain on 1.1 for one reason or another given that there is a 1,022 commit delta between 1.1 and master, which is 42% of the 2,460 commits presently on master. I don't mean this to say that releasing 1.2 shouldn't be a very high priority, but my impression is that the maintainers already strongly agree with us on that point.

[dimbleby]

Folk who remain on 1.1 are going to be out of support soon anyway, the question is whether this fix is important enough to divert maintainer bandwidth from the more important task of releasing 1.2.

IMO it's not, releasing 1.2 should be the sole priority right now, anyone who could be making that happen and is instead doing other poetry work is doing it wrong.

But if folk want to spend their time doing things that I consider less useful: well I'm not king of the world and can't stop them!

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