Skip to content

Commit b3b1a21

Browse files
derrickstoleegitster
derrickstolee
authored and
gitster
committed
sequencer: rewrite update-refs as user edits todo list
An interactive rebase provides opportunities for the user to edit the todo list. The --update-refs option initializes the list with some 'update-ref <ref>' steps, but the user could add these manually. Further, the user could add or remove these steps during pauses in the interactive rebase. Add a new method, todo_list_filter_update_refs(), that scans a todo_list and compares it to the stored update-refs file. There are two actions that can happen at this point: 1. If a '<ref>/<before>/<after>' triple in the update-refs file does not have a matching 'update-ref <ref>' command in the todo-list _and_ the <after> value is the null OID, then remove that triple. Here, the user removed the 'update-ref <ref>' command before it was executed, since if it was executed then the <after> value would store the commit at that position. 2. If a 'update-ref <ref>' command in the todo-list does not have a matching '<ref>/<before>/<after>' triple in the update-refs file, then insert a new one. Store the <before> value to be the current OID pointed at by <ref>. This is handled inside of the init_update_ref_record() helper method. We can test that this works by rewriting the todo-list several times in the course of a rebase. Check that each ref is locked or unlocked for updates after each todo-list update. We can also verify that the ref update fails if a concurrent process updates one of the refs after the rebase process records the "locked" ref location. To help these tests, add a new 'set_replace_editor' helper that will replace the todo-list with an exact file. Reported-by: Phillip Wood <[email protected]> Signed-off-by: Derrick Stolee <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
1 parent 89fc0b5 commit b3b1a21

File tree

5 files changed

+268
-0
lines changed

5 files changed

+268
-0
lines changed

‎rebase-interactive.c

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -146,6 +146,12 @@ int edit_todo_list(struct repository *r, struct todo_list *todo_list,
146146
return -4;
147147
}
148148

149+
/*
150+
* See if branches need to be added or removed from the update-refs
151+
* file based on the new todo list.
152+
*/
153+
todo_list_filter_update_refs(r, new_todo);
154+
149155
return 0;
150156
}
151157

‎sequencer.c

Lines changed: 96 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4168,6 +4168,102 @@ static int write_update_refs_state(struct string_list *refs_to_oids)
41684168
return result;
41694169
}
41704170

4171+
/*
4172+
* Parse the update-refs file for the current rebase, then remove the
4173+
* refs that do not appear in the todo_list (and have not had updated
4174+
* values stored) and add refs that are in the todo_list but not
4175+
* represented in the update-refs file.
4176+
*
4177+
* If there are changes to the update-refs list, then write the new state
4178+
* to disk.
4179+
*/
4180+
void todo_list_filter_update_refs(struct repository *r,
4181+
struct todo_list *todo_list)
4182+
{
4183+
int i;
4184+
int updated = 0;
4185+
struct string_list update_refs = STRING_LIST_INIT_DUP;
4186+
4187+
sequencer_get_update_refs_state(r->gitdir, &update_refs);
4188+
4189+
/*
4190+
* For each item in the update_refs list, if it has no updated
4191+
* value and does not appear in the todo_list, then remove it
4192+
* from the update_refs list.
4193+
*/
4194+
for (i = 0; i < update_refs.nr; i++) {
4195+
int j;
4196+
int found = 0;
4197+
const char *ref = update_refs.items[i].string;
4198+
size_t reflen = strlen(ref);
4199+
struct update_ref_record *rec = update_refs.items[i].util;
4200+
4201+
/* OID already stored as updated. */
4202+
if (!is_null_oid(&rec->after))
4203+
continue;
4204+
4205+
for (j = 0; !found && j < todo_list->total_nr; j++) {
4206+
struct todo_item *item = &todo_list->items[j];
4207+
const char *arg = todo_list->buf.buf + item->arg_offset;
4208+
4209+
if (item->command != TODO_UPDATE_REF)
4210+
continue;
4211+
4212+
if (item->arg_len != reflen ||
4213+
strncmp(arg, ref, reflen))
4214+
continue;
4215+
4216+
found = 1;
4217+
}
4218+
4219+
if (!found) {
4220+
free(update_refs.items[i].string);
4221+
free(update_refs.items[i].util);
4222+
4223+
update_refs.nr--;
4224+
MOVE_ARRAY(update_refs.items + i, update_refs.items + i + 1, update_refs.nr - i);
4225+
4226+
updated = 1;
4227+
i--;
4228+
}
4229+
}
4230+
4231+
/*
4232+
* For each todo_item, check if its ref is in the update_refs list.
4233+
* If not, then add it as an un-updated ref.
4234+
*/
4235+
for (i = 0; i < todo_list->total_nr; i++) {
4236+
struct todo_item *item = &todo_list->items[i];
4237+
const char *arg = todo_list->buf.buf + item->arg_offset;
4238+
int j, found = 0;
4239+
4240+
if (item->command != TODO_UPDATE_REF)
4241+
continue;
4242+
4243+
for (j = 0; !found && j < update_refs.nr; j++) {
4244+
const char *ref = update_refs.items[j].string;
4245+
4246+
found = strlen(ref) == item->arg_len &&
4247+
!strncmp(ref, arg, item->arg_len);
4248+
}
4249+
4250+
if (!found) {
4251+
struct string_list_item *inserted;
4252+
struct strbuf argref = STRBUF_INIT;
4253+
4254+
strbuf_add(&argref, arg, item->arg_len);
4255+
inserted = string_list_insert(&update_refs, argref.buf);
4256+
inserted->util = init_update_ref_record(argref.buf);
4257+
strbuf_release(&argref);
4258+
updated = 1;
4259+
}
4260+
}
4261+
4262+
if (updated)
4263+
write_update_refs_state(&update_refs);
4264+
string_list_clear(&update_refs, 1);
4265+
}
4266+
41714267
static int do_update_ref(struct repository *r, const char *refname)
41724268
{
41734269
struct string_list_item *item;

‎sequencer.h

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -133,6 +133,18 @@ void todo_list_release(struct todo_list *todo_list);
133133
const char *todo_item_get_arg(struct todo_list *todo_list,
134134
struct todo_item *item);
135135

136+
/*
137+
* Parse the update-refs file for the current rebase, then remove the
138+
* refs that do not appear in the todo_list (and have not had updated
139+
* values stored) and add refs that are in the todo_list but not
140+
* represented in the update-refs file.
141+
*
142+
* If there are changes to the update-refs list, then write the new state
143+
* to disk.
144+
*/
145+
void todo_list_filter_update_refs(struct repository *r,
146+
struct todo_list *todo_list);
147+
136148
/* Call this to setup defaults before parsing command line options */
137149
void sequencer_init_config(struct replay_opts *opts);
138150
int sequencer_pick_revisions(struct repository *repo,

‎t/lib-rebase.sh

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -207,3 +207,18 @@ check_reworded_commits () {
207207
>reword-log &&
208208
test_cmp reword-expected reword-log
209209
}
210+
211+
# usage: set_replace_editor <file>
212+
#
213+
# Replace the todo file with the exact contents of the given file.
214+
set_replace_editor () {
215+
cat >script <<-\EOF &&
216+
cat FILENAME >"$1"
217+
218+
echo 'rebase -i script after editing:'
219+
cat "$1"
220+
EOF
221+
222+
sed -e "s/FILENAME/$1/g" <script | write_script fake-editor.sh &&
223+
test_set_editor "$(pwd)/fake-editor.sh"
224+
}

‎t/t3404-rebase-interactive.sh

Lines changed: 139 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1834,6 +1834,145 @@ test_expect_success '--update-refs updates refs correctly' '
18341834
test_cmp_rev HEAD refs/heads/no-conflict-branch
18351835
'
18361836

1837+
test_expect_success 'respect user edits to update-ref steps' '
1838+
git checkout -B update-refs-break no-conflict-branch &&
1839+
git branch -f base HEAD~4 &&
1840+
git branch -f first HEAD~3 &&
1841+
git branch -f second HEAD~3 &&
1842+
git branch -f third HEAD~1 &&
1843+
git branch -f unseen base &&
1844+
1845+
# First, we will add breaks to the expected todo file
1846+
cat >fake-todo-1 <<-EOF &&
1847+
pick $(git rev-parse HEAD~3)
1848+
break
1849+
update-ref refs/heads/second
1850+
update-ref refs/heads/first
1851+
1852+
pick $(git rev-parse HEAD~2)
1853+
pick $(git rev-parse HEAD~1)
1854+
update-ref refs/heads/third
1855+
1856+
pick $(git rev-parse HEAD)
1857+
update-ref refs/heads/no-conflict-branch
1858+
EOF
1859+
1860+
# Second, we will drop some update-refs commands (and move one)
1861+
cat >fake-todo-2 <<-EOF &&
1862+
update-ref refs/heads/second
1863+
1864+
pick $(git rev-parse HEAD~2)
1865+
update-ref refs/heads/third
1866+
pick $(git rev-parse HEAD~1)
1867+
break
1868+
1869+
pick $(git rev-parse HEAD)
1870+
EOF
1871+
1872+
# Third, we will:
1873+
# * insert a new one (new-branch),
1874+
# * re-add an old one (first), and
1875+
# * add a second instance of a previously-stored one (second)
1876+
cat >fake-todo-3 <<-EOF &&
1877+
update-ref refs/heads/unseen
1878+
update-ref refs/heads/new-branch
1879+
pick $(git rev-parse HEAD)
1880+
update-ref refs/heads/first
1881+
update-ref refs/heads/second
1882+
EOF
1883+
1884+
(
1885+
set_replace_editor fake-todo-1 &&
1886+
git rebase -i --update-refs primary &&
1887+
1888+
# These branches are currently locked.
1889+
for b in first second third no-conflict-branch
1890+
do
1891+
test_must_fail git branch -f $b base || return 1
1892+
done &&
1893+
1894+
set_replace_editor fake-todo-2 &&
1895+
git rebase --edit-todo &&
1896+
1897+
# These branches are currently locked.
1898+
for b in second third
1899+
do
1900+
test_must_fail git branch -f $b base || return 1
1901+
done &&
1902+
1903+
# These branches are currently unlocked for checkout.
1904+
for b in first no-conflict-branch
1905+
do
1906+
git worktree add wt-$b $b &&
1907+
git worktree remove wt-$b || return 1
1908+
done &&
1909+
1910+
git rebase --continue &&
1911+
1912+
set_replace_editor fake-todo-3 &&
1913+
git rebase --edit-todo &&
1914+
1915+
# These branches are currently locked.
1916+
for b in second third first unseen
1917+
do
1918+
test_must_fail git branch -f $b base || return 1
1919+
done &&
1920+
1921+
# These branches are currently unlocked for checkout.
1922+
for b in no-conflict-branch
1923+
do
1924+
git worktree add wt-$b $b &&
1925+
git worktree remove wt-$b || return 1
1926+
done &&
1927+
1928+
git rebase --continue
1929+
) &&
1930+
1931+
test_cmp_rev HEAD~2 refs/heads/third &&
1932+
test_cmp_rev HEAD~1 refs/heads/unseen &&
1933+
test_cmp_rev HEAD~1 refs/heads/new-branch &&
1934+
test_cmp_rev HEAD refs/heads/first &&
1935+
test_cmp_rev HEAD refs/heads/second &&
1936+
test_cmp_rev HEAD refs/heads/no-conflict-branch
1937+
'
1938+
1939+
test_expect_success '--update-refs: check failed ref update' '
1940+
git checkout -B update-refs-error no-conflict-branch &&
1941+
git branch -f base HEAD~4 &&
1942+
git branch -f first HEAD~3 &&
1943+
git branch -f second HEAD~2 &&
1944+
git branch -f third HEAD~1 &&
1945+
1946+
cat >fake-todo <<-EOF &&
1947+
pick $(git rev-parse HEAD~3)
1948+
break
1949+
update-ref refs/heads/first
1950+
1951+
pick $(git rev-parse HEAD~2)
1952+
update-ref refs/heads/second
1953+
1954+
pick $(git rev-parse HEAD~1)
1955+
update-ref refs/heads/third
1956+
1957+
pick $(git rev-parse HEAD)
1958+
update-ref refs/heads/no-conflict-branch
1959+
EOF
1960+
1961+
(
1962+
set_replace_editor fake-todo &&
1963+
git rebase -i --update-refs base
1964+
) &&
1965+
1966+
# At this point, the values of first, second, and third are
1967+
# recorded in the update-refs file. We will force-update the
1968+
# "second" ref, but "git branch -f" will not work because of
1969+
# the lock in the update-refs file.
1970+
git rev-parse third >.git/refs/heads/second &&
1971+
1972+
git rebase --continue 2>err &&
1973+
grep "update_ref failed for ref '\''refs/heads/second'\''" err
1974+
'
1975+
18371976
# This must be the last test in this file
18381977
test_expect_success '$EDITOR and friends are unchanged' '
18391978
test_editor_unchanged

0 commit comments

Comments
 (0)