WIP: Setup CI #79

madig · 2018-03-02T15:59:27Z

We want a bundled shared FreeType library with a pinned version. If none comes bundled with the package, look for the system-wide one.

I copied and adapted what's done in ttfautohint-py, more testing needs to be done on macOS and Windows. Currently, no harfbuzz is built-in, not sure how to handle it yet (circular dependency freetype <-> harfbuzz).

Once the wheel build stuff works everywhere, I can experiment with Travis and Appveyor.

anthrotype

thanks for working on this!

anthrotype · 2018-03-02T16:10:45Z

freetype/raw.py

-        filename = os.path.join(os.path.realpath('.'), 'freetype.dll')
-    else:
-        filename = 'libfreetype.so.6'
+filename = pkg_resources.resource_filename('freetype', library_name)


using pkg_resources adds a runtime dependency on setuptools -- which nowadays is preinstalled on most python distributions alongside pip, but technically is not part of the python standard library.
The old-school approach to get a resource filename relative to __file__ works fine, unless you want to get a filename of a data file from a inside a zipped python package (like an .egg). But since we are including a precompiled native library, we have to set zip=False in setup.py in any case, otherwise we can't load it. So using pkg_resources has no concrete advantage IMO over os.path.dirname(__file__, library_name)

anthrotype · 2018-03-02T16:19:49Z

no harfbuzz is built-in, not sure how to handle it yet (circular dependency freetype <-> harfbuzz)

I guess you'll have to build freetype --without-harfbuzz, then build harfbuzz --with~~out~~-freetype, and finally freetype --with-harfbuzz

anthrotype · 2018-03-02T16:20:29Z

correction: of course, I meant harfbuzz --with-freetype

anthrotype · 2018-03-02T16:27:06Z

then we also need to figure out a way to make this optional, as there are some users that would like to simply pip install . or python setup.py install without building the embedded library, but using the system installed one (the current default behavior).
One way could be to add a command-line option or read from an environment variable that tells the setup.py to not add any ext_modules thus making the package "pure" python.

anthrotype · 2018-03-02T16:48:21Z

another thing with this setup borrowed from ttfautohint-py, is that the source distribution as generated with python setupt.py sdist will not include the freetype2 source. So, for example, running tox won't work, because tox by default creates the sdist, then installs it into the virtual environment, before running the tests inside it (ok, one can also do tox --develop or set usedevelop=True in tox.ini).

In any case, building the embedded library currently requires access to the freetype2 git submodule (thus one has to git clone --recursive ... or do git submodule update --init afterwards). And this submodule is not included in the python package sdist. I don't think it would be a good idea trying to add all the freetype2 sources from the git submodule to the python sdist package, because freetype's own build system probably detects whether it's running from a git repository or a released tarball.

One option instead of using git submodules would be to have the setup.py itself download the freetype2 released tarball from the official website, and do the build from there.
Using the released tarballs also has the advantage that you won't need to do ./autogen.sh but can do ./configure directly, so you don't need all the autotools but just make.

Actually, the only reason I used git submodules for ttfautohint-py was that at the time I wrote it, the stable ttfautohint release didn't have an option to build a shared library. If I were to redo it now, I would rather use the stable releases instead of the git repositories.

anthrotype · 2018-03-02T16:59:23Z

other than that, the current setup is already great, because it allows to build a wheel package that embeds a native library, works with both py2 and py3 (because it doesn't link with cpython like a regular extension module but is loaded via ctypes). And if you run this on Travis inside a manylinux1 docker container (like multibuild does), then you can have one universal wheel that works on almost every Linux distros (well, one for 64-bit, and another for 32 if you care about that), another one for Mac (a "fat" binary that works for both 64 and 32 architectures), and two others for Windows (32 and 64 bit) using Appveyor.

madig · 2018-03-02T17:35:37Z

Hm, running tox is actually on my want-list, so I guess I need to start looking into downloading a tarball... submodules seem to be quite a headache, anyway.

Thanks for your input on this :)

Will send a PR next to move freetype/ into src/python/.

madig · 2018-03-03T12:51:22Z

Alright, using a tarball works on Fedora 27. Now testing on macOS 10.13.

madig · 2018-03-03T13:41:23Z

It now builds and installs the module on Fedora 27 and macOS 10.13, running lsof | grep freetype confirms on both that the packaged library is used.

madig · 2018-03-03T20:57:13Z

I just did the move from freetype/ to src/freetype and added the Makefile to the manifest, running tox now passes :)

anthrotype · 2018-03-03T21:53:00Z

.travis.yml

+
+install:
+    # Maybe get and clean and patch source
+    - clean_code $REPO_DIR $BUILD_COMMIT


You don’t need this line

anthrotype · 2018-03-03T21:58:46Z

ci/multibuild_config.sh

+# Test for OSX with [ -n "$IS_OSX" ]
+
+function run_tests {
+    tox


This is going to build the shared library from source again, whereas you actually want to test the precompiled wheel (the one compiled inside the manylinux1 docker).
Use tox with —installpkg option, or just skip tox and run the tests directly. Multibuild already makes sure you are running the run_tests function from inside a fresh venv with the built wheel installed.
You can also lists tests requirements with an env variable

anthrotype · 2018-03-03T22:27:09Z

ci/multibuild_config.sh

@@ -2,5 +2,5 @@
 # Test for OSX with [ -n "$IS_OSX" ]

 function run_tests {
-    tox
+    pytest


Forgot to mention (though it’s in multibuild docs) that this function runs from within a new temporary subfolder, so if you want to run pytest from the root of th repo, you need to cd ..

https://github.com/fonttools/ttfautohint-py/blob/f099836e367bbc0d8f6652ac23f8e849d5b366b6/ci/multibuild_config.sh#L45

anthrotype · 2018-03-03T22:31:39Z

It’s working! 👍

madig · 2018-03-03T22:34:29Z

Okay, more work some other day. Get things to deploy on PyPI or get Appveyor up first?

anthrotype · 2018-03-03T22:35:16Z

Appveyor first. PyPI deployment is more complicated

madig · 2018-03-03T22:36:29Z

Kay :)

Oh, well, still need to give the option to not bundle FT. Maybe look for an env var and bundle FT only then? The var can be set in the build files. The default would be not to bundle.

anthrotype · 2018-03-03T22:37:42Z

@rougier needs to enable the repository with his Appveyor account for this to work, just like he did for Travis.

Yes, default should be not bundle

madig · 2018-03-05T10:57:48Z

Is it possible to cache the FT tarball somewhere? I feel uncomfortable downloading 2 Mb off the internet multiple times for every commit :(

madig · 2018-03-05T11:08:48Z

I'll be using https://ci.appveyor.com/project/madig/freetype-py until @rougier enables Appveyor on his repo :)

anthrotype · 2018-03-05T11:44:15Z

Is it possible to cache the FT tarball somewhere?

You could make a separate make target that just curls the tarball and make the other target depend on it

anthrotype · 2018-03-05T11:45:15Z

Oh you mean for every commit.. hm yes Travis has a cache feature, check the docs

madig · 2018-03-05T12:26:22Z

Will do.

The Windows MSYS2 build produces a libfreetype.dll.a, file says it is a "current ar" -- is this related to the dllwrap magic you did?

anthrotype · 2018-03-05T13:15:18Z

That’s just the import library (equivalent of *.lib when building with MSVC) for use when linking, you don’t need that.
You don’t get any DLL? Try configuring with —enable-shared, maybe it’s not enabled by default on MSYS2 builds.

anthrotype · 2018-03-05T13:27:36Z

I suggest you do your experiments on a real Windows machine or a local VM first, it’s easier.

Also use Dependency Walker to check that the built dll doesn’t depend on anything else but system libraries, we want to bundle a single freetype DLL.

http://www.dependencywalker.com

If you see it still depends on msys2 specific dlls, then try to create the DLL using the dllwrap tool and see if that makes it a standalone library.

(I’m at the airport typing from my phone)

anthrotype · 2018-03-05T13:34:48Z

Maybe also try passing -static-libgcc
https://stackoverflow.com/a/18138926

anthrotype · 2018-03-05T13:41:39Z

Also, technically we don’t have to use kings-w64 gcc to build the freetype DLL on windows, we could also build it with MSVC (there should be an nmake makefile in win32 folder). The reason I had to use msys2 for ttfautohint-py was because ttfautohint depends on gnulib and that can only be built with the autotools in a Unix-like environment.

anthrotype · 2018-03-05T13:41:58Z

mingw, not kings

HinTak · 2018-03-05T13:59:06Z

You don't need to use dependency walker if you are cross-compiling. ``` winedump -j import <freetype.dll> | grep offset ``` does the job on the typical Linux/Mac machines too, if you have wine installed.

madig · 2018-03-05T14:05:11Z

The .dll.a gets produced even when I do --enable-shared --disable-static. When I produce a static .a and use dllwrap (with --export-all-symbols), I get a proper .dll that only depends on kernel32.dll and msvcrt.dll.

Hm, I guess I'd have to make a separate (N)Makefile for Windows maybe. Although I guess that won't get us far if we want HarfBuzz, too... Oh, the build instructions mention vcpkg. More investigation needed I guess.

@HinTak: I'm cross-compiling to Windows.. on Windows 😀

AppVeyorBot · 2018-04-09T09:59:26Z

✅ Build freetype-py 1.0.18 completed (commit a4aed6f965 by @madig)

anthrotype

LGTM

AppVeyorBot · 2018-04-10T11:49:20Z

✅ Build freetype-py 1.0.19 completed (commit ddcb2211d1 by @madig)

madig · 2018-04-10T12:03:47Z

I don't have a PyPI account. @rougier do you want to fill in the deployment stuff yourself or hand that off to someone?

@anthrotype Why are you using "skip_cleanup: true", "distributions: check" and "on: repo: ..." in ttfautohint? Shouldn't 'distributions: "sdist bdist_wheel"' suffice?

anthrotype · 2018-04-10T12:17:57Z

Why are you using "skip_cleanup: true", "distributions: check" and "on: repo: ..." in ttfautohint? Shouldn't 'distributions: "sdist bdist_wheel"' suffice?

because we have already built the wheels inside the multibuild environment, we don't want Travis dpl tool to try rebuiding them again outside of it, which it will attempt to do.
The default distributions value is sdist bdist_wheel; these are simply forwarded by Travis to the setup.py script. I set it to check so that it doesn't actually do anything. The check setuptools command just checks the package metadata is correct. In other projects, I defined a pass custom command that does nothing, for the same reasons.
The skip_cleanup is to prevent Travis from deleting the wheels we just built.
I know, it sounds complicated, but it works.
The alternative would be to pip install twine and call twine ourselves in a custom deploy script, avoiding the use of Travis built-in PyPI intergration.

madig · 2018-04-10T12:21:56Z

And what about the "on: repo: ..." thing?

anthrotype · 2018-04-10T12:58:20Z

it's to prevent the deploy stage to trigger from forks.

Username and password need to be filled in still.

AppVeyorBot · 2018-04-10T13:04:26Z

❌ Build freetype-py 1.0.20 failed (commit f0b6329154 by @madig)

AppVeyorBot · 2018-04-10T13:05:01Z

❌ Build freetype-py 1.0.21 failed (commit f7f43e0c70 by @madig)

AppVeyorBot · 2018-04-10T13:14:00Z

✅ Build freetype-py 1.0.22 completed (commit 239e794eca by @madig)

madig · 2018-04-20T10:57:04Z

@rougier ping :)

rougier · 2018-04-23T07:34:11Z

Yes, sorry. I've not been reactive lately (lot of work). Is this ready to be merged ?

madig · 2018-04-23T08:05:18Z

Yes, the only thing left to do is filling in the PyPI upload credentials. You can do that yourself or follow Cosimo's suggestion (#79 (comment)).

rougier · 2018-04-23T11:00:46Z

Ok, I will add them later then. Found how to do it on with Travis but not with apppveyor.

anthrotype · 2018-04-23T11:25:09Z

For Appveyor, you need to encrypt your PyPI password by going to Account → Encrypt data. Store the secure value as TWINE_PASSWORD in appveyor.yml

rougier · 2018-04-23T11:38:41Z

Just dit it. Travis still failing. We need to add a travis/appveyor status in the README byt the way.

rougier · 2018-04-23T11:40:20Z

Traceback (most recent call last):
  File "ci/multibuild/supported_wheels.py", line 9, in <module>
    from pip.pep425tags import get_supported
ModuleNotFoundError: No module named 'pip.pep425tags'
ERROR: You must give at least one requirement to install (see "pip help install")

anthrotype · 2018-04-23T12:44:50Z

Try update the multibuild submodule ti latest master, or even try the dev branch if it still fails

rougier · 2018-04-23T13:35:21Z

It seems to be up to date and the PR was ok when not merged.

madig · 2018-04-23T13:56:52Z

https://github.com/matthew-brett/multibuild/issues/159 -- seems to be this?

rougier · 2018-04-23T13:58:38Z

Yes. Note sure how to switch to the devel branch on the module though. Maybe it's not worth the trouble.

madig · 2018-04-23T15:45:59Z

You can do follow https://stackoverflow.com/questions/5828324/update-git-submodule-to-latest-commit-on-origin and use the commit hash of the latest devel branch.

rougier · 2018-04-23T18:57:51Z

This seems to work. 🎉

@madig @anthrotype By the way, I forgot to say a huge thanks to for this huge PR that hopefully will make things easier for a lot of people! Again, many many thanks for your contribution.

anthrotype reviewed Mar 2, 2018

View reviewed changes

anthrotype reviewed Mar 3, 2018

View reviewed changes

Comment out pre_build()

1787f1f

anthrotype approved these changes Apr 9, 2018

View reviewed changes

Remove introduction.rst, point to GitHub for installation instructions

0f3b535

Prepare automatic deployment on tags

78e7251

Username and password need to be filled in still.

rougier merged commit 76e13a7 into rougier:master Apr 23, 2018

madig deleted the setup-ci branch April 23, 2018 13:55

madig mentioned this pull request May 9, 2018

Pin FreeType version? #78

Closed

bgermann mentioned this pull request Jun 11, 2018

New release? #88

Closed

WIP: Setup CI #79

WIP: Setup CI #79

Conversation

madig commented Mar 2, 2018 • edited

anthrotype left a comment

Choose a reason for hiding this comment

anthrotype Mar 2, 2018

Choose a reason for hiding this comment

anthrotype commented Mar 2, 2018 • edited

anthrotype commented Mar 2, 2018

anthrotype commented Mar 2, 2018

anthrotype commented Mar 2, 2018

anthrotype commented Mar 2, 2018

madig commented Mar 2, 2018 • edited

madig commented Mar 3, 2018

madig commented Mar 3, 2018 • edited

madig commented Mar 3, 2018

anthrotype Mar 3, 2018

Choose a reason for hiding this comment

anthrotype Mar 3, 2018

Choose a reason for hiding this comment

anthrotype Mar 3, 2018

Choose a reason for hiding this comment

anthrotype Mar 3, 2018

Choose a reason for hiding this comment

anthrotype commented Mar 3, 2018

madig commented Mar 3, 2018

anthrotype commented Mar 3, 2018

madig commented Mar 3, 2018

anthrotype commented Mar 3, 2018

madig commented Mar 5, 2018

madig commented Mar 5, 2018

anthrotype commented Mar 5, 2018

anthrotype commented Mar 5, 2018

madig commented Mar 5, 2018

anthrotype commented Mar 5, 2018

anthrotype commented Mar 5, 2018

anthrotype commented Mar 5, 2018

anthrotype commented Mar 5, 2018

anthrotype commented Mar 5, 2018

HinTak commented Mar 5, 2018 via email

madig commented Mar 5, 2018 • edited

AppVeyorBot commented Apr 9, 2018

anthrotype left a comment

Choose a reason for hiding this comment

AppVeyorBot commented Apr 10, 2018

madig commented Apr 10, 2018 • edited

anthrotype commented Apr 10, 2018

madig commented Apr 10, 2018

anthrotype commented Apr 10, 2018

AppVeyorBot commented Apr 10, 2018

AppVeyorBot commented Apr 10, 2018

AppVeyorBot commented Apr 10, 2018

madig commented Apr 20, 2018

rougier commented Apr 23, 2018 • edited

madig commented Apr 23, 2018

rougier commented Apr 23, 2018

anthrotype commented Apr 23, 2018

rougier commented Apr 23, 2018

rougier commented Apr 23, 2018

anthrotype commented Apr 23, 2018

rougier commented Apr 23, 2018

madig commented Apr 23, 2018

rougier commented Apr 23, 2018

madig commented Apr 23, 2018

rougier commented Apr 23, 2018

madig commented Mar 2, 2018 •

edited

anthrotype commented Mar 2, 2018 •

edited

madig commented Mar 2, 2018 •

edited

madig commented Mar 3, 2018 •

edited

madig commented Mar 5, 2018 •

edited

madig commented Apr 10, 2018 •

edited

rougier commented Apr 23, 2018 •

edited