Skip to content

Commit 93a7bc8

Browse files
vdyegitster
authored andcommitted
rebase --update-refs: avoid unintended ref deletion
In b3b1a21 (sequencer: rewrite update-refs as user edits todo list, 2022-07-19), the 'todo_list_filter_update_refs()' step was added to handle the removal of 'update-ref' lines from a 'rebase-todo'. Specifically, it removes potential ref updates from the "update refs state" if a ref does not have a corresponding 'update-ref' line. However, because 'write_update_refs_state()' will not update the state if the 'refs_to_oids' list was empty, removing *all* 'update-ref' lines will result in the state remaining unchanged from how it was initialized (with all refs' "after" OID being null). Then, when the ref update is applied, all refs will be updated to null and consequently deleted. To fix this, delete the 'update-refs' state file when 'refs_to_oids' is empty. Additionally, add a tests covering "all update-ref lines removed" cases. Reported-by: herr.kaste <[email protected]> Helped-by: Phillip Wood <[email protected]> Helped-by: Derrick Stolee <[email protected]> Signed-off-by: Victoria Dye <[email protected]> Signed-off-by: Taylor Blau <[email protected]>
1 parent e7e5c6f commit 93a7bc8

File tree

2 files changed

+113
-3
lines changed

2 files changed

+113
-3
lines changed

sequencer.c

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4130,11 +4130,14 @@ static int write_update_refs_state(struct string_list *refs_to_oids)
41304130
struct string_list_item *item;
41314131
char *path;
41324132

4133-
if (!refs_to_oids->nr)
4134-
return 0;
4135-
41364133
path = rebase_path_update_refs(the_repository->gitdir);
41374134

4135+
if (!refs_to_oids->nr) {
4136+
if (unlink(path) && errno != ENOENT)
4137+
result = error_errno(_("could not unlink: %s"), path);
4138+
goto cleanup;
4139+
}
4140+
41384141
if (safe_create_leading_directories(path)) {
41394142
result = error(_("unable to create leading directories of %s"),
41404143
path);

t/t3404-rebase-interactive.sh

Lines changed: 107 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1964,6 +1964,113 @@ test_expect_success 'respect user edits to update-ref steps' '
19641964
test_cmp_rev HEAD refs/heads/no-conflict-branch
19651965
'
19661966

1967+
test_expect_success '--update-refs: all update-ref lines removed' '
1968+
git checkout -b test-refs-not-removed no-conflict-branch &&
1969+
git branch -f base HEAD~4 &&
1970+
git branch -f first HEAD~3 &&
1971+
git branch -f second HEAD~3 &&
1972+
git branch -f third HEAD~1 &&
1973+
git branch -f tip &&
1974+
1975+
test_commit test-refs-not-removed &&
1976+
git commit --amend --fixup first &&
1977+
1978+
git rev-parse first second third tip no-conflict-branch >expect-oids &&
1979+
1980+
(
1981+
set_cat_todo_editor &&
1982+
test_must_fail git rebase -i --update-refs base >todo.raw &&
1983+
sed -e "/^update-ref/d" <todo.raw >todo
1984+
) &&
1985+
(
1986+
set_replace_editor todo &&
1987+
git rebase -i --update-refs base
1988+
) &&
1989+
1990+
# Ensure refs are not deleted and their OIDs have not changed
1991+
git rev-parse first second third tip no-conflict-branch >actual-oids &&
1992+
test_cmp expect-oids actual-oids
1993+
'
1994+
1995+
test_expect_success '--update-refs: all update-ref lines removed, then some re-added' '
1996+
git checkout -b test-refs-not-removed2 no-conflict-branch &&
1997+
git branch -f base HEAD~4 &&
1998+
git branch -f first HEAD~3 &&
1999+
git branch -f second HEAD~3 &&
2000+
git branch -f third HEAD~1 &&
2001+
git branch -f tip &&
2002+
2003+
test_commit test-refs-not-removed2 &&
2004+
git commit --amend --fixup first &&
2005+
2006+
git rev-parse first second third >expect-oids &&
2007+
2008+
(
2009+
set_cat_todo_editor &&
2010+
test_must_fail git rebase -i \
2011+
--autosquash --update-refs \
2012+
base >todo.raw &&
2013+
sed -e "/^update-ref/d" <todo.raw >todo
2014+
) &&
2015+
2016+
# Add a break to the end of the todo so we can edit later
2017+
echo "break" >>todo &&
2018+
2019+
(
2020+
set_replace_editor todo &&
2021+
git rebase -i --autosquash --update-refs base &&
2022+
echo "update-ref refs/heads/tip" >todo &&
2023+
git rebase --edit-todo &&
2024+
git rebase --continue
2025+
) &&
2026+
2027+
# Ensure first/second/third are unchanged, but tip is updated
2028+
git rev-parse first second third >actual-oids &&
2029+
test_cmp expect-oids actual-oids &&
2030+
test_cmp_rev HEAD tip
2031+
'
2032+
2033+
test_expect_success '--update-refs: --edit-todo with no update-ref lines' '
2034+
git checkout -b test-refs-not-removed3 no-conflict-branch &&
2035+
git branch -f base HEAD~4 &&
2036+
git branch -f first HEAD~3 &&
2037+
git branch -f second HEAD~3 &&
2038+
git branch -f third HEAD~1 &&
2039+
git branch -f tip &&
2040+
2041+
test_commit test-refs-not-removed3 &&
2042+
git commit --amend --fixup first &&
2043+
2044+
git rev-parse first second third tip no-conflict-branch >expect-oids &&
2045+
2046+
(
2047+
set_cat_todo_editor &&
2048+
test_must_fail git rebase -i \
2049+
--autosquash --update-refs \
2050+
base >todo.raw &&
2051+
sed -e "/^update-ref/d" <todo.raw >todo
2052+
) &&
2053+
2054+
# Add a break to the beginning of the todo so we can resume with no
2055+
# update-ref lines
2056+
echo "break" >todo.new &&
2057+
cat todo >>todo.new &&
2058+
2059+
(
2060+
set_replace_editor todo.new &&
2061+
git rebase -i --autosquash --update-refs base &&
2062+
2063+
# Make no changes when editing so update-refs is still empty
2064+
cat todo >todo.new &&
2065+
git rebase --edit-todo &&
2066+
git rebase --continue
2067+
) &&
2068+
2069+
# Ensure refs are not deleted and their OIDs have not changed
2070+
git rev-parse first second third tip no-conflict-branch >actual-oids &&
2071+
test_cmp expect-oids actual-oids
2072+
'
2073+
19672074
test_expect_success '--update-refs: check failed ref update' '
19682075
git checkout -B update-refs-error no-conflict-branch &&
19692076
git branch -f base HEAD~4 &&

0 commit comments

Comments
 (0)