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 Platform-Specific Wheel File Selection & Assign Markers (See #944 / #955) #1144

Closed
wants to merge 6 commits into from
Closed

Fix Platform-Specific Wheel File Selection & Assign Markers (See #944 / #955) #1144

wants to merge 6 commits into from

Conversation

KyleKing
Copy link
Contributor

@KyleKing KyleKing commented Jun 1, 2019

(22Aug2019)) Update: Based on the discussion in #955, I'll be implementing marker assignment based on the wheel filenames. See discussion starting at: #955 (comment)


This PR implements the same functionality as PR #955, but applies the changes to the sdispater/develop branch instead of sdispater/master

Please choose whichever PR should merged and the other can be closed


Resolves #944 & #807

Issue: If a package only has platform_specific_wheels, Poetry just downloads the first one, which is usually cp27 and MacOSX (i.e.: numpy)

Changes: This PR adds logic to determine the best wheel file to download for Poetry supported platforms. If the best wheel can't be identified, the method falls back on the original behavior to return the first platform wheel. This fixes the [OS Error] on Windows and improves matching for Linux and Mac.

Also, tested on a couple of different computers as a sanity check (Win7-64, Win10-64, and MacOSX Mojave)

Pull Request Check List

  • Added tests for changed code.
  • Updated documentation for changed code. {N/A - no changes necessary}

@Pluckerpluck
Copy link

Any update on this @sdispater? Poetry is flat out broken right now. How can I possibly support any system which has remained unusable for so long? I had such high hopes for poetry as well, but it looks like I may have to move back to pipenv.

Really, all I needed was dependency resolution. But maybe the answer is to ignore that, install manually, hope for the best and test a lot more... Then just lock those dependencies.

The other problem Windows is facing has also received no love. It's why the errors have been so unhelpful (this error masks other problems) and in a few unlucky cases again flat out breaks poetry on Windows.

@areeh
Copy link

areeh commented Jun 25, 2019

looks like this would fix most of our current build issues

@brycedrennan brycedrennan added the kind/feature Feature requests/implementations label Aug 17, 2019
# Conflicts:
#	poetry/repositories/pypi_repository.py
@KyleKing KyleKing changed the title Use Python Version and OS to Select the Appropriate Wheel (#944) {Against "develop" Branch} Fix Platform-Specific Wheel File Selection & Assign Markers (See #944 / #955) Aug 23, 2019
@KyleKing KyleKing changed the title Fix Platform-Specific Wheel File Selection & Assign Markers (See #944 / #955) {WIP} Fix Platform-Specific Wheel File Selection & Assign Markers (See #944 / #955) Aug 23, 2019
@KyleKing KyleKing changed the title {WIP} Fix Platform-Specific Wheel File Selection & Assign Markers (See #944 / #955) Fix Platform-Specific Wheel File Selection & Assign Markers (See #944 / #955) Sep 1, 2019
@KyleKing
Copy link
Contributor Author

KyleKing commented Sep 1, 2019

The PR is basically done, but I'm not going to push it further as this is no longer an issue now that poetry better picks the downloaded file. The current logic in poetry ^0.12.17 is sufficient

Copy link

github-actions bot commented Mar 1, 2024

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 Mar 1, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
kind/feature Feature requests/implementations
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants