* [PATCH v5 14/14] replay: stop assuming replayed branches do not diverge
From: Christian Couder @ 2023-10-10 12:38 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Patrick Steinhardt, Johannes Schindelin,
Elijah Newren, John Cai, Derrick Stolee, Phillip Wood, Calvin Wan,
Toon Claes, Dragan Simic, Linus Arver, Christian Couder
In-Reply-To: <20231010123847.2777056-1-christian.couder@gmail.com>
From: Elijah Newren <newren@gmail.com>
The replay command is able to replay multiple branches but when some of
them are based on other replayed branches, their commit should be
replayed onto already replayed commits.
For this purpose, let's store the replayed commit and its original
commit in a key value store, so that we can easily find and reuse a
replayed commit instead of the original one.
Co-authored-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
builtin/replay.c | 44 ++++++++++++++++++++++++++--------
t/t3650-replay-basics.sh | 52 ++++++++++++++++++++++++++++++++++++++++
2 files changed, 86 insertions(+), 10 deletions(-)
diff --git a/builtin/replay.c b/builtin/replay.c
index 12689f1c89..f74261b75e 100644
--- a/builtin/replay.c
+++ b/builtin/replay.c
@@ -223,20 +223,33 @@ static void determine_replay_mode(struct rev_cmdline_info *cmd_info,
strset_clear(&rinfo.positive_refs);
}
+static struct commit *mapped_commit(kh_oid_map_t *replayed_commits,
+ struct commit *commit,
+ struct commit *fallback)
+{
+ khint_t pos = kh_get_oid_map(replayed_commits, commit->object.oid);
+ if (pos == kh_end(replayed_commits))
+ return fallback;
+ return kh_value(replayed_commits, pos);
+}
+
static struct commit *pick_regular_commit(struct commit *pickme,
- struct commit *last_commit,
+ kh_oid_map_t *replayed_commits,
+ struct commit *onto,
struct merge_options *merge_opt,
struct merge_result *result)
{
- struct commit *base;
+ struct commit *base, *replayed_base;
struct tree *pickme_tree, *base_tree;
base = pickme->parents->item;
+ replayed_base = mapped_commit(replayed_commits, base, onto);
+ result->tree = repo_get_commit_tree(the_repository, replayed_base);
pickme_tree = repo_get_commit_tree(the_repository, pickme);
base_tree = repo_get_commit_tree(the_repository, base);
- merge_opt->branch1 = short_commit_name(last_commit);
+ merge_opt->branch1 = short_commit_name(replayed_base);
merge_opt->branch2 = short_commit_name(pickme);
merge_opt->ancestor = xstrfmt("parent of %s", merge_opt->branch2);
@@ -250,7 +263,7 @@ static struct commit *pick_regular_commit(struct commit *pickme,
merge_opt->ancestor = NULL;
if (!result->clean)
return NULL;
- return create_commit(result->tree, pickme, last_commit);
+ return create_commit(result->tree, pickme, replayed_base);
}
int cmd_replay(int argc, const char **argv, const char *prefix)
@@ -266,6 +279,7 @@ int cmd_replay(int argc, const char **argv, const char *prefix)
struct merge_options merge_opt;
struct merge_result result;
struct strset *update_refs = NULL;
+ kh_oid_map_t *replayed_commits;
int ret = 0;
const char * const replay_usage[] = {
@@ -324,21 +338,30 @@ int cmd_replay(int argc, const char **argv, const char *prefix)
init_merge_options(&merge_opt, the_repository);
memset(&result, 0, sizeof(result));
merge_opt.show_rename_progress = 0;
-
- result.tree = repo_get_commit_tree(the_repository, onto);
last_commit = onto;
+ replayed_commits = kh_init_oid_map();
while ((commit = get_revision(&revs))) {
const struct name_decoration *decoration;
+ khint_t pos;
+ int hr;
if (!commit->parents)
die(_("replaying down to root commit is not supported yet!"));
if (commit->parents->next)
die(_("replaying merge commits is not supported yet!"));
- last_commit = pick_regular_commit(commit, last_commit, &merge_opt, &result);
+ last_commit = pick_regular_commit(commit, replayed_commits, onto,
+ &merge_opt, &result);
if (!last_commit)
break;
+ /* Record commit -> last_commit mapping */
+ pos = kh_put_oid_map(replayed_commits, commit->object.oid, &hr);
+ if (hr == 0)
+ BUG("Duplicate rewritten commit: %s\n",
+ oid_to_hex(&commit->object.oid));
+ kh_value(replayed_commits, pos) = last_commit;
+
/* Update any necessary branches */
if (advance_name)
continue;
@@ -367,13 +390,14 @@ int cmd_replay(int argc, const char **argv, const char *prefix)
}
merge_finalize(&merge_opt, &result);
- ret = result.clean;
-
-cleanup:
+ kh_destroy_oid_map(replayed_commits);
if (update_refs) {
strset_clear(update_refs);
free(update_refs);
}
+ ret = result.clean;
+
+cleanup:
release_revisions(&revs);
/* Return */
diff --git a/t/t3650-replay-basics.sh b/t/t3650-replay-basics.sh
index d6286f9580..389670262e 100755
--- a/t/t3650-replay-basics.sh
+++ b/t/t3650-replay-basics.sh
@@ -143,4 +143,56 @@ test_expect_success 'using replay on bare repo to also rebase a contained branch
test_cmp expect result-bare
'
+test_expect_success 'using replay to rebase multiple divergent branches' '
+ git replay --onto main ^topic1 topic2 topic4 >result &&
+
+ test_line_count = 2 result &&
+ cut -f 3 -d " " result >new-branch-tips &&
+
+ git log --format=%s $(head -n 1 new-branch-tips) >actual &&
+ test_write_lines E D M L B A >expect &&
+ test_cmp expect actual &&
+
+ git log --format=%s $(tail -n 1 new-branch-tips) >actual &&
+ test_write_lines J I M L B A >expect &&
+ test_cmp expect actual &&
+
+ printf "update refs/heads/topic2 " >expect &&
+ printf "%s " $(head -n 1 new-branch-tips) >>expect &&
+ git rev-parse topic2 >>expect &&
+ printf "update refs/heads/topic4 " >>expect &&
+ printf "%s " $(tail -n 1 new-branch-tips) >>expect &&
+ git rev-parse topic4 >>expect &&
+
+ test_cmp expect result
+'
+
+test_expect_success 'using replay on bare repo to rebase multiple divergent branches, including contained ones' '
+ git -C bare replay --contained --onto main ^main topic2 topic3 topic4 >result &&
+
+ test_line_count = 4 result &&
+ cut -f 3 -d " " result >new-branch-tips &&
+
+ >expect &&
+ for i in 2 1 3 4
+ do
+ printf "update refs/heads/topic$i " >>expect &&
+ printf "%s " $(grep topic$i result | cut -f 3 -d " ") >>expect &&
+ git -C bare rev-parse topic$i >>expect || return 1
+ done &&
+
+ test_cmp expect result &&
+
+ test_write_lines F C M L B A >expect1 &&
+ test_write_lines E D C M L B A >expect2 &&
+ test_write_lines H G F C M L B A >expect3 &&
+ test_write_lines J I M L B A >expect4 &&
+
+ for i in 1 2 3 4
+ do
+ git -C bare log --format=%s $(grep topic$i result | cut -f 3 -d " ") >actual &&
+ test_cmp expect$i actual || return 1
+ done
+'
+
test_done
--
2.42.0.339.g663cbc8ab1
^ permalink raw reply related
* [PATCH v5 13/14] replay: add --contained to rebase contained branches
From: Christian Couder @ 2023-10-10 12:38 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Patrick Steinhardt, Johannes Schindelin,
Elijah Newren, John Cai, Derrick Stolee, Phillip Wood, Calvin Wan,
Toon Claes, Dragan Simic, Linus Arver, Christian Couder
In-Reply-To: <20231010123847.2777056-1-christian.couder@gmail.com>
From: Elijah Newren <newren@gmail.com>
Let's add a `--contained` option that can be used along with
`--onto` to rebase all the branches contained in the <revision-range>
argument.
Co-authored-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
Documentation/git-replay.txt | 12 +++++++++++-
builtin/replay.c | 12 ++++++++++--
t/t3650-replay-basics.sh | 29 +++++++++++++++++++++++++++++
3 files changed, 50 insertions(+), 3 deletions(-)
diff --git a/Documentation/git-replay.txt b/Documentation/git-replay.txt
index 2c09232c8f..e9697fcfb6 100644
--- a/Documentation/git-replay.txt
+++ b/Documentation/git-replay.txt
@@ -9,7 +9,7 @@ git-replay - Replay commits on a new base, works on bare repos too
SYNOPSIS
--------
[verse]
-'git replay' (--onto <newbase> | --advance <branch>) <revision-range>...
+'git replay' ([--contained] --onto <newbase> | --advance <branch>) <revision-range>...
DESCRIPTION
-----------
@@ -96,6 +96,16 @@ top of the exact same new base, they only differ in that the first
provides instructions to make mybranch point at the new commits and
the second provides instructions to make target point at them.
+What if you have a stack of branches, one depending upon another, and
+you'd really like to rebase the whole set?
+
+------------
+$ git replay --contained --onto origin/main origin/main..tipbranch
+update refs/heads/branch1 ${NEW_branch1_HASH} ${OLD_branch1_HASH}
+update refs/heads/branch2 ${NEW_branch2_HASH} ${OLD_branch2_HASH}
+update refs/heads/tipbranch ${NEW_tipbranch_HASH} ${OLD_tipbranch_HASH}
+------------
+
When calling `git replay`, one does not need to specify a range of
commits to replay using the syntax `A..B`; any range expression will
do:
diff --git a/builtin/replay.c b/builtin/replay.c
index d458837e9c..12689f1c89 100644
--- a/builtin/replay.c
+++ b/builtin/replay.c
@@ -258,6 +258,7 @@ int cmd_replay(int argc, const char **argv, const char *prefix)
const char *advance_name = NULL;
struct commit *onto = NULL;
const char *onto_name = NULL;
+ int contained = 0;
struct rev_info revs;
struct commit *last_commit = NULL;
@@ -268,7 +269,7 @@ int cmd_replay(int argc, const char **argv, const char *prefix)
int ret = 0;
const char * const replay_usage[] = {
- N_("git replay (--onto <newbase> | --advance <branch>) <revision-range>..."),
+ N_("git replay ([--contained] --onto <newbase> | --advance <branch>) <revision-range>..."),
NULL
};
struct option replay_options[] = {
@@ -278,6 +279,8 @@ int cmd_replay(int argc, const char **argv, const char *prefix)
OPT_STRING(0, "onto", &onto_name,
N_("revision"),
N_("replay onto given commit")),
+ OPT_BOOL(0, "contained", &contained,
+ N_("advance all branches contained in revision-range")),
OPT_END()
};
@@ -289,6 +292,10 @@ int cmd_replay(int argc, const char **argv, const char *prefix)
usage_with_options(replay_usage, replay_options);
}
+ if (advance_name && contained)
+ die(_("options '%s' and '%s' cannot be used together"),
+ "--advance", "--contained");
+
repo_init_revisions(the_repository, &revs, prefix);
argc = setup_revisions(argc, argv, &revs, NULL);
@@ -340,7 +347,8 @@ int cmd_replay(int argc, const char **argv, const char *prefix)
continue;
while (decoration) {
if (decoration->type == DECORATION_REF_LOCAL &&
- strset_contains(update_refs, decoration->name)) {
+ (contained || strset_contains(update_refs,
+ decoration->name))) {
printf("update %s %s %s\n",
decoration->name,
oid_to_hex(&last_commit->object.oid),
diff --git a/t/t3650-replay-basics.sh b/t/t3650-replay-basics.sh
index 68a87e7803..d6286f9580 100755
--- a/t/t3650-replay-basics.sh
+++ b/t/t3650-replay-basics.sh
@@ -114,4 +114,33 @@ test_expect_success 'replay fails when both --advance and --onto are omitted' '
test_must_fail git replay topic1..topic2 >result
'
+test_expect_success 'using replay to also rebase a contained branch' '
+ git replay --contained --onto main main..topic3 >result &&
+
+ test_line_count = 2 result &&
+ cut -f 3 -d " " result >new-branch-tips &&
+
+ git log --format=%s $(head -n 1 new-branch-tips) >actual &&
+ test_write_lines F C M L B A >expect &&
+ test_cmp expect actual &&
+
+ git log --format=%s $(tail -n 1 new-branch-tips) >actual &&
+ test_write_lines H G F C M L B A >expect &&
+ test_cmp expect actual &&
+
+ printf "update refs/heads/topic1 " >expect &&
+ printf "%s " $(head -n 1 new-branch-tips) >>expect &&
+ git rev-parse topic1 >>expect &&
+ printf "update refs/heads/topic3 " >>expect &&
+ printf "%s " $(tail -n 1 new-branch-tips) >>expect &&
+ git rev-parse topic3 >>expect &&
+
+ test_cmp expect result
+'
+
+test_expect_success 'using replay on bare repo to also rebase a contained branch' '
+ git -C bare replay --contained --onto main main..topic3 >result-bare &&
+ test_cmp expect result-bare
+'
+
test_done
--
2.42.0.339.g663cbc8ab1
^ permalink raw reply related
* [PATCH v5 12/14] replay: add --advance or 'cherry-pick' mode
From: Christian Couder @ 2023-10-10 12:38 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Patrick Steinhardt, Johannes Schindelin,
Elijah Newren, John Cai, Derrick Stolee, Phillip Wood, Calvin Wan,
Toon Claes, Dragan Simic, Linus Arver, Christian Couder
In-Reply-To: <20231010123847.2777056-1-christian.couder@gmail.com>
From: Elijah Newren <newren@gmail.com>
There is already a 'rebase' mode with `--onto`. Let's add an 'advance' or
'cherry-pick' mode with `--advance`. This new mode will make the target
branch advance as we replay commits onto it.
The replayed commits should have a single tip, so that it's clear where
the target branch should be advanced. If they have more than one tip,
this new mode will error out.
Co-authored-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
Documentation/git-replay.txt | 41 ++++++--
builtin/replay.c | 185 +++++++++++++++++++++++++++++++++--
t/t3650-replay-basics.sh | 34 +++++++
3 files changed, 243 insertions(+), 17 deletions(-)
diff --git a/Documentation/git-replay.txt b/Documentation/git-replay.txt
index 86be363f9b..2c09232c8f 100644
--- a/Documentation/git-replay.txt
+++ b/Documentation/git-replay.txt
@@ -9,7 +9,7 @@ git-replay - Replay commits on a new base, works on bare repos too
SYNOPSIS
--------
[verse]
-'git replay' --onto <newbase> <revision-range>...
+'git replay' (--onto <newbase> | --advance <branch>) <revision-range>...
DESCRIPTION
-----------
@@ -29,14 +29,25 @@ OPTIONS
Starting point at which to create the new commits. May be any
valid commit, and not just an existing branch name.
+
-The update-ref command(s) in the output will update the branch(es) in
-the revision range to point at the new commits, similar to the way how
-`git rebase --update-refs` updates multiple branches in the affected
-range.
+When `--onto` is specified, the update-ref command(s) in the output will
+update the branch(es) in the revision range to point at the new
+commits, similar to the way how `git rebase --update-refs` updates
+multiple branches in the affected range.
+
+--advance <branch>::
+ Starting point at which to create the new commits; must be a
+ branch name.
++
+When `--advance` is specified, the update-ref command(s) in the output
+will update the branch passed as an argument to `--advance` to point at
+the new commits (in other words, this mimics a cherry-pick operation).
<revision-range>::
- Range of commits to replay; see "Specifying Ranges" in
- linkgit:git-rev-parse and the "Commit Limiting" options below.
+ Range of commits to replay. More than one <revision-range> can
+ be passed, but in `--advance <branch>` mode, they should have
+ a single tip, so that it's clear where <branch> should point
+ to. See "Specifying Ranges" in linkgit:git-rev-parse and the
+ "Commit Limiting" options below.
include::rev-list-options.txt[]
@@ -51,7 +62,9 @@ input to `git update-ref --stdin`. It is of the form:
update refs/heads/branch3 ${NEW_branch3_HASH} ${OLD_branch3_HASH}
where the number of refs updated depends on the arguments passed and
-the shape of the history being replayed.
+the shape of the history being replayed. When using `--advance`, the
+number of refs updated is always one, but for `--onto`, it can be one
+or more (rebasing multiple branches simultaneously is supported).
EXIT STATUS
-----------
@@ -71,6 +84,18 @@ $ git replay --onto target origin/main..mybranch
update refs/heads/mybranch ${NEW_mybranch_HASH} ${OLD_mybranch_HASH}
------------
+To cherry-pick the commits from mybranch onto target:
+
+------------
+$ git replay --advance target origin/main..mybranch
+update refs/heads/target ${NEW_target_HASH} ${OLD_target_HASH}
+------------
+
+Note that the first two examples replay the exact same commits and on
+top of the exact same new base, they only differ in that the first
+provides instructions to make mybranch point at the new commits and
+the second provides instructions to make target point at them.
+
When calling `git replay`, one does not need to specify a range of
commits to replay using the syntax `A..B`; any range expression will
do:
diff --git a/builtin/replay.c b/builtin/replay.c
index 213ac2764e..d458837e9c 100644
--- a/builtin/replay.c
+++ b/builtin/replay.c
@@ -14,6 +14,7 @@
#include "parse-options.h"
#include "refs.h"
#include "revision.h"
+#include "strmap.h"
#include <oidset.h>
#include <tree.h>
@@ -82,6 +83,146 @@ static struct commit *create_commit(struct tree *tree,
return (struct commit *)obj;
}
+struct ref_info {
+ struct commit *onto;
+ struct strset positive_refs;
+ struct strset negative_refs;
+ int positive_refexprs;
+ int negative_refexprs;
+};
+
+static void get_ref_information(struct rev_cmdline_info *cmd_info,
+ struct ref_info *ref_info)
+{
+ int i;
+
+ ref_info->onto = NULL;
+ strset_init(&ref_info->positive_refs);
+ strset_init(&ref_info->negative_refs);
+ ref_info->positive_refexprs = 0;
+ ref_info->negative_refexprs = 0;
+
+ /*
+ * When the user specifies e.g.
+ * git replay origin/main..mybranch
+ * git replay ^origin/next mybranch1 mybranch2
+ * we want to be able to determine where to replay the commits. In
+ * these examples, the branches are probably based on an old version
+ * of either origin/main or origin/next, so we want to replay on the
+ * newest version of that branch. In contrast we would want to error
+ * out if they ran
+ * git replay ^origin/master ^origin/next mybranch
+ * git replay mybranch~2..mybranch
+ * the first of those because there's no unique base to choose, and
+ * the second because they'd likely just be replaying commits on top
+ * of the same commit and not making any difference.
+ */
+ for (i = 0; i < cmd_info->nr; i++) {
+ struct rev_cmdline_entry *e = cmd_info->rev + i;
+ struct object_id oid;
+ const char *refexpr = e->name;
+ char *fullname = NULL;
+ int can_uniquely_dwim = 1;
+
+ if (*refexpr == '^')
+ refexpr++;
+ if (repo_dwim_ref(the_repository, refexpr, strlen(refexpr), &oid, &fullname, 0) != 1)
+ can_uniquely_dwim = 0;
+
+ if (e->flags & BOTTOM) {
+ if (can_uniquely_dwim)
+ strset_add(&ref_info->negative_refs, fullname);
+ if (!ref_info->negative_refexprs)
+ ref_info->onto = lookup_commit_reference_gently(the_repository,
+ &e->item->oid, 1);
+ ref_info->negative_refexprs++;
+ } else {
+ if (can_uniquely_dwim)
+ strset_add(&ref_info->positive_refs, fullname);
+ ref_info->positive_refexprs++;
+ }
+
+ free(fullname);
+ }
+}
+
+static void determine_replay_mode(struct rev_cmdline_info *cmd_info,
+ const char *onto_name,
+ const char **advance_name,
+ struct commit **onto,
+ struct strset **update_refs)
+{
+ struct ref_info rinfo;
+
+ get_ref_information(cmd_info, &rinfo);
+ if (!rinfo.positive_refexprs)
+ die(_("need some commits to replay"));
+ if (onto_name && *advance_name)
+ die(_("--onto and --advance are incompatible"));
+ else if (onto_name) {
+ *onto = peel_committish(onto_name);
+ if (rinfo.positive_refexprs <
+ strset_get_size(&rinfo.positive_refs))
+ die(_("all positive revisions given must be references"));
+ } else if (*advance_name) {
+ struct object_id oid;
+ char *fullname = NULL;
+
+ *onto = peel_committish(*advance_name);
+ if (repo_dwim_ref(the_repository, *advance_name, strlen(*advance_name),
+ &oid, &fullname, 0) == 1) {
+ *advance_name = fullname;
+ } else {
+ die(_("argument to --advance must be a reference"));
+ }
+ if (rinfo.positive_refexprs > 1)
+ die(_("cannot advance target with multiple sources because ordering would be ill-defined"));
+ } else {
+ int positive_refs_complete = (
+ rinfo.positive_refexprs ==
+ strset_get_size(&rinfo.positive_refs));
+ int negative_refs_complete = (
+ rinfo.negative_refexprs ==
+ strset_get_size(&rinfo.negative_refs));
+ /*
+ * We need either positive_refs_complete or
+ * negative_refs_complete, but not both.
+ */
+ if (rinfo.negative_refexprs > 0 &&
+ positive_refs_complete == negative_refs_complete)
+ die(_("cannot implicitly determine whether this is an --advance or --onto operation"));
+ if (negative_refs_complete) {
+ struct hashmap_iter iter;
+ struct strmap_entry *entry;
+
+ if (rinfo.negative_refexprs == 0)
+ die(_("all positive revisions given must be references"));
+ else if (rinfo.negative_refexprs > 1)
+ die(_("cannot implicitly determine whether this is an --advance or --onto operation"));
+ else if (rinfo.positive_refexprs > 1)
+ die(_("cannot advance target with multiple source branches because ordering would be ill-defined"));
+
+ /* Only one entry, but we have to loop to get it */
+ strset_for_each_entry(&rinfo.negative_refs,
+ &iter, entry) {
+ *advance_name = entry->key;
+ }
+ } else { /* positive_refs_complete */
+ if (rinfo.negative_refexprs > 1)
+ die(_("cannot implicitly determine correct base for --onto"));
+ if (rinfo.negative_refexprs == 1)
+ *onto = rinfo.onto;
+ }
+ }
+ if (!*advance_name) {
+ *update_refs = xcalloc(1, sizeof(**update_refs));
+ **update_refs = rinfo.positive_refs;
+ memset(&rinfo.positive_refs, 0, sizeof(**update_refs));
+ }
+ strset_clear(&rinfo.negative_refs);
+ strset_clear(&rinfo.positive_refs);
+}
+
static struct commit *pick_regular_commit(struct commit *pickme,
struct commit *last_commit,
struct merge_options *merge_opt,
@@ -114,20 +255,26 @@ static struct commit *pick_regular_commit(struct commit *pickme,
int cmd_replay(int argc, const char **argv, const char *prefix)
{
- struct commit *onto;
+ const char *advance_name = NULL;
+ struct commit *onto = NULL;
const char *onto_name = NULL;
- struct commit *last_commit = NULL;
+
struct rev_info revs;
+ struct commit *last_commit = NULL;
struct commit *commit;
struct merge_options merge_opt;
struct merge_result result;
+ struct strset *update_refs = NULL;
int ret = 0;
const char * const replay_usage[] = {
- N_("git replay --onto <newbase> <revision-range>..."),
+ N_("git replay (--onto <newbase> | --advance <branch>) <revision-range>..."),
NULL
};
struct option replay_options[] = {
+ OPT_STRING(0, "advance", &advance_name,
+ N_("branch"),
+ N_("make replay advance given branch")),
OPT_STRING(0, "onto", &onto_name,
N_("revision"),
N_("replay onto given commit")),
@@ -137,13 +284,11 @@ int cmd_replay(int argc, const char **argv, const char *prefix)
argc = parse_options(argc, argv, prefix, replay_options, replay_usage,
PARSE_OPT_KEEP_ARGV0 | PARSE_OPT_KEEP_UNKNOWN_OPT);
- if (!onto_name) {
- error(_("option --onto is mandatory"));
+ if (!onto_name && !advance_name) {
+ error(_("option --onto or --advance is mandatory"));
usage_with_options(replay_usage, replay_options);
}
- onto = peel_committish(onto_name);
-
repo_init_revisions(the_repository, &revs, prefix);
argc = setup_revisions(argc, argv, &revs, NULL);
@@ -158,6 +303,12 @@ int cmd_replay(int argc, const char **argv, const char *prefix)
revs.topo_order = 1;
revs.simplify_history = 0;
+ determine_replay_mode(&revs.cmdline, onto_name, &advance_name,
+ &onto, &update_refs);
+
+ if (!onto) /* FIXME: Should handle replaying down to root commit */
+ die("Replaying down to root commit is not supported yet!");
+
if (prepare_revision_walk(&revs) < 0) {
ret = error(_("error preparing revisions"));
goto cleanup;
@@ -166,6 +317,7 @@ int cmd_replay(int argc, const char **argv, const char *prefix)
init_merge_options(&merge_opt, the_repository);
memset(&result, 0, sizeof(result));
merge_opt.show_rename_progress = 0;
+
result.tree = repo_get_commit_tree(the_repository, onto);
last_commit = onto;
while ((commit = get_revision(&revs))) {
@@ -180,12 +332,15 @@ int cmd_replay(int argc, const char **argv, const char *prefix)
if (!last_commit)
break;
+ /* Update any necessary branches */
+ if (advance_name)
+ continue;
decoration = get_name_decoration(&commit->object);
if (!decoration)
continue;
-
while (decoration) {
- if (decoration->type == DECORATION_REF_LOCAL) {
+ if (decoration->type == DECORATION_REF_LOCAL &&
+ strset_contains(update_refs, decoration->name)) {
printf("update %s %s %s\n",
decoration->name,
oid_to_hex(&last_commit->object.oid),
@@ -195,10 +350,22 @@ int cmd_replay(int argc, const char **argv, const char *prefix)
}
}
+ /* In --advance mode, advance the target ref */
+ if (result.clean == 1 && advance_name) {
+ printf("update %s %s %s\n",
+ advance_name,
+ oid_to_hex(&last_commit->object.oid),
+ oid_to_hex(&onto->object.oid));
+ }
+
merge_finalize(&merge_opt, &result);
ret = result.clean;
cleanup:
+ if (update_refs) {
+ strset_clear(update_refs);
+ free(update_refs);
+ }
release_revisions(&revs);
/* Return */
diff --git a/t/t3650-replay-basics.sh b/t/t3650-replay-basics.sh
index a1da4f9ef9..68a87e7803 100755
--- a/t/t3650-replay-basics.sh
+++ b/t/t3650-replay-basics.sh
@@ -80,4 +80,38 @@ test_expect_success 'using replay on bare repo to rebase with a conflict' '
test_expect_code 1 git -C bare replay --onto topic1 B..conflict
'
+test_expect_success 'using replay to perform basic cherry-pick' '
+ # The differences between this test and previous ones are:
+ # --advance vs --onto
+ # 2nd field of result is refs/heads/main vs. refs/heads/topic2
+ # 4th field of result is hash for main instead of hash for topic2
+
+ git replay --advance main topic1..topic2 >result &&
+
+ test_line_count = 1 result &&
+
+ git log --format=%s $(cut -f 3 -d " " result) >actual &&
+ test_write_lines E D M L B A >expect &&
+ test_cmp expect actual &&
+
+ printf "update refs/heads/main " >expect &&
+ printf "%s " $(cut -f 3 -d " " result) >>expect &&
+ git rev-parse main >>expect &&
+
+ test_cmp expect result
+'
+
+test_expect_success 'using replay on bare repo to perform basic cherry-pick' '
+ git -C bare replay --advance main topic1..topic2 >result-bare &&
+ test_cmp expect result-bare
+'
+
+test_expect_success 'replay on bare repo fails with both --advance and --onto' '
+ test_must_fail git -C bare replay --advance main --onto main topic1..topic2 >result-bare
+'
+
+test_expect_success 'replay fails when both --advance and --onto are omitted' '
+ test_must_fail git replay topic1..topic2 >result
+'
+
test_done
--
2.42.0.339.g663cbc8ab1
^ permalink raw reply related
* [PATCH v5 11/14] replay: use standard revision ranges
From: Christian Couder @ 2023-10-10 12:38 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Patrick Steinhardt, Johannes Schindelin,
Elijah Newren, John Cai, Derrick Stolee, Phillip Wood, Calvin Wan,
Toon Claes, Dragan Simic, Linus Arver, Christian Couder
In-Reply-To: <20231010123847.2777056-1-christian.couder@gmail.com>
From: Elijah Newren <newren@gmail.com>
Instead of the fixed "<oldbase> <branch>" arguments, the replay
command now accepts "<revision-range>..." arguments in a similar
way as many other Git commands. This makes its interface more
standard and more flexible.
This also enables many revision related options accepted and
eaten by setup_revisions(). If the replay command was a high level
one or had a high level mode, it would make sense to restrict some
of the possible options, like those generating non-contiguous
history, as they could be confusing for most users.
Also as the interface of the command is now mostly finalized,
we can add some documentation as well as testcases to make sure
the command will continue to work as designed in the future.
We only document the rev-list related options among all the
revision related options that are now accepted, as the rev-list
related ones are probably the most useful for now.
Helped-by: Dragan Simic <dsimic@manjaro.org>
Helped-by: Linus Arver <linusa@google.com>
Co-authored-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
Documentation/git-replay.txt | 92 ++++++++++++++++++++++++
builtin/replay.c | 21 ++----
t/t3650-replay-basics.sh | 83 +++++++++++++++++++++
t/t6429-merge-sequence-rename-caching.sh | 18 ++---
4 files changed, 188 insertions(+), 26 deletions(-)
create mode 100644 Documentation/git-replay.txt
create mode 100755 t/t3650-replay-basics.sh
diff --git a/Documentation/git-replay.txt b/Documentation/git-replay.txt
new file mode 100644
index 0000000000..86be363f9b
--- /dev/null
+++ b/Documentation/git-replay.txt
@@ -0,0 +1,92 @@
+git-replay(1)
+=============
+
+NAME
+----
+git-replay - Replay commits on a new base, works on bare repos too
+
+
+SYNOPSIS
+--------
+[verse]
+'git replay' --onto <newbase> <revision-range>...
+
+DESCRIPTION
+-----------
+
+Takes a range of commits and replays them onto a new location. Leaves
+the working tree and the index untouched, and updates no
+references. The output of this command is meant to be used as input to
+`git update-ref --stdin`, which would update the relevant branches
+(see the OUTPUT section below).
+
+THIS COMMAND IS EXPERIMENTAL. THE BEHAVIOR MAY CHANGE.
+
+OPTIONS
+-------
+
+--onto <newbase>::
+ Starting point at which to create the new commits. May be any
+ valid commit, and not just an existing branch name.
++
+The update-ref command(s) in the output will update the branch(es) in
+the revision range to point at the new commits, similar to the way how
+`git rebase --update-refs` updates multiple branches in the affected
+range.
+
+<revision-range>::
+ Range of commits to replay; see "Specifying Ranges" in
+ linkgit:git-rev-parse and the "Commit Limiting" options below.
+
+include::rev-list-options.txt[]
+
+OUTPUT
+------
+
+When there are no conflicts, the output of this command is usable as
+input to `git update-ref --stdin`. It is of the form:
+
+ update refs/heads/branch1 ${NEW_branch1_HASH} ${OLD_branch1_HASH}
+ update refs/heads/branch2 ${NEW_branch2_HASH} ${OLD_branch2_HASH}
+ update refs/heads/branch3 ${NEW_branch3_HASH} ${OLD_branch3_HASH}
+
+where the number of refs updated depends on the arguments passed and
+the shape of the history being replayed.
+
+EXIT STATUS
+-----------
+
+For a successful, non-conflicted replay, the exit status is 0. When
+the replay has conflicts, the exit status is 1. If the replay is not
+able to complete (or start) due to some kind of error, the exit status
+is something other than 0 or 1.
+
+EXAMPLES
+--------
+
+To simply rebase `mybranch` onto `target`:
+
+------------
+$ git replay --onto target origin/main..mybranch
+update refs/heads/mybranch ${NEW_mybranch_HASH} ${OLD_mybranch_HASH}
+------------
+
+When calling `git replay`, one does not need to specify a range of
+commits to replay using the syntax `A..B`; any range expression will
+do:
+
+------------
+$ git replay --onto origin/main ^base branch1 branch2 branch3
+update refs/heads/branch1 ${NEW_branch1_HASH} ${OLD_branch1_HASH}
+update refs/heads/branch2 ${NEW_branch2_HASH} ${OLD_branch2_HASH}
+update refs/heads/branch3 ${NEW_branch3_HASH} ${OLD_branch3_HASH}
+------------
+
+This will simultaneously rebase `branch1`, `branch2`, and `branch3`,
+all commits they have since `base`, playing them on top of
+`origin/main`. These three branches may have commits on top of `base`
+that they have in common, but that does not need to be the case.
+
+GIT
+---
+Part of the linkgit:git[1] suite
diff --git a/builtin/replay.c b/builtin/replay.c
index ace9ac6df0..213ac2764e 100644
--- a/builtin/replay.c
+++ b/builtin/replay.c
@@ -14,7 +14,6 @@
#include "parse-options.h"
#include "refs.h"
#include "revision.h"
-#include "strvec.h"
#include <oidset.h>
#include <tree.h>
@@ -118,16 +117,14 @@ int cmd_replay(int argc, const char **argv, const char *prefix)
struct commit *onto;
const char *onto_name = NULL;
struct commit *last_commit = NULL;
- struct strvec rev_walk_args = STRVEC_INIT;
struct rev_info revs;
struct commit *commit;
struct merge_options merge_opt;
struct merge_result result;
- struct strbuf branch_name = STRBUF_INIT;
int ret = 0;
const char * const replay_usage[] = {
- N_("git replay --onto <newbase> <oldbase> <branch>"),
+ N_("git replay --onto <newbase> <revision-range>..."),
NULL
};
struct option replay_options[] = {
@@ -145,20 +142,13 @@ int cmd_replay(int argc, const char **argv, const char *prefix)
usage_with_options(replay_usage, replay_options);
}
- if (argc != 3) {
- error(_("bad number of arguments"));
- usage_with_options(replay_usage, replay_options);
- }
-
onto = peel_committish(onto_name);
- strbuf_addf(&branch_name, "refs/heads/%s", argv[2]);
repo_init_revisions(the_repository, &revs, prefix);
- strvec_pushl(&rev_walk_args, "", argv[2], "--not", argv[1], NULL);
-
- if (setup_revisions(rev_walk_args.nr, rev_walk_args.v, &revs, NULL) > 1) {
- ret = error(_("unhandled options"));
+ argc = setup_revisions(argc, argv, &revs, NULL);
+ if (argc > 1) {
+ ret = error(_("unrecognized argument: %s"), argv[1]);
goto cleanup;
}
@@ -168,8 +158,6 @@ int cmd_replay(int argc, const char **argv, const char *prefix)
revs.topo_order = 1;
revs.simplify_history = 0;
- strvec_clear(&rev_walk_args);
-
if (prepare_revision_walk(&revs) < 0) {
ret = error(_("error preparing revisions"));
goto cleanup;
@@ -211,7 +199,6 @@ int cmd_replay(int argc, const char **argv, const char *prefix)
ret = result.clean;
cleanup:
- strbuf_release(&branch_name);
release_revisions(&revs);
/* Return */
diff --git a/t/t3650-replay-basics.sh b/t/t3650-replay-basics.sh
new file mode 100755
index 0000000000..a1da4f9ef9
--- /dev/null
+++ b/t/t3650-replay-basics.sh
@@ -0,0 +1,83 @@
+#!/bin/sh
+
+test_description='basic git replay tests'
+
+GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
+export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
+
+. ./test-lib.sh
+
+GIT_AUTHOR_NAME=author@name
+GIT_AUTHOR_EMAIL=bogus@email@address
+export GIT_AUTHOR_NAME GIT_AUTHOR_EMAIL
+
+test_expect_success 'setup' '
+ test_commit A &&
+ test_commit B &&
+
+ git switch -c topic1 &&
+ test_commit C &&
+ git switch -c topic2 &&
+ test_commit D &&
+ test_commit E &&
+ git switch topic1 &&
+ test_commit F &&
+ git switch -c topic3 &&
+ test_commit G &&
+ test_commit H &&
+ git switch -c topic4 main &&
+ test_commit I &&
+ test_commit J &&
+
+ git switch -c next main &&
+ test_commit K &&
+ git merge -m "Merge topic1" topic1 &&
+ git merge -m "Merge topic2" topic2 &&
+ git merge -m "Merge topic3" topic3 &&
+ >evil &&
+ git add evil &&
+ git commit --amend &&
+ git merge -m "Merge topic4" topic4 &&
+
+ git switch main &&
+ test_commit L &&
+ test_commit M &&
+
+ git switch -c conflict B &&
+ test_commit C.conflict C.t conflict
+'
+
+test_expect_success 'setup bare' '
+ git clone --bare . bare
+'
+
+test_expect_success 'using replay to rebase two branches, one on top of other' '
+ git replay --onto main topic1..topic2 >result &&
+
+ test_line_count = 1 result &&
+
+ git log --format=%s $(cut -f 3 -d " " result) >actual &&
+ test_write_lines E D M L B A >expect &&
+ test_cmp expect actual &&
+
+ printf "update refs/heads/topic2 " >expect &&
+ printf "%s " $(cut -f 3 -d " " result) >>expect &&
+ git rev-parse topic2 >>expect &&
+
+ test_cmp expect result
+'
+
+test_expect_success 'using replay on bare repo to rebase two branches, one on top of other' '
+ git -C bare replay --onto main topic1..topic2 >result-bare &&
+ test_cmp expect result-bare
+'
+
+test_expect_success 'using replay to rebase with a conflict' '
+ test_expect_code 1 git replay --onto topic1 B..conflict
+'
+
+test_expect_success 'using replay on bare repo to rebase with a conflict' '
+ test_expect_code 1 git -C bare replay --onto topic1 B..conflict
+'
+
+test_done
diff --git a/t/t6429-merge-sequence-rename-caching.sh b/t/t6429-merge-sequence-rename-caching.sh
index 099aefeffc..0f39ed0d08 100755
--- a/t/t6429-merge-sequence-rename-caching.sh
+++ b/t/t6429-merge-sequence-rename-caching.sh
@@ -71,7 +71,7 @@ test_expect_success 'caching renames does not preclude finding new ones' '
git switch upstream &&
- git replay --onto HEAD upstream~1 topic >out &&
+ git replay --onto HEAD upstream~1..topic >out &&
git update-ref --stdin <out &&
git checkout topic &&
@@ -141,7 +141,7 @@ test_expect_success 'cherry-pick both a commit and its immediate revert' '
GIT_TRACE2_PERF="$(pwd)/trace.output" &&
export GIT_TRACE2_PERF &&
- git replay --onto HEAD upstream~1 topic >out &&
+ git replay --onto HEAD upstream~1..topic >out &&
git update-ref --stdin <out &&
git checkout topic &&
@@ -201,7 +201,7 @@ test_expect_success 'rename same file identically, then reintroduce it' '
GIT_TRACE2_PERF="$(pwd)/trace.output" &&
export GIT_TRACE2_PERF &&
- git replay --onto HEAD upstream~1 topic >out &&
+ git replay --onto HEAD upstream~1..topic >out &&
git update-ref --stdin <out &&
git checkout topic &&
@@ -279,7 +279,7 @@ test_expect_success 'rename same file identically, then add file to old dir' '
GIT_TRACE2_PERF="$(pwd)/trace.output" &&
export GIT_TRACE2_PERF &&
- git replay --onto HEAD upstream~1 topic >out &&
+ git replay --onto HEAD upstream~1..topic >out &&
git update-ref --stdin <out &&
git checkout topic &&
@@ -357,7 +357,7 @@ test_expect_success 'cached dir rename does not prevent noticing later conflict'
GIT_TRACE2_PERF="$(pwd)/trace.output" &&
export GIT_TRACE2_PERF &&
- test_must_fail git replay --onto HEAD upstream~1 topic >output &&
+ test_must_fail git replay --onto HEAD upstream~1..topic >output &&
grep region_enter.*diffcore_rename trace.output >calls &&
test_line_count = 2 calls
@@ -456,7 +456,7 @@ test_expect_success 'dir rename unneeded, then add new file to old dir' '
GIT_TRACE2_PERF="$(pwd)/trace.output" &&
export GIT_TRACE2_PERF &&
- git replay --onto HEAD upstream~1 topic >out &&
+ git replay --onto HEAD upstream~1..topic >out &&
git update-ref --stdin <out &&
git checkout topic &&
@@ -523,7 +523,7 @@ test_expect_success 'dir rename unneeded, then rename existing file into old dir
GIT_TRACE2_PERF="$(pwd)/trace.output" &&
export GIT_TRACE2_PERF &&
- git replay --onto HEAD upstream~1 topic >out &&
+ git replay --onto HEAD upstream~1..topic >out &&
git update-ref --stdin <out &&
git checkout topic &&
@@ -626,7 +626,7 @@ test_expect_success 'caching renames only on upstream side, part 1' '
GIT_TRACE2_PERF="$(pwd)/trace.output" &&
export GIT_TRACE2_PERF &&
- git replay --onto HEAD upstream~1 topic >out &&
+ git replay --onto HEAD upstream~1..topic >out &&
git update-ref --stdin <out &&
git checkout topic &&
@@ -685,7 +685,7 @@ test_expect_success 'caching renames only on upstream side, part 2' '
GIT_TRACE2_PERF="$(pwd)/trace.output" &&
export GIT_TRACE2_PERF &&
- git replay --onto HEAD upstream~1 topic >out &&
+ git replay --onto HEAD upstream~1..topic >out &&
git update-ref --stdin <out &&
git checkout topic &&
--
2.42.0.339.g663cbc8ab1
^ permalink raw reply related
* [PATCH v5 09/14] replay: remove HEAD related sanity check
From: Christian Couder @ 2023-10-10 12:38 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Patrick Steinhardt, Johannes Schindelin,
Elijah Newren, John Cai, Derrick Stolee, Phillip Wood, Calvin Wan,
Toon Claes, Dragan Simic, Linus Arver, Christian Couder
In-Reply-To: <20231010123847.2777056-1-christian.couder@gmail.com>
From: Elijah Newren <newren@gmail.com>
We want replay to be a command that can be used on the server side on
any branch, not just the current one, so we are going to stop updating
HEAD in a future commit.
A "sanity check" that makes sure we are replaying the current branch
doesn't make sense anymore. Let's remove it.
Co-authored-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
builtin/replay.c | 8 +-------
1 file changed, 1 insertion(+), 7 deletions(-)
diff --git a/builtin/replay.c b/builtin/replay.c
index 08a7f68420..87fe655c6f 100644
--- a/builtin/replay.c
+++ b/builtin/replay.c
@@ -123,7 +123,6 @@ int cmd_replay(int argc, const char **argv, const char *prefix)
struct commit *onto;
const char *onto_name = NULL;
struct commit *last_commit = NULL, *last_picked_commit = NULL;
- struct object_id head;
struct lock_file lock = LOCK_INIT;
struct strvec rev_walk_args = STRVEC_INIT;
struct rev_info revs;
@@ -162,11 +161,6 @@ int cmd_replay(int argc, const char **argv, const char *prefix)
onto = peel_committish(onto_name);
strbuf_addf(&branch_name, "refs/heads/%s", argv[2]);
- /* Sanity check */
- if (repo_get_oid(the_repository, "HEAD", &head))
- die(_("Cannot read HEAD"));
- assert(oideq(&onto->object.oid, &head));
-
repo_hold_locked_index(the_repository, &lock, LOCK_DIE_ON_ERROR);
if (repo_read_index(the_repository) < 0)
BUG("Could not read index");
@@ -238,7 +232,7 @@ int cmd_replay(int argc, const char **argv, const char *prefix)
oid_to_hex(&last_picked_commit->object.oid));
if (update_ref(reflog_msg.buf, "HEAD",
&last_commit->object.oid,
- &head,
+ &onto->object.oid,
REF_NO_DEREF, UPDATE_REFS_MSG_ON_ERR)) {
error(_("could not update %s"), argv[2]);
die("Failed to update %s", argv[2]);
--
2.42.0.339.g663cbc8ab1
^ permalink raw reply related
* [PATCH v5 08/14] replay: remove progress and info output
From: Christian Couder @ 2023-10-10 12:38 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Patrick Steinhardt, Johannes Schindelin,
Elijah Newren, John Cai, Derrick Stolee, Phillip Wood, Calvin Wan,
Toon Claes, Dragan Simic, Linus Arver, Christian Couder
In-Reply-To: <20231010123847.2777056-1-christian.couder@gmail.com>
From: Elijah Newren <newren@gmail.com>
The replay command will be changed in a follow up commit, so that it
will not update refs directly, but instead it will print on stdout a
list of commands that can be consumed by `git update-ref --stdin`.
We don't want this output to be polluted by its current low value
output, so let's just remove the latter.
In the future, when the command gets an option to update refs by
itself, it will make a lot of sense to display a progress meter, but
we are not there yet.
Co-authored-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
builtin/replay.c | 7 +------
1 file changed, 1 insertion(+), 6 deletions(-)
diff --git a/builtin/replay.c b/builtin/replay.c
index dbadf3b9b4..08a7f68420 100644
--- a/builtin/replay.c
+++ b/builtin/replay.c
@@ -195,7 +195,7 @@ int cmd_replay(int argc, const char **argv, const char *prefix)
init_merge_options(&merge_opt, the_repository);
memset(&result, 0, sizeof(result));
- merge_opt.show_rename_progress = 1;
+ merge_opt.show_rename_progress = 0;
merge_opt.branch1 = "HEAD";
head_tree = repo_get_commit_tree(the_repository, onto);
result.tree = head_tree;
@@ -203,9 +203,6 @@ int cmd_replay(int argc, const char **argv, const char *prefix)
while ((commit = get_revision(&revs))) {
struct commit *pick;
- fprintf(stderr, "Rebasing %s...\r",
- oid_to_hex(&commit->object.oid));
-
if (!commit->parents)
die(_("replaying down to root commit is not supported yet!"));
if (commit->parents->next)
@@ -224,7 +221,6 @@ int cmd_replay(int argc, const char **argv, const char *prefix)
exit(128);
if (result.clean) {
- fprintf(stderr, "\nDone.\n");
strbuf_addf(&reflog_msg, "finish rebase %s onto %s",
oid_to_hex(&last_picked_commit->object.oid),
oid_to_hex(&last_commit->object.oid));
@@ -238,7 +234,6 @@ int cmd_replay(int argc, const char **argv, const char *prefix)
if (create_symref("HEAD", branch_name.buf, reflog_msg.buf) < 0)
die(_("unable to update HEAD"));
} else {
- fprintf(stderr, "\nAborting: Hit a conflict.\n");
strbuf_addf(&reflog_msg, "rebase progress up to %s",
oid_to_hex(&last_picked_commit->object.oid));
if (update_ref(reflog_msg.buf, "HEAD",
--
2.42.0.339.g663cbc8ab1
^ permalink raw reply related
* [PATCH v5 10/14] replay: make it a minimal server side command
From: Christian Couder @ 2023-10-10 12:38 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Patrick Steinhardt, Johannes Schindelin,
Elijah Newren, John Cai, Derrick Stolee, Phillip Wood, Calvin Wan,
Toon Claes, Dragan Simic, Linus Arver, Christian Couder
In-Reply-To: <20231010123847.2777056-1-christian.couder@gmail.com>
From: Elijah Newren <newren@gmail.com>
We want this command to be a minimal command that just does server side
picking of commits, displaying the results on stdout for higher level
scripts to consume.
So let's simplify it:
* remove the worktree and index reading/writing,
* remove the ref (and reflog) updating,
* remove the assumptions tying us to HEAD, since (a) this is not a
rebase and (b) we want to be able to pick commits in a bare repo,
i.e. to/from branches that are not checked out and not the main
branch,
* remove unneeded includes,
* handle rebasing multiple branches by printing on stdout the update
ref commands that should be performed.
The output can be piped into `git update-ref --stdin` for the ref
updates to happen.
In the future to make it easier for users to use this command
directly maybe an option can be added to automatically pipe its output
into `git update-ref`.
Co-authored-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
builtin/replay.c | 78 ++++++++----------------
t/t6429-merge-sequence-rename-caching.sh | 39 +++++++-----
2 files changed, 50 insertions(+), 67 deletions(-)
diff --git a/builtin/replay.c b/builtin/replay.c
index 87fe655c6f..ace9ac6df0 100644
--- a/builtin/replay.c
+++ b/builtin/replay.c
@@ -6,11 +6,7 @@
#include "git-compat-util.h"
#include "builtin.h"
-#include "cache-tree.h"
-#include "commit.h"
#include "environment.h"
-#include "gettext.h"
-#include "hash.h"
#include "hex.h"
#include "lockfile.h"
#include "merge-ort.h"
@@ -18,8 +14,6 @@
#include "parse-options.h"
#include "refs.h"
#include "revision.h"
-#include "sequencer.h"
-#include "setup.h"
#include "strvec.h"
#include <oidset.h>
#include <tree.h>
@@ -102,6 +96,7 @@ static struct commit *pick_regular_commit(struct commit *pickme,
pickme_tree = repo_get_commit_tree(the_repository, pickme);
base_tree = repo_get_commit_tree(the_repository, base);
+ merge_opt->branch1 = short_commit_name(last_commit);
merge_opt->branch2 = short_commit_name(pickme);
merge_opt->ancestor = xstrfmt("parent of %s", merge_opt->branch2);
@@ -122,15 +117,12 @@ int cmd_replay(int argc, const char **argv, const char *prefix)
{
struct commit *onto;
const char *onto_name = NULL;
- struct commit *last_commit = NULL, *last_picked_commit = NULL;
- struct lock_file lock = LOCK_INIT;
+ struct commit *last_commit = NULL;
struct strvec rev_walk_args = STRVEC_INIT;
struct rev_info revs;
struct commit *commit;
struct merge_options merge_opt;
- struct tree *head_tree;
struct merge_result result;
- struct strbuf reflog_msg = STRBUF_INIT;
struct strbuf branch_name = STRBUF_INIT;
int ret = 0;
@@ -161,10 +153,6 @@ int cmd_replay(int argc, const char **argv, const char *prefix)
onto = peel_committish(onto_name);
strbuf_addf(&branch_name, "refs/heads/%s", argv[2]);
- repo_hold_locked_index(the_repository, &lock, LOCK_DIE_ON_ERROR);
- if (repo_read_index(the_repository) < 0)
- BUG("Could not read index");
-
repo_init_revisions(the_repository, &revs, prefix);
strvec_pushl(&rev_walk_args, "", argv[2], "--not", argv[1], NULL);
@@ -190,58 +178,44 @@ int cmd_replay(int argc, const char **argv, const char *prefix)
init_merge_options(&merge_opt, the_repository);
memset(&result, 0, sizeof(result));
merge_opt.show_rename_progress = 0;
- merge_opt.branch1 = "HEAD";
- head_tree = repo_get_commit_tree(the_repository, onto);
- result.tree = head_tree;
+ result.tree = repo_get_commit_tree(the_repository, onto);
last_commit = onto;
while ((commit = get_revision(&revs))) {
- struct commit *pick;
+ const struct name_decoration *decoration;
if (!commit->parents)
die(_("replaying down to root commit is not supported yet!"));
if (commit->parents->next)
die(_("replaying merge commits is not supported yet!"));
- pick = pick_regular_commit(commit, last_commit, &merge_opt, &result);
- if (!pick)
+ last_commit = pick_regular_commit(commit, last_commit, &merge_opt, &result);
+ if (!last_commit)
break;
- last_commit = pick;
- last_picked_commit = commit;
+
+ decoration = get_name_decoration(&commit->object);
+ if (!decoration)
+ continue;
+
+ while (decoration) {
+ if (decoration->type == DECORATION_REF_LOCAL) {
+ printf("update %s %s %s\n",
+ decoration->name,
+ oid_to_hex(&last_commit->object.oid),
+ oid_to_hex(&commit->object.oid));
+ }
+ decoration = decoration->next;
+ }
}
merge_finalize(&merge_opt, &result);
+ ret = result.clean;
- if (result.clean < 0)
- exit(128);
-
- if (result.clean) {
- strbuf_addf(&reflog_msg, "finish rebase %s onto %s",
- oid_to_hex(&last_picked_commit->object.oid),
- oid_to_hex(&last_commit->object.oid));
- if (update_ref(reflog_msg.buf, branch_name.buf,
- &last_commit->object.oid,
- &last_picked_commit->object.oid,
- REF_NO_DEREF, UPDATE_REFS_MSG_ON_ERR)) {
- error(_("could not update %s"), argv[2]);
- die("Failed to update %s", argv[2]);
- }
- if (create_symref("HEAD", branch_name.buf, reflog_msg.buf) < 0)
- die(_("unable to update HEAD"));
- } else {
- strbuf_addf(&reflog_msg, "rebase progress up to %s",
- oid_to_hex(&last_picked_commit->object.oid));
- if (update_ref(reflog_msg.buf, "HEAD",
- &last_commit->object.oid,
- &onto->object.oid,
- REF_NO_DEREF, UPDATE_REFS_MSG_ON_ERR)) {
- error(_("could not update %s"), argv[2]);
- die("Failed to update %s", argv[2]);
- }
- }
- ret = (result.clean == 0);
cleanup:
- strbuf_release(&reflog_msg);
strbuf_release(&branch_name);
release_revisions(&revs);
- return ret;
+
+ /* Return */
+ if (ret < 0)
+ exit(128);
+ return ret ? 0 : 1;
}
diff --git a/t/t6429-merge-sequence-rename-caching.sh b/t/t6429-merge-sequence-rename-caching.sh
index 7670b72008..099aefeffc 100755
--- a/t/t6429-merge-sequence-rename-caching.sh
+++ b/t/t6429-merge-sequence-rename-caching.sh
@@ -71,8 +71,9 @@ test_expect_success 'caching renames does not preclude finding new ones' '
git switch upstream &&
- git replay --onto HEAD upstream~1 topic &&
- git reset --hard topic &&
+ git replay --onto HEAD upstream~1 topic >out &&
+ git update-ref --stdin <out &&
+ git checkout topic &&
git ls-files >tracked-files &&
test_line_count = 2 tracked-files &&
@@ -140,7 +141,9 @@ test_expect_success 'cherry-pick both a commit and its immediate revert' '
GIT_TRACE2_PERF="$(pwd)/trace.output" &&
export GIT_TRACE2_PERF &&
- git replay --onto HEAD upstream~1 topic &&
+ git replay --onto HEAD upstream~1 topic >out &&
+ git update-ref --stdin <out &&
+ git checkout topic &&
grep region_enter.*diffcore_rename trace.output >calls &&
test_line_count = 1 calls
@@ -198,8 +201,9 @@ test_expect_success 'rename same file identically, then reintroduce it' '
GIT_TRACE2_PERF="$(pwd)/trace.output" &&
export GIT_TRACE2_PERF &&
- git replay --onto HEAD upstream~1 topic &&
- git reset --hard topic &&
+ git replay --onto HEAD upstream~1 topic >out &&
+ git update-ref --stdin <out &&
+ git checkout topic &&
git ls-files >tracked &&
test_line_count = 2 tracked &&
@@ -275,8 +279,9 @@ test_expect_success 'rename same file identically, then add file to old dir' '
GIT_TRACE2_PERF="$(pwd)/trace.output" &&
export GIT_TRACE2_PERF &&
- git replay --onto HEAD upstream~1 topic &&
- git reset --hard topic &&
+ git replay --onto HEAD upstream~1 topic >out &&
+ git update-ref --stdin <out &&
+ git checkout topic &&
git ls-files >tracked &&
test_line_count = 4 tracked &&
@@ -451,8 +456,9 @@ test_expect_success 'dir rename unneeded, then add new file to old dir' '
GIT_TRACE2_PERF="$(pwd)/trace.output" &&
export GIT_TRACE2_PERF &&
- git replay --onto HEAD upstream~1 topic &&
- git reset --hard topic &&
+ git replay --onto HEAD upstream~1 topic >out &&
+ git update-ref --stdin <out &&
+ git checkout topic &&
grep region_enter.*diffcore_rename trace.output >calls &&
test_line_count = 2 calls &&
@@ -517,8 +523,9 @@ test_expect_success 'dir rename unneeded, then rename existing file into old dir
GIT_TRACE2_PERF="$(pwd)/trace.output" &&
export GIT_TRACE2_PERF &&
- git replay --onto HEAD upstream~1 topic &&
- git reset --hard topic &&
+ git replay --onto HEAD upstream~1 topic >out &&
+ git update-ref --stdin <out &&
+ git checkout topic &&
grep region_enter.*diffcore_rename trace.output >calls &&
test_line_count = 3 calls &&
@@ -619,8 +626,9 @@ test_expect_success 'caching renames only on upstream side, part 1' '
GIT_TRACE2_PERF="$(pwd)/trace.output" &&
export GIT_TRACE2_PERF &&
- git replay --onto HEAD upstream~1 topic &&
- git reset --hard topic &&
+ git replay --onto HEAD upstream~1 topic >out &&
+ git update-ref --stdin <out &&
+ git checkout topic &&
grep region_enter.*diffcore_rename trace.output >calls &&
test_line_count = 1 calls &&
@@ -677,8 +685,9 @@ test_expect_success 'caching renames only on upstream side, part 2' '
GIT_TRACE2_PERF="$(pwd)/trace.output" &&
export GIT_TRACE2_PERF &&
- git replay --onto HEAD upstream~1 topic &&
- git reset --hard topic &&
+ git replay --onto HEAD upstream~1 topic >out &&
+ git update-ref --stdin <out &&
+ git checkout topic &&
grep region_enter.*diffcore_rename trace.output >calls &&
test_line_count = 2 calls &&
--
2.42.0.339.g663cbc8ab1
^ permalink raw reply related
* [PATCH v5 06/14] replay: change rev walking options
From: Christian Couder @ 2023-10-10 12:38 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Patrick Steinhardt, Johannes Schindelin,
Elijah Newren, John Cai, Derrick Stolee, Phillip Wood, Calvin Wan,
Toon Claes, Dragan Simic, Linus Arver, Christian Couder
In-Reply-To: <20231010123847.2777056-1-christian.couder@gmail.com>
From: Elijah Newren <newren@gmail.com>
Let's set the rev walking options we need after calling
setup_revisions() instead of before. This enforces options we always
want.
We want the command to work from older commits to newer ones by default,
but we are Ok with letting users reverse that, using --reverse, if that's
what they really want.
Also we don't want history simplification, as we want to deal with all
the commits in the affected range.
Helped-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
Co-authored-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
builtin/replay.c | 15 ++++++---------
1 file changed, 6 insertions(+), 9 deletions(-)
diff --git a/builtin/replay.c b/builtin/replay.c
index c66888679b..5bab89857f 100644
--- a/builtin/replay.c
+++ b/builtin/replay.c
@@ -173,15 +173,6 @@ int cmd_replay(int argc, const char **argv, const char *prefix)
repo_init_revisions(the_repository, &revs, prefix);
- revs.verbose_header = 1;
- revs.max_parents = 1;
- revs.cherry_mark = 1;
- revs.limited = 1;
- revs.reverse = 1;
- revs.right_only = 1;
- revs.sort_order = REV_SORT_IN_GRAPH_ORDER;
- revs.topo_order = 1;
-
strvec_pushl(&rev_walk_args, "", argv[2], "--not", argv[1], NULL);
if (setup_revisions(rev_walk_args.nr, rev_walk_args.v, &revs, NULL) > 1) {
@@ -189,6 +180,12 @@ int cmd_replay(int argc, const char **argv, const char *prefix)
goto cleanup;
}
+ /* requirements/overrides for revs */
+ revs.reverse = !revs.reverse;
+ revs.sort_order = REV_SORT_IN_GRAPH_ORDER;
+ revs.topo_order = 1;
+ revs.simplify_history = 0;
+
strvec_clear(&rev_walk_args);
if (prepare_revision_walk(&revs) < 0) {
--
2.42.0.339.g663cbc8ab1
^ permalink raw reply related
* [PATCH v5 07/14] replay: add an important FIXME comment about gpg signing
From: Christian Couder @ 2023-10-10 12:38 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Patrick Steinhardt, Johannes Schindelin,
Elijah Newren, John Cai, Derrick Stolee, Phillip Wood, Calvin Wan,
Toon Claes, Dragan Simic, Linus Arver, Christian Couder
In-Reply-To: <20231010123847.2777056-1-christian.couder@gmail.com>
From: Elijah Newren <newren@gmail.com>
We want to be able to handle signed commits in some way in the future,
but we are not ready to do it now. So for the time being let's just add
a FIXME comment to remind us about it.
These are different ways we could handle them:
- in case of a cli user and if there was an interactive mode, we could
perhaps ask if the user wants to sign again
- we could add an option to just fail if there are signed commits
Co-authored-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
builtin/replay.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/builtin/replay.c b/builtin/replay.c
index 5bab89857f..dbadf3b9b4 100644
--- a/builtin/replay.c
+++ b/builtin/replay.c
@@ -62,7 +62,7 @@ static struct commit *create_commit(struct tree *tree,
struct object *obj;
struct commit_list *parents = NULL;
char *author;
- char *sign_commit = NULL;
+ char *sign_commit = NULL; /* FIXME: cli users might want to sign again */
struct commit_extra_header *extra;
struct strbuf msg = STRBUF_INIT;
const char *out_enc = get_commit_output_encoding();
--
2.42.0.339.g663cbc8ab1
^ permalink raw reply related
* [PATCH v5 05/14] replay: introduce pick_regular_commit()
From: Christian Couder @ 2023-10-10 12:38 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Patrick Steinhardt, Johannes Schindelin,
Elijah Newren, John Cai, Derrick Stolee, Phillip Wood, Calvin Wan,
Toon Claes, Dragan Simic, Linus Arver, Christian Couder
In-Reply-To: <20231010123847.2777056-1-christian.couder@gmail.com>
From: Elijah Newren <newren@gmail.com>
Let's refactor the code to handle a regular commit (a commit that is
neither a root commit nor a merge commit) into a single function instead
of keeping it inside cmd_replay().
This is good for separation of concerns, and this will help further work
in the future to replay merge commits.
Co-authored-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
builtin/replay.c | 54 ++++++++++++++++++++++++++++++------------------
1 file changed, 34 insertions(+), 20 deletions(-)
diff --git a/builtin/replay.c b/builtin/replay.c
index f3fdbe48c9..c66888679b 100644
--- a/builtin/replay.c
+++ b/builtin/replay.c
@@ -89,6 +89,35 @@ static struct commit *create_commit(struct tree *tree,
return (struct commit *)obj;
}
+static struct commit *pick_regular_commit(struct commit *pickme,
+ struct commit *last_commit,
+ struct merge_options *merge_opt,
+ struct merge_result *result)
+{
+ struct commit *base;
+ struct tree *pickme_tree, *base_tree;
+
+ base = pickme->parents->item;
+
+ pickme_tree = repo_get_commit_tree(the_repository, pickme);
+ base_tree = repo_get_commit_tree(the_repository, base);
+
+ merge_opt->branch2 = short_commit_name(pickme);
+ merge_opt->ancestor = xstrfmt("parent of %s", merge_opt->branch2);
+
+ merge_incore_nonrecursive(merge_opt,
+ base_tree,
+ result->tree,
+ pickme_tree,
+ result);
+
+ free((char*)merge_opt->ancestor);
+ merge_opt->ancestor = NULL;
+ if (!result->clean)
+ return NULL;
+ return create_commit(result->tree, pickme, last_commit);
+}
+
int cmd_replay(int argc, const char **argv, const char *prefix)
{
struct commit *onto;
@@ -100,7 +129,7 @@ int cmd_replay(int argc, const char **argv, const char *prefix)
struct rev_info revs;
struct commit *commit;
struct merge_options merge_opt;
- struct tree *next_tree, *base_tree, *head_tree;
+ struct tree *head_tree;
struct merge_result result;
struct strbuf reflog_msg = STRBUF_INIT;
struct strbuf branch_name = STRBUF_INIT;
@@ -175,7 +204,7 @@ int cmd_replay(int argc, const char **argv, const char *prefix)
result.tree = head_tree;
last_commit = onto;
while ((commit = get_revision(&revs))) {
- struct commit *base;
+ struct commit *pick;
fprintf(stderr, "Rebasing %s...\r",
oid_to_hex(&commit->object.oid));
@@ -185,26 +214,11 @@ int cmd_replay(int argc, const char **argv, const char *prefix)
if (commit->parents->next)
die(_("replaying merge commits is not supported yet!"));
- base = commit->parents->item;
-
- next_tree = repo_get_commit_tree(the_repository, commit);
- base_tree = repo_get_commit_tree(the_repository, base);
-
- merge_opt.branch2 = short_commit_name(commit);
- merge_opt.ancestor = xstrfmt("parent of %s", merge_opt.branch2);
-
- merge_incore_nonrecursive(&merge_opt,
- base_tree,
- result.tree,
- next_tree,
- &result);
-
- free((char*)merge_opt.ancestor);
- merge_opt.ancestor = NULL;
- if (!result.clean)
+ pick = pick_regular_commit(commit, last_commit, &merge_opt, &result);
+ if (!pick)
break;
+ last_commit = pick;
last_picked_commit = commit;
- last_commit = create_commit(result.tree, commit, last_commit);
}
merge_finalize(&merge_opt, &result);
--
2.42.0.339.g663cbc8ab1
^ permalink raw reply related
* [PATCH v5 04/14] replay: die() instead of failing assert()
From: Christian Couder @ 2023-10-10 12:38 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Patrick Steinhardt, Johannes Schindelin,
Elijah Newren, John Cai, Derrick Stolee, Phillip Wood, Calvin Wan,
Toon Claes, Dragan Simic, Linus Arver, Christian Couder
In-Reply-To: <20231010123847.2777056-1-christian.couder@gmail.com>
From: Elijah Newren <newren@gmail.com>
It's not a good idea for regular Git commands to use an assert() to
check for things that could happen but are not supported.
Let's die() with an explanation of the issue instead.
Co-authored-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
builtin/replay.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/builtin/replay.c b/builtin/replay.c
index d6dec7c866..f3fdbe48c9 100644
--- a/builtin/replay.c
+++ b/builtin/replay.c
@@ -179,7 +179,12 @@ int cmd_replay(int argc, const char **argv, const char *prefix)
fprintf(stderr, "Rebasing %s...\r",
oid_to_hex(&commit->object.oid));
- assert(commit->parents && !commit->parents->next);
+
+ if (!commit->parents)
+ die(_("replaying down to root commit is not supported yet!"));
+ if (commit->parents->next)
+ die(_("replaying merge commits is not supported yet!"));
+
base = commit->parents->item;
next_tree = repo_get_commit_tree(the_repository, commit);
--
2.42.0.339.g663cbc8ab1
^ permalink raw reply related
* [PATCH v5 02/14] replay: introduce new builtin
From: Christian Couder @ 2023-10-10 12:38 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Patrick Steinhardt, Johannes Schindelin,
Elijah Newren, John Cai, Derrick Stolee, Phillip Wood, Calvin Wan,
Toon Claes, Dragan Simic, Linus Arver, Christian Couder
In-Reply-To: <20231010123847.2777056-1-christian.couder@gmail.com>
From: Elijah Newren <newren@gmail.com>
For now, this is just a rename from `t/helper/test-fast-rebase.c` into
`builtin/replay.c` with minimal changes to make it build appropriately.
Subsequent commits will flesh out its capabilities and make it a more
standard regular builtin.
Helped-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
Co-authored-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
.gitignore | 1 +
Makefile | 2 +-
builtin.h | 1 +
.../test-fast-rebase.c => builtin/replay.c | 27 ++++++-------------
command-list.txt | 1 +
git.c | 1 +
t/helper/test-tool.c | 1 -
t/helper/test-tool.h | 1 -
t/t6429-merge-sequence-rename-caching.sh | 27 +++++++------------
9 files changed, 22 insertions(+), 40 deletions(-)
rename t/helper/test-fast-rebase.c => builtin/replay.c (89%)
diff --git a/.gitignore b/.gitignore
index 5e56e471b3..612c0f6a0f 100644
--- a/.gitignore
+++ b/.gitignore
@@ -135,6 +135,7 @@
/git-remote-ext
/git-repack
/git-replace
+/git-replay
/git-request-pull
/git-rerere
/git-reset
diff --git a/Makefile b/Makefile
index 003e63b792..b5e3843d7f 100644
--- a/Makefile
+++ b/Makefile
@@ -799,7 +799,6 @@ TEST_BUILTINS_OBJS += test-dump-split-index.o
TEST_BUILTINS_OBJS += test-dump-untracked-cache.o
TEST_BUILTINS_OBJS += test-env-helper.o
TEST_BUILTINS_OBJS += test-example-decorate.o
-TEST_BUILTINS_OBJS += test-fast-rebase.o
TEST_BUILTINS_OBJS += test-fsmonitor-client.o
TEST_BUILTINS_OBJS += test-genrandom.o
TEST_BUILTINS_OBJS += test-genzeros.o
@@ -1286,6 +1285,7 @@ BUILTIN_OBJS += builtin/remote-fd.o
BUILTIN_OBJS += builtin/remote.o
BUILTIN_OBJS += builtin/repack.o
BUILTIN_OBJS += builtin/replace.o
+BUILTIN_OBJS += builtin/replay.o
BUILTIN_OBJS += builtin/rerere.o
BUILTIN_OBJS += builtin/reset.o
BUILTIN_OBJS += builtin/rev-list.o
diff --git a/builtin.h b/builtin.h
index d560baa661..28280636da 100644
--- a/builtin.h
+++ b/builtin.h
@@ -211,6 +211,7 @@ int cmd_remote(int argc, const char **argv, const char *prefix);
int cmd_remote_ext(int argc, const char **argv, const char *prefix);
int cmd_remote_fd(int argc, const char **argv, const char *prefix);
int cmd_repack(int argc, const char **argv, const char *prefix);
+int cmd_replay(int argc, const char **argv, const char *prefix);
int cmd_rerere(int argc, const char **argv, const char *prefix);
int cmd_reset(int argc, const char **argv, const char *prefix);
int cmd_restore(int argc, const char **argv, const char *prefix);
diff --git a/t/helper/test-fast-rebase.c b/builtin/replay.c
similarity index 89%
rename from t/helper/test-fast-rebase.c
rename to builtin/replay.c
index 2bfab66b1b..e102749ab6 100644
--- a/t/helper/test-fast-rebase.c
+++ b/builtin/replay.c
@@ -1,17 +1,11 @@
/*
- * "git fast-rebase" builtin command
- *
- * FAST: Forking Any Subprocesses (is) Taboo
- *
- * This is meant SOLELY as a demo of what is possible. sequencer.c and
- * rebase.c should be refactored to use the ideas here, rather than attempting
- * to extend this file to replace those (unless Phillip or Dscho say that
- * refactoring is too hard and we need a clean slate, but I'm guessing that
- * refactoring is the better route).
+ * "git replay" builtin command
*/
#define USE_THE_INDEX_VARIABLE
-#include "test-tool.h"
+#include "git-compat-util.h"
+
+#include "builtin.h"
#include "cache-tree.h"
#include "commit.h"
#include "environment.h"
@@ -27,7 +21,8 @@
#include "sequencer.h"
#include "setup.h"
#include "strvec.h"
-#include "tree.h"
+#include <oidset.h>
+#include <tree.h>
static const char *short_commit_name(struct commit *commit)
{
@@ -94,7 +89,7 @@ static struct commit *create_commit(struct tree *tree,
return (struct commit *)obj;
}
-int cmd__fast_rebase(int argc, const char **argv)
+int cmd_replay(int argc, const char **argv, const char *prefix)
{
struct commit *onto;
struct commit *last_commit = NULL, *last_picked_commit = NULL;
@@ -110,12 +105,6 @@ int cmd__fast_rebase(int argc, const char **argv)
struct strbuf branch_name = STRBUF_INIT;
int ret = 0;
- /*
- * test-tool stuff doesn't set up the git directory by default; need to
- * do that manually.
- */
- setup_git_directory();
-
if (argc == 2 && !strcmp(argv[1], "-h")) {
printf("Sorry, I am not a psychiatrist; I can not give you the help you need. Oh, you meant usage...\n");
exit(129);
@@ -136,7 +125,7 @@ int cmd__fast_rebase(int argc, const char **argv)
if (repo_read_index(the_repository) < 0)
BUG("Could not read index");
- repo_init_revisions(the_repository, &revs, NULL);
+ repo_init_revisions(the_repository, &revs, prefix);
revs.verbose_header = 1;
revs.max_parents = 1;
revs.cherry_mark = 1;
diff --git a/command-list.txt b/command-list.txt
index 54b2a50f5f..c4cd0f352b 100644
--- a/command-list.txt
+++ b/command-list.txt
@@ -160,6 +160,7 @@ git-reflog ancillarymanipulators complete
git-remote ancillarymanipulators complete
git-repack ancillarymanipulators complete
git-replace ancillarymanipulators complete
+git-replay plumbingmanipulators
git-request-pull foreignscminterface complete
git-rerere ancillaryinterrogators
git-reset mainporcelain history
diff --git a/git.c b/git.c
index c67e44dd82..7068a184b0 100644
--- a/git.c
+++ b/git.c
@@ -594,6 +594,7 @@ static struct cmd_struct commands[] = {
{ "remote-fd", cmd_remote_fd, NO_PARSEOPT },
{ "repack", cmd_repack, RUN_SETUP },
{ "replace", cmd_replace, RUN_SETUP },
+ { "replay", cmd_replay, RUN_SETUP },
{ "rerere", cmd_rerere, RUN_SETUP },
{ "reset", cmd_reset, RUN_SETUP },
{ "restore", cmd_restore, RUN_SETUP | NEED_WORK_TREE },
diff --git a/t/helper/test-tool.c b/t/helper/test-tool.c
index 621ac3dd10..3109ba5c7a 100644
--- a/t/helper/test-tool.c
+++ b/t/helper/test-tool.c
@@ -30,7 +30,6 @@ static struct test_cmd cmds[] = {
{ "dump-untracked-cache", cmd__dump_untracked_cache },
{ "env-helper", cmd__env_helper },
{ "example-decorate", cmd__example_decorate },
- { "fast-rebase", cmd__fast_rebase },
{ "fsmonitor-client", cmd__fsmonitor_client },
{ "genrandom", cmd__genrandom },
{ "genzeros", cmd__genzeros },
diff --git a/t/helper/test-tool.h b/t/helper/test-tool.h
index a641c3a81d..d3e55c3f4e 100644
--- a/t/helper/test-tool.h
+++ b/t/helper/test-tool.h
@@ -24,7 +24,6 @@ int cmd__dump_untracked_cache(int argc, const char **argv);
int cmd__dump_reftable(int argc, const char **argv);
int cmd__env_helper(int argc, const char **argv);
int cmd__example_decorate(int argc, const char **argv);
-int cmd__fast_rebase(int argc, const char **argv);
int cmd__fsmonitor_client(int argc, const char **argv);
int cmd__genrandom(int argc, const char **argv);
int cmd__genzeros(int argc, const char **argv);
diff --git a/t/t6429-merge-sequence-rename-caching.sh b/t/t6429-merge-sequence-rename-caching.sh
index 75d3fd2dba..7670b72008 100755
--- a/t/t6429-merge-sequence-rename-caching.sh
+++ b/t/t6429-merge-sequence-rename-caching.sh
@@ -71,9 +71,8 @@ test_expect_success 'caching renames does not preclude finding new ones' '
git switch upstream &&
- test-tool fast-rebase --onto HEAD upstream~1 topic &&
+ git replay --onto HEAD upstream~1 topic &&
git reset --hard topic &&
- #git cherry-pick upstream~1..topic
git ls-files >tracked-files &&
test_line_count = 2 tracked-files &&
@@ -141,8 +140,7 @@ test_expect_success 'cherry-pick both a commit and its immediate revert' '
GIT_TRACE2_PERF="$(pwd)/trace.output" &&
export GIT_TRACE2_PERF &&
- test-tool fast-rebase --onto HEAD upstream~1 topic &&
- #git cherry-pick upstream~1..topic &&
+ git replay --onto HEAD upstream~1 topic &&
grep region_enter.*diffcore_rename trace.output >calls &&
test_line_count = 1 calls
@@ -200,9 +198,8 @@ test_expect_success 'rename same file identically, then reintroduce it' '
GIT_TRACE2_PERF="$(pwd)/trace.output" &&
export GIT_TRACE2_PERF &&
- test-tool fast-rebase --onto HEAD upstream~1 topic &&
+ git replay --onto HEAD upstream~1 topic &&
git reset --hard topic &&
- #git cherry-pick upstream~1..topic &&
git ls-files >tracked &&
test_line_count = 2 tracked &&
@@ -278,9 +275,8 @@ test_expect_success 'rename same file identically, then add file to old dir' '
GIT_TRACE2_PERF="$(pwd)/trace.output" &&
export GIT_TRACE2_PERF &&
- test-tool fast-rebase --onto HEAD upstream~1 topic &&
+ git replay --onto HEAD upstream~1 topic &&
git reset --hard topic &&
- #git cherry-pick upstream~1..topic &&
git ls-files >tracked &&
test_line_count = 4 tracked &&
@@ -356,8 +352,7 @@ test_expect_success 'cached dir rename does not prevent noticing later conflict'
GIT_TRACE2_PERF="$(pwd)/trace.output" &&
export GIT_TRACE2_PERF &&
- test_must_fail test-tool fast-rebase --onto HEAD upstream~1 topic >output &&
- #git cherry-pick upstream..topic &&
+ test_must_fail git replay --onto HEAD upstream~1 topic >output &&
grep region_enter.*diffcore_rename trace.output >calls &&
test_line_count = 2 calls
@@ -456,9 +451,8 @@ test_expect_success 'dir rename unneeded, then add new file to old dir' '
GIT_TRACE2_PERF="$(pwd)/trace.output" &&
export GIT_TRACE2_PERF &&
- test-tool fast-rebase --onto HEAD upstream~1 topic &&
+ git replay --onto HEAD upstream~1 topic &&
git reset --hard topic &&
- #git cherry-pick upstream..topic &&
grep region_enter.*diffcore_rename trace.output >calls &&
test_line_count = 2 calls &&
@@ -523,9 +517,8 @@ test_expect_success 'dir rename unneeded, then rename existing file into old dir
GIT_TRACE2_PERF="$(pwd)/trace.output" &&
export GIT_TRACE2_PERF &&
- test-tool fast-rebase --onto HEAD upstream~1 topic &&
+ git replay --onto HEAD upstream~1 topic &&
git reset --hard topic &&
- #git cherry-pick upstream..topic &&
grep region_enter.*diffcore_rename trace.output >calls &&
test_line_count = 3 calls &&
@@ -626,9 +619,8 @@ test_expect_success 'caching renames only on upstream side, part 1' '
GIT_TRACE2_PERF="$(pwd)/trace.output" &&
export GIT_TRACE2_PERF &&
- test-tool fast-rebase --onto HEAD upstream~1 topic &&
+ git replay --onto HEAD upstream~1 topic &&
git reset --hard topic &&
- #git cherry-pick upstream..topic &&
grep region_enter.*diffcore_rename trace.output >calls &&
test_line_count = 1 calls &&
@@ -685,9 +677,8 @@ test_expect_success 'caching renames only on upstream side, part 2' '
GIT_TRACE2_PERF="$(pwd)/trace.output" &&
export GIT_TRACE2_PERF &&
- test-tool fast-rebase --onto HEAD upstream~1 topic &&
+ git replay --onto HEAD upstream~1 topic &&
git reset --hard topic &&
- #git cherry-pick upstream..topic &&
grep region_enter.*diffcore_rename trace.output >calls &&
test_line_count = 2 calls &&
--
2.42.0.339.g663cbc8ab1
^ permalink raw reply related
* [PATCH v5 03/14] replay: start using parse_options API
From: Christian Couder @ 2023-10-10 12:38 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Patrick Steinhardt, Johannes Schindelin,
Elijah Newren, John Cai, Derrick Stolee, Phillip Wood, Calvin Wan,
Toon Claes, Dragan Simic, Linus Arver, Christian Couder
In-Reply-To: <20231010123847.2777056-1-christian.couder@gmail.com>
From: Elijah Newren <newren@gmail.com>
Instead of manually parsing arguments, let's start using the parse_options
API. This way this new builtin will look more standard, and in some
upcoming commits will more easily be able to handle more command line
options.
Note that we plan to later use standard revision ranges instead of
hardcoded "<oldbase> <branch>" arguments. When we will use standard
revision ranges, it will be easier to check if there are no spurious
arguments if we keep ARGV[0], so let's call parse_options() with
PARSE_OPT_KEEP_ARGV0 even if we don't need ARGV[0] right now to avoid
some useless code churn.
Co-authored-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
builtin/replay.c | 45 ++++++++++++++++++++++++++++++++-------------
1 file changed, 32 insertions(+), 13 deletions(-)
diff --git a/builtin/replay.c b/builtin/replay.c
index e102749ab6..d6dec7c866 100644
--- a/builtin/replay.c
+++ b/builtin/replay.c
@@ -15,7 +15,7 @@
#include "lockfile.h"
#include "merge-ort.h"
#include "object-name.h"
-#include "read-cache-ll.h"
+#include "parse-options.h"
#include "refs.h"
#include "revision.h"
#include "sequencer.h"
@@ -92,6 +92,7 @@ static struct commit *create_commit(struct tree *tree,
int cmd_replay(int argc, const char **argv, const char *prefix)
{
struct commit *onto;
+ const char *onto_name = NULL;
struct commit *last_commit = NULL, *last_picked_commit = NULL;
struct object_id head;
struct lock_file lock = LOCK_INIT;
@@ -105,16 +106,32 @@ int cmd_replay(int argc, const char **argv, const char *prefix)
struct strbuf branch_name = STRBUF_INIT;
int ret = 0;
- if (argc == 2 && !strcmp(argv[1], "-h")) {
- printf("Sorry, I am not a psychiatrist; I can not give you the help you need. Oh, you meant usage...\n");
- exit(129);
+ const char * const replay_usage[] = {
+ N_("git replay --onto <newbase> <oldbase> <branch>"),
+ NULL
+ };
+ struct option replay_options[] = {
+ OPT_STRING(0, "onto", &onto_name,
+ N_("revision"),
+ N_("replay onto given commit")),
+ OPT_END()
+ };
+
+ argc = parse_options(argc, argv, prefix, replay_options, replay_usage,
+ PARSE_OPT_KEEP_ARGV0 | PARSE_OPT_KEEP_UNKNOWN_OPT);
+
+ if (!onto_name) {
+ error(_("option --onto is mandatory"));
+ usage_with_options(replay_usage, replay_options);
}
- if (argc != 5 || strcmp(argv[1], "--onto"))
- die("usage: read the code, figure out how to use it, then do so");
+ if (argc != 3) {
+ error(_("bad number of arguments"));
+ usage_with_options(replay_usage, replay_options);
+ }
- onto = peel_committish(argv[2]);
- strbuf_addf(&branch_name, "refs/heads/%s", argv[4]);
+ onto = peel_committish(onto_name);
+ strbuf_addf(&branch_name, "refs/heads/%s", argv[2]);
/* Sanity check */
if (repo_get_oid(the_repository, "HEAD", &head))
@@ -126,6 +143,7 @@ int cmd_replay(int argc, const char **argv, const char *prefix)
BUG("Could not read index");
repo_init_revisions(the_repository, &revs, prefix);
+
revs.verbose_header = 1;
revs.max_parents = 1;
revs.cherry_mark = 1;
@@ -134,7 +152,8 @@ int cmd_replay(int argc, const char **argv, const char *prefix)
revs.right_only = 1;
revs.sort_order = REV_SORT_IN_GRAPH_ORDER;
revs.topo_order = 1;
- strvec_pushl(&rev_walk_args, "", argv[4], "--not", argv[3], NULL);
+
+ strvec_pushl(&rev_walk_args, "", argv[2], "--not", argv[1], NULL);
if (setup_revisions(rev_walk_args.nr, rev_walk_args.v, &revs, NULL) > 1) {
ret = error(_("unhandled options"));
@@ -197,8 +216,8 @@ int cmd_replay(int argc, const char **argv, const char *prefix)
&last_commit->object.oid,
&last_picked_commit->object.oid,
REF_NO_DEREF, UPDATE_REFS_MSG_ON_ERR)) {
- error(_("could not update %s"), argv[4]);
- die("Failed to update %s", argv[4]);
+ error(_("could not update %s"), argv[2]);
+ die("Failed to update %s", argv[2]);
}
if (create_symref("HEAD", branch_name.buf, reflog_msg.buf) < 0)
die(_("unable to update HEAD"));
@@ -210,8 +229,8 @@ int cmd_replay(int argc, const char **argv, const char *prefix)
&last_commit->object.oid,
&head,
REF_NO_DEREF, UPDATE_REFS_MSG_ON_ERR)) {
- error(_("could not update %s"), argv[4]);
- die("Failed to update %s", argv[4]);
+ error(_("could not update %s"), argv[2]);
+ die("Failed to update %s", argv[2]);
}
}
ret = (result.clean == 0);
--
2.42.0.339.g663cbc8ab1
^ permalink raw reply related
* [PATCH v5 00/14] Introduce new `git replay` command
From: Christian Couder @ 2023-10-10 12:38 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Patrick Steinhardt, Johannes Schindelin,
Elijah Newren, John Cai, Derrick Stolee, Phillip Wood, Calvin Wan,
Toon Claes, Dragan Simic, Linus Arver, Christian Couder
In-Reply-To: <20230907092521.733746-1-christian.couder@gmail.com>
# Intro
`git replay` has initially been developed entirely by Elijah Newren
mostly last year (2022) at:
https://github.com/newren/git/commits/replay
I took over this year to polish and upstream it as GitLab is
interested in replacing libgit2, and for that purpose needs a command
to do server side (so without using a worktree) rebases, cherry-picks
and reverts.
I reduced the number of commits and features in this first patch
series, compared to what Elijah already developed. Especially I
stopped short of replaying merge commits and replaying
interactively. These and other features might be upstreamed in the
future after this patch series has graduated.
The focus in this series is to make it a good plumbing command that
can already be used server side and that replaces the "fast-rebase"
test-tool command. So things to make it easier to use on the command
line, and more advanced features (like replaying merges) are left out.
It looks like GitHub has actually already been using version 3 of this
patch series in production with good results. See:
https://github.blog/2023-07-27-scaling-merge-ort-across-github/
https://lore.kernel.org/git/304f2a49-5e05-7655-9f87-2011606df5db@gmx.de/
# Content of this cover letter
The "Quick Overview" and "Reasons for diverging from cherry-pick &
rebase" sections just below are describing the purpose of the new
command in the big scheme of things. They are taken from Elijah's
design notes
(https://github.com/newren/git/blob/replay/replay-design-notes.txt)
and describe what we want this command to become and the reasons for
that, not what the command is after only this patch series. Also these
design notes were written at least one year ago, so parts of those 2
sections are not true anymore. I have added Phillip Wood's or Felipe
Contreras' notes (thanks to them) where that's the case, but some now
flawed parts may have missed.
After these two sections, starting with the "Important limitations"
section, you will find sections describing what is actually in this
patch series.
More interesting material is available in Elijah's design notes like
an "Intro via examples"
(https://github.com/newren/git/blob/replay/replay-design-notes.txt#L37-L132),
a discussion about "Preserving topology, replaying merges"
(https://github.com/newren/git/blob/replay/replay-design-notes.txt#L264-L341)
and a "Current status" section describing Elijah's work
(https://github.com/newren/git/blob/replay/replay-design-notes.txt#L344-L392)
before I started working on upstreaming it.
I have not included this material here though, as the documentation
added by this patch series for the `git replay` command already
includes an "EXAMPLES" section, and other sections of Elijah's design
notes might not be interesting for now. Also this cover letter is
already pretty long. But reviewers can refer to the links above if
they think it can help.
# Quick Overview (from Elijah's design notes)
`git replay`, at a basic level, can perhaps be thought of as a
"default-to-dry-run rebase" -- meaning no updates to the working tree,
or to the index, or to any references. However, it differs from
rebase in that it:
* Works for branches that aren't checked out
* Works in a bare repository
* Can replay multiple branches simultaneously (with or without common
history in the range being replayed)
* Preserves relative topology by default (merges are replayed too in
Elijah's original work, not in this series)
* Focuses on performance
* Has several altered defaults as a result of the above
I sometimes think of `git replay` as "fast-replay", a patch-based
analogue to the snapshot-based fast-export & fast-import tools.
# Reasons for diverging from cherry-pick & rebase (from Elijah's
design notes)
There are multiple reasons to diverge from the defaults in cherry-pick and
rebase.
* Server side needs
* Both cherry-pick and rebase, via the sequencer, are heavily tied
to updating the working tree, index, some refs, and a lot of
control files with every commit replayed, and invoke a mess of
hooks[1] that might be hard to avoid for backward compatibility
reasons (at least, that's been brought up a few times on the
list).
* cherry-pick and rebase both fork various subprocesses
unnecessarily, but somewhat intrinsically in part to ensure the
same hooks are called that old scripted implementations would have
called.
Note: since 356ee4659bb (sequencer: try to commit without forking
'git commit', 2017-11-24) cherry-pick and rebase do not fork
subprocesses other than hooks for the cases covered by this patch
series (i.e. they do not fork "git commit" for simple picks).
* "Dry run" behavior, where there are no updates to worktree, index,
or even refs might be important.
* Should not assume users only want to operate on HEAD (see next
section)
* Decapitate HEAD-centric assumptions
* cherry-pick forces commits to be played on top of HEAD;
inflexible.
* rebase assumes the range of commits to be replayed is
upstream..HEAD by default, though it allows one to replay
upstream..otherbranch -- but it still forcibly and needlessly
checks out 'otherbranch' before starting to replay things.
Note: since 767a9c417eb (rebase -i: stop checking out the tip of
the branch to rebase, 2020-01-24) it's not true that rebase
forcibly and needlessly checks out 'otherbranch'.
* Assuming HEAD is involved severely limits replaying multiple
(possibly divergent) branches.
Note: since 89fc0b53fdb (rebase: update refs from 'update-ref'
commands, 2022-07-19) the sequencer can update multiple
branches. The issue with divergent branch is with command line
arguments and the todo list generation rather than the
capabilities of the sequencer.
* Once you stop assuming HEAD has a certain meaning, there's not
much reason to have two separate commands anymore (except for the
funny extra not-necessarily-compatible options both have gained
over time).
* (Micro issue: Assuming HEAD is involved also makes it harder for
new users to learn what rebase means and does; it makes command
lines hard to parse. Not sure I want to harp on this too much, as
I have a suspicion I might be creating a tool for experts with
complicated use cases, but it's a minor quibble.)
* Performance
* jj is slaughtering us on rebase speed[2]. I would like us to become
competitive. (I dropped a few comments in the link at [2] about why
git is currently so bad.)
* From [3], there was a simple 4-patch series in linux.git that took
53 seconds to rebase. Switching to ort dropped it to 16 seconds.
While that sounds great, only 11 *milliseconds* were needed to do
the actual merges. That means almost *all* the time (>99%) was
overhead! Big offenders:
* --reapply-cherry-picks should be the default
* can_fast_forward() should be ripped out, and perhaps other extraneous
revision walks
Note: d42c9ffa0f (rebase: factor out branch_base calculation,
2022-10-17) might already deal with that (according to Felipe
Contreras).
* avoid updating working tree, index, refs, reflogs, and control
structures except when needed (e.g. hitting a conflict, or operation
finished)
* Other performance ideas (mostly for future work, not in this
series)
* single-file control structures instead of directory of files
(when doing interactive things which is in Elijah's original
work, but not in this series)
* avoid forking subprocesses unless explicitly requested (e.g.
--exec, --strategy, --run-hooks). For example, definitely do not
invoke `git commit` or `git merge`.
* Sanitize hooks:
* dispense with all per-commit hooks for sure (pre-commit,
post-commit, post-checkout).
* pre-rebase also seems to assume exactly 1 ref is written, and
invoking it repeatedly would be stupid. Plus, it's specific
to "rebase". So...ignore? (Stolee's --ref-update option for
rebase probably broke the pre-rebase assumptions already...)
* post-rewrite hook might make sense, but fast-import got
exempted, and I think of replay like a patch-based analogue
to the snapshot-based fast-import.
* When not running server side, resolve conflicts in a sparse-cone
sparse-index worktree to reduce number of files written to a
working tree. (See below as well.)
* [High risk of possible premature optimization] Avoid large
numbers of newly created loose objects, when replaying large
numbers of commits. Two possibilities: (1) Consider using
tmp-objdir and pack objects from the tmp-objdir at end of
exercise, (2) Lift code from git-fast-import to immediately
stuff new objects into a pack?
* Multiple branches and non-checked out branches
* The ability to operate on non-checked out branches also implies
that we should generally be able to replay when in a dirty working
tree (exception being when we expect to update HEAD and any of the
dirty files is one that needs to be updated by the replay).
* Also, if we are operating locally on a non-checked out branch and
hit a conflict, we should have a way to resolve the conflict
without messing with the user's work on their current
branch. (This is not is this patch series though.)
* Idea: new worktree with sparse cone + sparse index checkout,
containing only files in the root directory, and whatever is
necessary to get the conflicts
* Companion to above idea: control structures should be written to
$GIT_COMMON_DIR/replay-${worktree}, so users can have multiple
replay sessions, and so we know which worktrees are associated
with which replay operations.
- [1] https://lore.kernel.org/git/pull.749.v3.git.git.1586044818132.gitgitgadget@gmail.com/
- [2] https://github.com/martinvonz/jj/discussions/49
- [3] https://lore.kernel.org/git/CABPp-BE48=97k_3tnNqXPjSEfA163F8hoE+HY0Zvz1SWB2B8EA@mail.gmail.com/
# Important limitations
* The code exits with code 1 if there are any conflict. No
resumability. No nice output. No interactivity. No special exit code
depending on the reason.
* When a commit becomes empty as it is replayed, it is still replayed
as an empty commit, instead of being dropped.
* No replaying merges, nor root commits. Only regular commits.
* Signed commits are not properly handled. It's not clear what to do
to such commits when replaying on the server side.
* Notes associated with replayed commits are not updated nor
duplicated. (Thanks to Phillip Wood for noticing.)
# Commit overview
* 1/14 t6429: remove switching aspects of fast-rebase
Preparatory commit to make it easier to later replace the
fast-rebase test-tool by `git replay` without breaking existing
tests.
* 2/14 replay: introduce new builtin
This creates a minimal `git replay` command by moving the code
from the `fast-rebase` test helper from `t/helper/` into
`builtin/` and doing some renames and a few other needed changes.
* - 3/14 replay: start using parse_options API
- 4/14 replay: die() instead of failing assert()
- 5/14 replay: introduce pick_regular_commit()
- 6/14 replay: change rev walking options
- 7/14 replay: add an important FIXME comment about gpg signing
- 8/14 replay: remove progress and info output
- 9/14 replay: remove HEAD related sanity check
These slowly change the command to make it behave more like
regular commands and to start cleaning up its output.
* 10/14 replay: make it a minimal server side command
After the cleaning up in previous commits, it's now time to
radically change the way it works by stopping it to do ref
updates, to update the index and worktree, to consider HEAD as
special. Instead just make it output commands that should be
passed to `git update-ref --stdin`.
* 11/14 replay: use standard revision ranges
Start addind new interesting features and also documentation and
tests, as the interface of the command is cristalizing into its
final form.
* - 12/14 replay: add --advance or 'cherry-pick' mode
- 13/14 replay: add --contained to rebase contained branches
Add new options and features to the command.
* 14/14 replay: stop assuming replayed branches do not diverge
This adds another interesting feature, as well as related
documentation and tests.
# Notes about `fast-rebase`, tests and documentation
The `fast-rebase` test-tool helper was developed by Elijah to
experiment with a rebasing tool that would be developed from scratch
based on his merge-ort work, could be used to test that merge-ort
work, and would not have the speed and interface limitations of `git
rebase` or `git cherry-pick`.
This `fast-rebase` helper was used before this series in:
t6429-merge-sequence-rename-caching.sh
So when `git replay` is created from `fast-rebase` in patch 2/14, the
t6429 test script is also converted to use `git replay`. This ensures
that `git replay` doesn't break too badly during the first 10 patches
in this patch series.
Tests and documentation are introduced specifically for `git replay`
only in 11/14 and later patches as it doesn't make much sense to
document and test behavior that we know is going to change soon. So
it's only when the command is crystalizing towards its final form that
we start documenting and testing it.
# Possibly controversial issues
* bare or not bare: this series works towards a plumbing command with
the end goal of it being usable and used first on bare repos,
contrary to existing commands like `git rebase` and `git
cherry-pick`. The tests check that the command works on both bare
and non-bare repo though.
* exit status: a successful, non-conflicted replay exits with code
0. When the replay has conflicts, the exit status is 1. If the
replay is not able to complete (or start) due to some kind of error,
the exit status is something other than 0 or 1. There are a few
tests checking that. It has been suggested in an internal review
that conflicts might want to get a more specific error code as an
error code of 1 might be quite easy to return by accident. It
doesn't seem to me from their docs (which might want to be improved,
I didn't look at the code) that other commands like `git merge` and
`git rebase` exit with a special error code in case of conflict.
* make worktree and index changes optional: commit 10/14 stops
updating the index and worktree, but it might be better especially
for cli users to make that optional. The issue is that this would
make the command more complex while we are developing a number of
important features so that the command can be used on bare repos. It
seems that this should rather be done in an iterative improvement
after the important features have landed.
* when and where to add tests and docs: although t6429 has tests that
are changed to use the new command instead of the fast-rebase
test-tool command as soon as the former is introduced, there is no
specific test script and no doc for the new command until commit
11/14 when standard revision ranges are used. This is done to avoid
churn in tests and docs while the final form of the command hasn't
crystalized enough. Adding tests and doc at this point makes this
commit quite big and possibly more difficult to review than if they
were in separate commits though. On the other hand when tests and
docs are added in specific commits some reviewers say it would be
better to introduce them when the related changes are made.
* --advance and --contained: these two advanced options might not
belong to this first series and could perhaps be added in a followup
series in separate commits. On the other hand the code for
--contained seems involved with the code of --advance and it's nice
to see soon that git replay can indeed do cherry-picking and rebase
many refs at once, and this way fullfil these parts of its promise.
* replaying diverging branches: 14/14 the last patch in the series,
which allow replaying diverging branches, can be seen as a
fundamental fix or alternatively as adding an interesting
feature. So it's debatable if it should be in its own patch along
with its own tests as in this series, or if it should be merged into
a previous patch and which one.
* only 2 patches: this patch series can be seen from a high level
point of view as 1) introducing the new `git replay` command, and 2)
using `git replay` to replace, and get rid of, the fast-rebase
test-tool command. The fact that not much of the original
fast-rebase code and interface is left would agree with that point
of view. On the other hand, fast-rebase can also be seen as a first
iteration towards `git replay`. So it can also make sense to see how
`git replay` evolved from it.
# Changes between v4 and v5
Thanks to Dscho, Linus Arver and Dragan Simic for their suggestions on
the previous version! The few changes compared to v4 are:
* The patch series was rebased onto master at 3a06386e31 (The
fifteenth batch, 2023-10-04). This is to fix possible conflicts
especially related to header changes.
* Patch 12/15 (replay: disallow revision specific options and
pathspecs) in version 4 has been removed, so there are now only 14
patches instead of 15 in the series. This follows a suggestion by
Dscho, and goes in the direction Elijah initially wanted before
Derrick Stolee argued for disallowing revision specific options and
pathspecs.
* In patch 2/14 (replay: introduce new builtin) the command is now in
the "plumbingmanipulators" category instead of the "mainporcelain"
category. This is more in line with the goal of introducing it as a
plumbing command for now. Thanks to a suggestion from Dscho.
* In patch 6/14 (replay: change rev walking options) the commit
message has been improved, including its subject, to better match
and explain what the patch does. Also instead of forcing reverse
order we use the reverse order by default but allow it to be changed
using `--reverse`. Thanks to Dscho.
* In patch 11/14 (replay: use standard revision ranges) the commit
message has been improved to discuss a bit the new options that are
now accepted and eaten by setup_revisions().
* In patch 11/14 too, the documentation has been improved thanks to
suggestions by Linus Arver and Dragan Simic. It also now includes
`rev-list-options.txt` to document the new rev walking options
accepted by setup_revisions().
All CI tests seem to pass according to:
https://github.com/chriscool/git/actions/runs/6468200080
# Range-diff between v4 and v5
1: 1eaca9b788 = 1: 72c34a0eba t6429: remove switching aspects of fast-rebase
2: 5ac4beb1ae ! 2: f85e6c823c replay: introduce new builtin
@@ Commit message
Subsequent commits will flesh out its capabilities and make it a more
standard regular builtin.
+ Helped-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
Co-authored-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
@@ command-list.txt: git-reflog ancillarymanipulators
git-remote ancillarymanipulators complete
git-repack ancillarymanipulators complete
git-replace ancillarymanipulators complete
-+git-replay mainporcelain history
++git-replay plumbingmanipulators
git-request-pull foreignscminterface complete
git-rerere ancillaryinterrogators
git-reset mainporcelain history
3: 299381aa9b = 3: 11abb9d120 replay: start using parse_options API
4: 3b825c9be0 = 4: 9e568eae84 replay: die() instead of failing assert()
5: b1e890745d = 5: e7ebf3c5ef replay: introduce pick_regular_commit()
6: ec51351889 ! 6: 37d545d5d6 replay: don't simplify history
@@ Metadata
Author: Elijah Newren <newren@gmail.com>
## Commit message ##
- replay: don't simplify history
+ replay: change rev walking options
Let's set the rev walking options we need after calling
- setup_revisions() instead of before. This makes it clearer which options
- we need.
+ setup_revisions() instead of before. This enforces options we always
+ want.
+
+ We want the command to work from older commits to newer ones by default,
+ but we are Ok with letting users reverse that, using --reverse, if that's
+ what they really want.
Also we don't want history simplification, as we want to deal with all
the commits in the affected range.
+ Helped-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
Co-authored-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
@@ builtin/replay.c: int cmd_replay(int argc, const char **argv, const char *prefix
}
+ /* requirements/overrides for revs */
-+ revs.reverse = 1;
++ revs.reverse = !revs.reverse;
+ revs.sort_order = REV_SORT_IN_GRAPH_ORDER;
+ revs.topo_order = 1;
+ revs.simplify_history = 0;
7: cd4ed07d2d = 7: 2943f08926 replay: add an important FIXME comment about gpg signing
8: e45a55917c = 8: f81962ba41 replay: remove progress and info output
9: 0587a76cbb = 9: 236747497e replay: remove HEAD related sanity check
10: d10368e87a = 10: 3374d5be23 replay: make it a minimal server side command
11: 4e09572c43 ! 11: 197d076a93 replay: use standard revision ranges
@@ Commit message
way as many other Git commands. This makes its interface more
standard and more flexible.
+ This also enables many revision related options accepted and
+ eaten by setup_revisions(). If the replay command was a high level
+ one or had a high level mode, it would make sense to restrict some
+ of the possible options, like those generating non-contiguous
+ history, as they could be confusing for most users.
+
Also as the interface of the command is now mostly finalized,
we can add some documentation as well as testcases to make sure
the command will continue to work as designed in the future.
+ We only document the rev-list related options among all the
+ revision related options that are now accepted, as the rev-list
+ related ones are probably the most useful for now.
+
+ Helped-by: Dragan Simic <dsimic@manjaro.org>
+ Helped-by: Linus Arver <linusa@google.com>
Co-authored-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
@@ Documentation/git-replay.txt (new)
+
+NAME
+----
-+git-replay - Replay commits on a different base, without touching working tree
++git-replay - Replay commits on a new base, works on bare repos too
+
+
+SYNOPSIS
@@ Documentation/git-replay.txt (new)
+DESCRIPTION
+-----------
+
-+Takes a range of commits, and replays them onto a new location. Does
-+not touch the working tree or index, and does not update any
-+references. However, the output of this command is meant to be used
-+as input to `git update-ref --stdin`, which would update the relevant
-+branches.
++Takes a range of commits and replays them onto a new location. Leaves
++the working tree and the index untouched, and updates no
++references. The output of this command is meant to be used as input to
++`git update-ref --stdin`, which would update the relevant branches
++(see the OUTPUT section below).
+
+THIS COMMAND IS EXPERIMENTAL. THE BEHAVIOR MAY CHANGE.
+
@@ Documentation/git-replay.txt (new)
+
+<revision-range>::
+ Range of commits to replay; see "Specifying Ranges" in
-+ linkgit:git-rev-parse.
++ linkgit:git-rev-parse and the "Commit Limiting" options below.
++
++include::rev-list-options.txt[]
+
+OUTPUT
+------
+
+When there are no conflicts, the output of this command is usable as
-+input to `git update-ref --stdin`. It is basically of the form:
++input to `git update-ref --stdin`. It is of the form:
+
+ update refs/heads/branch1 ${NEW_branch1_HASH} ${OLD_branch1_HASH}
+ update refs/heads/branch2 ${NEW_branch2_HASH} ${OLD_branch2_HASH}
@@ Documentation/git-replay.txt (new)
+EXAMPLES
+--------
+
-+To simply rebase mybranch onto target:
++To simply rebase `mybranch` onto `target`:
+
+------------
+$ git replay --onto target origin/main..mybranch
@@ Documentation/git-replay.txt (new)
+update refs/heads/branch3 ${NEW_branch3_HASH} ${OLD_branch3_HASH}
+------------
+
-+This will simultaneously rebase branch1, branch2, and branch3 -- all
-+commits they have since base, playing them on top of origin/main.
-+These three branches may have commits on top of base that they have in
-+common, but that does not need to be the case.
++This will simultaneously rebase `branch1`, `branch2`, and `branch3`,
++all commits they have since `base`, playing them on top of
++`origin/main`. These three branches may have commits on top of `base`
++that they have in common, but that does not need to be the case.
+
+GIT
+---
12: 64b803f1cf < -: ---------- replay: disallow revision specific options and pathspecs
13: 04f27d81ab ! 12: e52d8b961c replay: add --advance or 'cherry-pick' mode
@@ Commit message
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
## Documentation/git-replay.txt ##
-@@ Documentation/git-replay.txt: git-replay - Replay commits on a different base, without touching working tree
+@@ Documentation/git-replay.txt: git-replay - Replay commits on a new base, works on bare repos too
SYNOPSIS
--------
[verse]
@@ Documentation/git-replay.txt: OPTIONS
<revision-range>::
- Range of commits to replay; see "Specifying Ranges" in
-- linkgit:git-rev-parse.
+- linkgit:git-rev-parse and the "Commit Limiting" options below.
+ Range of commits to replay. More than one <revision-range> can
+ be passed, but in `--advance <branch>` mode, they should have
+ a single tip, so that it's clear where <branch> should point
-+ to. See "Specifying Ranges" in linkgit:git-rev-parse.
++ to. See "Specifying Ranges" in linkgit:git-rev-parse and the
++ "Commit Limiting" options below.
- OUTPUT
- ------
-@@ Documentation/git-replay.txt: input to `git update-ref --stdin`. It is basically of the form:
+ include::rev-list-options.txt[]
+
+@@ Documentation/git-replay.txt: input to `git update-ref --stdin`. It is of the form:
update refs/heads/branch3 ${NEW_branch3_HASH} ${OLD_branch3_HASH}
where the number of refs updated depends on the arguments passed and
@@ builtin/replay.c: static struct commit *pick_regular_commit(struct commit *pickm
struct merge_options merge_opt;
struct merge_result result;
+ struct strset *update_refs = NULL;
- int ret = 0, i;
+ int ret = 0;
const char * const replay_usage[] = {
- N_("git replay --onto <newbase> <revision-range>..."),
@@ builtin/replay.c: static struct commit *pick_regular_commit(struct commit *pickm
N_("revision"),
N_("replay onto given commit")),
@@ builtin/replay.c: int cmd_replay(int argc, const char **argv, const char *prefix)
- usage_with_options(replay_usage, replay_options);
- }
+ argc = parse_options(argc, argv, prefix, replay_options, replay_usage,
+ PARSE_OPT_KEEP_ARGV0 | PARSE_OPT_KEEP_UNKNOWN_OPT);
- if (!onto_name) {
- error(_("option --onto is mandatory"));
@@ builtin/replay.c: int cmd_replay(int argc, const char **argv, const char *prefix
/* Return */
## t/t3650-replay-basics.sh ##
-@@ t/t3650-replay-basics.sh: test_expect_success 'using replay on bare repo with disallowed pathspec' '
- test_must_fail git -C bare replay --onto main topic1..topic2 -- A.t
+@@ t/t3650-replay-basics.sh: test_expect_success 'using replay on bare repo to rebase with a conflict' '
+ test_expect_code 1 git -C bare replay --onto topic1 B..conflict
'
+test_expect_success 'using replay to perform basic cherry-pick' '
14: 9ed0919ad5 ! 13: fc79a930b5 replay: add --contained to rebase contained branches
@@ Commit message
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
## Documentation/git-replay.txt ##
-@@ Documentation/git-replay.txt: git-replay - Replay commits on a different base, without touching working tree
+@@ Documentation/git-replay.txt: git-replay - Replay commits on a new base, works on bare repos too
SYNOPSIS
--------
[verse]
@@ builtin/replay.c: int cmd_replay(int argc, const char **argv, const char *prefix
struct rev_info revs;
struct commit *last_commit = NULL;
@@ builtin/replay.c: int cmd_replay(int argc, const char **argv, const char *prefix)
- int ret = 0, i;
+ int ret = 0;
const char * const replay_usage[] = {
- N_("git replay (--onto <newbase> | --advance <branch>) <revision-range>..."),
15: cf8c984877 ! 14: 1160ff54e6 replay: stop assuming replayed branches do not diverge
@@ builtin/replay.c: int cmd_replay(int argc, const char **argv, const char *prefix
struct merge_result result;
struct strset *update_refs = NULL;
+ kh_oid_map_t *replayed_commits;
- int ret = 0, i;
+ int ret = 0;
const char * const replay_usage[] = {
@@ builtin/replay.c: int cmd_replay(int argc, const char **argv, const char *prefix)
Elijah Newren (14):
t6429: remove switching aspects of fast-rebase
replay: introduce new builtin
replay: start using parse_options API
replay: die() instead of failing assert()
replay: introduce pick_regular_commit()
replay: change rev walking options
replay: add an important FIXME comment about gpg signing
replay: remove progress and info output
replay: remove HEAD related sanity check
replay: make it a minimal server side command
replay: use standard revision ranges
replay: add --advance or 'cherry-pick' mode
replay: add --contained to rebase contained branches
replay: stop assuming replayed branches do not diverge
.gitignore | 1 +
Documentation/git-replay.txt | 127 +++++++
Makefile | 2 +-
builtin.h | 1 +
builtin/replay.c | 407 +++++++++++++++++++++++
command-list.txt | 1 +
git.c | 1 +
t/helper/test-fast-rebase.c | 241 --------------
t/helper/test-tool.c | 1 -
t/helper/test-tool.h | 1 -
t/t3650-replay-basics.sh | 198 +++++++++++
t/t6429-merge-sequence-rename-caching.sh | 45 +--
12 files changed, 762 insertions(+), 264 deletions(-)
create mode 100644 Documentation/git-replay.txt
create mode 100644 builtin/replay.c
delete mode 100644 t/helper/test-fast-rebase.c
create mode 100755 t/t3650-replay-basics.sh
--
2.42.0.339.g663cbc8ab1
^ permalink raw reply
* [PATCH v5 01/14] t6429: remove switching aspects of fast-rebase
From: Christian Couder @ 2023-10-10 12:38 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Patrick Steinhardt, Johannes Schindelin,
Elijah Newren, John Cai, Derrick Stolee, Phillip Wood, Calvin Wan,
Toon Claes, Dragan Simic, Linus Arver, Christian Couder
In-Reply-To: <20231010123847.2777056-1-christian.couder@gmail.com>
From: Elijah Newren <newren@gmail.com>
At the time t6429 was written, merge-ort was still under development,
did not have quite as many tests, and certainly was not widely deployed.
Since t6429 was exercising some codepaths just a little differently, we
thought having them also test the "merge_switch_to_result()" bits of
merge-ort was useful even though they weren't intrinsic to the real
point of these tests.
However, the value provided by doing extra testing of the
"merge_switch_to_result()" bits has decreased a bit over time, and it's
actively making it harder to refactor `test-tool fast-rebase` into `git
replay`, which we are going to do in following commits. Dispense with
these bits.
Co-authored-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
t/helper/test-fast-rebase.c | 9 +--------
t/t6429-merge-sequence-rename-caching.sh | 9 +++++++--
2 files changed, 8 insertions(+), 10 deletions(-)
diff --git a/t/helper/test-fast-rebase.c b/t/helper/test-fast-rebase.c
index cac20a72b3..2bfab66b1b 100644
--- a/t/helper/test-fast-rebase.c
+++ b/t/helper/test-fast-rebase.c
@@ -194,7 +194,7 @@ int cmd__fast_rebase(int argc, const char **argv)
last_commit = create_commit(result.tree, commit, last_commit);
}
- merge_switch_to_result(&merge_opt, head_tree, &result, 1, !result.clean);
+ merge_finalize(&merge_opt, &result);
if (result.clean < 0)
exit(128);
@@ -213,9 +213,6 @@ int cmd__fast_rebase(int argc, const char **argv)
}
if (create_symref("HEAD", branch_name.buf, reflog_msg.buf) < 0)
die(_("unable to update HEAD"));
-
- prime_cache_tree(the_repository, the_repository->index,
- result.tree);
} else {
fprintf(stderr, "\nAborting: Hit a conflict.\n");
strbuf_addf(&reflog_msg, "rebase progress up to %s",
@@ -228,10 +225,6 @@ int cmd__fast_rebase(int argc, const char **argv)
die("Failed to update %s", argv[4]);
}
}
- if (write_locked_index(&the_index, &lock,
- COMMIT_LOCK | SKIP_IF_UNCHANGED))
- die(_("unable to write %s"), get_index_file());
-
ret = (result.clean == 0);
cleanup:
strbuf_release(&reflog_msg);
diff --git a/t/t6429-merge-sequence-rename-caching.sh b/t/t6429-merge-sequence-rename-caching.sh
index d02fa16614..75d3fd2dba 100755
--- a/t/t6429-merge-sequence-rename-caching.sh
+++ b/t/t6429-merge-sequence-rename-caching.sh
@@ -72,6 +72,7 @@ test_expect_success 'caching renames does not preclude finding new ones' '
git switch upstream &&
test-tool fast-rebase --onto HEAD upstream~1 topic &&
+ git reset --hard topic &&
#git cherry-pick upstream~1..topic
git ls-files >tracked-files &&
@@ -200,6 +201,7 @@ test_expect_success 'rename same file identically, then reintroduce it' '
export GIT_TRACE2_PERF &&
test-tool fast-rebase --onto HEAD upstream~1 topic &&
+ git reset --hard topic &&
#git cherry-pick upstream~1..topic &&
git ls-files >tracked &&
@@ -277,6 +279,7 @@ test_expect_success 'rename same file identically, then add file to old dir' '
export GIT_TRACE2_PERF &&
test-tool fast-rebase --onto HEAD upstream~1 topic &&
+ git reset --hard topic &&
#git cherry-pick upstream~1..topic &&
git ls-files >tracked &&
@@ -356,8 +359,6 @@ test_expect_success 'cached dir rename does not prevent noticing later conflict'
test_must_fail test-tool fast-rebase --onto HEAD upstream~1 topic >output &&
#git cherry-pick upstream..topic &&
- grep CONFLICT..rename/rename output &&
-
grep region_enter.*diffcore_rename trace.output >calls &&
test_line_count = 2 calls
)
@@ -456,6 +457,7 @@ test_expect_success 'dir rename unneeded, then add new file to old dir' '
export GIT_TRACE2_PERF &&
test-tool fast-rebase --onto HEAD upstream~1 topic &&
+ git reset --hard topic &&
#git cherry-pick upstream..topic &&
grep region_enter.*diffcore_rename trace.output >calls &&
@@ -522,6 +524,7 @@ test_expect_success 'dir rename unneeded, then rename existing file into old dir
export GIT_TRACE2_PERF &&
test-tool fast-rebase --onto HEAD upstream~1 topic &&
+ git reset --hard topic &&
#git cherry-pick upstream..topic &&
grep region_enter.*diffcore_rename trace.output >calls &&
@@ -624,6 +627,7 @@ test_expect_success 'caching renames only on upstream side, part 1' '
export GIT_TRACE2_PERF &&
test-tool fast-rebase --onto HEAD upstream~1 topic &&
+ git reset --hard topic &&
#git cherry-pick upstream..topic &&
grep region_enter.*diffcore_rename trace.output >calls &&
@@ -682,6 +686,7 @@ test_expect_success 'caching renames only on upstream side, part 2' '
export GIT_TRACE2_PERF &&
test-tool fast-rebase --onto HEAD upstream~1 topic &&
+ git reset --hard topic &&
#git cherry-pick upstream..topic &&
grep region_enter.*diffcore_rename trace.output >calls &&
--
2.42.0.339.g663cbc8ab1
^ permalink raw reply related
* [PATCH] doc/git-worktree: mention "refs/rewritten" as per-worktree refs
From: Patrick Steinhardt @ 2023-10-10 11:01 UTC (permalink / raw)
To: git; +Cc: Johannes Schindelin
[-- Attachment #1: Type: text/plain, Size: 2648 bytes --]
Some references are special in the context of worktrees as they are
considered to be per-worktree instead of shared across all of the
worktrees. Most importantly, this includes "refs/worktree/" that have
explicitly been designed such that users can create per-woorktree refs.
But there are also special references that have an associated meaning
like "refs/bisect/", which is used to track state of git-bisect(1).
These special per-worktree references are documented in git-worktree(1),
but one instance is missing. In a9be29c9817 (sequencer: make refs
generated by the `label` command worktree-local, 2018-04-25), we have
converted "refs/rewritten/" to be a per-worktree reference as well.
These references are used by our sequencer infrastructure to generate
labels for rebased commits. So in order to allow for multiple concurrent
rebases to happen in different worktrees, these references need to be
tracked per worktree.
We forgot to update our documentation to mention these new per-worktree
references, which is fixed by this patch.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
Documentation/git-worktree.txt | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/Documentation/git-worktree.txt b/Documentation/git-worktree.txt
index a4fbf5e838..93d76f5d66 100644
--- a/Documentation/git-worktree.txt
+++ b/Documentation/git-worktree.txt
@@ -286,7 +286,8 @@ rules and how to access refs of one worktree from another.
In general, all pseudo refs are per-worktree and all refs starting with
`refs/` are shared. Pseudo refs are ones like `HEAD` which are directly
under `$GIT_DIR` instead of inside `$GIT_DIR/refs`. There are exceptions,
-however: refs inside `refs/bisect` and `refs/worktree` are not shared.
+however: refs inside `refs/bisect`, `refs/worktree` and `refs/rewritten` are
+not shared.
Refs that are per-worktree can still be accessed from another worktree via
two special paths, `main-worktree` and `worktrees`. The former gives
@@ -363,8 +364,8 @@ linked worktree `git rev-parse --git-path HEAD` returns
`/path/other/test-next/.git/HEAD` or `/path/main/.git/HEAD`) while `git
rev-parse --git-path refs/heads/master` uses
`$GIT_COMMON_DIR` and returns `/path/main/.git/refs/heads/master`,
-since refs are shared across all worktrees, except `refs/bisect` and
-`refs/worktree`.
+since refs are shared across all worktrees, except `refs/bisect`,
+`refs/worktree` and `refs/rewritten`.
See linkgit:gitrepository-layout[5] for more information. The rule of
thumb is do not make any assumption about whether a path belongs to
--
2.42.0
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply related
* Re: [PATCH v4 0/4] Preliminary patches before git-std-lib
From: phillip.wood123 @ 2023-10-10 10:05 UTC (permalink / raw)
To: Jonathan Tan, git; +Cc: Calvin Wan, Junio C Hamano
In-Reply-To: <cover.1696021277.git.jonathantanmy@google.com>
Hi Jonathan
On 29/09/2023 22:20, Jonathan Tan wrote:
> Calvin will be away for a few weeks and I'll be handling the git-std-lib
> effort in the meantime. My goals will be:
>
> - Get the preliminary patches in Calvin's patch set (patches 1-4) merged
> first.
>
> - Updating patches 5-6 based on reviewer feedback (including my
> feedback). I have several aims including reducing or eliminating the
> need for the GIT_STD_LIB preprocessor variable, and making stubs a test-
> only concern (I think Phillip has some similar ideas [1] but I haven't
> looked at their repo on GitHub yet).
It sounds like we're thinking along similar lines, do feel free get in
touch on or off the list if you want to ask anything about those patches
I pushed to github.
> [1] https://lore.kernel.org/git/98f3edcf-7f37-45ff-abd2-c0038d4e0589@gmail.com/
>
> This patch set is in service of the first goal. Because the libification
> patches are no longer included in this patch set, I have rewritten the
> commit messages to justify the patches in terms of code organization.
> There are no changes in the code itself. Also, I have retained Calvin's
> name as the author.
I agree it makes sense to get the preliminary patches merged on their
own. I think the argument that they reduce the scope of includes is a
reasonable justification on its own. I've left a couple of comments but
they're looking pretty good.
Best Wishes
Phillip
^ permalink raw reply
* Re: [PATCH v4 4/4] parse: separate out parsing functions from config.h
From: phillip.wood123 @ 2023-10-10 10:00 UTC (permalink / raw)
To: Jonathan Tan, git; +Cc: Calvin Wan, Junio C Hamano
In-Reply-To: <5d9f0b3de08ab8541482b9b640db06b6d3000b86.1696021277.git.jonathantanmy@google.com>
Hi Jonathan
On 29/09/2023 22:20, Jonathan Tan wrote:
> diff --git a/parse.h b/parse.h
> new file mode 100644
> index 0000000000..07d2193d69
> --- /dev/null
> +++ b/parse.h
> @@ -0,0 +1,20 @@
> +#ifndef PARSE_H
> +#define PARSE_H
> +
> +int git_parse_signed(const char *value, intmax_t *ret, intmax_t max);
Previously this function was private to config.c, now it needs to be
public because it is still called by
git_config_get_expiry_date_in_days(). As this is essentially an internal
helper for git_parse_int() and friends it is a bit unfortunate that it
is now public. Perhaps we should change
git_config_get_expiry_date_in_days() to call git_parse_int() instead.
Then we can keep git_parse_signed() and git_parse_unsigned() private to
parse.c.
> +int git_parse_ssize_t(const char *, ssize_t *);
> +int git_parse_ulong(const char *, unsigned long *);
> +int git_parse_int(const char *value, int *ret);
> +int git_parse_int64(const char *value, int64_t *ret);
This was previously private but I think it makes sense for it to be
publicly available.
> +/**
> + * Same as `git_config_bool`, except that it returns -1 on error rather
> + * than dying.
> + */
> +int git_parse_maybe_bool(const char *);
> +int git_parse_maybe_bool_text(const char *value);
This used to be private to config.c and now has callers in parse.c and
config.c. We should make it clear that non-config code is likely to want
git_parse_maybe_bool() rather than this function.
Best Wishes
Phillip
^ permalink raw reply
* Re: [PATCH v4 2/4] wrapper: reduce scope of remove_or_warn()
From: phillip.wood123 @ 2023-10-10 9:59 UTC (permalink / raw)
To: Jonathan Tan, git; +Cc: Calvin Wan, Junio C Hamano
In-Reply-To: <c9e7cd78576527571fd70b953e340b5bdd196221.1696021277.git.jonathantanmy@google.com>
Hi Jonathan
On 29/09/2023 22:20, Jonathan Tan wrote:
> From: Calvin Wan <calvinwan@google.com>
>
> remove_or_warn() is only used by entry.c and apply.c, but it is
> currently declared and defined in wrapper.{h,c}, so it has a scope much
> greater than it needs. This needlessly large scope also causes wrapper.c
> to need to include object.h, when this file is largely unconcerned with
> Git objects.
>
> Move remove_or_warn() to entry.{h,c}. The file apply.c still has access
> to it, since it already includes entry.h for another reason.
This looks good. On a related note wrapper.c includes repository.h but
does use anything declared in that header.
Best Wishes
Phillip
^ permalink raw reply
* Re: [PATCH v8 1/3] unit tests: Add a project plan document
From: Oswald Buddenhagen @ 2023-10-10 8:57 UTC (permalink / raw)
To: Josh Steadmon; +Cc: git, phillip.wood123, linusa, calvinwan, gitster, rsbecker
In-Reply-To: <81c5148a1267b8f9ce432a950340f0fa16b4d773.1696889530.git.steadmon@google.com>
On Mon, Oct 09, 2023 at 03:21:20PM -0700, Josh Steadmon wrote:
>+=== Comparison
>+
>+[format="csv",options="header",width="33%"]
>+|=====
>+Framework,"<<license,License>>","<<vendorable-or-ubiquitous,Vendorable or ubiquitous>>","<<maintainable-extensible,Maintainable / extensible>>","<<major-platform-support,Major platform support>>","<<tap-support,TAP support>>","<<diagnostic-output,Diagnostic output>>","<<runtime--skippable-tests,Runtime- skippable tests>>","<<parallel-execution,Parallel execution>>","<<mock-support,Mock support>>","<<signal-error-handling,Signal & error handling>>","<<project-kloc,Project KLOC>>","<<adoption,Adoption>>"
>
the redundancy seems unnecessary; asciidoc should automatically use each
target's section title as the xreflabel.
>+https://lore.kernel.org/git/c902a166-98ce-afba-93f2-ea6027557176@gmail.com/[Custom Git impl.],[lime-background]#GPL v2#,[lime-background]#True#,[lime-background]#True#,[lime-background]#True#,[lime-background]#True#,[lime-background]#True#,[lime-background]#True#,[red-background]#False#,[red-background]#False#,[red-background]#False#,1,0
>+https://github.com/silentbicycle/greatest[Greatest],[lime-background]#ISC#,[lime-background]#True#,[yellow-background]#Partial#,[lime-background]#True#,[yellow-background]#Partial#,[lime-background]#True#,[lime-background]#True#,[red-background]#False#,[red-background]#False#,[red-background]#False#,3,1400
>+https://github.com/Snaipe/Criterion[Criterion],[lime-background]#MIT#,[red-background]#False#,[yellow-background]#Partial#,[lime-background]#True#,[lime-background]#True#,[lime-background]#True#,[lime-background]#True#,[lime-background]#True#,[red-background]#False#,[lime-background]#True#,19,1800
>+https://github.com/rra/c-tap-harness/[C TAP],[lime-background]#Expat#,[lime-background]#True#,[yellow-background]#Partial#,[yellow-background]#Partial#,[lime-background]#True#,[red-background]#False#,[lime-background]#True#,[red-background]#False#,[red-background]#False#,[red-background]#False#,4,33
>+https://libcheck.github.io/check/[Check],[lime-background]#LGPL v2.1#,[red-background]#False#,[yellow-background]#Partial#,[lime-background]#True#,[lime-background]#True#,[lime-background]#True#,[red-background]#False#,[red-background]#False#,[red-background]#False#,[lime-background]#True#,17,973
>+|=====
>+
i find this totally unreadable in its raw form.
consider user-defined document-attributes for specific cell contents.
externalizing the urls would probably help as well (i'm not sure how to
do that best).
regards
^ permalink raw reply
* Re: [PATCH 0/4] Performance improvement & cleanup in loose ref iteration
From: Patrick Steinhardt @ 2023-10-10 7:21 UTC (permalink / raw)
To: Victoria Dye; +Cc: Victoria Dye via GitGitGadget, git
In-Reply-To: <28ae03f5-7091-d3f3-8a70-56aba6639640@github.com>
[-- Attachment #1: Type: text/plain, Size: 8706 bytes --]
On Mon, Oct 09, 2023 at 02:49:14PM -0700, Victoria Dye wrote:
> Patrick Steinhardt wrote:
> > On Fri, Oct 06, 2023 at 06:09:25PM +0000, Victoria Dye via GitGitGadget wrote:
> >> While investigating ref iteration performance in builtins like
> >> 'for-each-ref' and 'show-ref', I found two small improvement opportunities.
> >>
> >> The first patch tweaks the logic around prefix matching in
> >> 'cache_ref_iterator_advance' so that we correctly skip refs that do not
> >> actually match a given prefix. The unnecessary iteration doesn't seem to be
> >> causing any bugs in the ref iteration commands that I've tested, but it
> >> doesn't hurt to be more precise (and it helps with some other patches I'm
> >> working on ;) ).
> >>
> >> The next three patches update how 'loose_fill_ref_dir' determines the type
> >> of ref cache entry to create (directory or regular). On platforms that
> >> include d_type information in 'struct dirent' (as far as I can tell, all
> >> except NonStop & certain versions of Cygwin), this allows us to skip calling
> >> 'stat'. In ad-hoc testing, this improved performance of 'git for-each-ref'
> >> by about 20%.
> >
> > I've done a small set of benchmarks with my usual test repositories,
> > which is linux.git with a bunch of references added. The repository
> > comes in four sizes:
> >
> > - small: 50k references
> > - medium: 500k references
> > - high: 1.1m references
> > - huge: 12m references
> >
> > Unfortunately, I couldn't really reproduce the performance improvements.
> > In fact, the new version runs consistently a tiny bit slower than the
> > old version:
> >
> > # Old version, which is 3a06386e31 (The fifteenth batch, 2023-10-04).
> >
> > Benchmark 1: git for-each-ref (revision=old,refcount=small)
> > Time (mean ± σ): 135.5 ms ± 1.2 ms [User: 76.4 ms, System: 59.0 ms]
> > Range (min … max): 134.8 ms … 136.9 ms 3 runs
> >
> > Benchmark 2: git for-each-ref (revision=old,refcount=medium)
> > Time (mean ± σ): 822.7 ms ± 2.2 ms [User: 697.4 ms, System: 125.1 ms]
> > Range (min … max): 821.1 ms … 825.2 ms 3 runs
> >
> > Benchmark 3: git for-each-ref (revision=old,refcount=high)
> > Time (mean ± σ): 1.960 s ± 0.015 s [User: 1.702 s, System: 0.257 s]
> > Range (min … max): 1.944 s … 1.973 s 3 runs
> >
> > # New version, which is your tip.
> >
> > Benchmark 4: git for-each-ref (revision=old,refcount=huge)
> > Time (mean ± σ): 16.815 s ± 0.054 s [User: 15.091 s, System: 1.722 s]
> > Range (min … max): 16.760 s … 16.869 s 3 runs
> >
> > Benchmark 5: git for-each-ref (revision=new,refcount=small)
> > Time (mean ± σ): 136.0 ms ± 0.2 ms [User: 78.8 ms, System: 57.1 ms]
> > Range (min … max): 135.8 ms … 136.2 ms 3 runs
> >
> > Benchmark 6: git for-each-ref (revision=new,refcount=medium)
> > Time (mean ± σ): 830.4 ms ± 21.2 ms [User: 691.3 ms, System: 138.7 ms]
> > Range (min … max): 814.2 ms … 854.5 ms 3 runs
> >
> > Benchmark 7: git for-each-ref (revision=new,refcount=high)
> > Time (mean ± σ): 1.966 s ± 0.013 s [User: 1.717 s, System: 0.249 s]
> > Range (min … max): 1.952 s … 1.978 s 3 runs
> >
> > Benchmark 8: git for-each-ref (revision=new,refcount=huge)
> > Time (mean ± σ): 16.945 s ± 0.037 s [User: 15.182 s, System: 1.760 s]
> > Range (min … max): 16.910 s … 16.983 s 3 runs
> >
> > Summary
> > git for-each-ref (revision=old,refcount=small) ran
> > 1.00 ± 0.01 times faster than git for-each-ref (revision=new,refcount=small)
> > 6.07 ± 0.06 times faster than git for-each-ref (revision=old,refcount=medium)
> > 6.13 ± 0.17 times faster than git for-each-ref (revision=new,refcount=medium)
> > 14.46 ± 0.17 times faster than git for-each-ref (revision=old,refcount=high)
> > 14.51 ± 0.16 times faster than git for-each-ref (revision=new,refcount=high)
> > 124.09 ± 1.15 times faster than git for-each-ref (revision=old,refcount=huge)
> > 125.05 ± 1.12 times faster than git for-each-ref (revision=new,refcount=huge)
> >
> > The performance regression isn't all that concerning, but it makes me
> > wonder why I see things becoming slower rather than faster. My guess is
> > that this is because all my test repositories are well-packed and don't
> > have a lot of loose references. But I just wanted to confirm how you
> > benchmarked your change and what the underlying shape of your test repo
> > was.
>
> I ran my benchmark on my (Intel) Mac with a test repository (single commit,
> one file) containing:
>
> - 10k refs/heads/ references
> - 10k refs/tags/ references
> - 10k refs/special/ references
>
> All refs in the repository are loose. My Mac has historically been somewhat
> slow and inconsistent when it comes to perf testing, though, so I re-ran the
> benchmark a bit more formally on an Ubuntu VM (3 warmup iterations followed
> by at least 10 iterations per test):
>
> ---
>
> Benchmark 1: git for-each-ref (revision=old,refcount=3k)
> Time (mean ± σ): 40.6 ms ± 3.9 ms [User: 13.2 ms, System: 27.1 ms]
> Range (min … max): 37.2 ms … 59.1 ms 76 runs
>
> Warning: Statistical outliers were detected. Consider re-running this benchmark on a quiet system without any interferences from other programs. It might help to use the '--warmup' or '--prepare' options.
>
> Benchmark 2: git for-each-ref (revision=new,refcount=3k)
> Time (mean ± σ): 38.7 ms ± 4.4 ms [User: 13.8 ms, System: 24.5 ms]
> Range (min … max): 35.1 ms … 57.2 ms 71 runs
>
> Warning: Statistical outliers were detected. Consider re-running this benchmark on a quiet system without any interferences from other programs. It might help to use the '--warmup' or '--prepare' options.
>
> Benchmark 3: git for-each-ref (revision=old,refcount=30k)
> Time (mean ± σ): 419.4 ms ± 43.9 ms [User: 136.4 ms, System: 274.1 ms]
> Range (min … max): 385.1 ms … 528.7 ms 10 runs
>
> Benchmark 4: git for-each-ref (revision=new,refcount=30k)
> Time (mean ± σ): 390.4 ms ± 27.2 ms [User: 133.1 ms, System: 251.6 ms]
> Range (min … max): 360.3 ms … 447.6 ms 10 runs
>
> Benchmark 5: git for-each-ref (revision=old,refcount=300k)
> Time (mean ± σ): 4.171 s ± 0.052 s [User: 1.400 s, System: 2.715 s]
> Range (min … max): 4.118 s … 4.283 s 10 runs
>
> Benchmark 6: git for-each-ref (revision=new,refcount=300k)
> Time (mean ± σ): 3.939 s ± 0.054 s [User: 1.403 s, System: 2.466 s]
> Range (min … max): 3.858 s … 4.026 s 10 runs
>
> Summary
> 'git for-each-ref (revision=new,refcount=3k)' ran
> 1.05 ± 0.16 times faster than 'git for-each-ref (revision=old,refcount=3k)'
> 10.08 ± 1.34 times faster than 'git for-each-ref (revision=new,refcount=30k)'
> 10.83 ± 1.67 times faster than 'git for-each-ref (revision=old,refcount=30k)'
> 101.68 ± 11.63 times faster than 'git for-each-ref (revision=new,refcount=300k)'
> 107.67 ± 12.30 times faster than 'git for-each-ref (revision=old,refcount=300k)'
>
> ---
>
> So it's not the 20% speedup I saw on my local test repo (it's more like
> 5-8%), but there does appear to be a consistent improvement.
Thanks a bunch for re-doing the benchmark with a documented setup.
> As for your results, the changes in this series shouldn't affect
> packed ref operations, and the difference between old & new doesn't
> seem to indicate a regression.
Yeah, I've also been surprised to see the performance regression for
packed-refs files. The regression is persistent and reproducable on my
machine though, so even though I tend to agree that the patches
shouldn't negatively impact packed-refs performance they somehow do. It
could just as well be something like different optimization choices by
the compler due to the added patches, or hitting different cache lines.
I dunno.
Anyway, I agree with your assessment. The regression I see is less than
1% for packed-refs, while the improvements for loose refs are a lot more
significant and conceptually make a lot of sense. So I didn't intend to
say that we shouldn't do these optimizations because of the miniscule
peformance regression with packed-refs.
Or in other words: this series looks good to me.
Thanks!
Patrick
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH v2 1/4] ref-cache.c: fix prefix matching in ref iteration
From: Patrick Steinhardt @ 2023-10-10 7:21 UTC (permalink / raw)
To: Victoria Dye via GitGitGadget; +Cc: git, Victoria Dye
In-Reply-To: <402176246ea9d722a71a0ca4e970dfce8a4bf776.1696888736.git.gitgitgadget@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 6056 bytes --]
On Mon, Oct 09, 2023 at 09:58:53PM +0000, Victoria Dye via GitGitGadget wrote:
> From: Victoria Dye <vdye@github.com>
>
> Update 'cache_ref_iterator_advance' to skip over refs that are not matched
> by the given prefix.
>
> Currently, a ref entry is considered "matched" if the entry name is fully
> contained within the prefix:
>
> * prefix: "refs/heads/v1"
> * entry: "refs/heads/v1.0"
>
> OR if the prefix is fully contained in the entry name:
>
> * prefix: "refs/heads/v1.0"
> * entry: "refs/heads/v1"
>
> The first case is always correct, but the second is only correct if the ref
> cache entry is a directory, for example:
>
> * prefix: "refs/heads/example"
> * entry: "refs/heads/"
>
> Modify the logic in 'cache_ref_iterator_advance' to reflect these
> expectations:
>
> 1. If 'overlaps_prefix' returns 'PREFIX_EXCLUDES_DIR', then the prefix and
> ref cache entry do not overlap at all. Skip this entry.
> 2. If 'overlaps_prefix' returns 'PREFIX_WITHIN_DIR', then the prefix matches
> inside this entry if it is a directory. Skip if the entry is not a
> directory, otherwise iterate over it.
> 3. Otherwise, 'overlaps_prefix' returned 'PREFIX_CONTAINS_DIR', indicating
> that the cache entry (directory or not) is fully contained by or equal to
> the prefix. Iterate over this entry.
>
> Note that condition 2 relies on the names of directory entries having the
> appropriate trailing slash. The existing function documentation of
> 'create_dir_entry' explicitly calls out the trailing slash requirement, so
> this is a safe assumption to make.
>
> This bug generally doesn't have any user-facing impact, since it requires:
>
> 1. using a non-empty prefix without a trailing slash in an iteration like
> 'for_each_fullref_in',
> 2. the callback to said iteration not reapplying the original filter (as
> for-each-ref does) to ensure unmatched refs are skipped, and
> 3. the repository having one or more refs that match part of, but not all
> of, the prefix.
>
> However, there are some niche scenarios that meet those criteria
> (specifically, 'rev-parse --bisect' and '(log|show|shortlog) --bisect'). Add
> tests covering those cases to demonstrate the fix in this patch.
>
> Signed-off-by: Victoria Dye <vdye@github.com>
> ---
> refs/ref-cache.c | 3 ++-
> t/t1500-rev-parse.sh | 23 +++++++++++++++++++++++
> t/t4205-log-pretty-formats.sh | 30 ++++++++++++++++++++++++++++++
> 3 files changed, 55 insertions(+), 1 deletion(-)
>
> diff --git a/refs/ref-cache.c b/refs/ref-cache.c
> index 2294c4564fb..6e3b725245c 100644
> --- a/refs/ref-cache.c
> +++ b/refs/ref-cache.c
> @@ -412,7 +412,8 @@ static int cache_ref_iterator_advance(struct ref_iterator *ref_iterator)
>
> if (level->prefix_state == PREFIX_WITHIN_DIR) {
> entry_prefix_state = overlaps_prefix(entry->name, iter->prefix);
> - if (entry_prefix_state == PREFIX_EXCLUDES_DIR)
> + if (entry_prefix_state == PREFIX_EXCLUDES_DIR ||
> + (entry_prefix_state == PREFIX_WITHIN_DIR && !(entry->flag & REF_DIR)))
> continue;
> } else {
> entry_prefix_state = level->prefix_state;
> diff --git a/t/t1500-rev-parse.sh b/t/t1500-rev-parse.sh
> index 37ee5091b5c..3f9e7f62e45 100755
> --- a/t/t1500-rev-parse.sh
> +++ b/t/t1500-rev-parse.sh
> @@ -264,4 +264,27 @@ test_expect_success 'rev-parse --since= unsqueezed ordering' '
> test_cmp expect actual
> '
>
> +test_expect_success 'rev-parse --bisect includes bad, excludes good' '
> + test_commit_bulk 6 &&
> +
> + git update-ref refs/bisect/bad-1 HEAD~1 &&
> + git update-ref refs/bisect/b HEAD~2 &&
> + git update-ref refs/bisect/bad-3 HEAD~3 &&
> + git update-ref refs/bisect/good-3 HEAD~3 &&
> + git update-ref refs/bisect/bad-4 HEAD~4 &&
> + git update-ref refs/bisect/go HEAD~4 &&
> +
> + # Note: refs/bisect/b and refs/bisect/go should be ignored because they
> + # do not match the refs/bisect/bad or refs/bisect/good prefixes.
> + cat >expect <<-EOF &&
> + refs/bisect/bad-1
> + refs/bisect/bad-3
> + refs/bisect/bad-4
> + ^refs/bisect/good-3
> + EOF
> +
> + git rev-parse --symbolic-full-name --bisect >actual &&
> + test_cmp expect actual
> +'
> +
> test_done
> diff --git a/t/t4205-log-pretty-formats.sh b/t/t4205-log-pretty-formats.sh
> index 16626e4fe96..62c7bfed5d7 100755
> --- a/t/t4205-log-pretty-formats.sh
> +++ b/t/t4205-log-pretty-formats.sh
> @@ -956,6 +956,36 @@ test_expect_success '%S in git log --format works with other placeholders (part
> test_cmp expect actual
> '
>
> +test_expect_success 'setup more commits for %S with --bisect' '
> + test_commit four &&
> + test_commit five &&
> +
> + head1=$(git rev-parse --verify HEAD~0) &&
> + head2=$(git rev-parse --verify HEAD~1) &&
> + head3=$(git rev-parse --verify HEAD~2) &&
> + head4=$(git rev-parse --verify HEAD~3)
> +'
> +
> +test_expect_success '%S with --bisect labels commits with refs/bisect/bad ref' '
> + git update-ref refs/bisect/bad-$head1 $head1 &&
> + git update-ref refs/bisect/go $head1 &&
> + git update-ref refs/bisect/bad-$head2 $head2 &&
> + git update-ref refs/bisect/b $head3 &&
> + git update-ref refs/bisect/bad-$head4 $head4 &&
> + git update-ref refs/bisect/good-$head4 $head4 &&
> +
> + # We expect to see the range of commits betwee refs/bisect/good-$head4
Nit: s/betwee/between. Probably not worth rerolling this series only
because of this typo though.
Patrick
> + # and refs/bisect/bad-$head1. The "source" ref is the nearest bisect ref
> + # from which the commit is reachable.
> + cat >expect <<-EOF &&
> + $head1 refs/bisect/bad-$head1
> + $head2 refs/bisect/bad-$head2
> + $head3 refs/bisect/bad-$head2
> + EOF
> + git log --bisect --format="%H %S" >actual &&
> + test_cmp expect actual
> +'
> +
> test_expect_success 'log --pretty=reference' '
> git log --pretty="tformat:%h (%s, %as)" >expect &&
> git log --pretty=reference >actual &&
> --
> gitgitgadget
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH 7/7] builtin/merge-tree.c: implement support for `--write-pack`
From: Patrick Steinhardt @ 2023-10-10 6:36 UTC (permalink / raw)
To: Taylor Blau
Cc: Junio C Hamano, git, Elijah Newren, Eric W. Biederman, Jeff King
In-Reply-To: <ZSQlgLDv+MrYmSp8@nand.local>
[-- Attachment #1: Type: text/plain, Size: 3748 bytes --]
On Mon, Oct 09, 2023 at 12:08:32PM -0400, Taylor Blau wrote:
> On Mon, Oct 09, 2023 at 12:54:12PM +0200, Patrick Steinhardt wrote:
> > In Gitaly, we usually set up quarantine directories for all operations
> > that create objects. This allows us to discard any newly written objects
> > in case either the RPC call gets cancelled or in case our access checks
> > determine that the change should not be allowed. The logic is rather
> > simple:
> >
> > 1. Create a new temporary directory.
> >
> > 2. Set up the new temporary directory as main object database via
> > the `GIT_OBJECT_DIRECTORY` environment variable.
> >
> > 3. Set up the main repository's object database via the
> > `GIT_ALTERNATE_OBJECT_DIRECTORIES` environment variable.
>
> Is there a reason not to run Git in the quarantine environment and list
> the main repository as an alternate via $GIT_DIR/objects/info/alternates
> instead of the GIT_ALTERNATE_OBJECT_DIRECTORIES environment variable?
The quarantine environment as we use it is really only a single object
database, so you cannot run Git inside of it directly.
> > 4. Execute Git commands that write objects with these environment
> > variables set up. The new objects will end up neatly contained in
> > the temporary directory.
> >
> > 5. Once done, either discard the temporary object database or
> > migrate objects into the main object daatabase.
>
> Interesting. I'm curious why you don't use the builtin tmp_objdir
> mechanism in Git itself. Do you need to run more than one command in the
> quarantine environment? If so, that makes sense that you'd want to have
> a scratch repository that lasts beyond the lifetime of a single process.
It's a mixture of things:
- Many commands simply don't set up a temporary object directory.
- We want to check the result after the objects have been generated.
Many of the commands don't provide hooks to do so in a reasonable
way. So we want to check the result _after_ the command has exited
already, and objects should not yet have been migrated into the
target object database at that point.
- Sometimes we indeed want to run multiple Git commands. We use this
e.g. for worktreeless rebases, where we run a succession of
commands to rebase every single commit.
So ultimately, our own quarantine directory sits at a conceptually
higher level than what Git commands would be able to provide.
> > I wonder whether this would be a viable approach for you, as well.
>
> I think that the main problem that we are trying to solve with this
> series is creating a potentially large number of loose objects. I think
> that you could do something like what you propose above, with a 'git
> repacks -adk' before moving its objects over back to the main repository.
Yeah, that's certainly possible. I had the feeling that there are two
different problems that we're trying to solve in this thread:
- The problem that the objects are of temporary nature. Regardless
of whether they are in a packfile or loose, it doesn't make much
sense to put them into the main object database as they would be
unreachable anyway and thus get pruned after some time.
- The problem that writing many loose objects can be slow.
My proposal only addresses the first problem.
> But since we're working in a single process only when doing a merge-tree
> operation, I think it is probably more expedient to write the pack's
> bytes directly.
If it's about efficiency and not about how we discard those objects
after the command has ran to completion then yes, I agree.
Patrick
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH 0/3] rev-list: add support for commits in `--missing`
From: Patrick Steinhardt @ 2023-10-10 6:19 UTC (permalink / raw)
To: Karthik Nayak; +Cc: git
In-Reply-To: <20231009105528.17777-1-karthik.188@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 2054 bytes --]
On Mon, Oct 09, 2023 at 12:55:25PM +0200, Karthik Nayak wrote:
> The `--missing` option in git-rev-list(1) was introduced intitally
> to deal with missing blobs in the context of promissory notes.
> Eventually the option was extended to also support tree objects in
> 7c0fe330d5 (rev-list: handle missing tree objects properly,2018-10-05).
>
> This patch series extends the `--missing` option to also add support for
> commit objects. We do this by introducing a new flag `MISSING` which is
> added whenever we encounter a missing commit during traversal. Then in
> `builtin/rev-list` we check for this flag and take the appropriate
> action based on the `--missing=*` option used.
>
> This series is an alternate to the patch series I had posted earlier:
> https://lore.kernel.org/git/20230908174208.249184-1-karthik.188@gmail.com/.
> In that patch, we introduced an option `--ignore-missing-links` which
> was added to expose the `ignore_missing_links` bit to the user. The
> issue in that patch was that, the option `--ignore-missing-links` didn't
> play well the pre-existing `--missing` option. This series avoids that
> route and just extends the `--missing` option for commits to solve the
> same problem.
I had already reviewed the patches internally at GitLab, so for what
it's worth please feel free to add my Reviewed-by.
Patrick
> Karthik Nayak (3):
> revision: rename bit to `do_not_die_on_missing_objects`
> rev-list: move `show_commit()` to the bottom
> rev-list: add commit object support in `--missing` option
>
> builtin/reflog.c | 2 +-
> builtin/rev-list.c | 93 +++++++++++++++++++------------------
> list-objects.c | 2 +-
> object.h | 2 +-
> revision.c | 9 +++-
> revision.h | 20 ++++----
> t/t6022-rev-list-missing.sh | 74 +++++++++++++++++++++++++++++
> 7 files changed, 145 insertions(+), 57 deletions(-)
> create mode 100755 t/t6022-rev-list-missing.sh
>
> --
> 2.41.0
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH] git-diff: Introduce --index and deprecate --cached.
From: Russell Nelson @ 2023-10-10 3:46 UTC (permalink / raw)
To: junkio; +Cc: ae, git
Sent from my iPhone
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox