Verify that only non-technical breaking changes are applied to libc

[gnzlbg]

Closes #270 .

cc @alexcrichton so this would be a solution to #270 that uses rust-semverver to check that the API of libc contains only non-technical breaking changes.

This is a WIP and uses a fork of rust-semverver for now, but I've sent PRs upstream already. This is the only idea I have for solving #270 . rust-semverver is not perfect, but it can deal with functions, consts, and simple structs just fine, and that's pretty much everything that libc uses.

cc @ibabushkin

Some other notes:

• we have to compile rust-semverver for each toolchain version, and it depends on cargo so we have to build ~160 dependencies. Using cache: cargo breaks everything.

[rust-highfive]

r? @alexcrichton

(rust_highfive has picked a reviewer for you, use r? to override)

[gnzlbg]

So I've made a commit that does an API breaking change, and the error messages are pretty good: https://travis-ci.org/rust-lang/libc/jobs/460316258#L384

version bump: 0.2.44 -> (breaking) -> 0.2.45
error: breaking changes in `tcflow`
    --> /Users/travis/build/rust-lang/libc/src/unix/mod.rs:1092:5
     |
1092 |     pub fn tcflow(fd: ::c_uint, action: ::c_int) -> ::c_int;
     |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
     |
     = warning: type error: expected i32, found u32 (breaking)
error: aborting due to previous error

I still need to fix a couple of issues with rust-semverver for this case:

• the version bump is incorrect: it should return 0.3.
• the return value is incorrect: cargo semver returns 0 (success) but since this is a breaking change it should probably return failure.
• it would be nicer if the error message would point to the exact place where the breaking change occurred: fd: ::c_uint in this case.

I'm closing the PR until I fix at least the return value and the version bump.

[alexcrichton]

Wow this seems quite promising though!

[gnzlbg]

The main problem that I don't know how to solve is that this requires compiling cargo as a library, which takes forever (~7 min of CI time on travis) and we can't cache it. Maybe there is a way to perform this only when bors is trying to do the merge, but not do it while people are working on PRs ?

[gnzlbg]

FYI this is still just a proof-of-concept test, but I think I fixed all of the blocking bugs in rust-semverver and rustc to make this reliable enough for production usage.

[gnzlbg]

So the rust-semverver tests that passed for libc did so because they were checking old enough libc versions that did not use a build.rs. Still need to add support for projects with build.rs here.

[gnzlbg]

Still need to add support for projects with build.rs here.

So I've added support for build.rss to `rust-semverver, and it appears that things are working now.

---

@semarie we can only detect semver breaking API changes for targets that ship a standard component with rustup. AFAICT the openbsd targets don't do that yet. I think it is now possible to ship a libcore only component with rustup, and that would be enough to run rust-semverver on them (it would also allow us to do some tests using cargo instead of xargo).

[gnzlbg]

I've rebased to something that we could merge @alexcrichton . This PR uses rust-semverver to compare the PR against the latest crates.io release of libc. If there is a semver API breaking change, it errors. If there is a change that requires updating the patch version, it succeeds instead of erroring that the patch version should be increased (we'll do that for a new release anyways).

Merging this is blocked on the review and merge of my PR to rust-semverver upstream, updating this PR to use a rust-semverver release containing those changes, and clarifying the maintenance of rust-semverver.

Right now there is only one maintainer, since the repo was migrated out of the nursery its CI has stopped working, and nightly changes does break this every now and then (just like clippy and rustfmt). So we need to clarify all of this. The tool is not perfect, but it is otherwise quite great.

In any case, I've set the build jobs as allowed to fail. We can check to see if they fail, and if so, whether the failure is spurious, or an API breaking change was introduced. Also, we do want to make breaking changes every now and then, what we don't want is to make them by mistake, so it appears to me that allowing these to fail by default is what we would do anyways.

[alexcrichton]

Sounds like a good plan to me!

[gnzlbg]

semverver was released with the fixes!

@bors: r+

[bors]

📌 Commit d6443f7 has been approved by gnzlbg

[bors]

⌛ Testing commit d6443f7 with merge 1236fed...

[bors]

☀️ Test successful - checks-cirrus, checks-travis, status-appveyor
Approved by: gnzlbg
Pushing 1236fed to master...