@matheusbsbs / matheus-bagliano / commits / 2f5ff2c339

rebase -i: fix counting of fixups after rebase --skip

When the sequencer processes a chain of "fixup" and "squash" commands it keeps a list of the commands that have been executed. If there are conflicts, then the list is saved when the rebase stops for the user to resolve them. When the rebase resumes, the list is loaded and is used to initialize the count of how many "fixup" and "squash" commands have been processed; if a command has been skipped with "git rebase --skip", then the last command needs to be popped off the end of the list. To count the number of commands, commit_staged_changes() uses the number of newlines in the file plus one. This is due to the slightly unusual way the list is constructed - instead of appending a newline when a command is added, a newline is inserted before the command if the current count is greater than zero. Therefore, when we pop a skipped command off the list, we should also remove the newline that precedes it. Otherwise, when a new command is added, a blank line will be left before it, which will contribute to the fixup count the next time the file is read. Unfortunately, the preceding newline is not removed, leading to an incorrect count. Fix this by removing the newline that appears before the skipped command. In addition to fixing the code that removes a skipped command from the list, the code that reads the list is fixed to skip blank lines. We have had reports of users starting a rebase with one version of git and continuing it with another. Often this happens because the version of git bundled with an IDE or TUI differs from the one used at the command line. By fixing both the reading and writing ends of the problem we ensure the count is correct when an older version of git reads the fixup file written by a newer version and vice versa. Triggering the incorrect count requires the user to skip two "fixup" or "squash" commands before the final command in the chain. An existing test is extended to prevent future regressions. The consequence of miscounting is not serious: we just print the wrong count in the header of the commit message template. Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Phillip Wood committed Jul 26, 2026 at 16:38 UTC 2f5ff2c339d90422dd4010b5d980d33d1959fdd9

2 files changed +42 -5

sequencer.c

+10 -1

@@ -3281,7 +3281,13 @@ static int read_populate_opts(struct replay_opts *opts)

3281 const char *p = ctx->current_fixups.buf;

3282 ctx->current_fixup_count = 1;

3283 while ((p = strchr(p, '\n'))) {

3284 - ctx->current_fixup_count++;

3284 + /*

3285 + * Older versions of git accidentally

3286 + * inserted blank lines when a fixup

3287 + * was skipped.

3288 + */

3289 + if (p[1] && p[1] != '\n')

3290 + ctx->current_fixup_count++;

3291 p++;

3292 }

3293 }

@@ -5354,6 +5360,9 @@ static int commit_staged_changes(struct repository *r,

5360 BUG("Incorrect current_fixups:\n%s", p);

5361 while (len && p[len - 1] != '\n')

5362 len--;

5363 + /* Remove trailing newline */

5364 + if (len)

5365 + len--;

5366 strbuf_setlen(&ctx->current_fixups, len);

5367 if (write_message(p, len, rebase_path_current_fixups(),

5368 0) < 0) {

t/t3418-rebase-continue.sh

+32 -4

@@ -134,6 +134,7 @@ test_expect_success '--skip after failed fixup cleans commit message' '

134 EOF

135

136 : skip and continue &&

137 + test_config commit.status false &&

138 echo "cp \"\$1\" .git/copy.txt" | write_script copy-editor.sh &&

139 (test_set_editor "$PWD/copy-editor.sh" && git rebase --skip) &&

140

@@ -145,7 +146,8 @@ test_expect_success '--skip after failed fixup cleans commit message' '

146

147 : now, let us ensure that "squash" is handled correctly &&

148 git reset --hard wants-fixup-3 &&

148 - test_must_fail env FAKE_LINES="1 squash 2 squash 1 squash 3 squash 1" \

149 + test_must_fail env \

150 + FAKE_LINES="1 squash 2 squash 1 squash 3 squash 1 squash 4 squash 1" \

151 git rebase -i HEAD~4 &&

152

153 : the second squash failed, but there are two more in the chain &&

@@ -171,6 +173,32 @@ test_expect_success '--skip after failed fixup cleans commit message' '

173 fixup 2

174 EOF

175

176 + (test_set_editor "$PWD/copy-editor.sh" &&

177 + test_must_fail git rebase --skip) &&

178 + : not the final squash, no need to edit the commit message &&

179 + test_path_is_missing .git/copy.txt &&

180 +

181 + : The first, third and fifth squashes succeeded, therefore: &&

182 + cat >expect <<-\EOF &&

183 + # This is a combination of 4 commits.

184 + # This is the 1st commit message:

185 +

186 + wants-fixup

187 +

188 + # This is the commit message #2:

189 +

190 + fixup 1

191 +

192 + # This is the commit message #3:

193 +

194 + fixup 2

195 +

196 + # This is the commit message #4:

197 +

198 + fixup 3

199 + EOF

200 + test_commit_message HEAD expect &&

201 +

202 (test_set_editor "$PWD/copy-editor.sh" && git rebase --skip) &&

203 test_commit_message HEAD <<-\EOF &&

204 wants-fixup

@@ -178,12 +206,12 @@ test_expect_success '--skip after failed fixup cleans commit message' '

206 fixup 1

207

208 fixup 2

209 +

210 + fixup 3

211 EOF

212

213 : Final squash failed, but there was still a squash &&

184 - head -n1 .git/copy.txt >first-line &&

185 - test_grep "# This is a combination of 3 commits" first-line &&

186 - test_grep "# This is the commit message #3:" .git/copy.txt

214 + test_cmp expect .git/copy.txt

215 '

216

217 test_expect_success 'setup rerere database' '