Small step toward wheel #178

bersace · 2020-03-31T15:03:53Z

Here is a first step to build wheels as requested by #121. This PR is a small step : it does not change release process, does not change tests, does not add CI, does not support every binary platform.

To upload wheel, you need docker-compose and twine. Then, simply run make upload. There is two new Make targets : distclean (just cleaning dist dir) and packages to build without uploading.

$ make distclean packages
...
$ tree dist/
dist/
├── bjoern-3.1.0-cp27-cp27mu-manylinux1_x86_64.whl
├── bjoern-3.1.0-cp27-cp27mu-manylinux2010_x86_64.whl
├── bjoern-3.1.0-cp35-cp35m-manylinux1_x86_64.whl
├── bjoern-3.1.0-cp35-cp35m-manylinux2010_x86_64.whl
├── bjoern-3.1.0-cp35-cp35m-manylinux2014_x86_64.whl
├── bjoern-3.1.0-cp36-cp36m-manylinux1_x86_64.whl
├── bjoern-3.1.0-cp36-cp36m-manylinux2010_x86_64.whl
├── bjoern-3.1.0-cp36-cp36m-manylinux2014_x86_64.whl
├── bjoern-3.1.0-cp37-cp37m-manylinux1_x86_64.whl
├── bjoern-3.1.0-cp37-cp37m-manylinux2010_x86_64.whl
├── bjoern-3.1.0-cp37-cp37m-manylinux2014_x86_64.whl
├── bjoern-3.1.0-cp38-cp38-manylinux1_x86_64.whl
├── bjoern-3.1.0-cp38-cp38-manylinux2010_x86_64.whl
├── bjoern-3.1.0-cp38-cp38-manylinux2014_x86_64.whl
└── bjoern-3.1.0.tar.gz

0 directories, 15 files
$

libev IS embedded in wheel. I'm wondering if this will trigger issue like with psycopg2 and libpq. The workaround is to use source tarball rather than wheel.

Each wheel is pen tested in a virtualenv with test_wsgi_compliance.py.

I hit a bug with cp27m. Instead of fighting this, I choose to blacklist it for now. Users will still use sources.

The docker-compose.yml should be easy to translate to a CI job matrix, using either CircleCI, GitHub Action, Travis, Azure pipelines and/or appveyor.

@jonashaag what do you think of this ?

cc @hoefling

packaging/build_wheels.sh

jonashaag · 2020-04-01T11:11:35Z

Looks really good. Generally speaking I dislike these kinds of sophisticated bash scripts (error handling, searching for strings in files, trap handlers, etc.) and I would suggest to write these kinds of programs in a proper programming language instead (e.g., Python). But that is no reason to rewrite the script now; it's fine.

Only concern I have is about embedded libev. Is this common practice? What are the advantages and disadvantages? I will have to give this some thought.

bersace · 2020-04-01T12:01:11Z

Only concern I have is about embedded libev. Is this common practice? What are the advantages and disadvantages? I will have to give this some thought.

For the sake of portability, a wheel must not depends on something else that manylinux* defined libraries. The official auditwheel tools is here to embed extra libs. Here is what's in pyscopg2-binary wheel :

$ unzip -l psycopg2_binary-2.8.4-cp37-cp37m-manylinux1_x86_64.whl  | grep .libs
     9000  2019-10-20 00:57   psycopg2/.libs/libkeyutils-1-ff31573b.2.so
    61008  2019-10-20 00:57   psycopg2/.libs/liblber-2-e85f761a.4.so.2.10.11
   251816  2019-10-20 00:57   psycopg2/.libs/libsepol-b4f5b513.so.1
  3217584  2019-10-20 00:57   psycopg2/.libs/libcrypto-3a9cf061.so.1.1.1d
   167808  2019-10-20 00:57   psycopg2/.libs/libk5crypto-622ef25b.so.3.1
   738056  2019-10-20 00:57   psycopg2/.libs/libkrb5-fb0d2caa.so.3.3
     8840  2019-10-20 00:57   psycopg2/.libs/libcom_err-beb60336.so.2.1
   106904  2019-10-20 00:57   psycopg2/.libs/libselinux-cf8f9094.so.1
   341408  2019-10-20 00:57   psycopg2/.libs/libpq-3d107b13.so.5.11
   662464  2019-10-20 00:57   psycopg2/.libs/libssl-8b4167bf.so.1.1.1d
   142720  2019-10-20 00:57   psycopg2/.libs/libsasl2-6f25e95f.so.3.0.0
    87848  2019-10-20 00:57   psycopg2/.libs/libz-a147dcb0.so.1.2.3
   240936  2019-10-20 00:57   psycopg2/.libs/libgssapi_krb5-174f8956.so.2.2
    46960  2019-10-20 00:57   psycopg2/.libs/libkrb5support-d7ce89d4.so.0.1
   428744  2019-10-20 00:57   psycopg2/.libs/libldap_r-2-b688a282.4.so.2.10.11
$

This way, bjoern does not require users to install libev through their own package manager or by hand. Everything is pipable.

The disadvandage is where a program mixes shared objects from bjoern wheel and other wheel or host system. This is the case pyscopg2 hit with libssl. I think that libev has not the same constraint than libssl.

See https://www.psycopg.org/articles/2018/02/08/psycopg-274-released/ for details on what issues psycopg2 hit by embedding shared objects in its wheel.

bersace · 2020-04-01T12:12:15Z

Generally speaking I dislike these kinds of sophisticated bash scripts (error handling, searching for strings in files, trap handlers, etc.) and I would suggest to write these kinds of programs in a proper programming language instead (e.g., Python). But that is no reason to rewrite the script now; it's fine.

I fully understand this. I'm used to advanced bash scripting and find it easier when it's all about running pip, yum, make, virtualenv, setup.py, auditwheel and python interpreters.

psycopg2 did implement this in Python. But the difference is that pyscopg2's script is meant to run on Windows. Hence the choice was between PowerShell and Python. PowerShell and bash are very differents.

Latest fixup removes grepping the log. I wish this make you more confident.

teardown & trap is a way to ensure that script running as root in docker does not pollute git clone with root files. This is because there is (usualy) no squash of uid with docker mounted volume.

If you prefer, I could extract a build_wheel.sh to build one single wheel for a given runtime and sdist (mostly build_wheel function) and rework build_wheels.sh as build_all.sh calling the first script, making it simpler.

packaging/build_wheels.sh

hoefling · 2020-04-01T12:36:11Z

Including libs in a self-contained wheel is what the manylinux spec is for, so including libev in the fat wheel is fine IMO. As long as the source dist is offered (and one can skip fat wheels via pip install bjoern --no-binary=bjoern), I wouldn't expect any issues with that.

bersace · 2020-04-01T14:43:06Z

@jonashaag ready for next review :-)

jonashaag · 2020-04-01T14:44:10Z

If you prefer, I could extract a build_wheel.sh to build one single wheel for a given runtime and sdist (mostly build_wheel function) and rework build_wheels.sh as build_all.sh calling the first script, making it simpler.

That would be great indeed.

Ok, it seems to be best practice to include requirements in the build, so let’s go with that. I’d be interested to know exactly why it is recommended to do it that way since it has many drawbacks (size of distribution, security, etc). I’ll try to find some sources for that info; it’s unrelated to this PR.

hoefling · 2020-04-01T15:11:14Z

@jonashaag PEP 513 describes the rationale.

bersace · 2020-04-01T15:47:23Z

That would be great indeed.

Done.

@hoefling finally, i found that wheel.pep425tags is available. I wish that won't change or wheeltag.py script will get harder.

This is a first step to produce wheels. The release process is unchanged. Releasing now requires docker-compose and twine. Only manylinux platforms are built as of this commit.

This target is strongly recommended before running upload to avoid reuploading packages to PyPI using twine.

bersace · 2020-04-01T15:49:37Z

@jonashaag are you working on a Mac ? Does it work properly on your environment ?

jonashaag · 2020-04-01T16:25:38Z

Yes, I have multiple Mac machines, will try soon.

bersace · 2020-04-02T12:42:04Z

@jonashaag If you want to upload wheels for older release, I can tweak it a bit to have the following post-release packaging process:

Clean with make cleandist
Download previous sdist from PyPI in dist/
make -C packaging all
twine upload dist/*.whl

jonashaag · 2020-08-20T06:49:56Z

Somehow, I totally forgot about this. Sorry. Testing it now.

jonashaag · 2020-08-20T07:21:06Z

Build seems to work, but got a problem with the chmod in teardown. Maybe that's Mac specific.

++ stat -c %u:%g /workspace/packaging/build_all.sh
+ uid_gid=0:0
+ chown -R 0:0 /workspace/dist
make[1]: *** [manylinux2014] Error 1
make: *** [packages] Error 2

UID/GID will always be 0.

bersace · 2020-08-20T15:58:04Z

Build seems to work, but got a problem with the chmod in teardown. Maybe that's Mac specific.

Right. When bind mounting volumes, it's better to sync ownership with git checkout so that upload tools have access and it's easy to cleanup dist directory.

I'll add a check on uid to avoid chmoding 0:0. Do you have a better trick ?

bersace · 2020-08-24T13:05:42Z

@jonashaag fixup pushed. Would you mind to test it ? Please wait the rebase before merging :-)

jonashaag · 2020-08-24T17:05:35Z

It builds ok now! Will review more asap

bersace · 2021-06-23T12:28:32Z

🆙

jonashaag · 2021-06-29T16:42:20Z

Sorry for the huge latency. I was mostly busy with other things, and also not being familiar with wheel packaging and advanced bash scripting didn't help. But finally got around to reviewing this and from what I understand it's great! Thanks!

jonashaag · 2021-06-29T16:46:53Z

When I run on Linux, this is what I get:

$ make
IMAGE=quay.io/pypa/manylinux2014_x86_64 docker-compose run --rm builder
Creating packaging_builder_run ... done
++ readlink -m /workspace/packaging/build_all.sh/../..
+ top_srcdir=/workspace
+ distdir=/workspace/dist
+ trap teardown INT EXIT TERM
+ cd /workspace
+ export PIP_DISABLE_PIP_VERSION_CHECK=off
+ PIP_DISABLE_PIP_VERSION_CHECK=off
+ export PIP_NO_PYTHON_VERSION_WARNING=on
+ PIP_NO_PYTHON_VERSION_WARNING=on
+ runtimes=(/opt/python/cp*)
++ /opt/python/cp310-cp310/bin/python setup.py --version
+ version=3.1.0
+ tarname=bjoern-3.1.0
+ sdist=/workspace/dist/bjoern-3.1.0.tar.gz
+ '[' -f /workspace/dist/bjoern-3.1.0.tar.gz ']'
+ echo 'Missing source tarball /workspace/dist/bjoern-3.1.0.tar.gz.'
Missing source tarball /workspace/dist/bjoern-3.1.0.tar.gz.
+ exit 1
+ teardown
++ stat -c %u:%g /workspace/packaging/build_all.sh
+ uid_gid=1000:0
+ '[' 1000:0 '!=' 0:0 ']'
+ chown -R 1000:0 /workspace/dist
chown: cannot access ‘/workspace/dist’: No such file or directory
ERROR: 1
make: *** [Makefile:4: manylinux2014] Error 1

bersace · 2021-06-30T06:15:19Z

Hi @jonashaag the scripts requires tarball :

...
+ echo 'Missing source tarball /workspace/dist/bjoern-3.1.0.tar.gz.'
Missing source tarball /workspace/dist/bjoern-3.1.0.tar.gz.
...

I can submit a patch to add better logging rather than bash verbose '+'. Also, the final chown fails if no dist dir, but that's not the root of the error.

jonashaag · 2021-06-30T09:09:05Z

Ah, didn't see that line!

jonashaag reviewed Apr 1, 2020

View reviewed changes

packaging/build_wheels.sh Outdated Show resolved Hide resolved

hoefling reviewed Apr 1, 2020

View reviewed changes

packaging/build_wheels.sh Outdated Show resolved Hide resolved

hoefling reviewed Apr 1, 2020

View reviewed changes

packaging/build_wheels.sh Outdated Show resolved Hide resolved

bersace added 2 commits April 1, 2020 17:48

Build wheel with docker-compose

fb5e363

This is a first step to produce wheels. The release process is unchanged. Releasing now requires docker-compose and twine. Only manylinux platforms are built as of this commit.

Add distclean target

cc60ca8

This target is strongly recommended before running upload to avoid reuploading packages to PyPI using twine.

fixup! Build wheel with docker-compose

c28d416

onekiloparsec mentioned this pull request Sep 2, 2020

Installation error on MacOS #191

Closed

jonashaag merged commit 54b0725 into jonashaag:master Jun 29, 2021

bersace mentioned this pull request Dec 10, 2021

Wheel support? #121

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Small step toward wheel #178

Small step toward wheel #178

bersace commented Mar 31, 2020 •

edited

Loading

jonashaag commented Apr 1, 2020

bersace commented Apr 1, 2020

bersace commented Apr 1, 2020

hoefling commented Apr 1, 2020

bersace commented Apr 1, 2020

jonashaag commented Apr 1, 2020

hoefling commented Apr 1, 2020

bersace commented Apr 1, 2020

bersace commented Apr 1, 2020

jonashaag commented Apr 1, 2020

bersace commented Apr 2, 2020

jonashaag commented Aug 20, 2020

jonashaag commented Aug 20, 2020

bersace commented Aug 20, 2020

bersace commented Aug 24, 2020

jonashaag commented Aug 24, 2020

bersace commented Jun 23, 2021

jonashaag commented Jun 29, 2021

jonashaag commented Jun 29, 2021 •

edited

Loading

bersace commented Jun 30, 2021

jonashaag commented Jun 30, 2021

Small step toward wheel #178

Small step toward wheel #178

Conversation

bersace commented Mar 31, 2020 • edited Loading

jonashaag commented Apr 1, 2020

bersace commented Apr 1, 2020

bersace commented Apr 1, 2020

hoefling commented Apr 1, 2020

bersace commented Apr 1, 2020

jonashaag commented Apr 1, 2020

hoefling commented Apr 1, 2020

bersace commented Apr 1, 2020

bersace commented Apr 1, 2020

jonashaag commented Apr 1, 2020

bersace commented Apr 2, 2020

jonashaag commented Aug 20, 2020

jonashaag commented Aug 20, 2020

bersace commented Aug 20, 2020

bersace commented Aug 24, 2020

jonashaag commented Aug 24, 2020

bersace commented Jun 23, 2021

jonashaag commented Jun 29, 2021

jonashaag commented Jun 29, 2021 • edited Loading

bersace commented Jun 30, 2021

jonashaag commented Jun 30, 2021

bersace commented Mar 31, 2020 •

edited

Loading

jonashaag commented Jun 29, 2021 •

edited

Loading