Hi Stolee
On 28/06/2022 14:25, Derrick Stolee via GitGitGadget wrote:
From: Derrick Stolee <derrickstolee@xxxxxxxxxx>
The previous change introduced the 'git rebase --update-refs' option
which added 'update-ref <ref>' commands to the todo list of an
interactive rebase.
Teach Git to record the HEAD position when reaching these 'update-ref'
commands. The ref/OID pair is stored in the
$GIT_DIR/rebase-merge/update-refs file. A previous change parsed this
file to avoid having other processes updating the refs in that file
while the rebase is in progress.
Not only do we update the file when the sequencer reaches these
'update-ref' commands, we then update the refs themselves at the end of
the rebase sequence. If the rebase is aborted before this final step,
then the refs are not updated.
This looks good, I've left a few comments but it seems basically sound
to me.
Signed-off-by: Derrick Stolee <derrickstolee@xxxxxxxxxx>
---
sequencer.c | 114 +++++++++++++++++++++++++++++++++-
t/t3404-rebase-interactive.sh | 23 +++++++
2 files changed, 136 insertions(+), 1 deletion(-)
diff --git a/sequencer.c b/sequencer.c
index 915d87a0336..4fd1c0b5bce 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -36,6 +36,7 @@
#include "rebase-interactive.h"
#include "reset.h"
#include "branch.h"
+#include "log-tree.h"
#define GIT_REFLOG_ACTION "GIT_REFLOG_ACTION"
@@ -148,6 +149,14 @@ static GIT_PATH_FUNC(rebase_path_squash_onto, "rebase-merge/squash-onto")
*/
static GIT_PATH_FUNC(rebase_path_refs_to_delete, "rebase-merge/refs-to-delete")
+/*
+ * The update-refs file stores a list of refs that will be updated at the end
+ * of the rebase sequence. The 'update-ref <ref>' commands in the todo file
+ * update the OIDs for the refs in this file, but the refs are not updated
+ * until the end of the rebase sequence.
+ */
+static GIT_PATH_FUNC(rebase_path_update_refs, "rebase-merge/update-refs")
+
/*
* The following files are written by git-rebase just after parsing the
* command-line.
@@ -4058,11 +4067,110 @@ leave_merge:
return ret;
}
-static int do_update_ref(struct repository *r, const char *ref_name)
+static int write_update_refs_state(struct string_list *refs_to_oids)
+{
+ int result = 0;
+ FILE *fp = NULL;
+ struct string_list_item *item;
+ char *path = xstrdup(rebase_path_update_refs());
This is leaked
+ if (safe_create_leading_directories(path)) {
+ result = error(_("unable to create leading directories of %s"),
+ path);
+ goto cleanup;
+ }
+
+ fp = fopen(path, "w");
+ if (!fp) {
+ result = error_errno(_("could not open '%s' for writing"), path);
+ goto cleanup;
+ }
Can we use fopen_or_warn() here? It ignores ENOENT and ENOTDIR but I
don't think that should matter here.
+
+ for_each_string_list_item(item, refs_to_oids)
+ fprintf(fp, "%s\n%s\n", item->string, oid_to_hex(item->util));
+
+cleanup:
+ if (fp)
+ fclose(fp);
+ return result;
+}
+compare_two_refs () {
+ git rev-parse $1 >expect &&
+ git rev-parse $2 >actual &&
+ test_cmp expect actual
+}
This is just test_cmp_rev
+test_expect_success '--update-refs updates refs correctly' '
+ git checkout -B update-refs no-conflict-branch &&
+ git branch -f base HEAD~4 &&
+ git branch -f first HEAD~3 &&
+ git branch -f second HEAD~3 &&
+ git branch -f third HEAD~1 &&
+ test_commit extra2 fileX &&
+ git commit --amend --fixup=L &&
+
+ git rebase -i --autosquash --update-refs primary &&
+
+ compare_two_refs HEAD~3 refs/heads/first &&
+ compare_two_refs HEAD~3 refs/heads/second &&
+ compare_two_refs HEAD~1 refs/heads/third &&
+ compare_two_refs HEAD refs/heads/no-conflict-branch
+'
Do we need a new test for this or can we just check the refs at the end
of one of the tests added in the last patch?
# This must be the last test in this file
test_expect_success '$EDITOR and friends are unchanged' '
test_editor_unchanged
I forgot to say on the last patch but you could maybe add a
test_editor_unchanged at the end of t2407 now that there are tests which
call test_set_editor
Thanks for working on this, it will be a really useful addition to rebase.
Phillip
