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 complicated export case #5711

Closed

Conversation

dimbleby
Copy link
Contributor

Related to python-poetry/poetry-core#381 (on which, as currently written, it relies) and python-poetry/poetry-plugin-export#32.

I don't yet have a testcase proving the point for poetry-export-plugin, but will get there eventually...

@radoering
Copy link
Member

What I don't like is that there seems to be no usage and no test for get_project_dependency_packages() in this repo. (Of course, that's not your fault but a result of moving export to its own repo.) I think it might really help, if there was a unit test here. However, I don't know how much effort is required for that and it may be too much to ask. (Anyway, I don't see a unit test in this repo being a blocker for merging this.)

@dimbleby
Copy link
Contributor Author

dimbleby commented May 30, 2022

This is of course blocked on there being a poetry-core that provides constraint_regions(), happy to wait for that rather than temporarily re-implement it here.

I've made an additional fix along the way though, to tighten up the code that selects a locked package so that it never can choose overlaps

  • this code previously said "if I've already selected a version of this package whose markers overlap with the current markers, then I must make the same choice again"
  • but it now also says "if there is more than one previously selected version of this package whose markers overlap with the current markers, then I have gone wrong"
  • that is, it detects the case where we've already chosen package p1 with markers m1, p2 with markers m2 (not overlapping m1) and we encounter a requirement for p with markers that overlap both m1 and m2

This was referenced Jun 26, 2022
dimbleby and others added 3 commits July 13, 2022 08:54
By treating different python version ranges independently, we buy the
flexibility needed to make better decisions.
fail more visibly if we take a wrong turn
@dimbleby
Copy link
Contributor Author

releases of poetry-core makes this ready to merge; but it's hard to negotiate the trip hazards set up by the cross-repository testing in the workflows.

The workflow identifies that this is "breaking" the released version of the plugin; though it's really fixing it. python-poetry/poetry-plugin-export#68 is the corresponding UT change.

@dimbleby
Copy link
Contributor Author

Some notes on disentangling the cross-repository mess in #5980 (comment)

TLDR suggest moving all the remaining export-specific code over to the export plugin.

That would put the code and its tests in the same place, which seems much more sensible.

@dimbleby
Copy link
Contributor Author

This should be ready to merge, modulo "what do we do about cross-repository testing".

@radoering
Copy link
Member

Superseded by: python-poetry/poetry-plugin-export#94

@radoering radoering closed this Aug 7, 2022
@dimbleby dimbleby deleted the fix-complicated-export branch August 13, 2022 16:08
Copy link

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 Feb 29, 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.

2 participants