Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

TravisCI: Windows + build stages #2342

Open
wants to merge 8 commits into
base: master
from

Conversation

Projects
None yet
3 participants
@bastimeyer
Copy link
Member

bastimeyer commented Mar 7, 2019

AppVeyor is failing once again with random network stuff and is breaking the builds. Let's see if Windows on TravisCI is already good enough for Streamlink (still in beta).

Resolves #2128

Changes

  • Added Windows VMs to the build matrix
    Based on @back-to's config mentioned here.
  • Added build stages
    This splits the test, documentation and deploy configs into consecutive and conditional stages instead of using one single python 3.5 VM with special build env vars. It should also improve the readability of the config. The deploy stage will only be run if all previous tests have passed in all environments. This wasn't the case previously.
  • Split sdistsign.sh into build-and-sign.sh and deploy-pypi.sh

To do

  • Remove appveyor.yml once webhook has been removed
  • Test deploy configs in a side-repo
@codecov

This comment has been minimized.

Copy link

codecov bot commented Mar 7, 2019

Codecov Report

Merging #2342 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master    #2342   +/-   ##
=======================================
  Coverage   52.59%   52.59%           
=======================================
  Files         237      237           
  Lines       14802    14802           
=======================================
  Hits         7785     7785           
  Misses       7017     7017

@bastimeyer bastimeyer force-pushed the bastimeyer:travisci branch from 381eac8 to ff504f6 Mar 7, 2019

@bastimeyer

This comment has been minimized.

Copy link
Member Author

bastimeyer commented Mar 7, 2019

Build config did not create any jobs.

https://travis-ci.org/streamlink/streamlink/requests

hm...

Is this because of the explicit build matrix (include keyword)?
https://docs.travis-ci.com/user/build-stages/#build-stages-and-build-matrix-expansion

The examples are all using the implicit build matrix:
https://github.com/travis-ci/build-stages-demo/blob/matrix-expansion/.travis.yml

@bastimeyer bastimeyer force-pushed the bastimeyer:travisci branch from ff504f6 to 100925f Mar 7, 2019

@bastimeyer

This comment has been minimized.

Copy link
Member Author

bastimeyer commented Mar 7, 2019

Travis has already finished the 11 test stages (only 4 ran in parallel) while AppVeyor is still running the 5th one. 😄

One issue I've noticed was that TRAVIS_PYTHON_VERSION was not set on the Windows VMs due to language: shell. This env var is missing in the coverage report and needs to be fixed.
https://codecov.io/gh/streamlink/streamlink/commit/100925fd5691ffac90d8e7020f760218f82dd423/build

Btw, all the env vars were missing the whole time in the coverage reports run by appveyor. Example:
https://codecov.io/gh/streamlink/streamlink/commit/a9aad6faeed72057e08ce51c44ae47316c14d2f8/build

@bastimeyer bastimeyer force-pushed the bastimeyer:travisci branch from bff344a to fae8e71 Mar 7, 2019

bastimeyer added some commits Mar 9, 2019

TravisCI: always build Windows installer
and be more dry by using YAML references
TravisCI: split sdistsign into build&sign + deploy
- run build-and-sign.sh in the release job's script field
- run deploy-pypi.sh in the release job's deploy field
- install build and deploy dependencies via Travis config
- move github_releases.py from installer job to release job (faster)
- rename SDIST_KEY_FILE env var to SIGNING_KEY_FILE
@bastimeyer

This comment has been minimized.

Copy link
Member Author

bastimeyer commented Mar 13, 2019

Apart from not having checked the new build config in a separate test-repo yet, I'm quite happy with the changes. A review would be appreciated. In case something's not clear, please see the individual commits. I can clean up my PR a bit, if it's needed.

@gravyboat

This comment has been minimized.

Copy link
Member

gravyboat commented Mar 14, 2019

Looks good to me @bastimeyer! I'll leave it open for a bit longer in case anyone else has opinions, but if not we'll get it merged in. Thanks!

@bastimeyer

This comment has been minimized.

Copy link
Member Author

bastimeyer commented Mar 14, 2019

Let's wait for a third (or fourth) person to confirm these changes first. It's a complete rewrite of the CI config and that shouldn't be merged without a proper review. I've already missed at least three critical things in the first commits and needed to fix that, which means that there's a chance I've missed something else, too.


Btw, another thing I just thought about: why does the wheel need to get uploaded to the Github releases page? How important is it to have it there when it's available on PyPI anyway? Yes, the 1.0.0 wheel has been downloaded ~400 times so far, but that's only a tiny fraction compared to the other release assets and stats from PyPI:

The problem is that with the new special wheels for Windows (win32, win-amd64 and cygwin), this will probably confuse many people, because the Windows installer, which is only being uploaded and available there, doesn't have a platform name included in its file name, whereas the new special wheels for Windows do.

Even without these CI changes, the next release page will include 1 platform independent wheel, 3 Windows specific wheels, 1 source archive, 5 pgp signatures and the Windows installer. A total number of 11 files, plus the two archives Github lists of the repo's tagged commit.

@beardypig

This comment has been minimized.

Copy link
Member

beardypig commented Mar 15, 2019

@bastimeyer I'd be in favour of reducing that list down to the Windows installer ;) That's the important one for the GitHub releases, everyone else will use PyPI - although might have to check with the package maintainers :)

@bastimeyer

This comment has been minimized.

Copy link
Member Author

bastimeyer commented Mar 15, 2019

I've left the tarball and its signature file. I think that's fair.

I'll test the deploy configs in a side-repo later today. If nothing goes wrong, this should be good to go.

@bastimeyer

This comment has been minimized.

Copy link
Member Author

bastimeyer commented Mar 15, 2019

Ok, did a bit of testing and learned a couple of things

https://travis-ci.org/bastimeyer-test/streamlink-test/builds/506724579
https://github.com/bastimeyer-test/streamlink-test/releases

  • allow_failures does not work, neither does specifying allow_failure: true on an individual job. This must be set on the global build matrix and all fields must match. I've googled this for at least 30 mins and only found a couple of other people having the same issue. That's why I'm ignoring this for now, since python: 3.8-dev seems to be working fine.
  • All Windows VMs failed in my tests during the dependency install via chocolatey for no apparent reason (I've removed them in my side-repo in the following test-releases). That's probably caused by the different default branch name, but I'm not sure...
    https://travis-ci.org/bastimeyer-test/streamlink-test/builds/506676918
  • Travis does a terrible job at documenting the build stage conditions. There's an issue with the tag and type/branch variables where either type or branch have a different value when tag is set. This needs a fix here. Glad I've tested it.
  • Didn't get doctr to work with the --token option and didn't want to read about creating a doctr deploy key, hence the simple echo commands in order to make the build work. Replaced the gpg signing key and openssl encryption key/iv for the actual release, though, and it's working fine.
@bastimeyer

This comment has been minimized.

Copy link
Member Author

bastimeyer commented Mar 16, 2019

Damn it, the Windows VMs are failing when secure env vars are set:
https://travis-ci.community/t/current-known-issues-please-read-this-before-posting-a-new-topic/264/10
😠 😠 😠

PRs don't have any secure vars, which is the reason why the VMs were working here. @back-to's build config also didn't have any secure env vars.

I guess we'll have to wait for a fix on Travis' side then. Or we could move the secure env vars into the config file itself and then only apply them to the deploy jobs running on Linux. I don't think it's worth it, though, as it would only bloat it up more.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.