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
A real-world example where such behavior is really unexpected - I have
a script that signs a series of commits like this:
head="$(git rev-parse HEAD)"
# Generate some commits (in my case it's cherry-picking commits from
public GitHub subtree repo to internal monorepo)
file="$(mktemp)"
git rebase $head --exec "git log -1 --pretty='%B' > $file" \
--exec "echo 'closes
'https://github.com/JetBrains/intellij-community/pull/$1' >> $file" \
--exec "git commit --amend -F $file"
But if none of the commits were generated then `git rebase --exec`
will amend the HEAD which is not expected. It means that I have to
process the case when 0 zero commits are generated separately.
If you agree that it's a bug then, most likely, it won't be possible
to fix it because it would break compatibility. Well, yes then this
bug report is JFYI.
[System Info]
git version:
git version 2.33.0
cpu: x86_64
no commit associated with this build
sizeof-long: 8
sizeof-size_t: 8
shell-path: /bin/sh
uname: Linux 5.4.98-1-lts #1 SMP Sat, 13 Feb 2021 19:22:14 +0000 x86_64
compiler info: gnuc: 11.1
libc info: glibc: 2.33
$SHELL (typically, interactive shell): /bin/zsh
[Enabled Hooks]
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. 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.
So why wouldn't doing the same for HEAD make sense?
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?
On Mon, Nov 29, 2021 at 2:25 PM Ævar Arnfjörð Bjarmason
[off-list ref] wrote:
On Fri, Nov 26 2021, Nikita Bobko wrote:
quoted
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.
On Mon, Nov 29, 2021 at 04:14:33PM -0800, Elijah Newren wrote:
On Mon, Nov 29, 2021 at 2:25 PM Ævar Arnfjörð Bjarmason
[off-list ref] wrote:
quoted
On Fri, Nov 26 2021, Nikita Bobko wrote:
quoted
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.
Thanks for quoting the docs here. When I ran this myself, I thought that
the docs must say something like "after every line" and not further
specify "... creating a commit".
But they do, so I agree with the original report from Nikita that
git rebase -x 'echo foo' HEAD
should be silent in order to be consistent with the docs.
quoted
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.
quoted
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.
Yep.
Thanks,
Taylor
On Mon, Nov 29, 2021 at 4:14 PM Elijah Newren [off-list ref] wrote:
On Mon, Nov 29, 2021 at 2:25 PM Ævar Arnfjörð Bjarmason
[off-list ref] wrote:
quoted
On Fri, Nov 26 2021, Nikita Bobko wrote:
quoted
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.
quoted
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.
quoted
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.
quoted
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.
Found over here:
https://lore.kernel.org/git/pull.1149.git.git.1638244719381.gitgitgadget@gmail.com/
On 29/11/2021 12:07, Ævar Arnfjörð Bjarmason wrote:
On Fri, Nov 26 2021, Nikita Bobko wrote:
quoted
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. 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.
I don't think we run 'make test' for base in that case, only after each
pick and base is not picked by the rebase.
Best Wishes
Phillip
So why wouldn't doing the same for HEAD make sense?
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?