Re: [PATCH 0/3] rebase: offer to reschedule failed exec commands automatically

From: Stefan Beller <sbeller@google.com>
Date: Dec 10, 2018, 22:56 UTC
Message-ID: <CAGZ79kY18SCaCBvkWyeVd+oeJ4EhoJK4=QxGhJ9a77iX2sR8ew@mail.gmail.com>
In-Reply-To: <d2c317fd-d10a-19c1-8b4f-a7311c69a52f@kdbg.org>

On Mon, Dec 10, 2018 at 2:08 PM Johannes Sixt <j6t@kdbg.org> wrote:

Show 13 quoted lines

>
> Am 10.12.18 um 20:04 schrieb Johannes Schindelin via GitGitGadget:
> > The idea was brought up by Paul Morelle.
> >
> > To be honest, this idea of rescheduling a failed exec makes so much sense
> > that I wish we had done this from the get-go.
>
> The status quo was actually not that bad a decision, because it made 'x
> false' as a substitute for 'break' very convenient.
>
> But now that we have a real 'break', I'm very much in favor of flipping
> the behavior over to rescheduling. (I'm actually not a user of the
> feature, but the proposed behavior is so compellingly logical.)

In rare cases I had commands that may be dangerous if rerun, but I'd just not run them with -y but with -x.

That brings me to some confusion I had in the last patch:
To catch a flaky test I surely would be tempted to
    git rebase -x make -y "make test"
but that might reschedule a compile failure as well,
as a single -y has implications on all other -x's.

I wonder if it might be better to push this mechanism one layer down: Instead of having a flag that changes the behavior of the "exec" instructions and having a handy '-y' short cut for the new mode, we'd rather have a new type of command that executes&retries a command ("exnrt", 'n'), which can still get the '-y' command line flag, but more importantly by having 2 separate sets of commands we'd have one set that is a one-shot, and the other that is retried. Then we can teach the user which is safe and which isn't for rescheduling.

By having two classes, I would anticipate fewer compatibility issues ('"Exec" behaves differently, and I forgot I had turned on the rescheduling').

Talking about rescheduling: In the above example the flaky test can flake more than once, so I'd be stuck with keeping 'git rebase --continue'ing after I see the test flaked once again.

My workflow with interactive rebase and fixing up things as I go always involves a manual final "make test" to check "for real", which I could lose now, which is nice.

Thanks, Stefan