Compiletest no longer recognizes the rustc exit code on nightly

[dtolnay]

rust-lang/rust#52197 changed the exit code of rustc from 101 to just 1 in the event of compile errors. This check needs to be updated.

Tests that used to work as of nightly-2018-07-19 now in nightly-2018-07-22 fail with the following output:

error: failure produced the wrong error: exit code: 1
status: exit code: 1

[SergioBenitez]

@laumann, @Manishearth Let's get a new release out with a7d5757

[laumann]

@SergioBenitez I'm putting together 0.3.11 now.

[laumann]

Of course, I meant 0.3.12

[JeanMertz]

This seems like a breaking change for the stable feature, isn't it?

I've written several tests locally, they all work as expected on my nightly toolchain, but the CI was failing.

I added features = ["stable"] to check if things would work on stable, but immediately all the tests failed with:

failure produced the wrong error: exit code: 101

[laumann]

@JeanMertz This was discussed recently in #124. The short summary is that supporting stable is something that'd be nice to do, but it's becoming harder to maintain.

The switch from error code 101 to 1 on nightly means even more maintenance for the stable feature, and I see that as just the beginning of an escalating headache.

But short answer, yes and sorry, the stable feature should probably be regarded as broken since 0.3.12.

[JeanMertz]

Ouch, okay.

I get where you're coming from. As a new Rustacean, it's quite annoying to see all these awesome tools out here that "force" you to use nightly to use them. That's not a complaint towards you, but to the ecosystem in general. I also see the upsides of that choice (combining both stability with the rapid experimentation on nightly), but it's still annoying to deal with (as it is as a maintainer as well, I'm sure).

[Manishearth]

It's improving -- RLS and Clippy will soon be available on stable via rustup. As for this tool, there aren't any plans, but perhaps once the custom test framework stuff happens?

[laumann]

@JeanMertz Well, I'm also lazy :-) And I understand your frustration. This project exists primarily as a Ctrl-C Ctrl-V from rustc/compiletest. I just wanted to make compiletest available outside of rustc somehow.

Ideally this project should be independent from rustc's version (I believe @SergioBenitez has made that argument before) and work on both stable and nightly. Maybe even targeting stable as the primary usecase would be the best. It'd be fair to call it a hack, but as it turns out a useful hack.

But I honestly don't actively develop this as much (because "development" really just means merging in the latest changes from rustc/compiletest). I'm very glad to take updates and proposed changes though!

[JeanMertz]

But I honestly don't actively develop this as much (because "development" really just means merging in the latest changes from rustc/compiletest). I'm very glad to take updates and proposed changes though!

Correct me if I'm wrong, but if I made a change that basically pulled out the current hard-coded 1 exit code integer into its own function, and add a second (equally named) function that returns 101, and then have either one enabled based on if you've enabled the stable feature, would at least make this problem go away, correct?

Is that a PR you'd be willing to take?

edit:

I see that it's defined as a constant, so that would have to be changed, but I don't think that would be an issue.

I could also copy the check_correct_failure_status implementation from both nightly and stable, and use the correct one based on the stable feature flag.

[laumann]

Maybe, I'd need some convincing arguments though :-) @fpoli actually made that specific change in #120 (see #120 (comment)) and I had him roll it back.

But I don't know if we should just roll with it for now? When the current nightly becomes stable, then the exit code should also "stabilize" to 1, right?

[JeanMertz]

Maybe, I'd need some convincing arguments though :-) @fpoli actually made that specific change in #120 (see #120 (comment)) and I had him roll it back.

Ah right, I missed that (collapsed) comment. I see your point about maintainability.

But I don't know if we should just roll with it for now? When the current nightly becomes stable, then the exit code should also "stabilize" to 1, right?

I'm not sure what determines when something migrates to stable, is it a given that the exit code change will be part of the next release?

Either way, I'm fine rolling with it for now and only run the compiletest test suites on nightly for now 👍

[laumann]

OK, thanks. (Re-reading my previous comment, I meant for us to roll with the added maintenance complexity and just make the change to support exit code 101 on stable :-) Sorry if it came off as snarky.)

If you want, we could make a PR to bring back stable support (just this return code stuff), and I'll try to get people's opinion on it? If more people want it, I see no harm in merging it.

[JeanMertz]

OK, thanks. (Re-reading my previous comment, I meant for us to roll with the added maintenance complexity and just make the change to support exit code 101 on stable :-) Sorry if it came off as snarky.)

Re-reading your comment, I now realise I took it the wrong way 😓

If you want, we could make a PR to bring back stable support (just this return code stuff), and I'll try to get people's opinion on it? If more people want it, I see no harm in merging it.

Thanks for the offer, I suggest we wait for the next stable release, and see if the new exit code becomes stable with that release. If not, I'll open up a PR with the required change.

Until then, I'll just run the compiletest tests on nightly builds, and the regular tests on stable.

[laumann]

OK, we'll do that. 👍