Re: [BUG REPORT] `git rebase --exec` shouldn't run the exec command when there is nothing to rebase

[Elijah Newren <newren@gmail.com>]

On Mon, Nov 29, 2021 at 2:25 PM Ævar Arnfjörð Bjarmason
<avarab@gmail.com> wrote:
>
> On Fri, Nov 26 2021, Nikita Bobko wrote:
>
> > Steps:
> > git rebase HEAD --exec "echo foo"
> >
> > EXPECTED: since 0 commits are going to be rebased then I expect "foo"
> > NOT to be printed
> > ACTUAL:   "foo" is printed
>
> I don't think this is a bug, but explicitly desired behavior.

My reading of the docs are such that I'd expect the same as Nikita here:

        Append "exec <cmd>" after each line creating a commit in the final
        history.
        ...
        If --autosquash is used, "exec" lines will not be appended for the
        intermediate commits, and will only appear at the end of each
        squash/fixup series.

There is no line creating a commit in the final history when you do a
git rebase -i --exec "echo foo" HEAD (there is only a noop line), so
there should be no exec line.

> When you do:
>
>     git rebase -x 'make test' BASE
>
> You expect to run 'make test' for all of BASE..HEAD inclusive of
> "base". E.g. for HEAD~1 we'll run 'make test' twice, and you know both
> your HEAD~ and HEAD passed tests.

This is not true.  Try `git rebase -i --exec HEAD~$N` for various
values of N>0.  base is not included.

> So why wouldn't doing the same for HEAD make sense?

Indeed; HEAD is weirdly inconsistent and should be brought in line
with the others.

> That being said perhaps some users would think an option or
> configuration to skip the injection of "exec" after "noop" would make
> sense in that case.
>
> But does this really have anything per-se to do with --exec? Wouldn't
> such an option/configuration be the same as rebase in general dying if
> there's no work to do?
>
> And wouldn't such a thing be more useful than a narrow change to make
> --exec a NOOP in these cases?
>
> E.g. if I've got a "topic" that has commit "A", that's since been
> integrated into my upstream and I have a script to "make test" on my
> topics, won't simply dying (and thus indicating that the topic is
> dead/integrated) be better than noop-ing?

Why do you suggest "dying" rather than early completion with success?

Anyway, rebase does early exit in non-interactive mode when there is
nothing to do, it's just that interactive mode suggests users might
want to do something special, so they get a TODO list containing only
"noop".  Since --exec was written in terms of interactive rebase by
editing the TODO list and inserting an exec command after each of the
picks, and it accidentally always added one at the end of the todo
list even if the last instruction (group) was not a pick/fixup/squash,
we hit this bug.

Anyway, I've got a patch I'll send in.