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

Bugfix: Empty and Any markers saved to lockfile #1650

Merged
merged 3 commits into from Dec 6, 2019

Conversation

@JBKahn
Copy link
Contributor

JBKahn commented Nov 28, 2019

When locking some private packages of ours (that have extras) it used to copy those extras over in 1.0.0b3 when we changed that behaviour it's now the Any marker and when it stringifies that when generating the lockfile it creates a format the lockfile doesn't support.

bugfix: Originally, the MarkerUnion didn't correctly respond to is_any or is_empty because it didn't evaluate the collection of markers which led to it trying to stringify those markers.

This on conjunction with the other fix means that the [Any, Any] case is simply excluded from the lockfile just like a single Any case.

bugfix
Before:
' or '.join([Any, Any]) -> ' or '
' or '.join([Any, REAL MARKER]) -> ' or REAL MARKER'
' or '.join([Empty, Empty]) -> '<empty> or <empty>'

Now
' or '.join([Any, Any]) -> ''
' or '.join([Any, REAL MARKER]) -> 'REAL MARKER'
' or '.join([Empty, Empty]) -> ''

This means if we have a mix of markers with any/empty that won't be propagated to the lockfile.

@JBKahn JBKahn changed the title do not write empty or any markers Bugfix: Empty and Any markers saved to lockfile Nov 28, 2019
@JBKahn JBKahn force-pushed the JBKahn:patch-1 branch from 998f2c2 to 1692495 Nov 28, 2019
@JBKahn JBKahn force-pushed the JBKahn:patch-1 branch from 1692495 to 0e7e820 Nov 28, 2019
@JBKahn

This comment has been minimized.

Copy link
Contributor Author

JBKahn commented Dec 3, 2019

@sdispater thoughts?

@JBKahn

This comment has been minimized.

Copy link
Contributor Author

JBKahn commented Dec 6, 2019

@finswimmer I see you're also able to commit on this repo, any thoughts?

poetry/version/markers.py Outdated Show resolved Hide resolved
@JBKahn

This comment has been minimized.

Copy link
Contributor Author

JBKahn commented Dec 6, 2019

Same for empty?

@JBKahn JBKahn closed this Dec 6, 2019
@JBKahn JBKahn reopened this Dec 6, 2019
Co-Authored-By: Sébastien Eustace <sebastien.eustace@gmail.com>
@sdispater

This comment has been minimized.

Copy link
Member

sdispater commented Dec 6, 2019

@JBKahn No, it's fine for is_empty() since all markers must be empty for the union to be empty as well.

@JBKahn

This comment has been minimized.

Copy link
Contributor Author

JBKahn commented Dec 6, 2019

K, then I think with that test change, this should be good for the next release?

@JBKahn

This comment has been minimized.

Copy link
Contributor Author

JBKahn commented Dec 6, 2019

@sdispater on a side note, I'm not super familiar with the code yet but if you're looking for help maintaining poetry, I'm happy to give some time to the project.

Copy link
Member

sdispater left a comment

Thanks a lot!

@sdispater sdispater merged commit b04faad into python-poetry:master Dec 6, 2019
16 checks passed
16 checks passed
Linting
Details
Linux (2.7)
Details
Linux (3.5)
Details
Linux (3.6)
Details
Linux (3.7)
Details
Linux (3.8)
Details
MacOS (2.7)
Details
MacOS (3.5)
Details
MacOS (3.6)
Details
MacOS (3.7)
Details
MacOS (3.8)
Details
Windows (2.7)
Details
Windows (3.5)
Details
Windows (3.6)
Details
Windows (3.7)
Details
Windows (3.8)
Details
@JBKahn JBKahn deleted the JBKahn:patch-1 branch Dec 6, 2019
shenek added a commit to shenek/poetry that referenced this pull request Dec 31, 2019
* do not write empty or any markers

* Update poetry/version/markers.py

Co-Authored-By: Sébastien Eustace <sebastien.eustace@gmail.com>

* Update test_markers.py
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.