bdist wheels: exclude build-time-only files #192

T-256 · 2023-12-26T15:51:34Z

Last chunk of #176

sdist wheel now includes all required build-from-source files.
bdist wheels now excludes build-time-only files.

perklet · 2023-12-27T03:34:09Z

I don't think this is the direction where are heading for.

The declaritive pyproject.toml is the future of python packaging, so I would like to keep setup.py as minimal as possible.
For platforms which we do not support, the sdist wheel will be downloded when pip installing, it's important to keep the build files so that they can be compiled with locally installed curl-impersonate.

T-256 · 2023-12-27T11:08:07Z

The declaritive pyproject.toml is the future of python packaging, so I would like to keep setup.py as minimal as possible

Then I think we should move out build.py out of curl_cffi folder. (this setup.py patch is the only way to exclude when exclude target is python file in that package)

2. For platforms which we do not support, the sdist wheel will be downloded when pip installing, it's important to keep the build files so that they can be compiled with locally installed curl-impersonate.

Makes senses. though in PR description what I said is: This PR excludes build-time-only files/folders from wheels dist But it seems it still applies on sdist.
I will try to prevent this exclusion for sdist build later today.

perklet · 2024-01-23T02:42:58Z

scripts/build.py

+    """
+        #include "shim.h"
+    """,
+    libraries=["curl-impersonate-chrome"] if system != "Windows" else ["libcurl"],


I think we may miss some steps for building windows dll, the name, if correctly configured, should be libcurl-impersonate-chrome.dll

Is this miss-configuration from curl-impersonate package?

Yes, curl-impersonate, possibly missing autoreconf on Windows.

Then, IIUC it's not blocker for merge this PR.

bjia56 · 2024-01-23T15:15:40Z

Testing again on 32-bit raspbian (armv7l):

pi@raspberrypi3:~ $ pip install /tmp/curl_cffi-0.6.0b8.tar.gz
Looking in indexes: https://pypi.org/simple, https://www.piwheels.org/simple
Processing /tmp/curl_cffi-0.6.0b8.tar.gz
  Installing build dependencies ... done
  Getting requirements to build wheel ... done
    Preparing wheel metadata ... done
Collecting cffi>=1.12.0
  Using cached https://www.piwheels.org/simple/cffi/cffi-1.16.0-cp39-cp39-linux_armv7l.whl (367 kB)
Requirement already satisfied: certifi in /usr/lib/python3/dist-packages (from curl_cffi==0.6.0b8) (2020.6.20)
Collecting pycparser
  Using cached https://www.piwheels.org/simple/pycparser/pycparser-2.21-py2.py3-none-any.whl (119 kB)
Building wheels for collected packages: curl-cffi
  Building wheel for curl-cffi (PEP 517) ... error
  ERROR: Command errored out with exit status 1:
   command: /usr/bin/python3 /tmp/tmpj8kc28s0_in_process.py build_wheel /tmp/tmpas0s8c7j
       cwd: /tmp/pip-req-build-gwupegko
  Complete output (5 lines):
  running bdist_wheel
  running build
  Downloading libcurl-impersonate-chrome from https://github.com/yifeikong/curl-impersonate/releases/download/v0.6.0b8/libcurl-impersonate-v0.6.0b8.arm-linux-gnu.tar.gz...
  Unpacking downloaded files...
  error: [Errno 13] Permission denied: '/usr/local/lib/libcurl-impersonate-chrome.a'
  ----------------------------------------
  ERROR: Failed building wheel for curl-cffi
Failed to build curl-cffi
ERROR: Could not build wheels for curl-cffi which use PEP 517 and cannot be installed directly

Not sure how important it is to allow the curl libs to be installed without root, though.

perklet · 2024-01-23T15:17:02Z

I think we need to use patchelf on the curl-impersonate side

bjia56 · 2024-01-23T15:30:08Z

The curl libraries are already self-contained after the curl-impersonate build pipeline. Above error is because the sdist attempts to download and install the libraries into /usr/local/lib, which fails when not run as root.

$ ldd -v libcurl-impersonate-chrome.so
        linux-vdso.so.1 (0x00007ffc8f614000)
        libz.so.1 => /lib/x86_64-linux-gnu/libz.so.1 (0x00007f6a014db000)
        libpthread.so.0 => /lib/x86_64-linux-gnu/libpthread.so.0 (0x00007f6a014d6000)
        libc.so.6 => /lib/x86_64-linux-gnu/libc.so.6 (0x00007f6a012ad000)
        /lib64/ld-linux-x86-64.so.2 (0x00007f6a017f4000)

        Version information:
        ./libcurl-impersonate-chrome.so:
                libpthread.so.0 (GLIBC_2.2.5) => /lib/x86_64-linux-gnu/libpthread.so.0
                libc.so.6 (GLIBC_2.16) => /lib/x86_64-linux-gnu/libc.so.6
                libc.so.6 (GLIBC_2.3) => /lib/x86_64-linux-gnu/libc.so.6
                libc.so.6 (GLIBC_2.7) => /lib/x86_64-linux-gnu/libc.so.6
                libc.so.6 (GLIBC_2.14) => /lib/x86_64-linux-gnu/libc.so.6
                libc.so.6 (GLIBC_2.17) => /lib/x86_64-linux-gnu/libc.so.6
                libc.so.6 (GLIBC_2.3.4) => /lib/x86_64-linux-gnu/libc.so.6
                libc.so.6 (GLIBC_2.2.5) => /lib/x86_64-linux-gnu/libc.so.6
        /lib/x86_64-linux-gnu/libz.so.1:
                libc.so.6 (GLIBC_2.14) => /lib/x86_64-linux-gnu/libc.so.6
                libc.so.6 (GLIBC_2.4) => /lib/x86_64-linux-gnu/libc.so.6
                libc.so.6 (GLIBC_2.2.5) => /lib/x86_64-linux-gnu/libc.so.6
                libc.so.6 (GLIBC_2.3.4) => /lib/x86_64-linux-gnu/libc.so.6
        /lib/x86_64-linux-gnu/libpthread.so.0:
                libc.so.6 (GLIBC_2.2.5) => /lib/x86_64-linux-gnu/libc.so.6
        /lib/x86_64-linux-gnu/libc.so.6:
                ld-linux-x86-64.so.2 (GLIBC_2.2.5) => /lib64/ld-linux-x86-64.so.2
                ld-linux-x86-64.so.2 (GLIBC_2.3) => /lib64/ld-linux-x86-64.so.2
                ld-linux-x86-64.so.2 (GLIBC_PRIVATE) => /lib64/ld-linux-x86-64.so.2

perklet · 2024-01-23T15:31:48Z

Well，they have to be there，because of their rpath，which can be fixed by patchelf

bjia56 · 2024-01-23T15:39:25Z

Not sure I understand - libcurl doesn't specify any other non-glibc libraries as dependencies, so it's more or less self contained. Auditwheel, on the other hand, will use patchelf to change library references to within the wheel itself, and copy the shared objects into the wheel. When cibuildwheel runs, auditwheel/delvewheel/delocate automatically runs to copy libraries into the final wheel.

T-256 · 2024-01-23T15:41:21Z

@yifeikong any suggestion to fix that? should it use chmod +x during sdist installation?

T-256 · 2024-01-23T15:44:44Z

When cibuildwheel runs, auditwheel/delvewheel/delocate automatically runs to copy libraries into the final wheel

FWIW, delvewheel dropped by #190 (windows-only), I believe we should do same for linux too. see #173.

T-256 · 2024-01-23T15:49:18Z

Above error is because the sdist attempts to download and install the libraries into /usr/local/lib, which fails when not run as root.

@bjia56 for clarification, do you verify build works when run as root?

bjia56 · 2024-01-23T15:50:36Z

Auditwheel is a standard tool for building native wheels on Linux, what's the reason for removing it?

bjia56 · 2024-01-23T15:51:21Z

@bjia56 for clarification, do you verify build works when run as root?

It does not yet work since the latest pre-release in curl-impersonate does not have the correct arm shared objects built. Another curl-impersonate tag will need to be cut.

perklet · 2024-01-23T15:52:33Z

When cibuildwheel runs, auditwheel/delvewheel/delocate automatically runs to copy libraries into the final wheel

FWIW, delvewheel dropped by #190 (windows-only), I believe we should do same for linux too. see #173.

No, we can not. Windows dlls do not have features like rpath, that’s why you can simply copy dlls to make them work.

perklet · 2024-01-24T08:05:10Z

scripts/build.py

@@ -0,0 +1,79 @@
+import os


I think we should move back this file to its original location for better tracking changes. It should be relocated next time.

As the purposes of this PR, it tries to exclude all unneeded files from bdist wheels. the changes applied on this file after move is not much, you can compare them easily.

setup.py

perklet · 2024-01-24T09:00:31Z

I tested and find out that rpath or similar attributes are not set for libcurl-impersonate-chrome.so, but for libcurl-impersonate-ff.so and libcurl-impersonte-chrome.dylib.

Which basically means that we can safely change the temporaray lib directory for all linux archs. Thus, sudo will not be needed. But for macOS, we have to download it to the buildtime directory. Luckily, we have bdist wheel on macOS, users don't need to care about it.

This PR is including two different features, I suggest that we split it into smaller chunks. First we make the sdist work, then maybe excludes useless files for bdist.

auditwheel and delocate are always needed for fixing the rpath of _wrapper.so.

T-256 · 2024-01-24T13:32:19Z

This PR is including two different features, I suggest that we split it into smaller chunks. First we make the sdist work, then maybe excludes useless files for bdist.

Ok, Go ahead with fix sdits in separated PR, I'll update this PR relatively after that. I opened #225 to track it.

This reverts commit 94d8c72.

perklet · 2024-01-25T02:35:29Z

This PR is including two different features, I suggest that we split it into smaller chunks. First we make the sdist work, then maybe excludes useless files for bdist.

Ok, Go ahead with fix sdits in separated PR, I'll update this PR relatively after that. I opened #225 to track it.

Cool, thanks.

T-256 · 2024-02-01T10:02:59Z

Too many files need for rebasing, created new PR instead: #236

T-256 marked this pull request as draft December 27, 2023 11:08

exclude build.py, ffi and include

af5bbc2

T-256 force-pushed the no-include branch 2 times, most recently from b6712d2 to 7a48b33 Compare December 27, 2023 12:55

T-256 changed the title ~~exclude build.py, ffi and include~~ bdist wheels: exclude build-time-only files Dec 27, 2023

move build.py, ffi and include out of package

0924e2a

T-256 force-pushed the no-include branch 22 times, most recently from 384e259 to e05b868 Compare December 27, 2023 16:13

T-256 force-pushed the no-include branch from 42347e6 to fb2314c Compare January 22, 2024 18:35

perklet reviewed Jan 23, 2024

View reviewed changes

perklet reviewed Jan 24, 2024

View reviewed changes

remove rare archs

07237e5

T-256 force-pushed the no-include branch from 59a32ab to 2911220 Compare January 24, 2024 10:54

T-256 mentioned this pull request Jan 24, 2024

[BUG] Cannot install from sdist wheel on linux without sudo #225

Closed

T-256 force-pushed the no-include branch 2 times, most recently from 3ee52a1 to 67d27d2 Compare January 24, 2024 15:31

POC: check sdist

94d8c72

T-256 force-pushed the no-include branch from 67d27d2 to 94d8c72 Compare January 24, 2024 15:33

Revert "POC: check sdist"

f9b3021

This reverts commit 94d8c72.

T-256 mentioned this pull request Feb 1, 2024

Improve wheels and sdist #236

Merged

T-256 closed this Feb 1, 2024

bjia56 mentioned this pull request Feb 3, 2024

Compile Linux binaries with Zig lexiforest/curl-impersonate#39

Merged

T-256 deleted the no-include branch April 16, 2024 19:35

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

bdist wheels: exclude build-time-only files #192

bdist wheels: exclude build-time-only files #192

T-256 commented Dec 26, 2023 •

edited

Loading

perklet commented Dec 27, 2023

T-256 commented Dec 27, 2023

perklet Jan 23, 2024

T-256 Jan 23, 2024

perklet Jan 24, 2024

T-256 Jan 24, 2024

bjia56 commented Jan 23, 2024

perklet commented Jan 23, 2024

bjia56 commented Jan 23, 2024 •

edited

Loading

perklet commented Jan 23, 2024

bjia56 commented Jan 23, 2024

T-256 commented Jan 23, 2024

T-256 commented Jan 23, 2024

T-256 commented Jan 23, 2024

bjia56 commented Jan 23, 2024

bjia56 commented Jan 23, 2024

perklet commented Jan 23, 2024

perklet Jan 24, 2024

T-256 Jan 24, 2024

perklet commented Jan 24, 2024 •

edited

Loading

T-256 commented Jan 24, 2024

perklet commented Jan 25, 2024

T-256 commented Feb 1, 2024

bdist wheels: exclude build-time-only files #192

bdist wheels: exclude build-time-only files #192

Conversation

T-256 commented Dec 26, 2023 • edited Loading

perklet commented Dec 27, 2023

T-256 commented Dec 27, 2023

perklet Jan 23, 2024

Choose a reason for hiding this comment

T-256 Jan 23, 2024

Choose a reason for hiding this comment

perklet Jan 24, 2024

Choose a reason for hiding this comment

T-256 Jan 24, 2024

Choose a reason for hiding this comment

bjia56 commented Jan 23, 2024

perklet commented Jan 23, 2024

bjia56 commented Jan 23, 2024 • edited Loading

perklet commented Jan 23, 2024

bjia56 commented Jan 23, 2024

T-256 commented Jan 23, 2024

T-256 commented Jan 23, 2024

T-256 commented Jan 23, 2024

bjia56 commented Jan 23, 2024

bjia56 commented Jan 23, 2024

perklet commented Jan 23, 2024

perklet Jan 24, 2024

Choose a reason for hiding this comment

T-256 Jan 24, 2024

Choose a reason for hiding this comment

perklet commented Jan 24, 2024 • edited Loading

T-256 commented Jan 24, 2024

perklet commented Jan 25, 2024

T-256 commented Feb 1, 2024

T-256 commented Dec 26, 2023 •

edited

Loading

bjia56 commented Jan 23, 2024 •

edited

Loading

perklet commented Jan 24, 2024 •

edited

Loading