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

Update our CI infrastructure? #387

Closed
azerupi opened this issue Aug 2, 2017 · 14 comments
Closed

Update our CI infrastructure? #387

azerupi opened this issue Aug 2, 2017 · 14 comments
Labels
A-Infrastructure Area: CI, Releases E-Medium Experience: Medium

Comments

@azerupi
Copy link
Contributor

azerupi commented Aug 2, 2017

A long time ago, Japaric contributed most of the Travis and AppVeyor configuration files to setup automatic deployments of release artefacts etc.

I think we should check if everything is still working fine and up to date. We can use his trust repository for this. It is a template for a fully configured CI using Travis and AppVeyor.

@azerupi azerupi added A-Infrastructure Area: CI, Releases E-Medium Experience: Medium T-Update labels Aug 2, 2017
@steveklabnik
Copy link
Member

I have a feeling it's way more complicated than it has to be; I've looked at it before and then been like "This is too complicated, I'll work on something else."

Maybe @azerupi for this thread we can enumerate what we want in our CI infra? That way we actually know what we're trying to evaluate.

@budziq
Copy link
Contributor

budziq commented Aug 2, 2017

I would suggest at least:

I'm not sure if we need to keep al these variants in Travis and Appveyor. I guess 9 could be enough (Linux+MacOS+Win) x (Stable, Beta, Nightly)

@azerupi
Copy link
Contributor Author

azerupi commented Aug 2, 2017

I have a feeling it's way more complicated than it has to be

Our infra or trust? Do you have any specifics about what you find too complicated and would remove?

To be honest, I have never been really comfortable with all the CI scripts and configurations. They were contributed by other people (mostly Japaric) and I never bothered to change them because they were working great. 😄

If you are talking about the build matrix, I agree. I don't think we need to run tests on so many platform-channel combinations. We could probably leave out the beta channel altogether.

From my point of view, here are the requirements:

  1. Test every PR (build+tests) this is the most important test so I would do all the platforms and at least stable + nightly. I would keep nightly because it helps us report regressions and get ahead of them if they won't be fixed. A recent example is: std::Command::new fails for some CMD scripts on Windows 10 msvc nightly 1.19/1.20 rust#42791
  2. Test commits to master (build+tests) this one is becoming less important because I don't often push directly to master anymore. But we can't just remove them because a passing PR does not necessarily mean that it will pass when merged (if something changed in master between the tests and the merge)
  3. Push to gh-pages, should happen only when testing a commit to master and only on one of the build configurations when the tests passed
  4. Generate pre-built binaries when releasing a new version (pushing a new tag) this is used for different purposes (speed up CI for other projects, package managers, ...)

@steveklabnik
Copy link
Member

Our infra or trust?

Our infra. trust is generally pretty good 😄

If you are talking about the build matrix, I agree. I don't think we need to run tests on so many platform-channel combinations. We could probably leave out the beta channel altogether.

Yeah, the build matrix was part of it.

It's mostly just that there is a lot there, including three whole shell scripts, which includes stuff like https://github.com/azerupi/mdBook/blob/master/ci/install.sh which shouldn't be needed anymore at all?

These requirements look 👍 to me.

@azerupi
Copy link
Contributor Author

azerupi commented Aug 2, 2017

which shouldn't be needed anymore at all?

Indeed, that is very probable. It was added a year ago when rustup was just emerging. There are probably better ways to do it now, that is why I linked to trust as a source of inspiration.

@azerupi
Copy link
Contributor Author

azerupi commented Sep 5, 2017

I pushed some changes to a new branch ci-infra where I started reworking the Travis config. We might want to merge this quickly because Travis is currently unusable.

But I had an idea for improvement:
For PRs and pushes we could run the tests only on stable and have a daily and weekly cron job for nightly and beta respectively.

It should be possible, but I am not sure how to implement this with Travis. If any Travis wizard knows how to do this, please leave a comment or PR 😉

@steveklabnik
Copy link
Member

I think this line needs a && after? https://github.com/azerupi/mdBook/compare/ci-infra#diff-354f30a63fb0907d4ad57269548329e3R23

I am not sure how to implement this with Travis.

https://docs.travis-ci.com/user/cron-jobs/ is the general docs; I'm not 100% sure how to build it though....

@steveklabnik
Copy link
Member

Looks like this may not be possible today travis-ci/travis-ci#7181

@azerupi
Copy link
Contributor Author

azerupi commented Sep 5, 2017

Looks like this may not be possible today

Ahn too bad 😢
It would have been very useful to speed up the tests I think.

I think this line needs a && after?

Indeed, good catch :)

@budziq
Copy link
Contributor

budziq commented Sep 5, 2017

Looks like this may not be possible today

It might not be possible to express it in the yml file alone but it would be perfectly possible with shellscript and env vars exported in matrix (also travis exports "${TRAVIS_EVENT_TYPE:-}" == "cron" )

We have done something along these lines in rust-cookbook

@azerupi
Copy link
Contributor Author

azerupi commented Sep 5, 2017

You can do that, but I was talking about the build matrix. You can't spawn more or less builds (for different rust versions) depending on if it is a regular PR / commit or cron job. At least that is how I understand it.

@budziq
Copy link
Contributor

budziq commented Sep 5, 2017

You can't spawn more or less builds (for different rust versions) depending on if it is a regular PR / commit or cron job

Right, but we can always spawn maximal number of builds and return early from the inconsequential ones. Or we can manually test few build variants in one session using rustup

@azerupi
Copy link
Contributor Author

azerupi commented Sep 9, 2017

Ok Travis has been revamped. It is very simple and doc / artefact deployment still has to be tested.
Next we should do the same for AppVeyor, I also noticed that AppVeyor does not do artefact deployment anymore so we should fix that.

@Michael-F-Bryan
Copy link
Contributor

I believe #516 fixes the last of this. Travis now tests all commits and PRs, plus it uploads the rendered user guide to github pages whenever a commit is made to master.

Appveyor is still a problem though... The best long-term solution would be to give me access to the rust-lang-nursery/mdBook build on Appveyor so I can sort out our Windows builds without having to go through the core team. Lets move Appveyor discussion to #482 though.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Infrastructure Area: CI, Releases E-Medium Experience: Medium
Projects
None yet
Development

No branches or pull requests

4 participants