rust-lang / cargo-bisect-rustc

Bisects rustc, either nightlies or CI artifacts
https://rust-lang.github.io/cargo-bisect-rustc/
Apache License 2.0
177 stars 55 forks source link

Add custom "Satisfies" terms #330

Closed apiraino closed 6 months ago

apiraino commented 7 months ago

cargo-bisect-rustc prints "Yes" or "No" on whether or not a build satisfies the condition that it is looking for (default: Yes = reproduces regression, No = does not reproduce the regression). However, this terminology is either confusing or backwards depending on what is being tested.

For example, one can use one of --regress options (f.e. --regress success) to find when a regression was fixed. In that sense, the "old" is "regression found" and the "new" is "regression fixed", which is backwards from the normal behavior.

Taking inspiration from git bisect, we are introducing custom terms for Satisfies. This is implemented with 2 new cli options:

--term-old, will apply for Satisfy::Yes (condition requested is matched) --term-new, will apply for Satisfy::No (condition requested is NOT matched)

This will allow the user to specify their own wording. Then, the --regress option could set the defaults for those terms appropriate for the regression type.

Fixes rust-lang#316

ehuss commented 7 months ago

Nice! I'm not sure if there is more work to do since this is in a Draft state.

One initial comment I have is that it seems like maybe the terms are backwards. My expectation is that this would work pretty much the same as git bisect where the default assumption is that you are looking for when a regression starts (and that is also cargo-bisect-rustc's default). With git bisect, the term "good" is synonymous with "old" which means that it successfully compiles. The term "bad" is synonymous with "new" which means that it fails to compile.

This PR seems to treat "good" meaning it found the regression, which is the opposite of git's behavior.

One of the reasons I suggested in #316 to use the old/new terminology is so that it is less ambiguous with good/bad. Good/bad could mean "good, I found the regression" or "good, it successfully compiles", which are opposites. I think with old/new, there is no ambiguity since there is only one direction of time.

apiraino commented 7 months ago

... since there is only one direction of time

@ehuss Hmm you're right. When I look at it from that point of view it makes def. more sense, sorry for not applying cleanly your suggestion.

Now it should work as expected. Can you check it one more time when you have a sec? Thanks

The draft state was because this patch stlll needs some documentation (I will update the book) and the tests are giving me headaches (the trycmd crate is not updating the .h .out files - looking into it...)

apiraino commented 7 months ago

@ehuss ok now tests are passing.

Feel free to review when you have a sec. I especially want to be sure the documentation explains the logic correctly and clearly.

thanks

apiraino commented 7 months ago

r? @ehuss

when you have a chance