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

A discrepancy in sys_tags handling causes poetry install to fail on macos #7161

Closed
4 tasks done
vxgmichel opened this issue Dec 9, 2022 · 14 comments
Closed
4 tasks done
Labels
kind/bug Something isn't working as expected status/triage This issue needs to be triaged

Comments

@vxgmichel
Copy link

vxgmichel commented Dec 9, 2022

  • Poetry version: 1.2.2
  • Python version: 3.9
  • OS version and name: macos-12
  • pyproject.toml:
[tool.poetry]
name = "macos-ci-testing"
version = "0.1.0"
description = "Just testing"
authors = []

[tool.poetry.dependencies]
python = "~3.9.0"
pyobjc-framework-SystemConfiguration = "^8"

[build-system]
requires = ["poetry-core"]
build-backend = "poetry.core.masonry.api"
  • I am on the latest stable Poetry version, installed using a recommended method.
  • I have searched the issues of this repo and believe that this is not a duplicate.
  • I have consulted the FAQ and blog for any relevant entries or release notes.
  • If an exception occurs when executing a command, I executed it again in debug mode (-vvv option) and have included the output below.

Issue

After an update in the macos-12 images from github actions, our CI job on macos started to fail during poetry install with the following error (fulltrace here):

...
ERROR: pyobjc_framework_SystemConfiguration-8.5.1-cp36-abi3-macosx_11_0_universal2.whl is not a supported wheel on this platform.
...

I managed to reproduce the error in a new repository with minimal configuration:
https://github.com/vxgmichel/macos-ci-testing

The workflow is really simple too:

  test-macos:
    name: macOS
    runs-on: macos-12
    steps:
      - uses: actions/checkout@v3
      - uses: actions/setup-python@v4
        with:
          python-version: "3.9"
      - name: Install poetry
        run: pip3 install poetry packaging
      - name: Run debug script
        run: python debug.py
      - name: Install project
        run: poetry install -vvv

After a quick investigation, I noticed something weird: packaging.tags.sys_tags and poetry.core._vendor.packaging.tags.sys_tags do not provide the same tag list.

In particular, this debug.py script yields:

Looking for 'cp36-abi3-macosx_11_0_universal2':
- in `sys_tags` provided by packaging: True
- in `sys_tags` provided by poetry: False

The full difference can be found here.

I suspect that different implementations of sys_tags are used to:

  • first, choose which wheel to download
  • then, when installing, check that the wheel is supported

This is as far as I can get with my limited knowledge of poetry, and I might be missing something else.

@vxgmichel vxgmichel added kind/bug Something isn't working as expected status/triage This issue needs to be triaged labels Dec 9, 2022
@vxgmichel
Copy link
Author

Also, any suggestion or workaround would be very helpful :)

@dimbleby
Copy link
Contributor

dimbleby commented Dec 9, 2022

poetry-core vendors packaging 21.3 which, up until two days ago, was the latest available version. What version do you have un-vendored?

Suggest that you make that unvendored version also be 21.3. If that requires you to upgrade then this probably isn't a very interesting report, if that requires you to downgrade then probably poetry has something to worry about.

@vxgmichel
Copy link
Author

What version do you have un-vendored?

The packaging version I installed for the debug.py script is 22.0 (released 2 days ago), which explains the differences found by the script as 22.0 includes the PR #513 that affects the macos platform tags.

if that requires you to downgrade then probably poetry has something to worry about.

I wouldn't even know what to downgrade exactly. The unvendored version of packaging I used was only for debugging. As you can see from the test repository I set up, a simple poetry install ends up failing with the not a supported wheel error. This lead me to believe that poetry ends up using both a vendored (21.3) and an unvendored (22.0) version during its install process.

@touilleMan
Copy link

If that requires you to upgrade then this probably isn't a very interesting report

I disagree: the point of vendoring packaging is to use a specific version and avoid clash which 3rd parties dependencies, which is precisely what this issue demonstrates.

Regarding on the reason this issue is present, I think this is due to how poetry and poetry-core handle dependencies:

  • when doing a pip install poetry-core, only poetry-core is installed, and it contains a vendoring folder with all it dependencies (including packaging)
  • on the other hand when doing pip install poetry, poetry deals with dependencies the regular way (i.e. dependencies are listed in the wheel and installed by pip alongside poetry)

Hence we end up with two separate version of packaging (and all other dependencies that are common to poetry and poetry-core): site-packages/poetry/core/_vendor/packaging/ (only used by code in site-packages/poetry/core/) and site-packages/packaging (used by everything else)

In theory poetry-core's vendored dependencies have precedence:
https://github.com/python-poetry/poetry-core/blob/c42f5a22f8ce7868331683eb5bbf0b95b00d80d5/src/poetry/core/__init__.py#L14-L15
But this is only once poetry-core has been loaded, so I guess it's possible for poetry to first load packaging, then poetry-core.

I see multiple solutions to solve this issue:

  1. Don't do vendoring in poetry-core when it is installed by poetry
  2. Do vendoring in poetry (hence poetry-core is included among the other dependencies and don't do itself a vendoring)
  3. Add at the very beginning of poetry/__init__.py a patch on sys.path to ensure poetry-core's vendored always have priority[1]
  4. Add at the very beginning of poetry/__init__.py a patch to disable the change poetry-core does on sys.path when it is loaded. This way poetry-core's vendored are always ignored[2]

Approaches 3&4 seems fragile to me (weird things can happen if poetry is not the very first module to be loaded...) and require to do some manual work to handle dependency (e.g. if poetry-core and poetry have different minimal version requirement on the same dependency)

PS: according to it changelog, packaging 22.0 contains a fix on macOS platform tags with old macOS SDK (pypa/packaging#513), that likely related to the current issue

@dimbleby
Copy link
Contributor

dimbleby commented Dec 10, 2022

the point of vendoring packaging is to use a specific version and avoid clash ...

this is not the reason that poetry-core vendors packaging. Indeed, as this issue demonstrates, that goal would be better achieved by not vendoring!

The reason that I would have viewed needing to upgrade as relatively uninteresting is that it's something that users could easily do, and that new installs would get automatically.

However since the actual problem is that poetry is now picking up a too-new version of packaging, it seems clear that Something Should Be Done.

While I agree there are tricks that could be played with paths etc, I suspect that the easiest thing to do is simply to upgrade the vendored version in poetry-core, and the required version in poetry, so that they both have packaging 22.0.

It's true that leaves poetry exposed to similar happening again one day and perhaps it will, but clearly this sort of thing doesn't happen very often; and upgrades are probably a good thing anyway.

@vxgmichel
Copy link
Author

it seems clear that Something Should Be Done.

In the meantime, a possible workaround for affected users is setting the SYSTEM_VERSION_COMPAT env var to 0.

env:
  SYSTEM_VERSION_COMPAT: "0"

vxgmichel/macos-ci-testing@5c70719

@dimbleby
Copy link
Contributor

Looking more closely at your CI failure, the discrepancy between poetry-core and poetry isn't the problem at all. Rather, it's the discrepancy between poetry and pip. Poetry is calling out to pip to install the package, and it is pip that is reporting that the wheel is not supported.

That'll be because pip has not yet picked up the new version of packaging.

So you'll want to encourage, or wait for, pip to update.

@vxgmichel
Copy link
Author

So you'll want to encourage, or wait for, pip to update.

So is that a wontfix?

@touilleMan
Copy link

touilleMan commented Dec 12, 2022

@dimbleby thanks for your feedback ;-)

Rather, it's the discrepancy between poetry and pip. Poetry is calling out to pip to install the package, and it is pip that is reporting that the wheel is not supported.

You're right about that, pip does it own vendoring of packaging (currently in version 21.3)
On the other hand, poetry's packaging requires >=20.4, hence installing it with pip/pipx result in having packaging 22.0 😢

Regarding the "Something Should Be Done" you mentioned 😄 , I agree with you that updating the minimal version in poetry/poetry-core requirements to < 22.0 would be a good fix for that issue (and later on remove this need once pip has updated it own vendoring) and, while not a perfect fix, should be "good enough" given this kind of issue shouldn't be too frequent.

Do you want a PR on that ?

@neersighted
Copy link
Member

That is an insufficient work around as we will just see it manifest in reverse with the next pip release. I have discussed another approach with a pip maintainer I will try to prepare today.

@dlech
Copy link

dlech commented Feb 18, 2023

Could Poetry used the vendored packaging from pip instead of vendoring it too or depending on an arbitrary version depedency? This way poetry and pip are guaranteed to always be using the same packaging package.

@neersighted
Copy link
Member

Poetry uses the pip installed in the target environment, which may not actually be compatible with Poetry's own code. We intend to resolve this in 1.4 by moving to the installer package instead of depending on pip. We won't fully be free of our use of pip, but it will mean that we won't be dependent on aligning with pip's interpretation of wheel tags.

@neersighted
Copy link
Member

Closing this as we merged #6205.

Copy link

This issue 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
kind/bug Something isn't working as expected status/triage This issue needs to be triaged
Projects
None yet
Development

No branches or pull requests

5 participants