setup multibuild for compiling multi-platform wheels #55

anthrotype · 2018-07-05T18:49:18Z

follow up from #42

the root .coveragerc is enough

always relative to the current working directory, so we can run test modules from anywhere

py.path is part of pytest (tmpdir fixture itself returns a py.path.local object)

remove parallel=True as it conficts with pytest-cov and pytest-xdist; add .tox site-packages folders to 'paths' for combining coverage.

so one can install them, e.g.: pip install -e .[testing]

Several changes, including: - we now use pytest-xdist plugin to distribute tests across several processes, depending on the CPU cores (-n auto) - py37 added to default envlist (but skipped if not present on local developer's machine) - pytest now searches in 'testpaths = tests' when no input is given; runs in verbose mode, with a summary report at the end - coverage data files for each tox environment are saved inside the .tox work dir, then are combined by a specific 'coverage' tox environment which must be run at the end, and which combines and produces reports in different formats: console, xml (for codecov), and html (saved in 'htmlcov') for inspecting locally in a browser.

we only need to run tox -e sdist on the CI before uploading to PyPI. I keep 'tox -e bdist' in case one may want to create the wheel locally

for now only 2.7 and 3.6 (64-bit) for both linux (wide unicode) and mac. No deploy stage yet.

run_tests will search for the precompiled psautohint wheel inside 'wheelhouse' dir and call tox with --installpkg (to skip rebuilding from sdist).

anthrotype · 2018-07-05T18:54:57Z

codacy is very picky.. didn't know it also linted bash! :)

need to tell tox where to find the path to the python interpreter on Windows (do I?)

https://app.codacy.com/app/frankrolf/psautohint/pullRequest?prid=1866893

…se it ourside docker container

anthrotype · 2018-07-05T19:35:21Z

appveyor is very slow, not so much the single build (5-6 min each) but the fact that unlike Travis, it doesn't run in parallel with the other matrix builds.
We might decide to drop 64-bit 2.7 and 32-bit 3.x, on the reasoning that legacy is legacy, and new OSes ==> modern python architecture?..

anyway, i'll complete this tomorrow.

installing pip seems to break multibuild osx 2.7 build somehow. https://travis-ci.org/adobe-type-tools/psautohint/jobs/400583973

the linux builds fail because they can't read/write the .tox folder..

I think these should work...

anthrotype · 2018-07-06T17:36:21Z

I'm not an admin on the appveyor account, so I cannot store my encrypted PyPI password as TWINE_PASSWORD secure environment variable in appveyor.yml.
For now I added the same encrypted credentials as the adobe-type-tools/afdko project. I believe that the secure encrypted variables in appveyor are the same across project (unlike Github), only vary based on the Appveyor account with which they are encrypted, and psautohint and afdko should belong to the same account.
We'll see if this work after we merge this PR and try to push a pre-release tag.

anthrotype · 2018-07-06T17:41:54Z

I didn't mean to be rude with our Coday bot, it's doing its job :)
The suggestion to use defused_xml to prevent XML attacks from untrusted sources is a bit overkill for our use case (using psautohint offline on one's own UFOs can be considered safe, or at least use-at-your-own-risk).

miguelsousa · 2018-07-06T17:51:35Z

The Codacy review link is pointing to the wrong page; #50 is tracking that. The correct link is https://app.codacy.com/app/adobe-type-tools/psautohint/pullRequest?prid=1867014

I'll look into providing twine credentials for this repo.

anthrotype · 2018-07-06T17:52:49Z

ok the pr is ready for review/merge now. I'm sure there'll be more fixes needed after we try to push the next tag.

anthrotype · 2018-07-06T18:02:28Z

for Travis I added my PyPI username/password since I'm a collaborator on both this repo and the linked PyPI page, so they will work.
If you prefer to replace them with yours, I'm ok. Just make sure to use this command:

$ echo "<your-password-here>" | travis encrypt -R adobe-type-tools/psautohint --add deploy.password

Then you'll notice that the .travis.yml file will have been reformatted with indentation and empty line separators all messed up. What I usually do is copy/paste the secure: ... password line somewhere else, then restore the .travis.yml file to its original state before the travis tool reformatted it (git checkout .travis.yml), and then paste the encrypted password to its correct place.

anthrotype · 2018-07-06T19:21:16Z

.travis.yml

@@ -24,6 +24,8 @@ env:
    - TEST_DEPENDS="-rdev-requirements.txt"
    - PLAT=x86_64
    - UNICODE_WIDTH=32
+    - TWINE_USERNAME=adobe-type-tools-ci
+    # TWINE_PASSWORD is set in Travis settings


@miguelsousa are you sure that setting these environment variable works with Travis deploy too?
It does also use twine under the hood, so it should work. But I've never tried. Anyway we'll see

that's how they're used in this repo https://github.com/joerick/pyinstrument_cext

actually, it failed to deploy without 'user' defined:
https://travis-ci.org/adobe-type-tools/psautohint/jobs/401042408#L878

I fixed it here: 65f9234

🤞

.. And also the 'password' field is required:
https://travis-ci.org/adobe-type-tools/psautohint/jobs/401053332#L878

I'm using mine just to test. The reason that alternative method works is because it calls twine directly (like we do with Appveyor), whereas in Travis we are using the dpl tool (which requires those settings in the .travis.yml, and in turns calls twine with them).

👌
the afdko repo uses a script to deploy to PyPI from Travis
https://github.com/adobe-type-tools/afdko/blob/develop/.travis.yml#L117

@miguelsousa can you please cancel this appveyor build for me?
https://ci.appveyor.com/project/adobe-type-tools/psautohint/build/1.0.256

I don't seem to have access to it, even despite I saw your invite in my email. The address is correct, not sure why it isn't working :-/
I'm logged in Appveyor with my github username @anthrotype

anyway we need to stop that appveyor build before travis pushes a new release which is going to be newer than the one appveyor is trying to push :(

@anthrotype cancelled. Try logging into AppVeyor using your email address. The invitation is still showing up as not having been accepted.

I did! The link asks me to "sign up" but of course I am already signed up... so I click the button to login with github and select "anthrotype" among several organizational accounts (e.g. fonttools, trufont, etc. but not afdko...), but then nothing happens. I don't seem to be able to cancel or restart the builds. Anyway, no problem. no big deal

@anthrotype I've reconfigured the access to AppVeyor to use GH teams. Accept the invitation to https://github.com/orgs/adobe-type-tools/teams/psautohint-team and then check AppVeyor to see if you're able to restart builds now

Thanks Miguel! Yeah, that should do it.

btw, I'm happy to report that auto-upload to pypi from travis/appveyor is now working:
https://pypi.org/project/psautohint/1.1.1a6/#files

anthrotype · 2018-07-06T19:23:02Z

oops you merged too fast, or we didn't coordinate :)

https://ci.appveyor.com/project/adobe-type-tools/psautohint/build/1.0.246/job/dojxioahaa9nd753#L198

I added some commits that broke the appveyor build

miguelsousa · 2018-07-06T19:25:21Z

you said this was ready to merge, so after changing the twine credentials I did it.

anthrotype · 2018-07-06T19:27:04Z

you're right, sorry about that :)
I'll fix it

anthrotype · 2018-07-06T19:45:35Z

@miguelsousa I pushed the fix to master, can you please stop the previous Appveyor builds, so that the last one can start? I can stop Travis builds, but not Appveyor's

anthrotype · 2018-07-06T19:58:26Z

the last couple of commits to .appveyor.yml were a minor optimization, to avoid having to compile the module from source twice: i.e. once when tox installs it into its test environments, and again before uploading the wheel to PyPI.
I changed it so that we build the wheel before running the tests, then call tox with --installpkg option, passing the full path to the compiled wheel file (with that dir command hack that uses a wildcard to get the filename, writes to a temp file, then reads the content into an environment variable.. windows batch can be weird, maybe a powershell command could have done the same thing, but I don't know that).
Then, when we upload with twine, we already have the wheel (which we just built and tested) so we can upload the same file without recompiling it again.

it's working now:
https://ci.appveyor.com/project/adobe-type-tools/psautohint/build/1.0.248/job/0a8hx3250c5464o2#L205

anthrotype · 2018-07-06T20:02:20Z

@miguelsousa I'll work on adding python3.7 wheels later (appveyor doesn't ship with 3.7 yet, but we can download/install it ourselves). I think you can try tagging a prerelease and see if it pushes the wheels. It never works at first try -- no matter how many times we set this things up over and over 😄

miguelsousa · 2018-07-06T20:13:57Z

@anthrotype I'm not sure what tags you guys want to use, so you go ahead with the prerelease

anthrotype · 2018-07-06T20:17:59Z

ok, I'll try in a bit. I noticed that we have two sets of version strings, one in setup.py (1.1.1.dev0) which is in sync with the git tag, and another one for the libpsautohint C library (1.6.0). The psautohint --version currently prints the latter.
Do we want to keep the two distinct. Or would it be better to merge them?
I'd prefer a single version for both, since the C library is not shipped separately from the python wrapper.
We could use setuptools_scm to keep that in sync with the git tags, and then we could be able to cut a release by simply pushing a tag (without needing to manually bump the version string in multiple places).
setuptools_scm has a feature that can automatically writes the computed version string to a text file, in our case this could be a C header which the AC_getVersion() function can use, instead of hard-coding it as it is now.

/cc @khaledhosny

miguelsousa · 2018-07-06T20:40:21Z

@anthrotype I prefer a single version as well. It should match the version of the wheel.

khaledhosny · 2018-07-06T23:10:20Z

This seems to have squashed and merged as a single commit, something I strongly dislike myself. Though keeping each of the original commits might be too much, at least commits should be grouped into some reasonable units e.g. why opening files in UTF-8 encoding is in the same commit with the changes to the test suite? If one of these two unrelated changes proved problematic in the future, reverting that bit will be unnecessarily complicated, the same goes for git bisecting to find the source of a regression.

anthrotype · 2018-07-06T23:13:26Z

I agree with Khaled. I never click merge&squash button on Github. I prefer to squash commits manually into logical groups. It was inevitable that there were many small commits because of the trial and error when dealing with CIs. Next time there's a big PR like this, we could let the dev do the squashing manually after the reviewer has approved.

khaledhosny · 2018-07-06T23:13:35Z

No objection to single version, it should probably be based on the current C library version. So probably 1.7.0something.

anthrotype · 2018-07-06T23:17:27Z

btw, the changes to open files as UTF-8 were necessary in order to let the tests pass on the manylinux1 docker which sets LANG=C locale and thus open text files as ascii by default, unless explicitly told encoding="utf-8".

miguelsousa · 2018-07-07T00:18:41Z

Sorry, going forward I'll leave the merging part up to you two. I prefer to merge&squash when there are too many minute changes that are toward the same goal.

anthrotype added 18 commits July 5, 2018 12:54

Add multibuild submodule (master branch)

5115aa0

remove redundant tests/.coveragerc

0b5e0e1

the root .coveragerc is enough

.gitignore: we store coverage files in .tox, already ignored

9c4e39b

.gitignore: add htmlcov/ folder

21f58db

tests/__init__.py: define DATA_DIR global path to 'tests/data'

55a8f3c

always relative to the current working directory, so we can run test modules from anywhere

test_hintfonts: use DATA_DIR global; save output to tmpdir

d8dd8dd

test_hintfontsmm: reference and outputs in tmpdir, use DATA_DIR

af4bfcf

py.path is part of pytest (tmpdir fixture itself returns a py.path.local object)

.coveragerc: fix source/paths, skip_covered, etc.

2044481

remove parallel=True as it conficts with pytest-cov and pytest-xdist; add .tox site-packages folders to 'paths' for combining coverage.

setup.py: add test requirements to 'testing' extra

31dd5ed

so one can install them, e.g.: pip install -e .[testing]

dev-requirements.txt: only tox; the rest is handled by tox

5cd503d

tox.ini: split 'sdist' and 'bdist' environments

d6627e7

we only need to run tox -e sdist on the CI before uploading to PyPI. I keep 'tox -e bdist' in case one may want to create the wheel locally

travis: set up multibuild

a808f1c

for now only 2.7 and 3.6 (64-bit) for both linux (wide unicode) and mac. No deploy stage yet.

add multibuild's config.sh setup file

76bbcc7

run_tests will search for the precompiled psautohint wheel inside 'wheelhouse' dir and call tox with --installpkg (to skip rebuilding from sdist).

tox.ini: set TOXPYTHON env variable for finding Appveyor pythons

83e9e4b

tox.ini: add 'wheel' and 'pypi' commands

c2a9ccd

appveyor: build wheels for both 32 and 64 bit, etc.

d5d95b8

tox.ini: passenv TWINE_USERNAME and TWINE_PASSWORD

ae22900

anthrotype added 7 commits July 5, 2018 20:01

tox.ini: must define 'basepython' with TOXPYTHON else Appveyor fails

e6a0ff1

need to tell tox where to find the path to the python interpreter on Windows (do I?)

config.sh: apply codacy suggestions

93bf66b

https://app.codacy.com/app/frankrolf/psautohint/pullRequest?prid=1866893

appveyor doesn't like my batch if statement.. let's try powershell

dcbb522

.travis: need to install tox on Travis default python to be able to u…

2bb40b7

…se it ourside docker container

tox.ini: need to create sdist to build wheel from it!

2c68fde

tox.ini: make 'pypi-wheel' environment; called by Appveyor on tags

a05737c

.travis: on osx we need to bootstrap pip in order to install tox...

b0d90e4

anthrotype added 3 commits July 5, 2018 20:39

travis: move the osx pip bootstrapping in 'after_success'

58c6132

installing pip seems to break multibuild osx 2.7 build somehow. https://travis-ci.org/adobe-type-tools/psautohint/jobs/400583973

appveyor: disable 2.7 64-bit and 3.6 32-bit.. too slow :(

b2a91cb

config.sh: remove .tox folder after run_tests to fix permission errors

4caa670

the linux builds fail because they can't read/write the .tox folder..

anthrotype added 2 commits July 6, 2018 18:26

travis: add encrypted pypi password

632b68c

appveyor: add same TWINE_USERNAME and TWINE_PASSWORD as afdko repo

35b6813

I think these should work...

tests/__init__: minor

87e1dcb

anthrotype changed the title ~~WIP: setup multibuild for compiling multi-platform wheels~~ setup multibuild for compiling multi-platform wheels Jul 6, 2018

anthrotype and others added 4 commits July 6, 2018 20:18

tox.ini: remove unused 'pypi-wheel' env; we call twine directly now

0ab12f5

[ci-travis] Configure twine [skip ci]

40741f7

[ci-appveyor] Configure twine

c46ed0c

appveyor: compile wheel first, then run tests; reuse same wheel for PyPI

3dcf59a

miguelsousa merged commit ee96f33 into adobe-type-tools:master Jul 6, 2018

anthrotype commented Jul 6, 2018

View reviewed changes

anthrotype deleted the multibuild branch July 6, 2018 19:41

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

setup multibuild for compiling multi-platform wheels #55

setup multibuild for compiling multi-platform wheels #55

anthrotype commented Jul 5, 2018

anthrotype commented Jul 5, 2018

anthrotype commented Jul 5, 2018 •

edited

Loading

anthrotype commented Jul 6, 2018

anthrotype commented Jul 6, 2018

miguelsousa commented Jul 6, 2018

anthrotype commented Jul 6, 2018

anthrotype commented Jul 6, 2018

anthrotype Jul 6, 2018

miguelsousa Jul 6, 2018

anthrotype Jul 6, 2018

anthrotype Jul 6, 2018

miguelsousa Jul 6, 2018

anthrotype Jul 6, 2018

miguelsousa Jul 6, 2018

anthrotype Jul 6, 2018

miguelsousa Jul 6, 2018

anthrotype Jul 6, 2018

anthrotype commented Jul 6, 2018

miguelsousa commented Jul 6, 2018

anthrotype commented Jul 6, 2018

anthrotype commented Jul 6, 2018 •

edited

Loading

anthrotype commented Jul 6, 2018

anthrotype commented Jul 6, 2018 •

edited

Loading

miguelsousa commented Jul 6, 2018

anthrotype commented Jul 6, 2018

miguelsousa commented Jul 6, 2018

khaledhosny commented Jul 6, 2018

anthrotype commented Jul 6, 2018

khaledhosny commented Jul 6, 2018

anthrotype commented Jul 6, 2018

miguelsousa commented Jul 7, 2018

setup multibuild for compiling multi-platform wheels #55

setup multibuild for compiling multi-platform wheels #55

Conversation

anthrotype commented Jul 5, 2018

anthrotype commented Jul 5, 2018

anthrotype commented Jul 5, 2018 • edited Loading

anthrotype commented Jul 6, 2018

anthrotype commented Jul 6, 2018

miguelsousa commented Jul 6, 2018

anthrotype commented Jul 6, 2018

anthrotype commented Jul 6, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

anthrotype commented Jul 6, 2018

miguelsousa commented Jul 6, 2018

anthrotype commented Jul 6, 2018

anthrotype commented Jul 6, 2018 • edited Loading

anthrotype commented Jul 6, 2018

anthrotype commented Jul 6, 2018 • edited Loading

miguelsousa commented Jul 6, 2018

anthrotype commented Jul 6, 2018

miguelsousa commented Jul 6, 2018

khaledhosny commented Jul 6, 2018

anthrotype commented Jul 6, 2018

khaledhosny commented Jul 6, 2018

anthrotype commented Jul 6, 2018

miguelsousa commented Jul 7, 2018

anthrotype commented Jul 5, 2018 •

edited

Loading

anthrotype commented Jul 6, 2018 •

edited

Loading

anthrotype commented Jul 6, 2018 •

edited

Loading