@matheusbsbs / matheus-bagliano / commits / b3b1a21d1a

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 <phillip.wood123@gmail.com> Signed-off-by: Derrick Stolee <derrickstolee@github.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Derrick Stolee committed Jul 19, 2022 at 18:33 UTC b3b1a21d1a5df87e81fa8b53c6c3cf9760ca12d4
5 files changed +268
rebase-interactive.c
+6
@@ -146,6 +146,12 @@ int edit_todo_list(struct repository *r, struct todo_list *todo_list,
146 return -4;
147 }
148
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 +
155 return 0;
156 }
157
sequencer.c
+96
@@ -4168,6 +4168,102 @@ cleanup:
4168 return result;
4169 }
4170
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 +
4267 static int do_update_ref(struct repository *r, const char *refname)
4268 {
4269 struct string_list_item *item;
sequencer.h
+12
@@ -133,6 +133,18 @@ void todo_list_release(struct todo_list *todo_list);
133 const char *todo_item_get_arg(struct todo_list *todo_list,
134 struct todo_item *item);
135
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 +
148 /* Call this to setup defaults before parsing command line options */
149 void sequencer_init_config(struct replay_opts *opts);
150 int sequencer_pick_revisions(struct repository *repo,
t/lib-rebase.sh
+15
@@ -207,3 +207,18 @@ check_reworded_commits () {
207 >reword-log &&
208 test_cmp reword-expected reword-log
209 }
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
+139
@@ -1834,6 +1834,145 @@ test_expect_success '--update-refs updates refs correctly' '
1834 test_cmp_rev HEAD refs/heads/no-conflict-branch
1835 '
1836
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 +
1976 # This must be the last test in this file
1977 test_expect_success '$EDITOR and friends are unchanged' '
1978 test_editor_unchanged