Fix toolchain name when a bisect range bound lands on the default nightly's date

[lqd]

As described in https://rust-lang.zulipchat.com/#narrow/stream/217417-t-compiler.2Fcargo-bisect-rustc/topic/default.20nightly, whenever a bisection range bound lands on the date of the default nightly, bisection will fail silently, but blame the error on an inability to reproduce the regression (which is incorrect).

Manually launching the --script would succeed in reproducing the regression: the silent error making reproduction fail is actually a rustup error, as cargo-bisect-rustc is trying to pass one of the non-standard nightly names used for downloaded nightlies, for a nightly it didn't download, the default nightly.

I'm not sure this is the best fix, as I don't know the ramifications elsewhere in the code. I just made the code refer to the nighly with the "nightly" name, and not "bisector-nightly-*" (and manually verified the RUSTUP_TOOLCHAIN env variable using the erroneous case described in the zulip thread above).

[lqd]

@spastorino tells me to try to use the actual toolchain name from rustup, so I'll try to do that instead of hardcoding the name "nightly".

[Mark-Simulacrum]

Presuming this has received some stamping, seems fine. I think the name can only be nightly (I don't think there's a way to have nonstandard toolchain names?).

I'm not entirely sure why this is necessary to be honest :)

But if it works in practice, let's merge!

[lqd]

I think it's unintended fallout from #37, which e.g. avoids installing a nightly you already have but elsewhere still refers to all nightlies using a bisector-* name.

Unfortunately I can't really say if it's "working in practice" all the time: I only tried on my confusingly-failing example, and I always pass complete ranges and use a --script. I wouldn't want to break other use-cases, especially the one implemented in #37.

I wasn't sure we could have non-standard toolchain names for channels either, and I don't think we interact with rustup here where one could find this name out ? (but rather use rustc_version which wouldn't make this information available).

Thinking about it, since this is about differences when using the default toolchain, maybe we also could avoid unconditionally using a +name altogether.

[Mark-Simulacrum]

I don't really have too much time to dig in here, I'm afraid, so I'll have to leave this up to you. I suspect that what we may want to do is change the "don't re-install installed default nightly" to do "rustup link" that nightly to bisector-nightly-FOO and then leave the logic this PR is changing as-is. That should give us the download benefits hopefully...

[spastorino]

@lqd I was wondering, should we apply this as is for now and release meanwhile we find a proper solution?. My feeling is that anything would be better to what we already have :).

[lqd]

It's a bit tough for me to answer the question: I'm not at all familiar with the code base compared to you all, and only have experience with a limited use-case, I wouldn't know how/if this could break other use cases. It does seem to work at least a little, so it may be interesting to merge now. If it does cause issues, however, let me know and I'll find the time to fix them.

[spastorino]

@lqd I was about to merge but ended with doubts. Did we say that the approach @Mark-Simulacrum has mentioned here #45 (comment) doesn't work? or what was the deal with it?.

[lqd]

I'm not sure the feature of avoiding downloading a single toolchain is really worth its complexity + more of our time to fix the bug.

I don't feel the "rustup link" approach is a clear-cut win either, the default nightly doesn't need a "+name" per se, you'd need to deal with more rustup failure points, and so on, maybe we could instead just avoid passing the RUSTUP_TOOLCHAIN env variable for the default nightly toolchain.

If you want me to quickly test that out in another PR, let me know. Otherwise, as mentioned on Zulip, I don't think I'll have the time to fully dig into the "rustup link" approach.

In any case it's clear we shouldn't land this PR as-is, so I'll close it (and won't see my ugly typo on the branch name nghitly :).