Linkcheck only changed files (except for cron jobs)

[mark-i-m]

Closes #886

In this PR, we change the CI setup to only check files that have changed in the
most recent git commit. This means that changes in PRs are less likely to get
blocked on unrelated breakage (it's still possible if that breakage happens to
be in the same file).

On the other hand, we do want to periodically check all files. Currently, there
is a cron job that runs every day. We change the CI script to check if the
Travis run is a cron job and if so do a full link check. The hope is that by
having the cron job run often enough and preserving the linkcheck cache, we can
continue to find breakage relatively soon while avoiding the 429 Too Many
Requests issues we've been seeing.

This is currently blocked on a new mdbook-linkcheck release.

[camelid]

$ cargo install mdbook-linkcheck --version '^0.7.1'

    Updating crates.io index

error: could not find `mdbook-linkcheck` in registry `https://github.com/rust-lang/crates.io-index` with version `^0.7.1`

The command "cargo install mdbook-linkcheck --version '^0.7.1'" failed and exited with 101 during .

Are we waiting on an mdbook-linkcheck release?

[mark-i-m]

@camelid Yes, and also good catch on the endpoint... I misread the docs.

[camelid]

We could use a commit hash version of mdbook-linkcheck until there's a point release; that way we can test to make sure the setup is working properly. What do you think?

[mark-i-m]

Hmm... somehow we are still messing up the git commit ranges, so there is something I don't understand. However, hopefully the current version works correctly.

[mark-i-m]

@jyn514 @camelid The latest commit pulls out the files I would expect it to... Now the question is: is it correct? You both suggested simpler alternatives. Could you explain why they are more correct than what we have now?

[camelid]

git diff is the plain diff command, and we're trying to do diffing, so that seems correct. Also it works locally for me, while diff-tree does not (it did before though...). Also, you should have three dots ... so that it does it from the merge-base instead of from the tip of master. jyn514's recommendation should work as well, but it's more verbose.

[mark-i-m]

Indeed, it does seem to work properly this way.

[mark-i-m]

Ok, I've squashed... I'm actually ok with just merging this, even though it will cause us to use master mdbook-linkcheck.

[camelid]

Do you want to pin to a particular commit though?

[mark-i-m]

Ah, yeah, that's a good idea.

[camelid]

I think this is ready! 🎉

[LeSeulArtichaut]

This is clear improvement for the CI workflow, though it would be awesome if we could somehow get rid of GitHub's ratelimits.
I don't really feel comfortable approving the shell script though, I'd prefer if someone else reviews it

[LeSeulArtichaut]

@mark-i-m @camelid If you are confident the script works I think we should merge this

[camelid]

I wouldn't say I'm "confident" - more than I'm pretty sure it'll work, and we have a regular cronjob to check all the links anyway. Maybe @jyn514 could review the script?

[camelid]

Also: worst-case scenario, we have a couple broken links :)

[LeSeulArtichaut]

Right. I also think this is pretty low-risk, but since we have very few PRs open right now, this can wait few hours/days for someone else to review

[jyn514]

error: unexpected state: COMMIT_RANGE must be non-empty in CI

I'm glad we added the assert! Not sure why it's failing though.

[jyn514]

Thanks so much for tackling this!