* [PATCH 7/7] sequencer: teach parser about CHERRY_PICK_HEAD
From: Ramkumar Ramachandra @ 2011-11-13 10:46 UTC (permalink / raw)
To: Git List; +Cc: Junio C Hamano, Jonathan Nieder, Christian Couder
In-Reply-To: <1321181181-23923-1-git-send-email-artagnon@gmail.com>
The final step in unifying the sequencer interface involves making
sure that a single-commit pick can be concluded with a '--continue'.
$ git cherry-pick foo
... conflict ...
$ echo "resolved" >problematicfile
$ git add problematicfile
$ git sequencer --continue
To do this, we have to update our parser that normally reads
'.git/sequencer/todo' to read '.git/CHERRY_PICK_HEAD' as a special
case before proceeding as usual. Add a new test in
't3510-sequencer.sh' to make sure that this works. Although we have
added a similar test for revert in the same patch for symmetry, the
test doesn't depend on this patch to work. A single-commit revert
operation does not create a '.git/CHERRY_PICK_HEAD' at all: it uses
the information in '.git/sequencer' as usual.
Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
---
sequencer.c | 32 ++++++++++++++++++++++++++++----
t/t3510-sequencer.sh | 44 ++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 72 insertions(+), 4 deletions(-)
diff --git a/sequencer.c b/sequencer.c
index 7b10b7b..84df926 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -569,12 +569,32 @@ static int parse_insn_buffer(char *buf, struct replay_insn_list **todo_list)
return 0;
}
-static void read_populate_todo(struct replay_insn_list **todo_list)
+static void read_populate_todo(struct replay_insn_list **todo_list, int cph_flag)
{
const char *todo_file = git_path(SEQ_TODO_FILE);
struct strbuf buf = STRBUF_INIT;
int fd, res;
+ if (cph_flag) {
+ struct replay_insn_list item = {0, NULL, NULL};
+ const char *name = "CHERRY_PICK_HEAD";
+ const char *CHERRY_PICK_HEAD = git_path(name);
+
+ fd = open(CHERRY_PICK_HEAD, O_RDONLY);
+ if (fd < 0)
+ die_errno(_("Could not open %s."), CHERRY_PICK_HEAD);
+
+ item.action = REPLAY_PICK;
+ item.operand = lookup_commit_reference_by_name(name);
+
+ if (!item.operand)
+ die(_("could not lookup commit %s"), name);
+ replay_insn_list_append(item.action, item.operand, todo_list);
+ close(fd);
+ strbuf_release(&buf);
+ return;
+ }
+
fd = open(todo_file, O_RDONLY);
if (fd < 0)
die_errno(_("Could not open %s."), todo_file);
@@ -792,10 +812,14 @@ int sequencer_pick_revisions(struct replay_opts *opts)
remove_sequencer_state(1);
return 0;
} else if (opts->subcommand == REPLAY_CONTINUE) {
- if (!file_exists(git_path(SEQ_TODO_FILE)))
- goto error;
+ if (!file_exists(git_path(SEQ_TODO_FILE))) {
+ if (!file_exists(git_path("CHERRY_PICK_HEAD")))
+ goto error;
+ read_populate_todo(&todo_list, 1);
+ }
+ else
+ read_populate_todo(&todo_list, 0);
read_populate_opts(&opts);
- read_populate_todo(&todo_list);
if (!index_differs_from("HEAD", 0))
todo_list = todo_list->next;
diff --git a/t/t3510-sequencer.sh b/t/t3510-sequencer.sh
index 65f2724..f7c3a37 100755
--- a/t/t3510-sequencer.sh
+++ b/t/t3510-sequencer.sh
@@ -45,6 +45,50 @@ test_expect_success '--continue complains when there are unresolved conflicts' '
test_must_fail git sequencer --continue
'
+test_expect_success '--continue continues single-commit cherry-pick' '
+ pristine_detach initial &&
+ test_must_fail git cherry-pick anotherpick &&
+ echo "c" >foo &&
+ git add foo &&
+ git sequencer --continue &&
+ test_path_is_missing .git/sequencer &&
+ {
+ git rev-list HEAD |
+ git diff-tree --root --stdin |
+ sed "s/$_x40/OBJID/g"
+ } >actual &&
+ cat >expect <<-\EOF &&
+ OBJID
+ :100644 100644 OBJID OBJID M foo
+ OBJID
+ :000000 100644 OBJID OBJID A foo
+ :000000 100644 OBJID OBJID A unrelated
+ EOF
+ test_cmp expect actual
+'
+
+test_expect_success '--continue continues single-commit revert' '
+ pristine_detach initial &&
+ test_must_fail git revert anotherpick &&
+ echo "c" >foo &&
+ git add foo &&
+ git sequencer --continue &&
+ test_path_is_missing .git/sequencer &&
+ {
+ git rev-list HEAD |
+ git diff-tree --root --stdin |
+ sed "s/$_x40/OBJID/g"
+ } >actual &&
+ cat >expect <<-\EOF &&
+ OBJID
+ :100644 100644 OBJID OBJID M foo
+ OBJID
+ :000000 100644 OBJID OBJID A foo
+ :000000 100644 OBJID OBJID A unrelated
+ EOF
+ test_cmp expect actual
+'
+
test_expect_success '--continue continues after conflicts are resolved' '
pristine_detach initial &&
test_must_fail git cherry-pick base..anotherpick &&
--
1.7.6.351.gb35ac.dirty
^ permalink raw reply related
* [PATCH 6/7] sequencer: teach '--continue' how to commit
From: Ramkumar Ramachandra @ 2011-11-13 10:46 UTC (permalink / raw)
To: Git List; +Cc: Junio C Hamano, Jonathan Nieder, Christian Couder
In-Reply-To: <1321181181-23923-1-git-send-email-artagnon@gmail.com>
As discussed earlier in the series, the commit-before-continue
convention has caused a lot of workflow inconsistencies. Teach
'--continue' how to commit so that you can now do:
$ git cherry-pick foo..bar
... conflict occured in bar~1 ...
$ echo "resolved" >problematicfile
$ git add problematicfile
$ git sequencer --continue
Modify existing tests in 't3510-sequencer.sh' to exclude the 'git
commit' step, and port relevant tests from
't3511-cherry-pick-sequence.sh' after removing the 'git commit' step.
Note that the tests in 't3511-cherry-pick-sequence.sh' (with the 'git
commit' step) still need to respect backward compatibility.
Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
---
sequencer.c | 10 +++++++-
t/t3510-sequencer.sh | 51 +++++++++++++++++++++++++++++++++++++++++++------
2 files changed, 52 insertions(+), 9 deletions(-)
diff --git a/sequencer.c b/sequencer.c
index 012d531..7b10b7b 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -269,7 +269,7 @@ static int run_git_commit(const char *defmsg, struct replay_opts *opts)
args[i++] = "-n";
if (opts->signoff)
args[i++] = "-s";
- if (!opts->edit) {
+ if (!opts->edit && defmsg) {
args[i++] = "-F";
args[i++] = defmsg;
}
@@ -776,6 +776,7 @@ int sequencer_pick_revisions(struct replay_opts *opts)
{
struct replay_insn_list *todo_list = NULL;
unsigned char sha1[20];
+ int res;
if (opts->subcommand == REPLAY_NONE)
assert(opts->revs);
@@ -796,9 +797,14 @@ int sequencer_pick_revisions(struct replay_opts *opts)
read_populate_opts(&opts);
read_populate_todo(&todo_list);
- /* Verify that the conflict has been resolved */
if (!index_differs_from("HEAD", 0))
todo_list = todo_list->next;
+ else if (!opts->no_commit && !read_cache_unmerged()) {
+ res = run_git_commit(NULL, opts);
+ if (res)
+ return res;
+ todo_list = todo_list->next;
+ }
} else {
/*
* Start a new cherry-pick/ revert sequence; but
diff --git a/t/t3510-sequencer.sh b/t/t3510-sequencer.sh
index 6b2e712..65f2724 100755
--- a/t/t3510-sequencer.sh
+++ b/t/t3510-sequencer.sh
@@ -45,12 +45,54 @@ test_expect_success '--continue complains when there are unresolved conflicts' '
test_must_fail git sequencer --continue
'
+test_expect_success '--continue continues after conflicts are resolved' '
+ pristine_detach initial &&
+ test_must_fail git cherry-pick base..anotherpick &&
+ echo "c" >foo &&
+ git add foo &&
+ git sequencer --continue &&
+ test_path_is_missing .git/sequencer &&
+ {
+ git rev-list HEAD |
+ git diff-tree --root --stdin |
+ sed "s/$_x40/OBJID/g"
+ } >actual &&
+ cat >expect <<-\EOF &&
+ OBJID
+ :100644 100644 OBJID OBJID M foo
+ OBJID
+ :100644 100644 OBJID OBJID M foo
+ OBJID
+ :100644 100644 OBJID OBJID M unrelated
+ OBJID
+ :000000 100644 OBJID OBJID A foo
+ :000000 100644 OBJID OBJID A unrelated
+ EOF
+ test_cmp expect actual
+'
+
+test_expect_success '--continue respects opts' '
+ pristine_detach initial &&
+ test_must_fail git cherry-pick -x base..anotherpick &&
+ echo "c" >foo &&
+ git add foo &&
+ git sequencer --continue &&
+ test_path_is_missing .git/sequencer &&
+ git cat-file commit HEAD >anotherpick_msg &&
+ git cat-file commit HEAD~1 >picked_msg &&
+ git cat-file commit HEAD~2 >unrelatedpick_msg &&
+ git cat-file commit HEAD~3 >initial_msg &&
+ test_must_fail grep "cherry picked from" initial_msg &&
+ grep "cherry picked from" unrelatedpick_msg &&
+ grep "cherry picked from" picked_msg &&
+ grep "cherry picked from" anotherpick_msg
+'
+
test_expect_success 'malformed instruction sheet 1' '
pristine_detach initial &&
test_must_fail git cherry-pick base..anotherpick &&
echo "resolved" >foo &&
git add foo &&
- git commit &&
sed "s/pick /pick/" .git/sequencer/todo >new_sheet &&
cp new_sheet .git/sequencer/todo &&
test_must_fail git sequencer --continue
@@ -61,7 +103,6 @@ test_expect_success 'malformed instruction sheet 2' '
test_must_fail git cherry-pick base..anotherpick &&
echo "resolved" >foo &&
git add foo &&
- git commit &&
sed "s/pick/revert/" .git/sequencer/todo >new_sheet &&
cp new_sheet .git/sequencer/todo &&
test_must_fail git sequencer --continue
@@ -72,7 +113,6 @@ test_expect_success 'malformed instruction sheet 3' '
test_must_fail git cherry-pick base..anotherpick &&
echo "resolved" >foo &&
git add foo &&
- git commit &&
sed "s/pick \([0-9a-f]*\)/pick $_r10/" .git/sequencer/todo >new_sheet &&
cp new_sheet .git/sequencer/todo &&
test_must_fail git sequencer --continue
@@ -83,7 +123,6 @@ test_expect_success 'commit descriptions in insn sheet are optional' '
test_must_fail git cherry-pick base..anotherpick &&
echo "c" >foo &&
git add foo &&
- git commit &&
cut -d" " -f1,2 .git/sequencer/todo >new_sheet &&
cp new_sheet .git/sequencer/todo &&
git sequencer --continue &&
@@ -97,7 +136,6 @@ test_expect_success 'revert --continue continues after cherry-pick' '
test_must_fail git cherry-pick base..anotherpick &&
echo "c" >foo &&
git add foo &&
- git commit &&
git revert --continue &&
test_path_is_missing .git/sequencer &&
{
@@ -120,12 +158,11 @@ test_expect_success 'revert --continue continues after cherry-pick' '
'
test_expect_success 'mixed pick and revert instructions' '
+ oldsha=`git rev-parse --short HEAD~2` &&
pristine_detach initial &&
test_must_fail git cherry-pick base..anotherpick &&
echo "c" >foo &&
git add foo &&
- git commit &&
- oldsha=`git rev-parse --short HEAD~1` &&
echo "revert $oldsha unrelatedpick" >>.git/sequencer/todo &&
git sequencer --continue &&
test_path_is_missing .git/sequencer &&
--
1.7.6.351.gb35ac.dirty
^ permalink raw reply related
* [PATCH 5/7] sequencer: introduce git-sequencer builtin
From: Ramkumar Ramachandra @ 2011-11-13 10:46 UTC (permalink / raw)
To: Git List; +Cc: Junio C Hamano, Jonathan Nieder, Christian Couder
In-Reply-To: <1321181181-23923-1-git-send-email-artagnon@gmail.com>
Introduce a new builtin 'git sequencer' that implements only
'--continue' and '--reset' features of the sequencer. As a result, it
can only be used to continue an existing sequencer operation, not to
start a new one. Now you can do:
$ git cherry-pick foo..bar
... conflict ...
$ echo "resolved" >problematicfile
$ git add problematicfile
$ git sequencer --continue
So, irrespective of the concrete implementation of the sequencer
invoked earlier, any sequencer operation can be continued with 'git
sequencer --continue' and reset with 'git sequencer --reset'. Also,
we plan to make the 'git sequencer' builtin implement a concrete
operation similar to 'git rebase -i' in the future.
While at it, change the advice printed by the sequencer library and
re-organize 't3510-cherry-pick-sequence.sh'. There are now two
separate sets of tests:
t3510-sequencer.sh is meant to test the continuation features of the
sequencer, using 'git cherry-pick' to initiate an operation.
t3511-cherry-pick-sequencer.sh is meant to test 'git cherry-pick' as
a concrete implementation of the sequencer, stressing on many of the
backward compatibility features specific to it.
In the future, when the sequencer grows more functionality that
doesn't directly impact the concrete implementation 'git cherry-pick',
the corresponding tests should go into 't3510-sequencer.sh'.
Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
---
.gitignore | 1 +
Documentation/git-sequencer.txt | 33 +++++
Makefile | 1 +
builtin.h | 1 +
builtin/sequencer.c | 52 +++++++
git.c | 1 +
sequencer.c | 6 +-
t/t3510-sequencer.sh | 153 ++++++++++++++++++++
...-sequence.sh => t3511-cherry-pick-sequencer.sh} | 120 +---------------
9 files changed, 251 insertions(+), 117 deletions(-)
create mode 100644 Documentation/git-sequencer.txt
create mode 100644 builtin/sequencer.c
create mode 100755 t/t3510-sequencer.sh
rename t/{t3510-cherry-pick-sequence.sh => t3511-cherry-pick-sequencer.sh} (61%)
diff --git a/.gitignore b/.gitignore
index 8572c8c..74ea408 100644
--- a/.gitignore
+++ b/.gitignore
@@ -128,6 +128,7 @@
/git-rm
/git-send-email
/git-send-pack
+/git-sequencer
/git-sh-i18n
/git-sh-i18n--envsubst
/git-sh-setup
diff --git a/Documentation/git-sequencer.txt b/Documentation/git-sequencer.txt
new file mode 100644
index 0000000..aa8a80b
--- /dev/null
+++ b/Documentation/git-sequencer.txt
@@ -0,0 +1,33 @@
+git-sequencer(1)
+================
+
+NAME
+----
+git-sequencer - Finish an existing sequencer operation
+
+
+SYNOPSIS
+--------
+[verse]
+'git sequencer' --reset
+'git sequencer' --continue
+
+DESCRIPTION
+-----------
+Given any arbitrary sequencer operation started using 'git
+cherry-pick' or 'git revert', facilitate continuing or resetting it
+via a uniform interface.
+
+
+OPTIONS
+-------
+include::sequencer.txt[]
+
+
+SEE ALSO
+--------
+linkgit:git-cherry-pick[1]
+
+GIT
+---
+Part of the linkgit:git[1] suite
diff --git a/Makefile b/Makefile
index 17404c4..e367310 100644
--- a/Makefile
+++ b/Makefile
@@ -777,6 +777,7 @@ BUILTIN_OBJS += builtin/rev-parse.o
BUILTIN_OBJS += builtin/revert.o
BUILTIN_OBJS += builtin/rm.o
BUILTIN_OBJS += builtin/send-pack.o
+BUILTIN_OBJS += builtin/sequencer.o
BUILTIN_OBJS += builtin/shortlog.o
BUILTIN_OBJS += builtin/show-branch.o
BUILTIN_OBJS += builtin/show-ref.o
diff --git a/builtin.h b/builtin.h
index 0e9da90..0335163 100644
--- a/builtin.h
+++ b/builtin.h
@@ -119,6 +119,7 @@ extern int cmd_rev_parse(int argc, const char **argv, const char *prefix);
extern int cmd_revert(int argc, const char **argv, const char *prefix);
extern int cmd_rm(int argc, const char **argv, const char *prefix);
extern int cmd_send_pack(int argc, const char **argv, const char *prefix);
+extern int cmd_sequencer(int argc, const char **argv, const char *prefix);
extern int cmd_shortlog(int argc, const char **argv, const char *prefix);
extern int cmd_show(int argc, const char **argv, const char *prefix);
extern int cmd_show_branch(int argc, const char **argv, const char *prefix);
diff --git a/builtin/sequencer.c b/builtin/sequencer.c
new file mode 100644
index 0000000..7364a52
--- /dev/null
+++ b/builtin/sequencer.c
@@ -0,0 +1,52 @@
+#include "cache.h"
+#include "builtin.h"
+#include "parse-options.h"
+#include "sequencer.h"
+
+static const char *const sequencer_usage[] = {
+ "git sequencer --continue",
+ "git sequencer --reset",
+ NULL
+};
+
+static void parse_args(int argc, const char **argv, struct replay_opts *opts)
+{
+ int reset = 0;
+ int contin = 0;
+ struct option options[] = {
+ OPT_BOOLEAN(0, "reset", &reset, "forget the current operation"),
+ OPT_BOOLEAN(0, "continue", &contin, "continue the current operation"),
+ OPT_END(),
+ };
+
+ argc = parse_options(argc, argv, NULL, options, sequencer_usage,
+ PARSE_OPT_KEEP_ARGV0 |
+ PARSE_OPT_KEEP_UNKNOWN);
+
+ /* Set the subcommand */
+ if (reset)
+ opts->subcommand = REPLAY_RESET;
+ else if (contin)
+ opts->subcommand = REPLAY_CONTINUE;
+ else
+ opts->subcommand = REPLAY_NONE;
+
+ /* Forbid REPLAY_NONE and stray command-line arguments */
+ if (opts->subcommand == REPLAY_NONE || argc > 1)
+ usage_with_options(sequencer_usage, options);
+}
+
+int cmd_sequencer(int argc, const char **argv, const char *prefix)
+{
+ struct replay_opts opts;
+ int res;
+
+ memset(&opts, 0, sizeof(opts));
+ opts.action = REPLAY_REVERT;
+ git_config(git_default_config, NULL);
+ parse_args(argc, argv, &opts);
+ res = sequencer_pick_revisions(&opts);
+ if (res < 0)
+ die(_("sequencer failed"));
+ return res;
+}
diff --git a/git.c b/git.c
index 8e34903..5262c9b 100644
--- a/git.c
+++ b/git.c
@@ -418,6 +418,7 @@ static void handle_internal_command(int argc, const char **argv)
{ "revert", cmd_revert, RUN_SETUP | NEED_WORK_TREE },
{ "rm", cmd_rm, RUN_SETUP },
{ "send-pack", cmd_send_pack, RUN_SETUP },
+ { "sequencer", cmd_sequencer, RUN_SETUP | NEED_WORK_TREE },
{ "shortlog", cmd_shortlog, RUN_SETUP_GENTLY | USE_PAGER },
{ "show", cmd_show, RUN_SETUP },
{ "show-branch", cmd_show_branch, RUN_SETUP },
diff --git a/sequencer.c b/sequencer.c
index 23fd3fe..012d531 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -808,9 +808,9 @@ int sequencer_pick_revisions(struct replay_opts *opts)
walk_revs_populate_todo(&todo_list, opts);
if (create_seq_dir() < 0) {
- error(_("A cherry-pick or revert is in progress."));
- advise(_("Use --continue to continue the operation"));
- advise(_("or --reset to forget about it"));
+ error(_("A sequencer operation is in progress."));
+ advise(_("Use git sequencer --continue to it"));
+ advise(_("or git sequencer --reset to forget about it"));
return -1;
}
if (get_sha1("HEAD", sha1)) {
diff --git a/t/t3510-sequencer.sh b/t/t3510-sequencer.sh
new file mode 100755
index 0000000..6b2e712
--- /dev/null
+++ b/t/t3510-sequencer.sh
@@ -0,0 +1,153 @@
+#!/bin/sh
+
+test_description='Test sequencer continuation features
+
+ + anotherpick: rewrites foo to d
+ + picked: rewrites foo to c
+ + unrelatedpick: rewrites unrelated to reallyunrelated
+ + base: rewrites foo to b
+ + initial: writes foo as a, unrelated as unrelated
+
+'
+
+. ./test-lib.sh
+
+# Repeat first match 10 times
+_r10='\1\1\1\1\1\1\1\1\1\1'
+
+pristine_detach () {
+ git cherry-pick --reset &&
+ git checkout -f "$1^0" &&
+ git read-tree -u --reset HEAD &&
+ git clean -d -f -f -q -x
+}
+
+test_expect_success setup '
+ echo unrelated >unrelated &&
+ git add unrelated &&
+ test_commit initial foo a &&
+ test_commit base foo b &&
+ test_commit unrelatedpick unrelated reallyunrelated &&
+ test_commit picked foo c &&
+ test_commit anotherpick foo d &&
+ git config advice.detachedhead false
+
+'
+
+test_expect_success '--continue complains when no sequencer operation is in progress' '
+ pristine_detach initial &&
+ test_must_fail git sequencer --continue
+'
+
+test_expect_success '--continue complains when there are unresolved conflicts' '
+ pristine_detach initial &&
+ test_must_fail git cherry-pick base..anotherpick &&
+ test_must_fail git sequencer --continue
+'
+
+test_expect_success 'malformed instruction sheet 1' '
+ pristine_detach initial &&
+ test_must_fail git cherry-pick base..anotherpick &&
+ echo "resolved" >foo &&
+ git add foo &&
+ git commit &&
+ sed "s/pick /pick/" .git/sequencer/todo >new_sheet &&
+ cp new_sheet .git/sequencer/todo &&
+ test_must_fail git sequencer --continue
+'
+
+test_expect_success 'malformed instruction sheet 2' '
+ pristine_detach initial &&
+ test_must_fail git cherry-pick base..anotherpick &&
+ echo "resolved" >foo &&
+ git add foo &&
+ git commit &&
+ sed "s/pick/revert/" .git/sequencer/todo >new_sheet &&
+ cp new_sheet .git/sequencer/todo &&
+ test_must_fail git sequencer --continue
+'
+
+test_expect_success 'malformed instruction sheet 3' '
+ pristine_detach initial &&
+ test_must_fail git cherry-pick base..anotherpick &&
+ echo "resolved" >foo &&
+ git add foo &&
+ git commit &&
+ sed "s/pick \([0-9a-f]*\)/pick $_r10/" .git/sequencer/todo >new_sheet &&
+ cp new_sheet .git/sequencer/todo &&
+ test_must_fail git sequencer --continue
+'
+
+test_expect_success 'commit descriptions in insn sheet are optional' '
+ pristine_detach initial &&
+ test_must_fail git cherry-pick base..anotherpick &&
+ echo "c" >foo &&
+ git add foo &&
+ git commit &&
+ cut -d" " -f1,2 .git/sequencer/todo >new_sheet &&
+ cp new_sheet .git/sequencer/todo &&
+ git sequencer --continue &&
+ test_path_is_missing .git/sequencer &&
+ git rev-list HEAD >commits
+ test_line_count = 4 commits
+'
+
+test_expect_success 'revert --continue continues after cherry-pick' '
+ pristine_detach initial &&
+ test_must_fail git cherry-pick base..anotherpick &&
+ echo "c" >foo &&
+ git add foo &&
+ git commit &&
+ git revert --continue &&
+ test_path_is_missing .git/sequencer &&
+ {
+ git rev-list HEAD |
+ git diff-tree --root --stdin |
+ sed "s/$_x40/OBJID/g"
+ } >actual &&
+ cat >expect <<-\EOF &&
+ OBJID
+ :100644 100644 OBJID OBJID M foo
+ OBJID
+ :100644 100644 OBJID OBJID M foo
+ OBJID
+ :100644 100644 OBJID OBJID M unrelated
+ OBJID
+ :000000 100644 OBJID OBJID A foo
+ :000000 100644 OBJID OBJID A unrelated
+ EOF
+ test_cmp expect actual
+'
+
+test_expect_success 'mixed pick and revert instructions' '
+ pristine_detach initial &&
+ test_must_fail git cherry-pick base..anotherpick &&
+ echo "c" >foo &&
+ git add foo &&
+ git commit &&
+ oldsha=`git rev-parse --short HEAD~1` &&
+ echo "revert $oldsha unrelatedpick" >>.git/sequencer/todo &&
+ git sequencer --continue &&
+ test_path_is_missing .git/sequencer &&
+ {
+ git rev-list HEAD |
+ git diff-tree --root --stdin |
+ sed "s/$_x40/OBJID/g"
+ } >actual &&
+ cat >expect <<-\EOF &&
+ OBJID
+ :100644 100644 OBJID OBJID M unrelated
+ OBJID
+ :100644 100644 OBJID OBJID M foo
+ OBJID
+ :100644 100644 OBJID OBJID M foo
+ OBJID
+ :100644 100644 OBJID OBJID M unrelated
+ OBJID
+ :000000 100644 OBJID OBJID A foo
+ :000000 100644 OBJID OBJID A unrelated
+ EOF
+ test_cmp expect actual
+'
+
+test_done
diff --git a/t/t3510-cherry-pick-sequence.sh b/t/t3511-cherry-pick-sequencer.sh
similarity index 61%
rename from t/t3510-cherry-pick-sequence.sh
rename to t/t3511-cherry-pick-sequencer.sh
index 09b9e65..a9c6ac1 100755
--- a/t/t3510-cherry-pick-sequence.sh
+++ b/t/t3511-cherry-pick-sequencer.sh
@@ -1,6 +1,6 @@
#!/bin/sh
-test_description='Test cherry-pick continuation features
+test_description='Test cherry-pick as a sequencer implementation
+ anotherpick: rewrites foo to d
+ picked: rewrites foo to c
@@ -12,9 +12,6 @@ test_description='Test cherry-pick continuation features
. ./test-lib.sh
-# Repeat first match 10 times
-_r10='\1\1\1\1\1\1\1\1\1\1'
-
pristine_detach () {
git cherry-pick --reset &&
git checkout -f "$1^0" &&
@@ -34,7 +31,7 @@ test_expect_success setup '
'
-test_expect_success 'cherry-pick persists data on failure' '
+test_expect_success 'sequencer persists data on failure' '
pristine_detach initial &&
test_must_fail git cherry-pick -s base..anotherpick &&
test_path_is_dir .git/sequencer &&
@@ -43,7 +40,7 @@ test_expect_success 'cherry-pick persists data on failure' '
test_path_is_file .git/sequencer/opts
'
-test_expect_success 'cherry-pick persists opts correctly' '
+test_expect_success 'sequencer persists opts correctly' '
pristine_detach initial &&
test_must_fail git cherry-pick -s -m 1 --strategy=recursive -X patience -X ours base..anotherpick &&
test_path_is_dir .git/sequencer &&
@@ -67,7 +64,7 @@ test_expect_success 'cherry-pick persists opts correctly' '
test_cmp expect actual
'
-test_expect_success 'cherry-pick cleans up sequencer state upon success' '
+test_expect_success 'sequencer cleans up state upon success' '
pristine_detach initial &&
git cherry-pick initial..picked &&
test_path_is_missing .git/sequencer
@@ -109,7 +106,7 @@ test_expect_success 'cherry-pick cleans up sequencer todo when one commit is lef
test_cmp expect actual
'
-test_expect_success 'cherry-pick does not implicitly stomp an existing operation' '
+test_expect_success 'sequencer does not implicitly stomp an existing operation' '
pristine_detach initial &&
test_must_fail git cherry-pick base..anotherpick &&
test-chmtime -v +0 .git/sequencer >expect &&
@@ -118,7 +115,7 @@ test_expect_success 'cherry-pick does not implicitly stomp an existing operation
test_cmp expect actual
'
-test_expect_success '--continue complains when no cherry-pick is in progress' '
+test_expect_success '--continue complains when no sequencer operation is in progress' '
pristine_detach initial &&
test_must_fail git cherry-pick --continue
'
@@ -192,109 +189,4 @@ test_expect_success '--signoff is not automatically propagated to resolved confl
grep "Signed-off-by:" anotherpick_msg
'
-test_expect_success 'malformed instruction sheet 1' '
- pristine_detach initial &&
- test_must_fail git cherry-pick base..anotherpick &&
- echo "resolved" >foo &&
- git add foo &&
- git commit &&
- sed "s/pick /pick/" .git/sequencer/todo >new_sheet &&
- cp new_sheet .git/sequencer/todo &&
- test_must_fail git cherry-pick --continue
-'
-
-test_expect_success 'malformed instruction sheet 2' '
- pristine_detach initial &&
- test_must_fail git cherry-pick base..anotherpick &&
- echo "resolved" >foo &&
- git add foo &&
- git commit &&
- sed "s/pick/revert/" .git/sequencer/todo >new_sheet &&
- cp new_sheet .git/sequencer/todo &&
- test_must_fail git cherry-pick --continue
-'
-
-test_expect_success 'malformed instruction sheet 3' '
- pristine_detach initial &&
- test_must_fail git cherry-pick base..anotherpick &&
- echo "resolved" >foo &&
- git add foo &&
- git commit &&
- sed "s/pick \([0-9a-f]*\)/pick $_r10/" .git/sequencer/todo >new_sheet &&
- cp new_sheet .git/sequencer/todo &&
- test_must_fail git cherry-pick --continue
-'
-
-test_expect_success 'commit descriptions in insn sheet are optional' '
- pristine_detach initial &&
- test_must_fail git cherry-pick base..anotherpick &&
- echo "c" >foo &&
- git add foo &&
- git commit &&
- cut -d" " -f1,2 .git/sequencer/todo >new_sheet &&
- cp new_sheet .git/sequencer/todo &&
- git cherry-pick --continue &&
- test_path_is_missing .git/sequencer &&
- git rev-list HEAD >commits
- test_line_count = 4 commits
-'
-
-test_expect_success 'revert --continue continues after cherry-pick' '
- pristine_detach initial &&
- test_must_fail git cherry-pick base..anotherpick &&
- echo "c" >foo &&
- git add foo &&
- git commit &&
- git revert --continue &&
- test_path_is_missing .git/sequencer &&
- {
- git rev-list HEAD |
- git diff-tree --root --stdin |
- sed "s/$_x40/OBJID/g"
- } >actual &&
- cat >expect <<-\EOF &&
- OBJID
- :100644 100644 OBJID OBJID M foo
- OBJID
- :100644 100644 OBJID OBJID M foo
- OBJID
- :100644 100644 OBJID OBJID M unrelated
- OBJID
- :000000 100644 OBJID OBJID A foo
- :000000 100644 OBJID OBJID A unrelated
- EOF
- test_cmp expect actual
-'
-
-test_expect_success 'mixed pick and revert instructions' '
- pristine_detach initial &&
- test_must_fail git cherry-pick base..anotherpick &&
- echo "c" >foo &&
- git add foo &&
- git commit &&
- oldsha=`git rev-parse --short HEAD~1` &&
- echo "revert $oldsha unrelatedpick" >>.git/sequencer/todo &&
- git cherry-pick --continue &&
- test_path_is_missing .git/sequencer &&
- {
- git rev-list HEAD |
- git diff-tree --root --stdin |
- sed "s/$_x40/OBJID/g"
- } >actual &&
- cat >expect <<-\EOF &&
- OBJID
- :100644 100644 OBJID OBJID M unrelated
- OBJID
- :100644 100644 OBJID OBJID M foo
- OBJID
- :100644 100644 OBJID OBJID M foo
- OBJID
- :100644 100644 OBJID OBJID M unrelated
- OBJID
- :000000 100644 OBJID OBJID A foo
- :000000 100644 OBJID OBJID A unrelated
- EOF
- test_cmp expect actual
-'
-
test_done
--
1.7.6.351.gb35ac.dirty
^ permalink raw reply related
* [PATCH 4/7] sequencer: handle cherry-pick conflict in last commit
From: Ramkumar Ramachandra @ 2011-11-13 10:46 UTC (permalink / raw)
To: Git List; +Cc: Junio C Hamano, Jonathan Nieder, Christian Couder
In-Reply-To: <1321181181-23923-1-git-send-email-artagnon@gmail.com>
The previous two commits in the series implement special handling for
the single-commit cherry-pick case. Although we can technically
revert the changes made by d3f4628e (revert: Remove sequencer state
when no commits are pending, 2011-06-06) without breaking any existing
tests now, there is one pending corner case: when a cherry-pick is
invoked on a commit range, and a conflict that occurs in the last
commit is concluded with a 'git commit'. Without d3f4628e, we'd have
the following unpleasant case:
$ git cherry-pick foo..bar
... .git/sequencer is created ...
... .git/CHERRY_PICK_HEAD is created ...
... conflict in bar ...
$ echo "resolved" >problematicfile
$ git add problematicfile
$ git commit
... .git/CHERRY_PICK_HEAD is removed ...
$ git cherry-pick moo
error: An existing cherry-pick or revert is in progress
$ git cherry-pick --continue
... .git/sequencer is removed ...
# We're in pristine shape now
While prematurely removing the entire sequencer state is an overkill,
we can revise our plan: prematurely remove only '.git/sequencer/todo'
in the 'REPLAY_PICK' case, because this is the exact case where the
information in '.git/sequencer/todo' can be inferred from
'.git/CHERRY_PICK_HEAD'. This will be compatible with our future plan
to implement '--continue' and '--reset' consistently.
Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
---
sequencer.c | 8 +++-----
t/t3510-cherry-pick-sequence.sh | 4 ++--
2 files changed, 5 insertions(+), 7 deletions(-)
diff --git a/sequencer.c b/sequencer.c
index b35fcc7..23fd3fe 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -753,15 +753,13 @@ static int pick_commits(struct replay_insn_list *todo_list,
save_todo(cur);
res = do_pick_commit(cur->operand, cur->action, opts);
if (res) {
- if (!cur->next)
+ if (!cur->next && opts->action == REPLAY_PICK)
/*
* An error was encountered while
* picking the last commit; the
- * sequencer state is useless now --
- * the user simply needs to resolve
- * the conflict and commit
+ * sequencer todo is useless now.
*/
- remove_sequencer_state(0);
+ unlink(git_path(SEQ_TODO_FILE));
return res;
}
}
diff --git a/t/t3510-cherry-pick-sequence.sh b/t/t3510-cherry-pick-sequence.sh
index 4b12244..09b9e65 100755
--- a/t/t3510-cherry-pick-sequence.sh
+++ b/t/t3510-cherry-pick-sequence.sh
@@ -85,10 +85,10 @@ test_expect_success '--reset cleans up sequencer state' '
test_path_is_missing .git/sequencer
'
-test_expect_success 'cherry-pick cleans up sequencer state when one commit is left' '
+test_expect_success 'cherry-pick cleans up sequencer todo when one commit is left' '
pristine_detach initial &&
test_must_fail git cherry-pick base..picked &&
- test_path_is_missing .git/sequencer &&
+ test_path_is_missing .git/sequencer/todo &&
echo "resolved" >foo &&
git add foo &&
git commit &&
--
1.7.6.351.gb35ac.dirty
^ permalink raw reply related
* [PATCH 3/7] sequencer: handle single-commit pick as special case
From: Ramkumar Ramachandra @ 2011-11-13 10:46 UTC (permalink / raw)
To: Git List; +Cc: Junio C Hamano, Jonathan Nieder, Christian Couder
In-Reply-To: <1321181181-23923-1-git-send-email-artagnon@gmail.com>
Prior to v1.7.2-rc1~4^2~7 (revert: allow cherry-picking more than one
commit, 2010-06-02), 'git cherry-pick' could only pick one commit at a
time, and it used '.git/CHERRY_PICK_HEAD' to pass on information to a
subsequent invocation in case of a conflict. While
'.git/CHERRY_PICK_HEAD' can only keep information about one commit,
the sequencer uses '.git/sequencer' to persist information in the
general case.
A problem arises because a single-commit cherry-pick operation can be
completed successfully using 'git commit'. This removes
'.git/CHERRY_PICK_HEAD' without informing the sequencer, leaving
behind a stale sequencer state as a result. We have worked around
this problem already by prematurely removing the sequencer state in
d3f4628e (revert: Remove sequencer state when no commits are pending,
2011-06-06). However, this gets in the way of our future plan to
eliminate a glaring workflow inconsistency:
$ git cherry-pick foo
... .git/sequencer is created ....
... .git/CHERRY_PICK_HEAD is created ...
... conflict ...
.... .git/sequencer is prematurely removed ...
$ echo "resolved" >problematicfile
$ git add problematicfile
$ git commit
... .git/CHERRY_PICK_HEAD is removed ...
$ git cherry-pick --continue
error: No cherry-pick in progress
$ git cherry-pick foo..bar
... .git/sequencer is created ....
... CHERRY_PICK_HEAD is created ...
... conflict in bar~1 ...
$ echo "resolved" >problematicfile
$ git add problematicfile
$ git commit
... CHERRY_PICK_HEAD is removed ...
$ git cherry-pick --continue # Success!
To eliminate this inconsistency, we have decided to make '--continue'
continue any general sequencer operation bypassing 'git commit'
completely (although preserving the existing workflow for backward
compatibility). For '--continue' and '--reset' to work uniformly,
they must use the information in:
1. '.git/sequencer/head', '.git/sequencer/opts', '.git/sequencer/todo'
in the general case.
2. '.git/sequencer/head', '.git/sequencer/opts', '.git/CHERRY_PICK_HEAD'
in case of a single-commit cherry-pick.
As a start, handle cherry-picking a single commit as a special case by
not creating '.git/sequencer/todo' in the first place. This will
eliminate the need for prematurely removing it in d3f4628e.
Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
---
sequencer.c | 8 ++++++++
1 files changed, 8 insertions(+), 0 deletions(-)
diff --git a/sequencer.c b/sequencer.c
index 8b2518c..b35fcc7 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -741,6 +741,14 @@ static int pick_commits(struct replay_insn_list *todo_list,
opts->record_origin || opts->edit));
read_and_refresh_cache(opts);
+ /*
+ * Backward compatibility hack: handle single-commit pick as a
+ * special case.
+ */
+ if (opts->subcommand == REPLAY_NONE &&
+ todo_list->next == NULL && todo_list->action == REPLAY_PICK)
+ return do_pick_commit(todo_list->operand, REPLAY_PICK, opts);
+
for (cur = todo_list; cur; cur = cur->next) {
save_todo(cur);
res = do_pick_commit(cur->operand, cur->action, opts);
--
1.7.6.351.gb35ac.dirty
^ permalink raw reply related
* [PATCH 2/7] sequencer: invalidate sequencer state without todo
From: Ramkumar Ramachandra @ 2011-11-13 10:46 UTC (permalink / raw)
To: Git List; +Cc: Junio C Hamano, Jonathan Nieder, Christian Couder
In-Reply-To: <1321181181-23923-1-git-send-email-artagnon@gmail.com>
To check whether an existing sequencer operation is in progress and
error out, we currently check for the existence of the
'.git/sequencer' directory.
$ git cherry-pick foo..bar
... conflict in bar~1 ...
... .git/sequencer is created ...
$ echo "resolved" >problematicfile
$ git add problematicfile
$ git commit # Success!
$ git cherry-pick moo # .git/sequencer exists
error: A cherry-pick or revert is in progress
Although the sequencer state is useless without '.git/sequencer/todo',
this case never occurs. However, in the light of the next patch,
where will handle single-commit picks as a special case by not
persisting '.git/sequencer/todo' in the first place, we are forced to
reconsider. So, when starting a fresh sequencer invocation, remove
the sequencer state carried over from a previous invocation if it's
missing '.git/sequencer/todo'.
Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
---
sequencer.c | 10 +++++++---
1 files changed, 7 insertions(+), 3 deletions(-)
diff --git a/sequencer.c b/sequencer.c
index 87f146b..8b2518c 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -649,11 +649,15 @@ static void walk_revs_populate_todo(struct replay_insn_list **todo_list,
static int create_seq_dir(void)
{
+ const char *todo_file = git_path(SEQ_TODO_FILE);
const char *seq_dir = git_path(SEQ_DIR);
- if (file_exists(seq_dir))
- return error(_("%s already exists."), seq_dir);
- else if (mkdir(seq_dir, 0777) < 0)
+ if (file_exists(todo_file))
+ return error(_("%s already exists."), todo_file);
+
+ /* If todo_file doesn't exist, discard sequencer state */
+ remove_sequencer_state(1);
+ if (mkdir(seq_dir, 0777) < 0)
die_errno(_("Could not create sequencer directory '%s'."), seq_dir);
return 0;
}
--
1.7.6.351.gb35ac.dirty
^ permalink raw reply related
* [PATCH 1/7] sequencer: factor code out of revert builtin
From: Ramkumar Ramachandra @ 2011-11-13 10:46 UTC (permalink / raw)
To: Git List; +Cc: Junio C Hamano, Jonathan Nieder, Christian Couder
In-Reply-To: <1321181181-23923-1-git-send-email-artagnon@gmail.com>
To build a generalized sequencer of which cherry-picking and reverting
are special cases, we must first expose the cherry-picking machinery
through a public API. Move code from revert.c into sequencer.c so as
to expose pick_revisions() as sequencer_pick_revisions() in
sequencer.h. Consequently, make the cherry-pick builtin a thin
wrapper around sequencer_pick_revisions() that additionally does
command-line argument parsing. In the future, we can write a new
"foo" builtin that calls into the sequencer like:
memset(&opts, 0, sizeof(opts));
opts.action = REPLAY_FOO;
opts.revisions = xmalloc(sizeof(*opts.revs));
parse_args_populate_opts(argc, argv, &opts);
init_revisions(opts.revs);
sequencer_pick_revisions(&opts);
This is intended to be almost a pure code movement patch with no
functional changes. Check with:
$ git blame -s -CCC HEAD^..HEAD -- sequencer.c | grep -C3 '^[^^]'
Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
---
builtin/revert.c | 821 +-----------------------------------------------------
sequencer.c | 802 ++++++++++++++++++++++++++++++++++++++++++++++++++++-
sequencer.h | 26 ++
3 files changed, 828 insertions(+), 821 deletions(-)
diff --git a/builtin/revert.c b/builtin/revert.c
index df9459b..c272920 100644
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -1,19 +1,9 @@
#include "cache.h"
#include "builtin.h"
-#include "object.h"
-#include "commit.h"
-#include "tag.h"
-#include "run-command.h"
-#include "exec_cmd.h"
-#include "utf8.h"
#include "parse-options.h"
-#include "cache-tree.h"
#include "diff.h"
#include "revision.h"
#include "rerere.h"
-#include "merge-recursive.h"
-#include "refs.h"
-#include "dir.h"
#include "sequencer.h"
/*
@@ -39,40 +29,11 @@ static const char * const cherry_pick_usage[] = {
NULL
};
-enum replay_subcommand { REPLAY_NONE, REPLAY_RESET, REPLAY_CONTINUE };
-
-struct replay_opts {
- enum replay_action action;
- enum replay_subcommand subcommand;
-
- /* Boolean options */
- int edit;
- int record_origin;
- int no_commit;
- int signoff;
- int allow_ff;
- int allow_rerere_auto;
-
- int mainline;
-
- /* Merge strategy */
- const char *strategy;
- const char **xopts;
- size_t xopts_nr, xopts_alloc;
-
- /* Only used by REPLAY_NONE */
- struct rev_info *revs;
-};
-
-#define GIT_REFLOG_ACTION "GIT_REFLOG_ACTION"
-
static const char *action_name(const struct replay_opts *opts)
{
return opts->action == REPLAY_REVERT ? "revert" : "cherry-pick";
}
-static char *get_encoding(const char *message);
-
static const char * const *revert_or_cherry_pick_usage(struct replay_opts *opts)
{
return opts->action == REPLAY_REVERT ? revert_usage : cherry_pick_usage;
@@ -222,784 +183,6 @@ static void parse_args(int argc, const char **argv, struct replay_opts *opts)
usage_with_options(usage_str, options);
}
-struct commit_message {
- char *parent_label;
- const char *label;
- const char *subject;
- char *reencoded_message;
- const char *message;
-};
-
-static int get_message(struct commit *commit, struct commit_message *out)
-{
- const char *encoding;
- const char *abbrev, *subject;
- int abbrev_len, subject_len;
- char *q;
-
- if (!commit->buffer)
- return -1;
- encoding = get_encoding(commit->buffer);
- if (!encoding)
- encoding = "UTF-8";
- if (!git_commit_encoding)
- git_commit_encoding = "UTF-8";
-
- out->reencoded_message = NULL;
- out->message = commit->buffer;
- if (strcmp(encoding, git_commit_encoding))
- out->reencoded_message = reencode_string(commit->buffer,
- git_commit_encoding, encoding);
- if (out->reencoded_message)
- out->message = out->reencoded_message;
-
- abbrev = find_unique_abbrev(commit->object.sha1, DEFAULT_ABBREV);
- abbrev_len = strlen(abbrev);
-
- subject_len = find_commit_subject(out->message, &subject);
-
- out->parent_label = xmalloc(strlen("parent of ") + abbrev_len +
- strlen("... ") + subject_len + 1);
- q = out->parent_label;
- q = mempcpy(q, "parent of ", strlen("parent of "));
- out->label = q;
- q = mempcpy(q, abbrev, abbrev_len);
- q = mempcpy(q, "... ", strlen("... "));
- out->subject = q;
- q = mempcpy(q, subject, subject_len);
- *q = '\0';
- return 0;
-}
-
-static void free_message(struct commit_message *msg)
-{
- free(msg->parent_label);
- free(msg->reencoded_message);
-}
-
-static char *get_encoding(const char *message)
-{
- const char *p = message, *eol;
-
- while (*p && *p != '\n') {
- for (eol = p + 1; *eol && *eol != '\n'; eol++)
- ; /* do nothing */
- if (!prefixcmp(p, "encoding ")) {
- char *result = xmalloc(eol - 8 - p);
- strlcpy(result, p + 9, eol - 8 - p);
- return result;
- }
- p = eol;
- if (*p == '\n')
- p++;
- }
- return NULL;
-}
-
-static void write_cherry_pick_head(struct commit *commit)
-{
- int fd;
- struct strbuf buf = STRBUF_INIT;
-
- strbuf_addf(&buf, "%s\n", sha1_to_hex(commit->object.sha1));
-
- fd = open(git_path("CHERRY_PICK_HEAD"), O_WRONLY | O_CREAT, 0666);
- if (fd < 0)
- die_errno(_("Could not open '%s' for writing"),
- git_path("CHERRY_PICK_HEAD"));
- if (write_in_full(fd, buf.buf, buf.len) != buf.len || close(fd))
- die_errno(_("Could not write to '%s'"), git_path("CHERRY_PICK_HEAD"));
- strbuf_release(&buf);
-}
-
-static void print_advice(int show_hint)
-{
- char *msg = getenv("GIT_CHERRY_PICK_HELP");
-
- if (msg) {
- fprintf(stderr, "%s\n", msg);
- /*
- * A conflict has occured but the porcelain
- * (typically rebase --interactive) wants to take care
- * of the commit itself so remove CHERRY_PICK_HEAD
- */
- unlink(git_path("CHERRY_PICK_HEAD"));
- return;
- }
-
- if (show_hint) {
- advise("after resolving the conflicts, mark the corrected paths");
- advise("with 'git add <paths>' or 'git rm <paths>'");
- advise("and commit the result with 'git commit'");
- }
-}
-
-static void write_message(struct strbuf *msgbuf, const char *filename)
-{
- static struct lock_file msg_file;
-
- int msg_fd = hold_lock_file_for_update(&msg_file, filename,
- LOCK_DIE_ON_ERROR);
- if (write_in_full(msg_fd, msgbuf->buf, msgbuf->len) < 0)
- die_errno(_("Could not write to %s."), filename);
- strbuf_release(msgbuf);
- if (commit_lock_file(&msg_file) < 0)
- die(_("Error wrapping up %s"), filename);
-}
-
-static struct tree *empty_tree(void)
-{
- return lookup_tree((const unsigned char *)EMPTY_TREE_SHA1_BIN);
-}
-
-static int error_dirty_index(struct replay_opts *opts)
-{
- if (read_cache_unmerged())
- return error_resolve_conflict(action_name(opts));
-
- /* Different translation strings for cherry-pick and revert */
- if (opts->action == REPLAY_PICK)
- error(_("Your local changes would be overwritten by cherry-pick."));
- else
- error(_("Your local changes would be overwritten by revert."));
-
- if (advice_commit_before_merge)
- advise(_("Commit your changes or stash them to proceed."));
- return -1;
-}
-
-static int fast_forward_to(const unsigned char *to, const unsigned char *from)
-{
- struct ref_lock *ref_lock;
-
- read_cache();
- if (checkout_fast_forward(from, to))
- exit(1); /* the callee should have complained already */
- ref_lock = lock_any_ref_for_update("HEAD", from, 0);
- return write_ref_sha1(ref_lock, to, "cherry-pick");
-}
-
-static int do_recursive_merge(struct commit *base, struct commit *next,
- const char *base_label, const char *next_label,
- unsigned char *head, struct strbuf *msgbuf,
- struct replay_opts *opts)
-{
- struct merge_options o;
- struct tree *result, *next_tree, *base_tree, *head_tree;
- int clean, index_fd;
- const char **xopt;
- static struct lock_file index_lock;
-
- index_fd = hold_locked_index(&index_lock, 1);
-
- read_cache();
-
- init_merge_options(&o);
- o.ancestor = base ? base_label : "(empty tree)";
- o.branch1 = "HEAD";
- o.branch2 = next ? next_label : "(empty tree)";
-
- head_tree = parse_tree_indirect(head);
- next_tree = next ? next->tree : empty_tree();
- base_tree = base ? base->tree : empty_tree();
-
- for (xopt = opts->xopts; xopt != opts->xopts + opts->xopts_nr; xopt++)
- parse_merge_opt(&o, *xopt);
-
- clean = merge_trees(&o,
- head_tree,
- next_tree, base_tree, &result);
-
- if (active_cache_changed &&
- (write_cache(index_fd, active_cache, active_nr) ||
- commit_locked_index(&index_lock)))
- /* TRANSLATORS: %s will be "revert" or "cherry-pick" */
- die(_("%s: Unable to write new index file"), action_name(opts));
- rollback_lock_file(&index_lock);
-
- if (!clean) {
- int i;
- strbuf_addstr(msgbuf, "\nConflicts:\n\n");
- for (i = 0; i < active_nr;) {
- struct cache_entry *ce = active_cache[i++];
- if (ce_stage(ce)) {
- strbuf_addch(msgbuf, '\t');
- strbuf_addstr(msgbuf, ce->name);
- strbuf_addch(msgbuf, '\n');
- while (i < active_nr && !strcmp(ce->name,
- active_cache[i]->name))
- i++;
- }
- }
- }
-
- return !clean;
-}
-
-/*
- * If we are cherry-pick, and if the merge did not result in
- * hand-editing, we will hit this commit and inherit the original
- * author date and name.
- * If we are revert, or if our cherry-pick results in a hand merge,
- * we had better say that the current user is responsible for that.
- */
-static int run_git_commit(const char *defmsg, struct replay_opts *opts)
-{
- /* 6 is max possible length of our args array including NULL */
- const char *args[6];
- int i = 0;
-
- args[i++] = "commit";
- args[i++] = "-n";
- if (opts->signoff)
- args[i++] = "-s";
- if (!opts->edit) {
- args[i++] = "-F";
- args[i++] = defmsg;
- }
- args[i] = NULL;
-
- return run_command_v_opt(args, RUN_GIT_CMD);
-}
-
-static int do_pick_commit(struct commit *commit, enum replay_action action,
- struct replay_opts *opts)
-{
- unsigned char head[20];
- struct commit *base, *next, *parent;
- const char *base_label, *next_label;
- struct commit_message msg = { NULL, NULL, NULL, NULL, NULL };
- char *defmsg = NULL;
- struct strbuf msgbuf = STRBUF_INIT;
- int res;
-
- if (opts->no_commit) {
- /*
- * We do not intend to commit immediately. We just want to
- * merge the differences in, so let's compute the tree
- * that represents the "current" state for merge-recursive
- * to work on.
- */
- if (write_cache_as_tree(head, 0, NULL))
- die (_("Your index file is unmerged."));
- } else {
- if (get_sha1("HEAD", head))
- return error(_("You do not have a valid HEAD"));
- if (index_differs_from("HEAD", 0))
- return error_dirty_index(opts);
- }
- discard_cache();
-
- if (!commit->parents) {
- parent = NULL;
- }
- else if (commit->parents->next) {
- /* Reverting or cherry-picking a merge commit */
- int cnt;
- struct commit_list *p;
-
- if (!opts->mainline)
- return error(_("Commit %s is a merge but no -m option was given."),
- sha1_to_hex(commit->object.sha1));
-
- for (cnt = 1, p = commit->parents;
- cnt != opts->mainline && p;
- cnt++)
- p = p->next;
- if (cnt != opts->mainline || !p)
- return error(_("Commit %s does not have parent %d"),
- sha1_to_hex(commit->object.sha1), opts->mainline);
- parent = p->item;
- } else if (0 < opts->mainline)
- return error(_("Mainline was specified but commit %s is not a merge."),
- sha1_to_hex(commit->object.sha1));
- else
- parent = commit->parents->item;
-
- if (opts->allow_ff && parent && !hashcmp(parent->object.sha1, head))
- return fast_forward_to(commit->object.sha1, head);
-
- if (parent && parse_commit(parent) < 0)
- /* TRANSLATORS: The first %s will be "revert" or
- "cherry-pick", the second %s a SHA1 */
- return error(_("%s: cannot parse parent commit %s"),
- action_name(opts), sha1_to_hex(parent->object.sha1));
-
- if (get_message(commit, &msg) != 0)
- return error(_("Cannot get commit message for %s"),
- sha1_to_hex(commit->object.sha1));
-
- /*
- * "commit" is an existing commit. We would want to apply
- * the difference it introduces since its first parent "prev"
- * on top of the current HEAD if we are cherry-pick. Or the
- * reverse of it if we are revert.
- */
-
- defmsg = git_pathdup("MERGE_MSG");
-
- if (action == REPLAY_REVERT) {
- base = commit;
- base_label = msg.label;
- next = parent;
- next_label = msg.parent_label;
- strbuf_addstr(&msgbuf, "Revert \"");
- strbuf_addstr(&msgbuf, msg.subject);
- strbuf_addstr(&msgbuf, "\"\n\nThis reverts commit ");
- strbuf_addstr(&msgbuf, sha1_to_hex(commit->object.sha1));
-
- if (commit->parents && commit->parents->next) {
- strbuf_addstr(&msgbuf, ", reversing\nchanges made to ");
- strbuf_addstr(&msgbuf, sha1_to_hex(parent->object.sha1));
- }
- strbuf_addstr(&msgbuf, ".\n");
- } else {
- const char *p;
-
- base = parent;
- base_label = msg.parent_label;
- next = commit;
- next_label = msg.label;
-
- /*
- * Append the commit log message to msgbuf; it starts
- * after the tree, parent, author, committer
- * information followed by "\n\n".
- */
- p = strstr(msg.message, "\n\n");
- if (p) {
- p += 2;
- strbuf_addstr(&msgbuf, p);
- }
-
- if (opts->record_origin) {
- strbuf_addstr(&msgbuf, "(cherry picked from commit ");
- strbuf_addstr(&msgbuf, sha1_to_hex(commit->object.sha1));
- strbuf_addstr(&msgbuf, ")\n");
- }
- }
-
- if (!opts->strategy || !strcmp(opts->strategy, "recursive") || action == REPLAY_REVERT) {
- res = do_recursive_merge(base, next, base_label, next_label,
- head, &msgbuf, opts);
- write_message(&msgbuf, defmsg);
- } else {
- struct commit_list *common = NULL;
- struct commit_list *remotes = NULL;
-
- write_message(&msgbuf, defmsg);
-
- commit_list_insert(base, &common);
- commit_list_insert(next, &remotes);
- res = try_merge_command(opts->strategy, opts->xopts_nr, opts->xopts,
- common, sha1_to_hex(head), remotes);
- free_commit_list(common);
- free_commit_list(remotes);
- }
-
- /*
- * If the merge was clean or if it failed due to conflict, we write
- * CHERRY_PICK_HEAD for the subsequent invocation of commit to use.
- * However, if the merge did not even start, then we don't want to
- * write it at all.
- */
- if (opts->action == REPLAY_PICK && !opts->no_commit && (res == 0 || res == 1))
- write_cherry_pick_head(commit);
-
- if (res) {
- error(action == REPLAY_REVERT
- ? _("could not revert %s... %s")
- : _("could not apply %s... %s"),
- find_unique_abbrev(commit->object.sha1, DEFAULT_ABBREV),
- msg.subject);
- print_advice(res == 1);
- rerere(opts->allow_rerere_auto);
- } else {
- if (!opts->no_commit)
- res = run_git_commit(defmsg, opts);
- }
-
- free_message(&msg);
- free(defmsg);
-
- return res;
-}
-
-static void prepare_revs(struct replay_opts *opts)
-{
- if (opts->action != REPLAY_REVERT)
- opts->revs->reverse ^= 1;
-
- if (prepare_revision_walk(opts->revs))
- die(_("revision walk setup failed"));
-
- if (!opts->revs->commits)
- die(_("empty commit set passed"));
-}
-
-static void read_and_refresh_cache(struct replay_opts *opts)
-{
- static struct lock_file index_lock;
- int index_fd = hold_locked_index(&index_lock, 0);
- if (read_index_preload(&the_index, NULL) < 0)
- die(_("git %s: failed to read the index"), action_name(opts));
- refresh_index(&the_index, REFRESH_QUIET|REFRESH_UNMERGED, NULL, NULL, NULL);
- if (the_index.cache_changed) {
- if (write_index(&the_index, index_fd) ||
- commit_locked_index(&index_lock))
- die(_("git %s: failed to refresh the index"), action_name(opts));
- }
- rollback_lock_file(&index_lock);
-}
-
-/*
- * Append a commit to the end of the commit_list.
- *
- * next starts by pointing to the variable that holds the head of an
- * empty commit_list, and is updated to point to the "next" field of
- * the last item on the list as new commits are appended.
- *
- * Usage example:
- *
- * struct commit_list *list;
- * struct commit_list **next = &list;
- *
- * next = commit_list_append(c1, next);
- * next = commit_list_append(c2, next);
- * assert(commit_list_count(list) == 2);
- * return list;
- */
-static struct replay_insn_list **replay_insn_list_append(enum replay_action action,
- struct commit *operand,
- struct replay_insn_list **next)
-{
- struct replay_insn_list *new = xmalloc(sizeof(*new));
- new->action = action;
- new->operand = operand;
- *next = new;
- new->next = NULL;
- return &new->next;
-}
-
-static int format_todo(struct strbuf *buf, struct replay_insn_list *todo_list)
-{
- struct replay_insn_list *cur;
-
- for (cur = todo_list; cur; cur = cur->next) {
- const char *sha1_abbrev, *action_str, *subject;
- int subject_len;
-
- action_str = cur->action == REPLAY_REVERT ? "revert" : "pick";
- sha1_abbrev = find_unique_abbrev(cur->operand->object.sha1, DEFAULT_ABBREV);
- subject_len = find_commit_subject(cur->operand->buffer, &subject);
- strbuf_addf(buf, "%s %s %.*s\n", action_str, sha1_abbrev,
- subject_len, subject);
- }
- return 0;
-}
-
-static int parse_insn_line(char *bol, char *eol, struct replay_insn_list *item)
-{
- unsigned char commit_sha1[20];
- char *end_of_object_name;
- int saved, status;
-
- if (!prefixcmp(bol, "pick ")) {
- item->action = REPLAY_PICK;
- bol += strlen("pick ");
- } else if (!prefixcmp(bol, "revert ")) {
- item->action = REPLAY_REVERT;
- bol += strlen("revert ");
- } else {
- size_t len = strchrnul(bol, '\n') - bol;
- if (len > 255)
- len = 255;
- return error(_("Unrecognized action: %.*s"), (int)len, bol);
- }
-
- end_of_object_name = bol + strcspn(bol, " \n");
- saved = *end_of_object_name;
- *end_of_object_name = '\0';
- status = get_sha1(bol, commit_sha1);
- *end_of_object_name = saved;
-
- if (status < 0)
- return error(_("Malformed object name: %s"), bol);
-
- item->operand = lookup_commit_reference(commit_sha1);
- if (!item->operand)
- return error(_("Not a valid commit: %s"), bol);
-
- item->next = NULL;
- return 0;
-}
-
-static int parse_insn_buffer(char *buf, struct replay_insn_list **todo_list)
-{
- struct replay_insn_list **next = todo_list;
- struct replay_insn_list item = {0, NULL, NULL};
- char *p = buf;
- int i;
-
- for (i = 1; *p; i++) {
- char *eol = strchrnul(p, '\n');
- if (parse_insn_line(p, eol, &item) < 0)
- return error(_("on line %d."), i);
- next = replay_insn_list_append(item.action, item.operand, next);
- p = *eol ? eol + 1 : eol;
- }
- if (!*todo_list)
- return error(_("No commits parsed."));
- return 0;
-}
-
-static void read_populate_todo(struct replay_insn_list **todo_list)
-{
- const char *todo_file = git_path(SEQ_TODO_FILE);
- struct strbuf buf = STRBUF_INIT;
- int fd, res;
-
- fd = open(todo_file, O_RDONLY);
- if (fd < 0)
- die_errno(_("Could not open %s."), todo_file);
- if (strbuf_read(&buf, fd, 0) < 0) {
- close(fd);
- strbuf_release(&buf);
- die(_("Could not read %s."), todo_file);
- }
- close(fd);
-
- res = parse_insn_buffer(buf.buf, todo_list);
- strbuf_release(&buf);
- if (res)
- die(_("Unusable instruction sheet: %s"), todo_file);
-}
-
-static int populate_opts_cb(const char *key, const char *value, void *data)
-{
- struct replay_opts *opts = data;
- int error_flag = 1;
-
- if (!value)
- error_flag = 0;
- else if (!strcmp(key, "options.no-commit"))
- opts->no_commit = git_config_bool_or_int(key, value, &error_flag);
- else if (!strcmp(key, "options.edit"))
- opts->edit = git_config_bool_or_int(key, value, &error_flag);
- else if (!strcmp(key, "options.signoff"))
- opts->signoff = git_config_bool_or_int(key, value, &error_flag);
- else if (!strcmp(key, "options.record-origin"))
- opts->record_origin = git_config_bool_or_int(key, value, &error_flag);
- else if (!strcmp(key, "options.allow-ff"))
- opts->allow_ff = git_config_bool_or_int(key, value, &error_flag);
- else if (!strcmp(key, "options.mainline"))
- opts->mainline = git_config_int(key, value);
- else if (!strcmp(key, "options.strategy"))
- git_config_string(&opts->strategy, key, value);
- else if (!strcmp(key, "options.strategy-option")) {
- ALLOC_GROW(opts->xopts, opts->xopts_nr + 1, opts->xopts_alloc);
- opts->xopts[opts->xopts_nr++] = xstrdup(value);
- } else
- return error(_("Invalid key: %s"), key);
-
- if (!error_flag)
- return error(_("Invalid value for %s: %s"), key, value);
-
- return 0;
-}
-
-static void read_populate_opts(struct replay_opts **opts_ptr)
-{
- const char *opts_file = git_path(SEQ_OPTS_FILE);
-
- if (!file_exists(opts_file))
- return;
- if (git_config_from_file(populate_opts_cb, opts_file, *opts_ptr) < 0)
- die(_("Malformed options sheet: %s"), opts_file);
-}
-
-static void walk_revs_populate_todo(struct replay_insn_list **todo_list,
- struct replay_opts *opts)
-{
- struct commit *commit;
- struct replay_insn_list **next;
-
- prepare_revs(opts);
-
- next = todo_list;
- while ((commit = get_revision(opts->revs)))
- next = replay_insn_list_append(opts->action, commit, next);
-}
-
-static int create_seq_dir(void)
-{
- const char *seq_dir = git_path(SEQ_DIR);
-
- if (file_exists(seq_dir))
- return error(_("%s already exists."), seq_dir);
- else if (mkdir(seq_dir, 0777) < 0)
- die_errno(_("Could not create sequencer directory '%s'."), seq_dir);
- return 0;
-}
-
-static void save_head(const char *head)
-{
- const char *head_file = git_path(SEQ_HEAD_FILE);
- static struct lock_file head_lock;
- struct strbuf buf = STRBUF_INIT;
- int fd;
-
- fd = hold_lock_file_for_update(&head_lock, head_file, LOCK_DIE_ON_ERROR);
- strbuf_addf(&buf, "%s\n", head);
- if (write_in_full(fd, buf.buf, buf.len) < 0)
- die_errno(_("Could not write to %s."), head_file);
- if (commit_lock_file(&head_lock) < 0)
- die(_("Error wrapping up %s."), head_file);
-}
-
-static void save_todo(struct replay_insn_list *todo_list)
-{
- const char *todo_file = git_path(SEQ_TODO_FILE);
- static struct lock_file todo_lock;
- struct strbuf buf = STRBUF_INIT;
- int fd;
-
- fd = hold_lock_file_for_update(&todo_lock, todo_file, LOCK_DIE_ON_ERROR);
- if (format_todo(&buf, todo_list) < 0)
- die(_("Could not format %s."), todo_file);
- if (write_in_full(fd, buf.buf, buf.len) < 0) {
- strbuf_release(&buf);
- die_errno(_("Could not write to %s."), todo_file);
- }
- if (commit_lock_file(&todo_lock) < 0) {
- strbuf_release(&buf);
- die(_("Error wrapping up %s."), todo_file);
- }
- strbuf_release(&buf);
-}
-
-static void save_opts(struct replay_opts *opts)
-{
- const char *opts_file = git_path(SEQ_OPTS_FILE);
-
- if (opts->no_commit)
- git_config_set_in_file(opts_file, "options.no-commit", "true");
- if (opts->edit)
- git_config_set_in_file(opts_file, "options.edit", "true");
- if (opts->signoff)
- git_config_set_in_file(opts_file, "options.signoff", "true");
- if (opts->record_origin)
- git_config_set_in_file(opts_file, "options.record-origin", "true");
- if (opts->allow_ff)
- git_config_set_in_file(opts_file, "options.allow-ff", "true");
- if (opts->mainline) {
- struct strbuf buf = STRBUF_INIT;
- strbuf_addf(&buf, "%d", opts->mainline);
- git_config_set_in_file(opts_file, "options.mainline", buf.buf);
- strbuf_release(&buf);
- }
- if (opts->strategy)
- git_config_set_in_file(opts_file, "options.strategy", opts->strategy);
- if (opts->xopts) {
- int i;
- for (i = 0; i < opts->xopts_nr; i++)
- git_config_set_multivar_in_file(opts_file,
- "options.strategy-option",
- opts->xopts[i], "^$", 0);
- }
-}
-
-static int pick_commits(struct replay_insn_list *todo_list,
- struct replay_opts *opts)
-{
- struct replay_insn_list *cur;
- int res;
-
- setenv(GIT_REFLOG_ACTION, action_name(opts), 0);
- if (opts->allow_ff)
- assert(!(opts->signoff || opts->no_commit ||
- opts->record_origin || opts->edit));
- read_and_refresh_cache(opts);
-
- for (cur = todo_list; cur; cur = cur->next) {
- save_todo(cur);
- res = do_pick_commit(cur->operand, cur->action, opts);
- if (res) {
- if (!cur->next)
- /*
- * An error was encountered while
- * picking the last commit; the
- * sequencer state is useless now --
- * the user simply needs to resolve
- * the conflict and commit
- */
- remove_sequencer_state(0);
- return res;
- }
- }
-
- /*
- * Sequence of picks finished successfully; cleanup by
- * removing the .git/sequencer directory
- */
- remove_sequencer_state(1);
- return 0;
-}
-
-static int pick_revisions(struct replay_opts *opts)
-{
- struct replay_insn_list *todo_list = NULL;
- unsigned char sha1[20];
-
- if (opts->subcommand == REPLAY_NONE)
- assert(opts->revs);
-
- read_and_refresh_cache(opts);
-
- /*
- * Decide what to do depending on the arguments; a fresh
- * cherry-pick should be handled differently from an existing
- * one that is being continued
- */
- if (opts->subcommand == REPLAY_RESET) {
- remove_sequencer_state(1);
- return 0;
- } else if (opts->subcommand == REPLAY_CONTINUE) {
- if (!file_exists(git_path(SEQ_TODO_FILE)))
- goto error;
- read_populate_opts(&opts);
- read_populate_todo(&todo_list);
-
- /* Verify that the conflict has been resolved */
- if (!index_differs_from("HEAD", 0))
- todo_list = todo_list->next;
- } else {
- /*
- * Start a new cherry-pick/ revert sequence; but
- * first, make sure that an existing one isn't in
- * progress
- */
-
- walk_revs_populate_todo(&todo_list, opts);
- if (create_seq_dir() < 0) {
- error(_("A cherry-pick or revert is in progress."));
- advise(_("Use --continue to continue the operation"));
- advise(_("or --reset to forget about it"));
- return -1;
- }
- if (get_sha1("HEAD", sha1)) {
- if (opts->action == REPLAY_REVERT)
- return error(_("Can't revert as initial commit"));
- return error(_("Can't cherry-pick into empty head"));
- }
- save_head(sha1_to_hex(sha1));
- save_opts(opts);
- }
- return pick_commits(todo_list, opts);
-error:
- return error(_("No %s in progress"), action_name(opts));
-}
-
int cmd_revert(int argc, const char **argv, const char *prefix)
{
struct replay_opts opts;
@@ -1011,7 +194,7 @@ int cmd_revert(int argc, const char **argv, const char *prefix)
opts.action = REPLAY_REVERT;
git_config(git_default_config, NULL);
parse_args(argc, argv, &opts);
- res = pick_revisions(&opts);
+ res = sequencer_pick_revisions(&opts);
if (res < 0)
die(_("revert failed"));
return res;
@@ -1026,7 +209,7 @@ int cmd_cherry_pick(int argc, const char **argv, const char *prefix)
opts.action = REPLAY_PICK;
git_config(git_default_config, NULL);
parse_args(argc, argv, &opts);
- res = pick_revisions(&opts);
+ res = sequencer_pick_revisions(&opts);
if (res < 0)
die(_("cherry-pick failed"));
return res;
diff --git a/sequencer.c b/sequencer.c
index bc2c046..87f146b 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -1,7 +1,27 @@
#include "cache.h"
-#include "sequencer.h"
-#include "strbuf.h"
+#include "object.h"
+#include "commit.h"
+#include "tag.h"
+#include "run-command.h"
+#include "exec_cmd.h"
+#include "utf8.h"
+#include "cache-tree.h"
+#include "diff.h"
+#include "revision.h"
+#include "rerere.h"
+#include "merge-recursive.h"
+#include "refs.h"
#include "dir.h"
+#include "sequencer.h"
+
+#define GIT_REFLOG_ACTION "GIT_REFLOG_ACTION"
+
+static const char *action_name(const struct replay_opts *opts)
+{
+ return opts->action == REPLAY_REVERT ? "revert" : "cherry-pick";
+}
+
+static char *get_encoding(const char *message);
void remove_sequencer_state(int aggressive)
{
@@ -17,3 +37,781 @@ void remove_sequencer_state(int aggressive)
strbuf_release(&seq_dir);
strbuf_release(&seq_old_dir);
}
+
+struct commit_message {
+ char *parent_label;
+ const char *label;
+ const char *subject;
+ char *reencoded_message;
+ const char *message;
+};
+
+static int get_message(struct commit *commit, struct commit_message *out)
+{
+ const char *encoding;
+ const char *abbrev, *subject;
+ int abbrev_len, subject_len;
+ char *q;
+
+ if (!commit->buffer)
+ return -1;
+ encoding = get_encoding(commit->buffer);
+ if (!encoding)
+ encoding = "UTF-8";
+ if (!git_commit_encoding)
+ git_commit_encoding = "UTF-8";
+
+ out->reencoded_message = NULL;
+ out->message = commit->buffer;
+ if (strcmp(encoding, git_commit_encoding))
+ out->reencoded_message = reencode_string(commit->buffer,
+ git_commit_encoding, encoding);
+ if (out->reencoded_message)
+ out->message = out->reencoded_message;
+
+ abbrev = find_unique_abbrev(commit->object.sha1, DEFAULT_ABBREV);
+ abbrev_len = strlen(abbrev);
+
+ subject_len = find_commit_subject(out->message, &subject);
+
+ out->parent_label = xmalloc(strlen("parent of ") + abbrev_len +
+ strlen("... ") + subject_len + 1);
+ q = out->parent_label;
+ q = mempcpy(q, "parent of ", strlen("parent of "));
+ out->label = q;
+ q = mempcpy(q, abbrev, abbrev_len);
+ q = mempcpy(q, "... ", strlen("... "));
+ out->subject = q;
+ q = mempcpy(q, subject, subject_len);
+ *q = '\0';
+ return 0;
+}
+
+static void free_message(struct commit_message *msg)
+{
+ free(msg->parent_label);
+ free(msg->reencoded_message);
+}
+
+static char *get_encoding(const char *message)
+{
+ const char *p = message, *eol;
+
+ while (*p && *p != '\n') {
+ for (eol = p + 1; *eol && *eol != '\n'; eol++)
+ ; /* do nothing */
+ if (!prefixcmp(p, "encoding ")) {
+ char *result = xmalloc(eol - 8 - p);
+ strlcpy(result, p + 9, eol - 8 - p);
+ return result;
+ }
+ p = eol;
+ if (*p == '\n')
+ p++;
+ }
+ return NULL;
+}
+
+static void write_cherry_pick_head(struct commit *commit)
+{
+ int fd;
+ struct strbuf buf = STRBUF_INIT;
+
+ strbuf_addf(&buf, "%s\n", sha1_to_hex(commit->object.sha1));
+
+ fd = open(git_path("CHERRY_PICK_HEAD"), O_WRONLY | O_CREAT, 0666);
+ if (fd < 0)
+ die_errno(_("Could not open '%s' for writing"),
+ git_path("CHERRY_PICK_HEAD"));
+ if (write_in_full(fd, buf.buf, buf.len) != buf.len || close(fd))
+ die_errno(_("Could not write to '%s'"), git_path("CHERRY_PICK_HEAD"));
+ strbuf_release(&buf);
+}
+
+static void print_advice(int show_hint)
+{
+ char *msg = getenv("GIT_CHERRY_PICK_HELP");
+
+ if (msg) {
+ fprintf(stderr, "%s\n", msg);
+ /*
+ * A conflict has occured but the porcelain
+ * (typically rebase --interactive) wants to take care
+ * of the commit itself so remove CHERRY_PICK_HEAD
+ */
+ unlink(git_path("CHERRY_PICK_HEAD"));
+ return;
+ }
+
+ if (show_hint) {
+ advise("after resolving the conflicts, mark the corrected paths");
+ advise("with 'git add <paths>' or 'git rm <paths>'");
+ advise("and commit the result with 'git commit'");
+ }
+}
+
+static void write_message(struct strbuf *msgbuf, const char *filename)
+{
+ static struct lock_file msg_file;
+
+ int msg_fd = hold_lock_file_for_update(&msg_file, filename,
+ LOCK_DIE_ON_ERROR);
+ if (write_in_full(msg_fd, msgbuf->buf, msgbuf->len) < 0)
+ die_errno(_("Could not write to %s."), filename);
+ strbuf_release(msgbuf);
+ if (commit_lock_file(&msg_file) < 0)
+ die(_("Error wrapping up %s"), filename);
+}
+
+static struct tree *empty_tree(void)
+{
+ return lookup_tree((const unsigned char *)EMPTY_TREE_SHA1_BIN);
+}
+
+static int error_dirty_index(struct replay_opts *opts)
+{
+ if (read_cache_unmerged())
+ return error_resolve_conflict(action_name(opts));
+
+ /* Different translation strings for cherry-pick and revert */
+ if (opts->action == REPLAY_PICK)
+ error(_("Your local changes would be overwritten by cherry-pick."));
+ else
+ error(_("Your local changes would be overwritten by revert."));
+
+ if (advice_commit_before_merge)
+ advise(_("Commit your changes or stash them to proceed."));
+ return -1;
+}
+
+static int fast_forward_to(const unsigned char *to, const unsigned char *from)
+{
+ struct ref_lock *ref_lock;
+
+ read_cache();
+ if (checkout_fast_forward(from, to))
+ exit(1); /* the callee should have complained already */
+ ref_lock = lock_any_ref_for_update("HEAD", from, 0);
+ return write_ref_sha1(ref_lock, to, "cherry-pick");
+}
+
+static int do_recursive_merge(struct commit *base, struct commit *next,
+ const char *base_label, const char *next_label,
+ unsigned char *head, struct strbuf *msgbuf,
+ struct replay_opts *opts)
+{
+ struct merge_options o;
+ struct tree *result, *next_tree, *base_tree, *head_tree;
+ int clean, index_fd;
+ const char **xopt;
+ static struct lock_file index_lock;
+
+ index_fd = hold_locked_index(&index_lock, 1);
+
+ read_cache();
+
+ init_merge_options(&o);
+ o.ancestor = base ? base_label : "(empty tree)";
+ o.branch1 = "HEAD";
+ o.branch2 = next ? next_label : "(empty tree)";
+
+ head_tree = parse_tree_indirect(head);
+ next_tree = next ? next->tree : empty_tree();
+ base_tree = base ? base->tree : empty_tree();
+
+ for (xopt = opts->xopts; xopt != opts->xopts + opts->xopts_nr; xopt++)
+ parse_merge_opt(&o, *xopt);
+
+ clean = merge_trees(&o,
+ head_tree,
+ next_tree, base_tree, &result);
+
+ if (active_cache_changed &&
+ (write_cache(index_fd, active_cache, active_nr) ||
+ commit_locked_index(&index_lock)))
+ /* TRANSLATORS: %s will be "revert" or "cherry-pick" */
+ die(_("%s: Unable to write new index file"), action_name(opts));
+ rollback_lock_file(&index_lock);
+
+ if (!clean) {
+ int i;
+ strbuf_addstr(msgbuf, "\nConflicts:\n\n");
+ for (i = 0; i < active_nr;) {
+ struct cache_entry *ce = active_cache[i++];
+ if (ce_stage(ce)) {
+ strbuf_addch(msgbuf, '\t');
+ strbuf_addstr(msgbuf, ce->name);
+ strbuf_addch(msgbuf, '\n');
+ while (i < active_nr && !strcmp(ce->name,
+ active_cache[i]->name))
+ i++;
+ }
+ }
+ }
+
+ return !clean;
+}
+
+/*
+ * If we are cherry-pick, and if the merge did not result in
+ * hand-editing, we will hit this commit and inherit the original
+ * author date and name.
+ * If we are revert, or if our cherry-pick results in a hand merge,
+ * we had better say that the current user is responsible for that.
+ */
+static int run_git_commit(const char *defmsg, struct replay_opts *opts)
+{
+ /* 6 is max possible length of our args array including NULL */
+ const char *args[6];
+ int i = 0;
+
+ args[i++] = "commit";
+ args[i++] = "-n";
+ if (opts->signoff)
+ args[i++] = "-s";
+ if (!opts->edit) {
+ args[i++] = "-F";
+ args[i++] = defmsg;
+ }
+ args[i] = NULL;
+
+ return run_command_v_opt(args, RUN_GIT_CMD);
+}
+
+static int do_pick_commit(struct commit *commit, enum replay_action action,
+ struct replay_opts *opts)
+{
+ unsigned char head[20];
+ struct commit *base, *next, *parent;
+ const char *base_label, *next_label;
+ struct commit_message msg = { NULL, NULL, NULL, NULL, NULL };
+ char *defmsg = NULL;
+ struct strbuf msgbuf = STRBUF_INIT;
+ int res;
+
+ if (opts->no_commit) {
+ /*
+ * We do not intend to commit immediately. We just want to
+ * merge the differences in, so let's compute the tree
+ * that represents the "current" state for merge-recursive
+ * to work on.
+ */
+ if (write_cache_as_tree(head, 0, NULL))
+ die (_("Your index file is unmerged."));
+ } else {
+ if (get_sha1("HEAD", head))
+ return error(_("You do not have a valid HEAD"));
+ if (index_differs_from("HEAD", 0))
+ return error_dirty_index(opts);
+ }
+ discard_cache();
+
+ if (!commit->parents) {
+ parent = NULL;
+ }
+ else if (commit->parents->next) {
+ /* Reverting or cherry-picking a merge commit */
+ int cnt;
+ struct commit_list *p;
+
+ if (!opts->mainline)
+ return error(_("Commit %s is a merge but no -m option was given."),
+ sha1_to_hex(commit->object.sha1));
+
+ for (cnt = 1, p = commit->parents;
+ cnt != opts->mainline && p;
+ cnt++)
+ p = p->next;
+ if (cnt != opts->mainline || !p)
+ return error(_("Commit %s does not have parent %d"),
+ sha1_to_hex(commit->object.sha1), opts->mainline);
+ parent = p->item;
+ } else if (0 < opts->mainline)
+ return error(_("Mainline was specified but commit %s is not a merge."),
+ sha1_to_hex(commit->object.sha1));
+ else
+ parent = commit->parents->item;
+
+ if (opts->allow_ff && parent && !hashcmp(parent->object.sha1, head))
+ return fast_forward_to(commit->object.sha1, head);
+
+ if (parent && parse_commit(parent) < 0)
+ /* TRANSLATORS: The first %s will be "revert" or
+ "cherry-pick", the second %s a SHA1 */
+ return error(_("%s: cannot parse parent commit %s"),
+ action_name(opts), sha1_to_hex(parent->object.sha1));
+
+ if (get_message(commit, &msg) != 0)
+ return error(_("Cannot get commit message for %s"),
+ sha1_to_hex(commit->object.sha1));
+
+ /*
+ * "commit" is an existing commit. We would want to apply
+ * the difference it introduces since its first parent "prev"
+ * on top of the current HEAD if we are cherry-pick. Or the
+ * reverse of it if we are revert.
+ */
+
+ defmsg = git_pathdup("MERGE_MSG");
+
+ if (action == REPLAY_REVERT) {
+ base = commit;
+ base_label = msg.label;
+ next = parent;
+ next_label = msg.parent_label;
+ strbuf_addstr(&msgbuf, "Revert \"");
+ strbuf_addstr(&msgbuf, msg.subject);
+ strbuf_addstr(&msgbuf, "\"\n\nThis reverts commit ");
+ strbuf_addstr(&msgbuf, sha1_to_hex(commit->object.sha1));
+
+ if (commit->parents && commit->parents->next) {
+ strbuf_addstr(&msgbuf, ", reversing\nchanges made to ");
+ strbuf_addstr(&msgbuf, sha1_to_hex(parent->object.sha1));
+ }
+ strbuf_addstr(&msgbuf, ".\n");
+ } else {
+ const char *p;
+
+ base = parent;
+ base_label = msg.parent_label;
+ next = commit;
+ next_label = msg.label;
+
+ /*
+ * Append the commit log message to msgbuf; it starts
+ * after the tree, parent, author, committer
+ * information followed by "\n\n".
+ */
+ p = strstr(msg.message, "\n\n");
+ if (p) {
+ p += 2;
+ strbuf_addstr(&msgbuf, p);
+ }
+
+ if (opts->record_origin) {
+ strbuf_addstr(&msgbuf, "(cherry picked from commit ");
+ strbuf_addstr(&msgbuf, sha1_to_hex(commit->object.sha1));
+ strbuf_addstr(&msgbuf, ")\n");
+ }
+ }
+
+ if (!opts->strategy || !strcmp(opts->strategy, "recursive") || action == REPLAY_REVERT) {
+ res = do_recursive_merge(base, next, base_label, next_label,
+ head, &msgbuf, opts);
+ write_message(&msgbuf, defmsg);
+ } else {
+ struct commit_list *common = NULL;
+ struct commit_list *remotes = NULL;
+
+ write_message(&msgbuf, defmsg);
+
+ commit_list_insert(base, &common);
+ commit_list_insert(next, &remotes);
+ res = try_merge_command(opts->strategy, opts->xopts_nr, opts->xopts,
+ common, sha1_to_hex(head), remotes);
+ free_commit_list(common);
+ free_commit_list(remotes);
+ }
+
+ /*
+ * If the merge was clean or if it failed due to conflict, we write
+ * CHERRY_PICK_HEAD for the subsequent invocation of commit to use.
+ * However, if the merge did not even start, then we don't want to
+ * write it at all.
+ */
+ if (opts->action == REPLAY_PICK && !opts->no_commit && (res == 0 || res == 1))
+ write_cherry_pick_head(commit);
+
+ if (res) {
+ error(action == REPLAY_REVERT
+ ? _("could not revert %s... %s")
+ : _("could not apply %s... %s"),
+ find_unique_abbrev(commit->object.sha1, DEFAULT_ABBREV),
+ msg.subject);
+ print_advice(res == 1);
+ rerere(opts->allow_rerere_auto);
+ } else {
+ if (!opts->no_commit)
+ res = run_git_commit(defmsg, opts);
+ }
+
+ free_message(&msg);
+ free(defmsg);
+
+ return res;
+}
+
+static void prepare_revs(struct replay_opts *opts)
+{
+ if (opts->action != REPLAY_REVERT)
+ opts->revs->reverse ^= 1;
+
+ if (prepare_revision_walk(opts->revs))
+ die(_("revision walk setup failed"));
+
+ if (!opts->revs->commits)
+ die(_("empty commit set passed"));
+}
+
+static void read_and_refresh_cache(struct replay_opts *opts)
+{
+ static struct lock_file index_lock;
+ int index_fd = hold_locked_index(&index_lock, 0);
+ if (read_index_preload(&the_index, NULL) < 0)
+ die(_("git %s: failed to read the index"), action_name(opts));
+ refresh_index(&the_index, REFRESH_QUIET|REFRESH_UNMERGED, NULL, NULL, NULL);
+ if (the_index.cache_changed) {
+ if (write_index(&the_index, index_fd) ||
+ commit_locked_index(&index_lock))
+ die(_("git %s: failed to refresh the index"), action_name(opts));
+ }
+ rollback_lock_file(&index_lock);
+}
+
+/*
+ * Append a commit to the end of the commit_list.
+ *
+ * next starts by pointing to the variable that holds the head of an
+ * empty commit_list, and is updated to point to the "next" field of
+ * the last item on the list as new commits are appended.
+ *
+ * Usage example:
+ *
+ * struct commit_list *list;
+ * struct commit_list **next = &list;
+ *
+ * next = commit_list_append(c1, next);
+ * next = commit_list_append(c2, next);
+ * assert(commit_list_count(list) == 2);
+ * return list;
+ */
+static struct replay_insn_list **replay_insn_list_append(enum replay_action action,
+ struct commit *operand,
+ struct replay_insn_list **next)
+{
+ struct replay_insn_list *new = xmalloc(sizeof(*new));
+ new->action = action;
+ new->operand = operand;
+ *next = new;
+ new->next = NULL;
+ return &new->next;
+}
+
+static int format_todo(struct strbuf *buf, struct replay_insn_list *todo_list)
+{
+ struct replay_insn_list *cur;
+
+ for (cur = todo_list; cur; cur = cur->next) {
+ const char *sha1_abbrev, *action_str, *subject;
+ int subject_len;
+
+ action_str = cur->action == REPLAY_REVERT ? "revert" : "pick";
+ sha1_abbrev = find_unique_abbrev(cur->operand->object.sha1, DEFAULT_ABBREV);
+ subject_len = find_commit_subject(cur->operand->buffer, &subject);
+ strbuf_addf(buf, "%s %s %.*s\n", action_str, sha1_abbrev,
+ subject_len, subject);
+ }
+ return 0;
+}
+
+static int parse_insn_line(char *bol, char *eol, struct replay_insn_list *item)
+{
+ unsigned char commit_sha1[20];
+ char *end_of_object_name;
+ int saved, status;
+
+ if (!prefixcmp(bol, "pick ")) {
+ item->action = REPLAY_PICK;
+ bol += strlen("pick ");
+ } else if (!prefixcmp(bol, "revert ")) {
+ item->action = REPLAY_REVERT;
+ bol += strlen("revert ");
+ } else {
+ size_t len = strchrnul(bol, '\n') - bol;
+ if (len > 255)
+ len = 255;
+ return error(_("Unrecognized action: %.*s"), (int)len, bol);
+ }
+
+ end_of_object_name = bol + strcspn(bol, " \n");
+ saved = *end_of_object_name;
+ *end_of_object_name = '\0';
+ status = get_sha1(bol, commit_sha1);
+ *end_of_object_name = saved;
+
+ if (status < 0)
+ return error(_("Malformed object name: %s"), bol);
+
+ item->operand = lookup_commit_reference(commit_sha1);
+ if (!item->operand)
+ return error(_("Not a valid commit: %s"), bol);
+
+ item->next = NULL;
+ return 0;
+}
+
+static int parse_insn_buffer(char *buf, struct replay_insn_list **todo_list)
+{
+ struct replay_insn_list **next = todo_list;
+ struct replay_insn_list item = {0, NULL, NULL};
+ char *p = buf;
+ int i;
+
+ for (i = 1; *p; i++) {
+ char *eol = strchrnul(p, '\n');
+ if (parse_insn_line(p, eol, &item) < 0)
+ return error(_("on line %d."), i);
+ next = replay_insn_list_append(item.action, item.operand, next);
+ p = *eol ? eol + 1 : eol;
+ }
+ if (!*todo_list)
+ return error(_("No commits parsed."));
+ return 0;
+}
+
+static void read_populate_todo(struct replay_insn_list **todo_list)
+{
+ const char *todo_file = git_path(SEQ_TODO_FILE);
+ struct strbuf buf = STRBUF_INIT;
+ int fd, res;
+
+ fd = open(todo_file, O_RDONLY);
+ if (fd < 0)
+ die_errno(_("Could not open %s."), todo_file);
+ if (strbuf_read(&buf, fd, 0) < 0) {
+ close(fd);
+ strbuf_release(&buf);
+ die(_("Could not read %s."), todo_file);
+ }
+ close(fd);
+
+ res = parse_insn_buffer(buf.buf, todo_list);
+ strbuf_release(&buf);
+ if (res)
+ die(_("Unusable instruction sheet: %s"), todo_file);
+}
+
+static int populate_opts_cb(const char *key, const char *value, void *data)
+{
+ struct replay_opts *opts = data;
+ int error_flag = 1;
+
+ if (!value)
+ error_flag = 0;
+ else if (!strcmp(key, "options.no-commit"))
+ opts->no_commit = git_config_bool_or_int(key, value, &error_flag);
+ else if (!strcmp(key, "options.edit"))
+ opts->edit = git_config_bool_or_int(key, value, &error_flag);
+ else if (!strcmp(key, "options.signoff"))
+ opts->signoff = git_config_bool_or_int(key, value, &error_flag);
+ else if (!strcmp(key, "options.record-origin"))
+ opts->record_origin = git_config_bool_or_int(key, value, &error_flag);
+ else if (!strcmp(key, "options.allow-ff"))
+ opts->allow_ff = git_config_bool_or_int(key, value, &error_flag);
+ else if (!strcmp(key, "options.mainline"))
+ opts->mainline = git_config_int(key, value);
+ else if (!strcmp(key, "options.strategy"))
+ git_config_string(&opts->strategy, key, value);
+ else if (!strcmp(key, "options.strategy-option")) {
+ ALLOC_GROW(opts->xopts, opts->xopts_nr + 1, opts->xopts_alloc);
+ opts->xopts[opts->xopts_nr++] = xstrdup(value);
+ } else
+ return error(_("Invalid key: %s"), key);
+
+ if (!error_flag)
+ return error(_("Invalid value for %s: %s"), key, value);
+
+ return 0;
+}
+
+static void read_populate_opts(struct replay_opts **opts_ptr)
+{
+ const char *opts_file = git_path(SEQ_OPTS_FILE);
+
+ if (!file_exists(opts_file))
+ return;
+ if (git_config_from_file(populate_opts_cb, opts_file, *opts_ptr) < 0)
+ die(_("Malformed options sheet: %s"), opts_file);
+}
+
+static void walk_revs_populate_todo(struct replay_insn_list **todo_list,
+ struct replay_opts *opts)
+{
+ struct commit *commit;
+ struct replay_insn_list **next;
+
+ prepare_revs(opts);
+
+ next = todo_list;
+ while ((commit = get_revision(opts->revs)))
+ next = replay_insn_list_append(opts->action, commit, next);
+}
+
+static int create_seq_dir(void)
+{
+ const char *seq_dir = git_path(SEQ_DIR);
+
+ if (file_exists(seq_dir))
+ return error(_("%s already exists."), seq_dir);
+ else if (mkdir(seq_dir, 0777) < 0)
+ die_errno(_("Could not create sequencer directory '%s'."), seq_dir);
+ return 0;
+}
+
+static void save_head(const char *head)
+{
+ const char *head_file = git_path(SEQ_HEAD_FILE);
+ static struct lock_file head_lock;
+ struct strbuf buf = STRBUF_INIT;
+ int fd;
+
+ fd = hold_lock_file_for_update(&head_lock, head_file, LOCK_DIE_ON_ERROR);
+ strbuf_addf(&buf, "%s\n", head);
+ if (write_in_full(fd, buf.buf, buf.len) < 0)
+ die_errno(_("Could not write to %s."), head_file);
+ if (commit_lock_file(&head_lock) < 0)
+ die(_("Error wrapping up %s."), head_file);
+}
+
+static void save_todo(struct replay_insn_list *todo_list)
+{
+ const char *todo_file = git_path(SEQ_TODO_FILE);
+ static struct lock_file todo_lock;
+ struct strbuf buf = STRBUF_INIT;
+ int fd;
+
+ fd = hold_lock_file_for_update(&todo_lock, todo_file, LOCK_DIE_ON_ERROR);
+ if (format_todo(&buf, todo_list) < 0)
+ die(_("Could not format %s."), todo_file);
+ if (write_in_full(fd, buf.buf, buf.len) < 0) {
+ strbuf_release(&buf);
+ die_errno(_("Could not write to %s."), todo_file);
+ }
+ if (commit_lock_file(&todo_lock) < 0) {
+ strbuf_release(&buf);
+ die(_("Error wrapping up %s."), todo_file);
+ }
+ strbuf_release(&buf);
+}
+
+static void save_opts(struct replay_opts *opts)
+{
+ const char *opts_file = git_path(SEQ_OPTS_FILE);
+
+ if (opts->no_commit)
+ git_config_set_in_file(opts_file, "options.no-commit", "true");
+ if (opts->edit)
+ git_config_set_in_file(opts_file, "options.edit", "true");
+ if (opts->signoff)
+ git_config_set_in_file(opts_file, "options.signoff", "true");
+ if (opts->record_origin)
+ git_config_set_in_file(opts_file, "options.record-origin", "true");
+ if (opts->allow_ff)
+ git_config_set_in_file(opts_file, "options.allow-ff", "true");
+ if (opts->mainline) {
+ struct strbuf buf = STRBUF_INIT;
+ strbuf_addf(&buf, "%d", opts->mainline);
+ git_config_set_in_file(opts_file, "options.mainline", buf.buf);
+ strbuf_release(&buf);
+ }
+ if (opts->strategy)
+ git_config_set_in_file(opts_file, "options.strategy", opts->strategy);
+ if (opts->xopts) {
+ int i;
+ for (i = 0; i < opts->xopts_nr; i++)
+ git_config_set_multivar_in_file(opts_file,
+ "options.strategy-option",
+ opts->xopts[i], "^$", 0);
+ }
+}
+
+static int pick_commits(struct replay_insn_list *todo_list,
+ struct replay_opts *opts)
+{
+ struct replay_insn_list *cur;
+ int res;
+
+ setenv(GIT_REFLOG_ACTION, action_name(opts), 0);
+ if (opts->allow_ff)
+ assert(!(opts->signoff || opts->no_commit ||
+ opts->record_origin || opts->edit));
+ read_and_refresh_cache(opts);
+
+ for (cur = todo_list; cur; cur = cur->next) {
+ save_todo(cur);
+ res = do_pick_commit(cur->operand, cur->action, opts);
+ if (res) {
+ if (!cur->next)
+ /*
+ * An error was encountered while
+ * picking the last commit; the
+ * sequencer state is useless now --
+ * the user simply needs to resolve
+ * the conflict and commit
+ */
+ remove_sequencer_state(0);
+ return res;
+ }
+ }
+
+ /*
+ * Sequence of picks finished successfully; cleanup by
+ * removing the .git/sequencer directory
+ */
+ remove_sequencer_state(1);
+ return 0;
+}
+
+int sequencer_pick_revisions(struct replay_opts *opts)
+{
+ struct replay_insn_list *todo_list = NULL;
+ unsigned char sha1[20];
+
+ if (opts->subcommand == REPLAY_NONE)
+ assert(opts->revs);
+
+ read_and_refresh_cache(opts);
+
+ /*
+ * Decide what to do depending on the arguments; a fresh
+ * cherry-pick should be handled differently from an existing
+ * one that is being continued
+ */
+ if (opts->subcommand == REPLAY_RESET) {
+ remove_sequencer_state(1);
+ return 0;
+ } else if (opts->subcommand == REPLAY_CONTINUE) {
+ if (!file_exists(git_path(SEQ_TODO_FILE)))
+ goto error;
+ read_populate_opts(&opts);
+ read_populate_todo(&todo_list);
+
+ /* Verify that the conflict has been resolved */
+ if (!index_differs_from("HEAD", 0))
+ todo_list = todo_list->next;
+ } else {
+ /*
+ * Start a new cherry-pick/ revert sequence; but
+ * first, make sure that an existing one isn't in
+ * progress
+ */
+
+ walk_revs_populate_todo(&todo_list, opts);
+ if (create_seq_dir() < 0) {
+ error(_("A cherry-pick or revert is in progress."));
+ advise(_("Use --continue to continue the operation"));
+ advise(_("or --reset to forget about it"));
+ return -1;
+ }
+ if (get_sha1("HEAD", sha1)) {
+ if (opts->action == REPLAY_REVERT)
+ return error(_("Can't revert as initial commit"));
+ return error(_("Can't cherry-pick into empty head"));
+ }
+ save_head(sha1_to_hex(sha1));
+ save_opts(opts);
+ }
+ return pick_commits(todo_list, opts);
+error:
+ return error(_("No %s in progress"), action_name(opts));
+}
diff --git a/sequencer.h b/sequencer.h
index f4db257..92b2d63 100644
--- a/sequencer.h
+++ b/sequencer.h
@@ -8,6 +8,30 @@
#define SEQ_OPTS_FILE "sequencer/opts"
enum replay_action { REPLAY_REVERT, REPLAY_PICK };
+enum replay_subcommand { REPLAY_NONE, REPLAY_RESET, REPLAY_CONTINUE };
+
+struct replay_opts {
+ enum replay_action action;
+ enum replay_subcommand subcommand;
+
+ /* Boolean options */
+ int edit;
+ int record_origin;
+ int no_commit;
+ int signoff;
+ int allow_ff;
+ int allow_rerere_auto;
+
+ int mainline;
+
+ /* Merge strategy */
+ const char *strategy;
+ const char **xopts;
+ size_t xopts_nr, xopts_alloc;
+
+ /* Only used by REPLAY_NONE */
+ struct rev_info *revs;
+};
struct replay_insn_list {
enum replay_action action;
@@ -25,4 +49,6 @@ struct replay_insn_list {
*/
void remove_sequencer_state(int aggressive);
+int sequencer_pick_revisions(struct replay_opts *opts);
+
#endif
--
1.7.6.351.gb35ac.dirty
^ permalink raw reply related
* [PATCH 0/7] New sequencer workflow!
From: Ramkumar Ramachandra @ 2011-11-13 10:46 UTC (permalink / raw)
To: Git List; +Cc: Junio C Hamano, Jonathan Nieder, Christian Couder
Hi,
After days of bending my mind around the backward-compatibility
problem, I'm happy to announce that we finally have an implementation!
This series preserves the old workflow:
$ git cherry-pick foo
... conflict ...
$ echo "resolved" >problematicfile
$ git add problematicfile
$ git commit
$ git cherry-pick foo..bar
... conflict ...
$ echo "resolved" >problematicfile
$ git add problematicfile
$ git commit
$ git revert --continue # No, there are no typos
And introduces a brand new workflow:
$ git cherry-pick foo
... conflict ...
$ echo "resolved" >problematicfile
$ git add problematicfile
$ git sequencer --continue
$ git cherry-pick foo..bar
... conflict ...
$ echo "resolved" >problematicfile
$ git add problematicfile
$ git sequencer --continue
$ git revert moo..baz
... conflict ...
$ echo "resolved" >problematicfile
$ git add problematicfile
$ git sequencer --continue
In other words, it just doesn't matter how you started what sequencing
operation: you can always continue a it with "git sequencer
--continue" and reset it with "git sequencer --reset".
This series is reasonably well-polished and based on
rr/revert-cherry-pick (since the topic hasn't made it to 'master'
yet). The only downside is that it's very complicated, and I've tried
my best to explain the motivation for everything in the commit
messages. Needless to say, it requires a huge amount of testing to
catch unpleasant corner cases.
Part 1 moves a lot of code from builtin/revert.c to sequencer.c.
Parts 2, 3, 4 carefully work around some complex issues related to
'.git/CHERRY_PICK_HEAD'.
Part 5 introduces a new git-sequencer builtin. Although it's
empty'ish now, I'd argue that adding it now helps think about the
unified interface. Any suggestions on what concrete function it
should implement? I'm thinking of something along the lines of
'rebase -i'.
Part 6 is the patch that tells the sequencer "how to continue". As
more commands grow sequencer functionalities, it will become clear
that committing isn't the only way to continue, but I think it's good
for the moment.
Part 7 is the icing on the cake. Enjoy.
I'd sent out $gmane/184859 last week as a sanity check: thanks to
Junio and Jonathan for looking at it. It's always comforting to know
that I haven't lost my head yet :)
If you're looking to get involved before this series hits 'pu', use
the "sequencer" branch of my Git fork on Github:
http://github.com/artagnon/git
Thanks.
-- Ram
Ramkumar Ramachandra (7):
sequencer: factor code out of revert builtin
sequencer: invalidate sequencer state without todo
sequencer: handle single-commit pick as special case
sequencer: handle cherry-pick conflict in last commit
sequencer: introduce git-sequencer builtin
sequencer: teach '--continue' how to commit
sequencer: teach parser about CHERRY_PICK_HEAD
.gitignore | 1 +
Documentation/git-sequencer.txt | 33 +
Makefile | 1 +
builtin.h | 1 +
builtin/revert.c | 821 +-------------------
builtin/sequencer.c | 52 ++
git.c | 1 +
sequencer.c | 842 +++++++++++++++++++-
sequencer.h | 26 +
...-cherry-pick-sequence.sh => t3510-sequencer.sh} | 140 +---
...-sequence.sh => t3511-cherry-pick-sequencer.sh} | 124 +---
11 files changed, 1002 insertions(+), 1040 deletions(-)
create mode 100644 Documentation/git-sequencer.txt
create mode 100644 builtin/sequencer.c
copy t/{t3510-cherry-pick-sequence.sh => t3510-sequencer.sh} (60%)
rename t/{t3510-cherry-pick-sequence.sh => t3511-cherry-pick-sequencer.sh} (59%)
--
1.7.6.351.gb35ac.dirty
^ permalink raw reply
* Re: [PATCH 3/5] sequencer: sequencer state is useless without todo
From: Ramkumar Ramachandra @ 2011-11-13 10:44 UTC (permalink / raw)
To: Jonathan Nieder; +Cc: Git List, Junio C Hamano, Christian Couder
In-Reply-To: <20111106002645.GE27272@elie.hsd1.il.comcast.net>
Hi,
Jonathan Nieder wrote:
> Ramkumar Ramachandra wrote:
> [...]
> The usual commit-message debugging strategy applies here: imagine you
> are a BIOS clone manufacturer, and for legal reasons you are not
> allowed to read this part of the git implementation embedded in the
> standard BIOS. However, you are allowed to read the commit message,
> and if that message is clear enough, it will explain the purpose and
> behavior of that code and you will be able to implement a compatible
> implementation addressing the same problem without scratching your
> head too much.
Ah, it helps to think about commit messages like this. Thanks.
>> --- a/sequencer.c
>> +++ b/sequencer.c
>> @@ -654,11 +654,15 @@ static void walk_revs_populate_todo(struct replay_insn_list **todo_list,
>>
>> static int create_seq_dir(void)
>> {
>> + const char *todo_file = git_path(SEQ_TODO_FILE);
>> const char *seq_dir = git_path(SEQ_DIR);
>
> Scary idiom.
What's scary about it?
-- Ram
^ permalink raw reply
* Re: [PATCH 0/5] Sequencer: working around historical mistakes
From: Ramkumar Ramachandra @ 2011-11-13 10:42 UTC (permalink / raw)
To: Jonathan Nieder; +Cc: Git List, Junio C Hamano, Christian Couder
In-Reply-To: <20111105234312.GB27272@elie.hsd1.il.comcast.net>
Hi,
Jonathan Nieder writes:
> Shouldn't it be based against rr/revert-cherry-pick, rather than
> "next" which is more of a moving target?
When I read this, I went: "Now why didn't I think of that?"
Thanks.
-- Ram
^ permalink raw reply
* Re: [PATCH 1/5] sequencer: factor code out of revert builtin
From: Ramkumar Ramachandra @ 2011-11-13 10:40 UTC (permalink / raw)
To: Jonathan Nieder; +Cc: Git List, Junio C Hamano, Christian Couder
In-Reply-To: <20111106001232.GC27272@elie.hsd1.il.comcast.net>
Hi,
Small note.
Jonathan Nieder writes:
>> --- a/sequencer.c
>> +++ b/sequencer.c
>> @@ -1,7 +1,27 @@
>> #include "cache.h"
>> +#include "object.h"
>> +#include "commit.h"
>> +#include "tag.h"
>> +#include "run-command.h"
>> +#include "exec_cmd.h"
>> +#include "utf8.h"
>> +#include "cache-tree.h"
>> +#include "diff.h"
>> +#include "revision.h"
>> +#include "rerere.h"
>> +#include "merge-recursive.h"
>> +#include "refs.h"
>> -#include "sequencer.h"
>> -#include "strbuf.h"
>> #include "dir.h"
>> +#include "sequencer.h"
>
> Why did sequencer.h move to after dir.h?
1. I like the convention of including the "foo.h" as the last header
in "foo.c". I suppose it has to do with the way I include standard
headers in my own code (for pet projects).
2. I didn't want to include many of these headers in sequencer.h again
-- it uses a lot of these data types. I've noticed that ordering of
header inclusion is important in many parts of Git, so the convention
just stuck.
Thanks.
-- Ram
^ permalink raw reply
* [PATCH 2/2] Copy resolve_ref() return value for longer use
From: Nguyễn Thái Ngọc Duy @ 2011-11-13 10:22 UTC (permalink / raw)
To: git, Junio C Hamano; +Cc: Nguyễn Thái Ngọc Duy
In-Reply-To: <1321179735-21890-1-git-send-email-pclouds@gmail.com>
resolve_ref() may return a pointer to a static buffer. Callers that
use this value longer than a couple of statements should copy the
value to avoid some hidden resolve_ref() call that may change the
static buffer's value.
The bug found by Tony Wang <wwwjfy@gmail.com> in builtin/merge.c
demonstrates this. The first call is in cmd_merge()
branch = resolve_ref("HEAD", head_sha1, 0, &flag);
Then deep in lookup_commit_or_die() a few lines after, resolve_ref()
may be called again and destroy "branch".
lookup_commit_or_die
lookup_commit_reference
lookup_commit_reference_gently
parse_object
lookup_replace_object
do_lookup_replace_object
prepare_replace_object
for_each_replace_ref
do_for_each_ref
get_loose_refs
get_ref_dir
get_ref_dir
resolve_ref
All call sites are checked and made sure that xstrdup() is called if
the value should be saved.
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
Unfortunately there are still 37 resolve_ref() calls. An API change
would touch all of them and potentially introduce more bugs along the
way, either leak more memory or free() the wrong way.
So I'd stick with this for v1.7.8.
builtin/branch.c | 5 +++-
builtin/checkout.c | 4 ++-
builtin/commit.c | 3 +-
builtin/fmt-merge-msg.c | 6 ++++-
builtin/merge.c | 62 ++++++++++++++++++++++++++++++----------------
builtin/notes.c | 6 ++++-
builtin/receive-pack.c | 3 ++
reflog-walk.c | 5 +++-
8 files changed, 66 insertions(+), 28 deletions(-)
diff --git a/builtin/branch.c b/builtin/branch.c
index 0fe9c4d..5b6d839 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -115,8 +115,10 @@ static int branch_merged(int kind, const char *name,
branch->merge[0] &&
branch->merge[0]->dst &&
(reference_name =
- resolve_ref(branch->merge[0]->dst, sha1, 1, NULL)) != NULL)
+ resolve_ref(branch->merge[0]->dst, sha1, 1, NULL)) != NULL) {
+ reference_name = xstrdup(reference_name);
reference_rev = lookup_commit_reference(sha1);
+ }
}
if (!reference_rev)
reference_rev = head_rev;
@@ -141,6 +143,7 @@ static int branch_merged(int kind, const char *name,
" '%s', even though it is merged to HEAD."),
name, reference_name);
}
+ free((char*)reference_name);
return merged;
}
diff --git a/builtin/checkout.c b/builtin/checkout.c
index beeaee4..c6919f1 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -699,7 +699,9 @@ static int switch_branches(struct checkout_opts *opts, struct branch_info *new)
unsigned char rev[20];
int flag;
memset(&old, 0, sizeof(old));
- old.path = xstrdup(resolve_ref("HEAD", rev, 0, &flag));
+ old.path = resolve_ref("HEAD", rev, 0, &flag);
+ if (old.path)
+ old.path = xstrdup(old.path);
old.commit = lookup_commit_reference_gently(rev, 1);
if (!(flag & REF_ISSYMREF)) {
free((char *)old.path);
diff --git a/builtin/commit.c b/builtin/commit.c
index c46f2d1..f3a6ed2 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -1259,7 +1259,7 @@ static void print_summary(const char *prefix, const unsigned char *sha1,
struct commit *commit;
struct strbuf format = STRBUF_INIT;
unsigned char junk_sha1[20];
- const char *head = resolve_ref("HEAD", junk_sha1, 0, NULL);
+ const char *head;
struct pretty_print_context pctx = {0};
struct strbuf author_ident = STRBUF_INIT;
struct strbuf committer_ident = STRBUF_INIT;
@@ -1304,6 +1304,7 @@ static void print_summary(const char *prefix, const unsigned char *sha1,
rev.diffopt.break_opt = 0;
diff_setup_done(&rev.diffopt);
+ head = resolve_ref("HEAD", junk_sha1, 0, NULL);
printf("[%s%s ",
!prefixcmp(head, "refs/heads/") ?
head + 11 :
diff --git a/builtin/fmt-merge-msg.c b/builtin/fmt-merge-msg.c
index 7e2f225..6fe9102 100644
--- a/builtin/fmt-merge-msg.c
+++ b/builtin/fmt-merge-msg.c
@@ -268,6 +268,7 @@ static int do_fmt_merge_msg(int merge_title, struct strbuf *in,
die("No current branch");
if (!prefixcmp(current_branch, "refs/heads/"))
current_branch += 11;
+ current_branch = xstrdup(current_branch);
/* get a line */
while (pos < in->len) {
@@ -283,8 +284,10 @@ static int do_fmt_merge_msg(int merge_title, struct strbuf *in,
die ("Error in line %d: %.*s", i, len, p);
}
- if (!srcs.nr)
+ if (!srcs.nr) {
+ free((char*)current_branch);
return 0;
+ }
if (merge_title)
do_fmt_merge_msg_title(out, current_branch);
@@ -306,6 +309,7 @@ static int do_fmt_merge_msg(int merge_title, struct strbuf *in,
shortlog(origins.items[i].string, origins.items[i].util,
head, &rev, shortlog_len, out);
}
+ free((char*)current_branch);
return 0;
}
diff --git a/builtin/merge.c b/builtin/merge.c
index 42b4f9e..33be6c8 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -1082,7 +1082,7 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
struct commit *head_commit;
struct strbuf buf = STRBUF_INIT;
const char *head_arg;
- int flag, i;
+ int flag, i, ret = 0;
int best_cnt = -1, merge_was_ok = 0, automerge_was_ok = 0;
struct commit_list *common = NULL;
const char *best_strategy = NULL, *wt_strategy = NULL;
@@ -1096,8 +1096,11 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
* current branch.
*/
branch = resolve_ref("HEAD", head_sha1, 0, &flag);
- if (branch && !prefixcmp(branch, "refs/heads/"))
- branch += 11;
+ if (branch) {
+ if (!prefixcmp(branch, "refs/heads/"))
+ branch += 11;
+ branch = xstrdup(branch);
+ }
if (!branch || is_null_sha1(head_sha1))
head_commit = NULL;
else
@@ -1121,7 +1124,8 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
die(_("There is no merge to abort (MERGE_HEAD missing)."));
/* Invoke 'git reset --merge' */
- return cmd_reset(nargc, nargv, prefix);
+ ret = cmd_reset(nargc, nargv, prefix);
+ goto done;
}
if (read_cache_unmerged())
@@ -1205,7 +1209,7 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
read_empty(remote_head->sha1, 0);
update_ref("initial pull", "HEAD", remote_head->sha1, NULL, 0,
DIE_ON_ERR);
- return 0;
+ goto done;
} else {
struct strbuf merge_names = STRBUF_INIT;
@@ -1292,7 +1296,7 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
* but first the most common case of merging one remote.
*/
finish_up_to_date("Already up-to-date.");
- return 0;
+ goto done;
} else if (allow_fast_forward && !remoteheads->next &&
!common->next &&
!hashcmp(common->item->object.sha1, head_commit->object.sha1)) {
@@ -1313,15 +1317,20 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
strbuf_addstr(&msg,
" (no commit created; -m option ignored)");
o = want_commit(sha1_to_hex(remoteheads->item->object.sha1));
- if (!o)
- return 1;
+ if (!o) {
+ ret = 1;
+ goto done;
+ }
- if (checkout_fast_forward(head_commit->object.sha1, remoteheads->item->object.sha1))
- return 1;
+ if (checkout_fast_forward(head_commit->object.sha1,
+ remoteheads->item->object.sha1)) {
+ ret = 1;
+ goto done;
+ }
finish(head_commit, o->sha1, msg.buf);
drop_save();
- return 0;
+ goto done;
} else if (!remoteheads->next && common->next)
;
/*
@@ -1339,8 +1348,11 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
git_committer_info(IDENT_ERROR_ON_NO_NAME);
printf(_("Trying really trivial in-index merge...\n"));
if (!read_tree_trivial(common->item->object.sha1,
- head_commit->object.sha1, remoteheads->item->object.sha1))
- return merge_trivial(head_commit);
+ head_commit->object.sha1,
+ remoteheads->item->object.sha1)) {
+ ret = merge_trivial(head_commit);
+ goto done;
+ }
printf(_("Nope.\n"));
}
} else {
@@ -1368,7 +1380,7 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
}
if (up_to_date) {
finish_up_to_date("Already up-to-date. Yeeah!");
- return 0;
+ goto done;
}
}
@@ -1450,9 +1462,11 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
* If we have a resulting tree, that means the strategy module
* auto resolved the merge cleanly.
*/
- if (automerge_was_ok)
- return finish_automerge(head_commit, common, result_tree,
- wt_strategy);
+ if (automerge_was_ok) {
+ ret = finish_automerge(head_commit, common, result_tree,
+ wt_strategy);
+ goto done;
+ }
/*
* Pick the result from the best strategy and have the user fix
@@ -1466,7 +1480,8 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
else
fprintf(stderr, _("Merge with strategy %s failed.\n"),
use_strategies[0]->name);
- return 2;
+ ret = 2;
+ goto done;
} else if (best_strategy == wt_strategy)
; /* We already have its result in the working tree. */
else {
@@ -1482,10 +1497,13 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
else
write_merge_state();
- if (merge_was_ok) {
+ if (merge_was_ok)
fprintf(stderr, _("Automatic merge went well; "
"stopped before committing as requested\n"));
- return 0;
- } else
- return suggest_conflicts(option_renormalize);
+ else
+ ret = suggest_conflicts(option_renormalize);
+
+done:
+ free((char*)branch);
+ return ret;
}
diff --git a/builtin/notes.c b/builtin/notes.c
index f8e437d..ccff770 100644
--- a/builtin/notes.c
+++ b/builtin/notes.c
@@ -804,6 +804,7 @@ static int merge_commit(struct notes_merge_options *o)
struct notes_tree *t;
struct commit *partial;
struct pretty_print_context pretty_ctx;
+ int ret;
/*
* Read partial merge result from .git/NOTES_MERGE_PARTIAL,
@@ -828,6 +829,7 @@ static int merge_commit(struct notes_merge_options *o)
o->local_ref = resolve_ref("NOTES_MERGE_REF", sha1, 0, NULL);
if (!o->local_ref)
die("Failed to resolve NOTES_MERGE_REF");
+ o->local_ref = xstrdup(o->local_ref);
if (notes_merge_commit(o, t, partial, sha1))
die("Failed to finalize notes merge");
@@ -843,7 +845,9 @@ static int merge_commit(struct notes_merge_options *o)
free_notes(t);
strbuf_release(&msg);
- return merge_abort(o);
+ ret = merge_abort(o);
+ free((char*)o->local_ref);
+ return ret;
}
static int merge(int argc, const char **argv, const char *prefix)
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index 7ec68a1..d3fe42a 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -695,7 +695,10 @@ static void execute_commands(struct command *commands, const char *unpacker_erro
check_aliased_updates(commands);
+ free((char*)head_name);
head_name = resolve_ref("HEAD", sha1, 0, NULL);
+ if (head_name)
+ head_name = xstrdup(head_name);
for (cmd = commands; cmd; cmd = cmd->next)
if (!cmd->skip_update)
diff --git a/reflog-walk.c b/reflog-walk.c
index 5d81d39..efd13ad 100644
--- a/reflog-walk.c
+++ b/reflog-walk.c
@@ -51,8 +51,11 @@ static struct complete_reflogs *read_complete_reflog(const char *ref)
if (reflogs->nr == 0) {
unsigned char sha1[20];
const char *name = resolve_ref(ref, sha1, 1, NULL);
- if (name)
+ if (name) {
+ name = xstrdup(name);
for_each_reflog_ent(name, read_one_reflog, reflogs);
+ free((char*)name);
+ }
}
if (reflogs->nr == 0) {
int len = strlen(ref);
--
1.7.4.74.g639db
^ permalink raw reply related
* [PATCH 1/2] Convert many resolve_ref() calls to read_ref*() and ref_exists()
From: Nguyễn Thái Ngọc Duy @ 2011-11-13 10:22 UTC (permalink / raw)
To: git, Junio C Hamano; +Cc: Nguyễn Thái Ngọc Duy
In-Reply-To: <7vobwgo3l5.fsf@alter.siamese.dyndns.org>
resolve_ref() may return a pointer to a static buffer, which is not
safe for long-term use because if another resolve_ref() call happens,
the buffer may be changed. Many call sites though do not care about
this buffer. They simply check if the return value is NULL or not.
Convert all these call sites to new wrappers to reduce resolve_ref()
calls from 57 to 34. If we change resolve_ref() prototype later on
to avoid passing static buffer out, this helps reduce changes.
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
On Sun, Nov 13, 2011 at 2:59 PM, Junio C Hamano <gitster@pobox.com> wrote:
> But if we were to go that route, as the first step, it might make sense to
> rewrite "only checks if it returns NULL and uses sha1" callers to call
> either read_ref() or ref_exists(), so that we do not have to worry about
> leaking at such callers when we later decide to return allocated memory
> from resolve_ref().
Good idea.
builtin/branch.c | 5 ++---
builtin/checkout.c | 4 ++--
builtin/merge.c | 4 ++--
builtin/remote.c | 8 +++-----
builtin/replace.c | 4 ++--
builtin/show-ref.c | 2 +-
builtin/tag.c | 4 ++--
bundle.c | 2 +-
cache.h | 2 ++
notes-merge.c | 2 +-
refs.c | 27 ++++++++++++++++-----------
remote.c | 4 ++--
12 files changed, 36 insertions(+), 32 deletions(-)
diff --git a/builtin/branch.c b/builtin/branch.c
index 51ca6a0..0fe9c4d 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -186,7 +186,7 @@ static int delete_branches(int argc, const char **argv, int force, int kinds)
free(name);
name = xstrdup(mkpath(fmt, bname.buf));
- if (!resolve_ref(name, sha1, 1, NULL)) {
+ if (read_ref(name, sha1)) {
error(_("%sbranch '%s' not found."),
remote, bname.buf);
ret = 1;
@@ -565,7 +565,6 @@ static int print_ref_list(int kinds, int detached, int verbose, int abbrev, stru
static void rename_branch(const char *oldname, const char *newname, int force)
{
struct strbuf oldref = STRBUF_INIT, newref = STRBUF_INIT, logmsg = STRBUF_INIT;
- unsigned char sha1[20];
struct strbuf oldsection = STRBUF_INIT, newsection = STRBUF_INIT;
int recovery = 0;
@@ -577,7 +576,7 @@ static void rename_branch(const char *oldname, const char *newname, int force)
* Bad name --- this could be an attempt to rename a
* ref that we used to allow to be created by accident.
*/
- if (resolve_ref(oldref.buf, sha1, 1, NULL))
+ if (ref_exists(oldref.buf))
recovery = 1;
else
die(_("Invalid branch name: '%s'"), oldname);
diff --git a/builtin/checkout.c b/builtin/checkout.c
index 2a80772..beeaee4 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -288,7 +288,7 @@ static int checkout_paths(struct tree *source_tree, const char **pathspec,
commit_locked_index(lock_file))
die(_("unable to write new index file"));
- resolve_ref("HEAD", rev, 0, &flag);
+ read_ref_full("HEAD", rev, 0, &flag);
head = lookup_commit_reference_gently(rev, 1);
errs |= post_checkout_hook(head, head, 0);
@@ -866,7 +866,7 @@ static int parse_branchname_arg(int argc, const char **argv,
setup_branch_path(new);
if (!check_refname_format(new->path, 0) &&
- resolve_ref(new->path, branch_rev, 1, NULL))
+ !read_ref(new->path, branch_rev))
hashcpy(rev, branch_rev);
else
new->path = NULL; /* not an existing branch */
diff --git a/builtin/merge.c b/builtin/merge.c
index dffd5ec..42b4f9e 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -420,7 +420,7 @@ static struct object *want_commit(const char *name)
static void merge_name(const char *remote, struct strbuf *msg)
{
struct object *remote_head;
- unsigned char branch_head[20], buf_sha[20];
+ unsigned char branch_head[20];
struct strbuf buf = STRBUF_INIT;
struct strbuf bname = STRBUF_INIT;
const char *ptr;
@@ -479,7 +479,7 @@ static void merge_name(const char *remote, struct strbuf *msg)
strbuf_addstr(&truname, "refs/heads/");
strbuf_addstr(&truname, remote);
strbuf_setlen(&truname, truname.len - len);
- if (resolve_ref(truname.buf, buf_sha, 1, NULL)) {
+ if (ref_exists(truname.buf)) {
strbuf_addf(msg,
"%s\t\tbranch '%s'%s of .\n",
sha1_to_hex(remote_head->sha1),
diff --git a/builtin/remote.c b/builtin/remote.c
index c810643..407abfb 100644
--- a/builtin/remote.c
+++ b/builtin/remote.c
@@ -343,8 +343,7 @@ static int get_ref_states(const struct ref *remote_refs, struct ref_states *stat
states->tracked.strdup_strings = 1;
states->stale.strdup_strings = 1;
for (ref = fetch_map; ref; ref = ref->next) {
- unsigned char sha1[20];
- if (!ref->peer_ref || read_ref(ref->peer_ref->name, sha1))
+ if (!ref->peer_ref || !ref_exists(ref->peer_ref->name))
string_list_append(&states->new, abbrev_branch(ref->name));
else
string_list_append(&states->tracked, abbrev_branch(ref->name));
@@ -710,7 +709,7 @@ static int mv(int argc, const char **argv)
int flag = 0;
unsigned char sha1[20];
- resolve_ref(item->string, sha1, 1, &flag);
+ read_ref_full(item->string, sha1, 1, &flag);
if (!(flag & REF_ISSYMREF))
continue;
if (delete_ref(item->string, NULL, REF_NODEREF))
@@ -1220,10 +1219,9 @@ static int set_head(int argc, const char **argv)
usage_with_options(builtin_remote_sethead_usage, options);
if (head_name) {
- unsigned char sha1[20];
strbuf_addf(&buf2, "refs/remotes/%s/%s", argv[0], head_name);
/* make sure it's valid */
- if (!resolve_ref(buf2.buf, sha1, 1, NULL))
+ if (!ref_exists(buf2.buf))
result |= error("Not a valid ref: %s", buf2.buf);
else if (create_symref(buf.buf, buf2.buf, "remote set-head"))
result |= error("Could not setup %s", buf.buf);
diff --git a/builtin/replace.c b/builtin/replace.c
index 517fa10..4a8970e 100644
--- a/builtin/replace.c
+++ b/builtin/replace.c
@@ -58,7 +58,7 @@ static int for_each_replace_name(const char **argv, each_replace_name_fn fn)
had_error = 1;
continue;
}
- if (!resolve_ref(ref, sha1, 1, NULL)) {
+ if (read_ref(ref, sha1)) {
error("replace ref '%s' not found.", *p);
had_error = 1;
continue;
@@ -97,7 +97,7 @@ static int replace_object(const char *object_ref, const char *replace_ref,
if (check_refname_format(ref, 0))
die("'%s' is not a valid ref name.", ref);
- if (!resolve_ref(ref, prev, 1, NULL))
+ if (read_ref(ref, prev))
hashclr(prev);
else if (!force)
die("replace ref '%s' already exists", ref);
diff --git a/builtin/show-ref.c b/builtin/show-ref.c
index fafb6dd..3911661 100644
--- a/builtin/show-ref.c
+++ b/builtin/show-ref.c
@@ -225,7 +225,7 @@ int cmd_show_ref(int argc, const char **argv, const char *prefix)
unsigned char sha1[20];
if (!prefixcmp(*pattern, "refs/") &&
- resolve_ref(*pattern, sha1, 1, NULL)) {
+ !read_ref(*pattern, sha1)) {
if (!quiet)
show_one(*pattern, sha1);
}
diff --git a/builtin/tag.c b/builtin/tag.c
index 9b6fd95..439249d 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -174,7 +174,7 @@ static int for_each_tag_name(const char **argv, each_tag_name_fn fn)
had_error = 1;
continue;
}
- if (!resolve_ref(ref, sha1, 1, NULL)) {
+ if (read_ref(ref, sha1)) {
error(_("tag '%s' not found."), *p);
had_error = 1;
continue;
@@ -518,7 +518,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
if (strbuf_check_tag_ref(&ref, tag))
die(_("'%s' is not a valid tag name."), tag);
- if (!resolve_ref(ref.buf, prev, 1, NULL))
+ if (read_ref(ref.buf, prev))
hashclr(prev);
else if (!force)
die(_("tag '%s' already exists"), tag);
diff --git a/bundle.c b/bundle.c
index 08020bc..4742f27 100644
--- a/bundle.c
+++ b/bundle.c
@@ -320,7 +320,7 @@ int create_bundle(struct bundle_header *header, const char *path,
continue;
if (dwim_ref(e->name, strlen(e->name), sha1, &ref) != 1)
continue;
- if (!resolve_ref(e->name, sha1, 1, &flag))
+ if (read_ref_full(e->name, sha1, 1, &flag))
flag = 0;
display_ref = (flag & REF_ISSYMREF) ? e->name : ref;
diff --git a/cache.h b/cache.h
index 2e6ad36..5badece 100644
--- a/cache.h
+++ b/cache.h
@@ -832,6 +832,8 @@ static inline int get_sha1_with_context(const char *str, unsigned char *sha1, st
extern int get_sha1_hex(const char *hex, unsigned char *sha1);
extern char *sha1_to_hex(const unsigned char *sha1); /* static buffer result! */
+extern int read_ref_full(const char *filename, unsigned char *sha1,
+ int reading, int *flags);
extern int read_ref(const char *filename, unsigned char *sha1);
/*
diff --git a/notes-merge.c b/notes-merge.c
index e9e4199..e33c2c9 100644
--- a/notes-merge.c
+++ b/notes-merge.c
@@ -568,7 +568,7 @@ int notes_merge(struct notes_merge_options *o,
o->local_ref, o->remote_ref);
/* Dereference o->local_ref into local_sha1 */
- if (!resolve_ref(o->local_ref, local_sha1, 0, NULL))
+ if (read_ref_full(o->local_ref, local_sha1, 0, NULL))
die("Failed to resolve local notes ref '%s'", o->local_ref);
else if (!check_refname_format(o->local_ref, 0) &&
is_null_sha1(local_sha1))
diff --git a/refs.c b/refs.c
index e69ba26..44c1c86 100644
--- a/refs.c
+++ b/refs.c
@@ -334,7 +334,7 @@ static void get_ref_dir(const char *submodule, const char *base,
flag |= REF_ISBROKEN;
}
} else
- if (!resolve_ref(ref, sha1, 1, &flag)) {
+ if (read_ref_full(ref, sha1, 1, &flag)) {
hashclr(sha1);
flag |= REF_ISBROKEN;
}
@@ -612,13 +612,18 @@ struct ref_filter {
void *cb_data;
};
-int read_ref(const char *ref, unsigned char *sha1)
+int read_ref_full(const char *ref, unsigned char *sha1, int reading, int *flags)
{
- if (resolve_ref(ref, sha1, 1, NULL))
+ if (resolve_ref(ref, sha1, reading, flags))
return 0;
return -1;
}
+int read_ref(const char *ref, unsigned char *sha1)
+{
+ return read_ref_full(ref, sha1, 1, NULL);
+}
+
#define DO_FOR_EACH_INCLUDE_BROKEN 01
static int do_one_ref(const char *base, each_ref_fn fn, int trim,
int flags, void *cb_data, struct ref_entry *entry)
@@ -663,7 +668,7 @@ int peel_ref(const char *ref, unsigned char *sha1)
goto fallback;
}
- if (!resolve_ref(ref, base, 1, &flag))
+ if (read_ref_full(ref, base, 1, &flag))
return -1;
if ((flag & REF_ISPACKED)) {
@@ -746,7 +751,7 @@ static int do_head_ref(const char *submodule, each_ref_fn fn, void *cb_data)
return 0;
}
- if (resolve_ref("HEAD", sha1, 1, &flag))
+ if (!read_ref_full("HEAD", sha1, 1, &flag))
return fn("HEAD", sha1, flag, cb_data);
return 0;
@@ -826,7 +831,7 @@ int head_ref_namespaced(each_ref_fn fn, void *cb_data)
int flag;
strbuf_addf(&buf, "%sHEAD", get_git_namespace());
- if (resolve_ref(buf.buf, sha1, 1, &flag))
+ if (!read_ref_full(buf.buf, sha1, 1, &flag))
ret = fn(buf.buf, sha1, flag, cb_data);
strbuf_release(&buf);
@@ -1022,7 +1027,7 @@ int refname_match(const char *abbrev_name, const char *full_name, const char **r
static struct ref_lock *verify_lock(struct ref_lock *lock,
const unsigned char *old_sha1, int mustexist)
{
- if (!resolve_ref(lock->ref_name, lock->old_sha1, mustexist, NULL)) {
+ if (read_ref_full(lock->ref_name, lock->old_sha1, mustexist, NULL)) {
error("Can't verify ref %s", lock->ref_name);
unlock_ref(lock);
return NULL;
@@ -1377,7 +1382,8 @@ int rename_ref(const char *oldref, const char *newref, const char *logmsg)
goto rollback;
}
- if (resolve_ref(newref, sha1, 1, &flag) && delete_ref(newref, sha1, REF_NODEREF)) {
+ if (!read_ref_full(newref, sha1, 1, &flag) &&
+ delete_ref(newref, sha1, REF_NODEREF)) {
if (errno==EISDIR) {
if (remove_empty_directories(git_path("%s", newref))) {
error("Directory not empty: %s", newref);
@@ -1929,7 +1935,7 @@ static int do_for_each_reflog(const char *base, each_ref_fn fn, void *cb_data)
retval = do_for_each_reflog(log, fn, cb_data);
} else {
unsigned char sha1[20];
- if (!resolve_ref(log, sha1, 0, NULL))
+ if (read_ref_full(log, sha1, 0, NULL))
retval = error("bad ref for %s", log);
else
retval = fn(log, sha1, 0, cb_data);
@@ -2072,7 +2078,6 @@ char *shorten_unambiguous_ref(const char *ref, int strict)
*/
for (j = 0; j < rules_to_fail; j++) {
const char *rule = ref_rev_parse_rules[j];
- unsigned char short_objectname[20];
char refname[PATH_MAX];
/* skip matched rule */
@@ -2086,7 +2091,7 @@ char *shorten_unambiguous_ref(const char *ref, int strict)
*/
mksnpath(refname, sizeof(refname),
rule, short_name_len, short_name);
- if (!read_ref(refname, short_objectname))
+ if (ref_exists(refname))
break;
}
diff --git a/remote.c b/remote.c
index e2ef991..6655bb0 100644
--- a/remote.c
+++ b/remote.c
@@ -1507,13 +1507,13 @@ int stat_tracking_info(struct branch *branch, int *num_ours, int *num_theirs)
* nothing to report.
*/
base = branch->merge[0]->dst;
- if (!resolve_ref(base, sha1, 1, NULL))
+ if (read_ref(base, sha1))
return 0;
theirs = lookup_commit_reference(sha1);
if (!theirs)
return 0;
- if (!resolve_ref(branch->refname, sha1, 1, NULL))
+ if (read_ref(branch->refname, sha1))
return 0;
ours = lookup_commit_reference(sha1);
if (!ours)
--
1.7.4.74.g639db
^ permalink raw reply related
* Re: [PATCH] Copy resolve_ref() return value for longer use
From: Junio C Hamano @ 2011-11-13 7:59 UTC (permalink / raw)
To: Nguyen Thai Ngoc Duy; +Cc: git, Tony Wang
In-Reply-To: <CACsJy8ARAzNWjZfXwnNG0AprCFXLCkiDrE+eFj9icbeNX14xKw@mail.gmail.com>
Nguyen Thai Ngoc Duy <pclouds@gmail.com> writes:
> I went through all of them. Most of them only checks if return value
> is NULL, or does one-time string comparison. Others do xstrdup() to
> save the return value. Will update the patch message to reflect this.
Thanks. Given your analysis, I think the potential change I alluded to to
return allocated memory from the function is probably overkill in the
current state of the code.
But as the codepaths around the existing callsites evolve, some of these
call sites that are not problematic in today's code can become problematic
if we are not careful. I was primarily wondering if an updated API could
force us to be careful when making future changes.
And a change along the lines of your suggestion
> ... Though if you don't mind a bigger patch, we could turn
>
> const char *resolve_ref(const char *path, const char *sha1, int
> reading, int *flag);
>
> to
>
> int resolve_ref(const char *path, const char *sha1, int reading, int
> *flag, char **ref);
>
> where *ref is the current return value and is only allocated by
> resolve_ref() if ref != NULL.
might be one such updated API that makes mistakes harder to make. I dunno.
But if we were to go that route, as the first step, it might make sense to
rewrite "only checks if it returns NULL and uses sha1" callers to call
either read_ref() or ref_exists(), so that we do not have to worry about
leaking at such callers when we later decide to return allocated memory
from resolve_ref().
builtin/branch.c | 3 +--
builtin/merge.c | 4 ++--
2 files changed, 3 insertions(+), 4 deletions(-)
diff --git a/builtin/branch.c b/builtin/branch.c
index 51ca6a0..94948a4 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -565,7 +565,6 @@ static int print_ref_list(int kinds, int detached, int verbose, int abbrev, stru
static void rename_branch(const char *oldname, const char *newname, int force)
{
struct strbuf oldref = STRBUF_INIT, newref = STRBUF_INIT, logmsg = STRBUF_INIT;
- unsigned char sha1[20];
struct strbuf oldsection = STRBUF_INIT, newsection = STRBUF_INIT;
int recovery = 0;
@@ -577,7 +576,7 @@ static void rename_branch(const char *oldname, const char *newname, int force)
* Bad name --- this could be an attempt to rename a
* ref that we used to allow to be created by accident.
*/
- if (resolve_ref(oldref.buf, sha1, 1, NULL))
+ if (ref_exists(oldref.buf))
recovery = 1;
else
die(_("Invalid branch name: '%s'"), oldname);
diff --git a/builtin/merge.c b/builtin/merge.c
index dffd5ec..42b4f9e 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -420,7 +420,7 @@ static struct object *want_commit(const char *name)
static void merge_name(const char *remote, struct strbuf *msg)
{
struct object *remote_head;
- unsigned char branch_head[20], buf_sha[20];
+ unsigned char branch_head[20];
struct strbuf buf = STRBUF_INIT;
struct strbuf bname = STRBUF_INIT;
const char *ptr;
@@ -479,7 +479,7 @@ static void merge_name(const char *remote, struct strbuf *msg)
strbuf_addstr(&truname, "refs/heads/");
strbuf_addstr(&truname, remote);
strbuf_setlen(&truname, truname.len - len);
- if (resolve_ref(truname.buf, buf_sha, 1, NULL)) {
+ if (ref_exists(truname.buf)) {
strbuf_addf(msg,
"%s\t\tbranch '%s'%s of .\n",
sha1_to_hex(remote_head->sha1),
^ permalink raw reply related
* [ANNOUNCE] Git 1.7.8.rc2
From: Junio C Hamano @ 2011-11-13 7:27 UTC (permalink / raw)
To: git; +Cc: Linux Kernel
A release candidate Git 1.7.8.rc2 is available for testing.
Since rc0, we killed all known regressions. Because there won't be any
more new feature merged until the 1.7.8 final, it is a good time for the
coolest kids on the block to start using the upcoming release before
others do.
The release tarballs are found at:
http://code.google.com/p/git-core/downloads/list
and their SHA-1 checksums are:
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
b1cb030dee2b9ae024f4076fe5fadfea43edec4e git-1.7.8.rc2.tar.gz
cd30ce92f9518920ff8f9cdde0a8da5c856f6193 git-htmldocs-1.7.8.rc2.tar.gz
97d72c0c56e557eb3f11b9a3dcdb971e38eaee49 git-manpages-1.7.8.rc2.tar.gz
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.10 (GNU/Linux)
iQIcBAEBAgAGBQJOv287AAoJELC16IaWr+bLGJwP/1ZL+9NcBiIWshqxrYaBACF6
dgLsK1M7iJBn95ye6xBp57uPeLGK9iv5qLvo5Wxoog8raWgceR3qXjZL3snfYlLO
Hz2P8Zb9EF80QfVs7RjkQYAJRaT/WzbSxoQF/ZUXHyLk2BHpw6YfYA0Vj0JJZ72l
bCYMnppi18uxLuUyf6/0ftlkadxv3L58VIaNWAp8NwuLuskucx64LZgYzwkaRFfO
YNWgV0zwimK+9SF/gnQ5cw55GCurs69HWoDVLJbnQuJbjiU3Kl9jehYBwTo/rPSn
vo03Foh3mV6u3DTovZARLF54goJvvc+JKKkFAdeY+dNKLdZQRdO+hrldGTeGKxVk
XissJxmsHm/DkXc15yIPu+iV5wmAXVc8BpTzK1NgleuOyz2qnvVrmY+ZOVQfM1BA
4zS6PMp/sgEJa6ybmwXuYY/JpJlgmEBsDk6MJOJ8RKt+q0qovB+2ltMJbFpqVnQR
VEswTvhBOmUSfKhiY68UmqE4H6vmSO5yOmo1VQKgCKzN7glcG/3wpIHNK9+QW9yE
eOQMSDdBQGGgzz7Y+bVwpbOpSUsc49MYit8T9wx/haNNCA3Fud1UUKbT+060/zQw
Tgv2gi/6L/vCcDhj7oersNYfpgilggYBeiADKsI1fw6SHzBpbWblJZTtjr1yplaZ
YG+o9yPd2EwA2tkosG51
=hXu/
-----END PGP SIGNATURE-----
Also the following public repositories all have a copy of the v1.7.8.rc2
tag and the master branch that the tag points at:
url = git://repo.or.cz/alt-git.git
url = https://code.google.com/p/git-core/
url = git://git.sourceforge.jp/gitroot/git-core/git.git
url = git://git-core.git.sourceforge.net/gitroot/git-core/git-core
url = https://github.com/gitster/git
----------------------------------------------------------------
Changes since v1.7.8-rc1 are as follows:
Felipe Contreras (1):
remote: fix remote set-url usage
Jeff King (1):
docs: don't mention --quiet or --exit-code in git-log(1)
Junio C Hamano (5):
remote: fix set-branches usage
docs: Update install-doc-quick
Git 1.7.7.3
Update draft release notes to 1.7.8
Git 1.7.8-rc2
Liu Yuan (1):
mktree: fix a memory leak in write_tree()
SZEDER Gábor (1):
completion: don't leak variable from the prompt into environment
^ permalink raw reply
* Re: [PATCH] Copy resolve_ref() return value for longer use
From: Nguyen Thai Ngoc Duy @ 2011-11-13 7:09 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Tony Wang
In-Reply-To: <7vehxcpns5.fsf@alter.siamese.dyndns.org>
2011/11/13 Junio C Hamano <gitster@pobox.com>:
> The above log message with the callchain would be a perfect explanation
> for a patch to fix builtin/merge.c and nothing else, but among 50+
> callsites of resolve_ref(), only 5 places are modified in this patch, and
> it is not explained in the commit log message at all how they were chosen.
> Were the other 40+ or so places inspected and deemed to be OK? Or is this
> "I just did a few of them in addition to the immediate need of fixing what
> was reported, and the rest are left as an exercise to the readers" patch?
I went through all of them. Most of them only checks if return value
is NULL, or does one-time string comparison. Others do xstrdup() to
save the return value. Will update the patch message to reflect this.
> Some variables that receive xstrdup()'ed result with this patch are
> globals whose lifetime lasts while the process is alive, and I do not
> think it is a huge problem that this patch does not free it, but it seems
> the patch does not free some other function scope variables once their use
> is done even when it is fairly easy to do so.
Will fix.
> How much overhead would it incur if we return xstrdup()'ed memory from the
> resolve_ref() and make it the caller's responsibility to free it? Would it
> make the calling site too ugly?
Given a lot of callsites do "if (resolve_ref(...)) ...", yes it may
look ugly. Though if you don't mind a bigger patch, we could turn
const char *resolve_ref(const char *path, const char *sha1, int
reading, int *flag);
to
int resolve_ref(const char *path, const char *sha1, int reading, int
*flag, char **ref);
where *ref is the current return value and is only allocated by
resolve_ref() if ref != NULL.
--
Duy
^ permalink raw reply
* Re: [PATCH] Copy resolve_ref() return value for longer use
From: Junio C Hamano @ 2011-11-13 5:57 UTC (permalink / raw)
To: Nguyễn Thái Ngọc Duy; +Cc: git, Tony Wang
In-Reply-To: <1320719428-1802-1-git-send-email-pclouds@gmail.com>
Nguyễn Thái Ngọc Duy <pclouds@gmail.com> writes:
> resolve_ref() may return a pointer to a static buffer. Callers that
> use this value outside of a block should copy the value to avoid some
> hidden resolve_ref() call that may change the static buffer's value.
>
> The bug found by Tony Wang <wwwjfy@gmail.com> in builtin/merge.c
> demonstrates this. The first call is in cmd_merge()
>
> branch = resolve_ref("HEAD", head_sha1, 0, &flag);
>
> Then deep in lookup_commit_or_die() a few lines after, resolve_ref()
> may be called again and destroy "branch".
>
> lookup_commit_or_die
> lookup_commit_reference
> lookup_commit_reference_gently
> parse_object
> lookup_replace_object
> do_lookup_replace_object
> prepare_replace_object
> for_each_replace_ref
> do_for_each_ref
> get_loose_refs
> get_ref_dir
> get_ref_dir
> resolve_ref
>
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
> ---
Thanks.
The above log message with the callchain would be a perfect explanation
for a patch to fix builtin/merge.c and nothing else, but among 50+
callsites of resolve_ref(), only 5 places are modified in this patch, and
it is not explained in the commit log message at all how they were chosen.
Were the other 40+ or so places inspected and deemed to be OK? Or is this
"I just did a few of them in addition to the immediate need of fixing what
was reported, and the rest are left as an exercise to the readers" patch?
Some variables that receive xstrdup()'ed result with this patch are
globals whose lifetime lasts while the process is alive, and I do not
think it is a huge problem that this patch does not free it, but it seems
the patch does not free some other function scope variables once their use
is done even when it is fairly easy to do so.
How much overhead would it incur if we return xstrdup()'ed memory from the
resolve_ref() and make it the caller's responsibility to free it? Would it
make the calling site too ugly?
^ permalink raw reply
* Re: git diff --numstat <branch> always shows dirty submodules
From: Gelonida N @ 2011-11-13 0:45 UTC (permalink / raw)
To: git
In-Reply-To: <4EBED0ED.7060005@web.de>
Hi Jens,
Thanks a lot for your answer.
On 11/12/2011 09:02 PM, Jens Lehmann wrote:
> Am 12.11.2011 14:29, schrieb Gelonida N:
>> I recently started using submodules and they behave mostly as I like to.
>> Normally I use diff --numstat<branch>
>> to check quickly whether I am aligned with another branch or not.
>> . . .
>> Is there any quick way flag / helper script / . . .
>> to show differences between two branches without raising the fact, that
>> submodules are dirty?
>
> Yes, there is the "--ignore-submodules" command line option and the
> diff.ignoreSubmodules (which can be set globally and/or per repo) and
> submodule.<name.>ignore configuration settings. They can be set to
> "untracked", "dirty" or "all" to control what you want to see.
I tried this immediately and at works perfectly for git diff.
Please see also my comments at the end of this post
>
> Did you check areas in the Documentation where you did expect to find
> them mentioned but they weren't? Then please say so that we can fix
> that.
>
Apologies. My bad. I must have read the output of
git help diff
too quickly.
It is there plain as the day. :-(
>>> From a user perspective I don't see why this is reported.
>> I am not being warned about dirty files in the top level repository
>
> This is so you can't forget to add new files inside the submodule,
> which can lead to breakage when other people clone the superproject
> but won't get the new files from the submodule because you didn't
> commit them there.
Well I wouldn't expect to find this kind of info (by default) in the
output of a git-diff.
If you git-diff two branches of a project without submodules it doesn't
tell you either, that you have untracked files in the repository.
I would have expected this kind of output just as result of
'git status'
(as it is already today)
# modified: submodule (untracked content)
Git status reports untracked files in the super project AND in it
reports, that there are untracked files in the submodule.
So this seems to be more consistent to me than the diff case.
On the other hand I would consider it usefult if git status could
optionally report the complete list of untracked files also for the
submodules
(So far I didn't search in depth in doc whether there is a switch
for it.)
A first shot was reading the output of git help status:
> If status.submodulesummary is set to a non zero number or true (identical to -1 or an unlimited number), the
> submodule summary will be enabled for the long format and a summary of commits for modified submodules will
> be shown (see --summary-limit option of git-submodule(1)).
I put thus following lines in .git/config of my repository
status]
submodulesummary = true
but the untracked files of my submodule were not reported. (will follow
the doc of git-submodule)
What is also confusing to me is, that the setting
diff.ignoreSubmodules is also being used by git status.
There seems to be no variable
status.ignoreSubmodules
So it seems impossible (without aliases) to
have git diff NOT report the untracked files,
but git status report them.
I guess I'll go for a solution with git aliases
^ permalink raw reply
* Re: [PATCH 5/5] sequencer: revert d3f4628e
From: Jonathan Nieder @ 2011-11-12 22:40 UTC (permalink / raw)
To: Ramkumar Ramachandra; +Cc: Git List, Junio C Hamano, Christian Couder
In-Reply-To: <CALkWK0=QHUeKH6ccVLYJVW_RxXbEaLfwafTVzJ94+s49j=8QjA@mail.gmail.com>
Ramkumar Ramachandra wrote:
> I'm trying to
> "effectively port the inverse of the changes made by d3f4628e in
> revert.c to sequencer.c" -- would you still like to see a git-revert
> style commit message? Don't you think it'll be misleading?
My main complaint is that the subject line (and then the body) didn't
tell me what effect the patch would have in a self-contained way.
I don't think a git-revert style commit message would be misleading.
Couldn't you avoid confusing people by providing the relevant
information directly? "This commit was not made with 'git revert',
since there has been too much code reorganization in the meantime;
instead, I applied the inverse of the changes made by d3f4628e by
hand. This patch also tweaks the test added in that commit instead of
removing it."
> Sorry about the shoddy commit messages though: I'm polishing the
> series now that I'm convinced that it's heading in the right
> direction. Hopefully, I'll have more to show soon.
Thanks. I'll try not to be distracted and to just focus on the code
for the next round.
^ permalink raw reply
* Re: [RFC/PATCH] remote: add new sync command
From: Felipe Contreras @ 2011-11-12 22:07 UTC (permalink / raw)
To: Jeff King; +Cc: git
In-Reply-To: <20111111181352.GA16055@sigill.intra.peff.net>
On Fri, Nov 11, 2011 at 8:13 PM, Jeff King <peff@peff.net> wrote:
> On Fri, Nov 11, 2011 at 02:30:48PM +0200, Felipe Contreras wrote:
>
>> > Doesn't --all mean all refs, including tags and branches?
>>
>> Nope, only branches, try it. I also found it strange. And what is
>> more, you can't use --all and --tags at the same time. Totally
>> strange.
>
> Yeah, you're right. Sorry for my confusion.
>
> So in that sense, it is poorly named, and "--branches" (or "--heads")
> would be more accurate. At the same time, it is probably more likely
> what the user wants to do (you almost never want to push "refs/remotes",
> for example).
But you do want to push tags, and --all --tags doesn't sound right; if
I'm pushing everything, why do I specify I want to push more stuff.
And then, why it --all --tags disallowed?
> So I am a little hesitant to suggest changing it, even
> with a warning and deprecation period.
It is confusing and wrong, what more reason do you need?
>> And yes, in this particular use-case that's what I am trying to avoid,
>> but in other use-cases (like creating a new repo and pushing
>> *everything*), a *true* --all would be nice.
>
> Right. It looks like that is just spelled "--mirror" (which gives you
> pruning also), or "refs/*:refs/*" (without pruning). The latter is even
> more flexible, as you could do "refs/*:refs/foo/*" to keep several
> related backups in one upstream repo.
So, we agree that --all is the same as 'refs/heads/*'. Therefore we
already have this mixture of refspecs and options.
> Just to get back to the original patch for a second: are we in agreement
> that what you want to do with "sync" is almost possible now (modulo a
> separate --prune option), and that there is a separate issue of making
> friendlier syntax for a few common refspecs?
Yes.
>> > We could add syntactic sugar to make
>> > --branches mean "refs/heads/*". But I do worry that pseudo-options like
>> > that just end up creating more confusion (I seem to recall there being a
>> > lot of confusion about "--tags", which is more or less the same thing).
>>
>> But it's not, that could explain part of the confusion. I think these
>> would be totally intuitive.
>>
>> --branches
>> --tags
>> --other
>> --all
>> --update
>> --prune
>
> My problem with them syntactically is that you have option-looking
> things that are really not flags, but rather syntactic placeholders for
> refspecs.
We already have those: --all -> 'ref/heads/*', --tags -> 'refs/tags/*'.
> So you have:
>
> git push --prune remote --branches
>
> which looks wrong from the perspective of usual option-parsing rules.
> And does:
>
> git push remote --prune --branches
>
> work? Or:
>
> git push --prune --branches remote
>
> ?
>
> I think we could make them all work if we want. But somehow the syntaxes
> just look wrong to me. Using a non-dashed placeholder for special
> refspecs makes more sense to me like:
I don't see any problem with making them all work.
> git push --prune remote BRANCHES
>
> and then it really is just a special way of spelling "refs/heads/*". But
> then, I also think it's good for users to understand that the options
> are refspecs, and what that means. It's not a hard concept, and then
> when they inevitably say "how can I do BRANCHES, except put the result
> somewhere else in the remote namespace", it's a very natural extension
> to learn about the right-hand side o the refspec.
>
> Of course I also think BRANCHES looks ugly, and people should just learn
> "refs/heads/*".
Look, I'm all in favor of people learning stuff, but I have been
involved in Git since basically day 1, and up to this day I was (am?)
not familiar with refspecs, I don't use them regularly, and never
really had a need to, and that's fine. People are already complaining
about the learning curve of git, and what you are suggesting is that:
Instead of doing:
% git push remote --branches --tags
They should do:
% git push remote 'refs/heads/*' 'refs/tags/*'
I disagree, I think refspecs should remain as plumbing, and there must
be a porcelain way to achieve the options I proposed.
> I dunno. Maybe my brain is fried from knowing too much about how git
> works. I don't have much of an "ordinary user" perspective anymore.
Maybe :)
>> But what about 'git fetch'? You didn't comment anything. I think we
>> should try to be consistent in these imaginary future options, maybe
>> to devise a transition, or at least to identify good names for the new
>> options.
>
> Yeah, fetch makes it harder because the options may have subtly
> different meanings. Using non-option placeholders would work around
> that. You would still have options for pruning, which to me is not
> really a refspec, but an option that acts on the refspecs.
>
> From the list in your previous email, you wrote:
>
>> --prune -> rename to --prune-tracking?
>> --prune-local (new; prune local branches that don't exist on the remote)
>
> I think --prune wouldn't need renaming. If you fetch into tracking
> branches, then pruning would prune tracking branches. If you fetch as a
> mirror (refs/*:refs/*), then you would prune everything.
I'm not going to investigate the subtleties of these different setups,
I'm going to put my common user hat and ask; how do I fetch as a
mirror?
> And "--prune-local" doesn't seem like a fetch operation to me. Either
> you are mirroring, and --prune already handles it as above. Or you are
> interested in getting rid of branches whose upstream has gone away. But
> that's not a fetch operation; that's a branch operation.
This would make things more confusing to the user.
Say on one side I do this push?
% git push test --prune 'refs/heads/*' 'refs/tags/*'
What do I do in the other side to synchronize the repo?
% git fetch test --prune-local 'refs/heads/*:refs/heads/*'
'refs/tags/*:refs/tags/*'
I would prefer this of course:
% git fetch test --all --prune-local
But you are saying it should be:
% git fetch test 'refs/heads/*:refs/heads/*' 'refs/tags/*:refs/tags/*'
% git branch --prune-remote test
That doesn't sound right to me; mixing branch operations with a specific remote?
Again, I think conceptually it's much easier to think about these
operations to be related to a specific remote, sure, fetch and push
mostly deal with specific remotes, but even more 'git remote'.
--
Felipe Contreras
^ permalink raw reply
* Re: Update Linux kernel branch source from GIT
From: Thiago Farina @ 2011-11-12 20:59 UTC (permalink / raw)
To: Eric Kom; +Cc: Junio C Hamano, git
In-Reply-To: <4EBCB737.4090100@kom.za.net>
On Fri, Nov 11, 2011 at 3:48 AM, Eric Kom <erickom@kom.za.net> wrote:
> Good day,
>
> Am new on this list, and also compile the kernel linux from git after
> clone. since I don't use the tar kernel version, am use to clone a new
> kernel branch instead to update it via patch.
>
> Please can you explain to me how to make a patch after clone it?
>
To make a patch?
I think you want something like this (uncompleted/untested offhand):
$ git checkout -b my-patch origin/master
# hack on some files
$ git commit -a -s
# edit commit message
$ git format-patch
$ git send-email
Hugely these are the commands I'd use to make a change and send it out to LKML.
^ permalink raw reply
* Re: git diff --numstat <branch> always shows dirty submodules
From: Jens Lehmann @ 2011-11-12 20:02 UTC (permalink / raw)
To: Gelonida N; +Cc: git
In-Reply-To: <j9lsc1$4uf$1@dough.gmane.org>
Am 12.11.2011 14:29, schrieb Gelonida N:
> I recently started using submodules and they behave mostly as I like to.
Good to hear that.
> Normally I use diff --numstat<branch>
> to check quickly whether I am aligned with another branch or not.
>
> The (for me) annoying feature of submodules is, that they are always
> reported to be different due to files, which are not under git.
>
>
> I type git diff --numstat master
> I get
> 1 1 mysubmodule
>
>
> Now I check the differences with git diff master mysubmodule
> diff --git a/mysubmodule b/mysubmodule
> index 1382b73..f4f1f1d 160000
> --- a/mysubmodule
> +++ b/mysubmodule
> @@ -1 +1 @@
> -Subproject commit xxxxxxxxx
> +Subproject commit xxxxxxxxx-dirty
>
> So the only difference (which I wasn't interested in) is, that the
> submodule is dirty.
>
> Is there any quick way flag / helper script / . . .
> to show differences between two branches without raising the fact, that
> submodules are dirty?
Yes, there is the "--ignore-submodules" command line option and the
diff.ignoreSubmodules (which can be set globally and/or per repo) and
submodule.<name.>ignore configuration settings. They can be set to
"untracked", "dirty" or "all" to control what you want to see.
Did you check areas in the Documentation where you did expect to find
them mentioned but they weren't? Then please say so that we can fix
that.
>> From a user perspective I don't see why this is reported.
> I am not being warned about dirty files in the top level repository
This is so you can't forget to add new files inside the submodule,
which can lead to breakage when other people clone the superproject
but won't get the new files from the submodule because you didn't
commit them there.
^ permalink raw reply
* Re: Verifying recent tags in git.git
From: Stefan Naewe @ 2011-11-12 19:55 UTC (permalink / raw)
To: git
In-Reply-To: <4EBEA053.6040109@ramsay1.demon.co.uk>
Ramsay Jones <ramsay <at> ramsay1.demon.co.uk> writes:
>
> Hi Junio,
>
> I noticed that the v1.7.8-rc1 tag took about 24 hours to appear in the
> kernel.org (and repo.or.cz) repository after you announced it and actually
> pushed the branch out.
>
> [...]
> Note the key ID 96AFE6CB.
>
> Are you planning to create an junio-gpg-pub-v2 tag? (or are you making it
> available from a keyserver?)
What about this:
http://pgp.mit.edu:11371/pks/lookup?search=0x96AFE6CB&op=index&fingerprint=on
Stefan
^ permalink raw reply
* RFC: post-fetch hook
From: Joey Hess @ 2011-11-12 19:44 UTC (permalink / raw)
To: git
[-- Attachment #1: Type: text/plain, Size: 1149 bytes --]
A post-fetch hook would run on the local repository after a git-fetch or
git-pull. It would probably make sense for the hook to be sent the same
format input as the post-receive hook (although I don't need that information
myself). Like post-receive, it would not affect the outcome of the fetch. It
should be called late enough that it can manipulate the git repository
using the fetched refs.
My use case is in git-annex. It maintains its own, rather notes-like
branch. When remote versions of the branch have changed, they are
automatically merged into the local branch, using a union merge,
the next time git-annex is run. So normally it does not need this hook.
But, consider the case where git-annex has locally modified its branch,
and the remote tracking branch has also been modified. Now the user does
"git pull; git push". With the two git-annex branches diverged the
push fails. The user has to manually run "git annex merge" before
pushing to handle this.
If there was a post-fetch hook, git-annex could make it always run
git annex merge, and users would not need to deal with this situation.
--
see shy jo
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 828 bytes --]
^ permalink raw reply
* Re: Git: Unexpected behaviour?
From: Junio C Hamano @ 2011-11-12 19:37 UTC (permalink / raw)
To: J.V.; +Cc: git
In-Reply-To: <4EBDBCA2.7070603@gmail.com>
"J.V." <jvsrvcs@gmail.com> writes:
> OK so "work tree" is a new term for me. I thought we were in isolated
> sandboxes called "branches" and changes made in a branch would stay in
> that branch regardless.
Do not think of "branches" as isolated _sandboxes_.
Rather, "branches" are where the independent states are to be _recorded_.
The recorded states only exist in the git repository, and to use its
contents (e.g. view in the pager or browser, edit in the editor, run the
compiler on,...), you need to materialize the contents of the branch
somewhere on the filesystem. Such a set of files on the filesystem form
the working tree. The act of doing so is called "checking out a branch".
After you check out a branch, your working tree can be used to record an
updated state to the branch, but notice the "can be" part. Changes you
make to the working tree are _not_ associated to the branch until you make
them so by committing. They are floating on top of the branch you have
currently checked out in your working tree, and "floating" was the key
part lacking in your understanding that started this thread. You are
allowed to check out another branch while you have a local change in the
working tree, and this is deliberately so. People often start working on a
branch (that is, they want to make a change and check out a branch, or
they happen to have a check-out of a branch and then the find something
they want to change), and then realize that the change logically does not
belong to the currently checked-out branch but some other branch. They
need to be able to check out another branch without losing the change they
already made in their working tree, and for such usage, the workflow
should look like:
$ git checkout master ;# on master branch
$ edit hello.c ;# some feature being added
... realize that this change does not belong to the master
... branch but is part of the "hello" branch you have been
... working on for the past few days
$ git checkout hello ;# check out the correct branch
... this keeps the local modification in hello.c (as long as
... the file you modified are the same between master and hello
... branches). Keep working on it and then finally...
$ git commit ;# on hello branch.
There is another unrelated use case in which people have local changes in
their working tree, but need to check out a different branch. The most
common is while you are working on a large feature that is not finished
yet on your "feature" branch, you hear from your boss that one trivial fix
for an urgent bug must be committed and pushed out on the "master" branch.
The feature you have in your working tree does not have anything to do
with the bug or the fix you are going to make for your boss. In such a
case, you would want to save away the changes for the feature, check out
the "master" branch and commit the fix, i.e.
$ git checkout feature ;# on feature
$ edit foo bar baz ;# some complicated change
... boss comes
$ git stash ;# stash away the current change
or
$ git commit -a -m 'wip'
$ git checkout master ;# emergency
$ edit ... ;# quickfix
$ git commit -m 'urgent fix...'
... emergency dealt with
$ git checkout feature
... back to what was being worked on
$ git stash pop
or
$ git reset HEAD^
^ 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