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

From: Stefan Beller <sbeller@google.com>
To: Johannes Sixt <j6t@kdbg.org>
Cc: gitgitgadget@gmail.com, git <git@vger.kernel.org>,
	Junio C Hamano <gitster@pobox.com>
Subject: Re: [PATCH 0/3] rebase: offer to reschedule failed exec commands automatically
Date: Mon, 10 Dec 2018 14:56:47 -0800	[thread overview]
Message-ID: <CAGZ79kY18SCaCBvkWyeVd+oeJ4EhoJK4=QxGhJ9a77iX2sR8ew@mail.gmail.com> (raw)
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:
>
> 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