adafruit / Adafruit_Blinka

Add CircuitPython hardware API and libraries to MicroPython & CPython devices
https://learn.adafruit.com/circuitpython-on-raspberrypi-linux
MIT License
453 stars 340 forks source link

pip try to install sysv-ipc on Windows #858

Closed GuillaumeBeaudin closed 3 months ago

GuillaumeBeaudin commented 3 months ago

Board Name

FT232H

Steps

  1. Try to install adafruit-blinka with pip in a virtual envitonment : python -m pip install adafruit-blinka

Description

When I do that on Windows, I get the following error :

Building wheels for collected packages: sysv-ipc
  Building wheel for sysv-ipc (setup.py) ... error
  error: subprocess-exited-with-error

  × python setup.py bdist_wheel did not run successfully.
  │ exit code: 1
  ╰─> [18 lines of output]
      ******************************************************************************
      * Setup can't determine the value of PAGE_SIZE on your system, so it will
      * default to 4096 which may not be correct.
      *
      * Please report this message and your operating system info to the package
      * maintainer listed in the README file.
      ******************************************************************************
      running bdist_wheel
      running build
      running build_ext
      building 'sysv_ipc' extension
      creating build
      creating build\temp.win-amd64-cpython-312
      creating build\temp.win-amd64-cpython-312\Release
      "C:\Program Files (x86)\Microsoft Visual Studio\2019\Community\VC\Tools\MSVC\14.26.28801\bin\HostX86\x64\cl.exe" /c /nologo /O2 /W3 /GL /DNDEBUG /MD -IF:\Python\MyFT232H\venv\include -IC:\Users\guill\anaconda3\envs\py312_lite\include -IC:\Users\guill\anaconda3\envs\py312_lite\Include "-IC:\Program Files (x86)\Microsoft Visual Studio\2019\Community\VC\Tools\MSVC\14.26.28801\include" "-IC:\Program Files (x86)\Windows Kits\10\include\10.0.18362.0\ucrt" "-IC:\Program Files (x86)\Windows Kits\10\include\10.0.18362.0\shared" "-IC:\Program Files (x86)\Windows Kits\10\include\10.0.18362.0\um" "-IC:\Program Files (x86)\Windows Kits\10\include\10.0.18362.0\winrt" "-IC:\Program Files (x86)\Windows Kits\10\include\10.0.18362.0\cppwinrt" /Tccommon.c /Fobuild\temp.win-amd64-cpython-312\Release\common.obj
      common.c
      C:\Users\guill\AppData\Local\Temp\pip-install-j1jh955c\sysv-ipc_e1374884c2ad43d8a43e7cc4e7e0ff66\common.h(7): fatal error C1083: Cannot open include file: 'sys/ipc.h': No such file or directory
      error: command 'C:\\Program Files (x86)\\Microsoft Visual Studio\\2019\\Community\\VC\\Tools\\MSVC\\14.26.28801\\bin\\HostX86\\x64\\cl.exe' failed with exit code 2
      [end of output]

  note: This error originates from a subprocess, and is likely not a problem with pip.
  ERROR: Failed building wheel for sysv-ipc
  Running setup.py clean for sysv-ipc
Failed to build sysv-ipc
ERROR: Could not build wheels for sysv-ipc, which is required to install pyproject.toml-based projects

Additional information

The version I'm trying to install is 8.44. This problem is not present for 8.43.

makermelissa commented 3 months ago

Thanks for letting us know, likely this is caused by #856, and it needs a bit of adjusting. I'll work on a fix right away.

2bndy5 commented 3 months ago

I just came accross this too. Researching, I found https://github.com/adafruit/Adafruit_Blinka/blob/a1329a84de429cf75375904c003e40df224d507e/setup.py#L97-L106 where platform_reqs is defined beforehand https://github.com/adafruit/Adafruit_Blinka/blob/a1329a84de429cf75375904c003e40df224d507e/setup.py#L26 and then conditionally altered https://github.com/adafruit/Adafruit_Blinka/blob/a1329a84de429cf75375904c003e40df224d507e/setup.py#L49-L50

git blames https://github.com/adafruit/Adafruit_Blinka/commit/903c5ea5db4b4cc9d55561bc4518dba098cec4e8 as the latest relevant change.

I'm not sure what the exact problem is here. Technically, the condition guarding the dependence on sys-ipc should not be satisfied on Windows

Python 3.12.4 (tags/v3.12.4:8e8a4ba, Jun  6 2024, 19:30:16) [MSC v.1940 64 bit (AMD64)] on win32
Type "help", "copyright", "credits" or "license" for more information.
>>> import sys
>>> sys.platform
'win32'
>>> import platform
>>> platform.machine
<function machine at 0x0000023AAE8CB9C0>
>>> platform.machine()
'AMD64'
>>> sys.platform == 'linux' and platform.machine != 'mips'
False

Above you'll observe that platform.machine is a function object, not a static string. So, platform.machine != 'mips' is technically true. But, sys.platform == 'linux' is false.

makermelissa commented 3 months ago

Ah, ok. Yeah, the machine one should probably be updated. However, still strange behavior.

2bndy5 commented 3 months ago

Strange indeed. Just for clarity, I came here because pip names this lib as the reason for installing sys-ipc:

Collecting sysv-ipc>=1.1.0 (from Adafruit-Blinka->circuitpython-nrf24l01==2.1.3.post1.dev16)

makermelissa commented 3 months ago

I fixed the script locally on a windows machine with the platform.machine() fix and it worked. Creating a PR now.

2bndy5 commented 3 months ago

I just confirmed that your PR works on Windows using

pip install git+https://github.com/makermelissa/Adafruit_Blinka#main

Thanks!

makermelissa commented 3 months ago

Cool. However, pip install adafruit-blinka is still not working on my system, so I'm going to try and continue tracking down the issue.

2bndy5 commented 3 months ago

FYI, I used a fresh venv. Not sure if that helps.

makermelissa commented 3 months ago

I might give that a try. I was installing it without a venv.

2bndy5 commented 3 months ago

Damn, same here. pip cache purge didn't help; v8.44.1 still fails to install.

2bndy5 commented 3 months ago

maybe switch to platform.system()?

>>> platform.system()
'Windows'

on WSL ubuntu:

>>> platform.system()
'Linux'

Underneath, platform.system() uses platform.uname().system.

2bndy5 commented 3 months ago

I think I have a more understandable flaw. You're using ubuntu-latest in CI to build the bdist. This is understandable since the workflow uses bash's sed to replace version 0.0.0-auto placeholders with the actual tagged version. But sys.platform will always resolve to `linux' if your CI runner is a Linux OS.

therefore, us windows users (for v8.44.0 and v8.44.1) have to install without using the bdist:

pip install --no-binary adafruit-blinka adafruit-blinka

Installing collected packages: adafruit-blinka Successfully installed adafruit-blinka-8.44.1

makermelissa commented 3 months ago

Huh, ok. That kind of makes sense. Well, I may just adjust it to check cpu then. It's only needed for Raspberry Pi and amlogic a311d.

2bndy5 commented 3 months ago

My lazy gut instinct thinks: For the meantime, just upload the sdist and rely on piwheels to provide the bdist for RPi OS.

It would be nicer if the bdist could be augmented to include the platform tag instead of presuming a universal tag. Maybe cibuildwheel could do that for you, IDK.

2bndy5 commented 3 months ago

all we really need on windows is a Dependency specifier (see grammar)

sys-ipc>=1.1.0;sys_platform=='linux'
makermelissa commented 3 months ago

all we really need on windows is a Dependency specifier (see grammar)

sys-ipc>=1.1.0;sys_platform=='linux'

Yeah, that would have worked too with the exception of it possibly failing on the mips board. However, using the CPUs for the only boards that need it worked as well. :)

2bndy5 commented 3 months ago

IHMO, I still think uploading a universal bdist with so many machine-specific constraints is a flawed approach.

makermelissa commented 3 months ago

Fair enough. I'll update it so it's using the same string as in the requirements.txt from before I changed it.

2bndy5 commented 3 months ago

Ci just needs to

python -m build -s

instead of using pypa/build's default behavior

makermelissa commented 3 months ago

Ci just needs to

python -m build -s

instead of using pypa/build's default behavior

What effect will this change have?

2bndy5 commented 3 months ago

python -m build does 2 things (by default with no args):

  1. create a sdist (.tar.gz) from the project's current state
  2. create a bdist (.whl) from the sdist created in step 1. This is what you want to skip. Note, this does not actually use the project source. Rather this step is akin to
    python -m pip wheel -w dist --no-deps dist/adafruit-blinka-<major>.<minor>.<patch>.tar.gz

python -m build -s will only perform step 1 and skip step 2. More detail https://build.pypa.io/en/stable/ (or in python -m build -h)

2bndy5 commented 3 months ago

Python's build back-ends simply aren't made to support creating a platform-specific bdist from a pure python source. There are hacks out there, but there's no guarantee that some new PEP implementation will not break the build process.

I understand the itch to upload bdists because they install faster, but that's why the piwheels project exists. Piwheels also helps with creating 32-bit bdists for c-extensions because we can't use a docker image to reliably create a 32-bit bdist for armv6l platforms. For aarch64, I have to build the bdist (with cibuildwheels) using a qemu VM in CI.

makermelissa commented 3 months ago

It makes sense for Blinka in particular (and possibly PlatformDetect). Want to submit a PR?

2bndy5 commented 3 months ago

Sure. I could have a look at PlatformDetect as well.

2bndy5 commented 3 months ago

PlatformDetect doesn't have any install deps (requirements.txt dynamically loaded by build backend from pyproject.toml). Unless I missed something, it looks like all platform-specific code is used at runtime. So, a universal bdist makes sense for that lib.

makermelissa commented 3 months ago

Thanks, makes sense.