* Re: t5800-*.sh: Intermittent test failures
From: Sverre Rabbelier @ 2011-08-11 21:39 UTC (permalink / raw)
To: Ramsay Jones; +Cc: GIT Mailing-list, Jeff King, Jonathan Nieder, Junio C Hamano
In-Reply-To: <4E417CB4.50007@ramsay1.demon.co.uk>
Heya,
On Tue, Aug 9, 2011 at 20:30, Ramsay Jones <ramsay@ramsay1.demon.co.uk> wrote:
> The git-fast-import is hung in the read() syscall waiting for data which will
> never arrive. This is because the git(fast-export) process, started by the above
> git(push), executes (producing it's data on stdout) and completes successfully
> and exits *before* the above git-fast-import process starts.
>
> I haven't looked to see how the git(fast-export)/git-fast-import processes are
> plumbed together, but there seems to be a synchronization problem somewhere ...
This seems odd, before the fast-export process is even started it's
stdout are wired to the stdin of the helper (and thus the fast-import
process). What indication do you have that fast-import hasn't started
and that fast-export has finished?
Also, you say git remote-test everywhere, but it should be git
remote-testgit, typo?
--
Cheers,
Sverre Rabbelier
^ permalink raw reply
* Re: [PATCH 6/6] sequencer: Remove sequencer state after final commit
From: Jonathan Nieder @ 2011-08-11 20:17 UTC (permalink / raw)
To: Ramkumar Ramachandra
Cc: Git List, Junio C Hamano, Christian Couder, Daniel Barkalow,
Jeff King
In-Reply-To: <1313088705-32222-7-git-send-email-artagnon@gmail.com>
Hi,
Ramkumar Ramachandra wrote:
> --- a/builtin/commit.c
> +++ b/builtin/commit.c
> @@ -26,6 +26,7 @@
> #include "unpack-trees.h"
> #include "quote.h"
> #include "submodule.h"
> +#include "sequencer.h"
>
> static const char * const builtin_commit_usage[] = {
> "git commit [options] [--] <filepattern>...",
> @@ -1521,7 +1522,11 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
> unlink(git_path("MERGE_MODE"));
> unlink(git_path("SQUASH_MSG"));
>
> - if (commit_index_files())
> + /* Remove sequencer state if we just finished the last insn */
> + if (sequencer_count_todo() == 1)
> + remove_sequencer_state(1);
> +
> + if (commit_index_files())
> die (_("Repository has been updated, but unable to write\n"
Whitespace damage?
I think even this is too much knowledge of sequencer details on "git
commit"'s part. It should be enough to say
Dear sequencer, I'm commiting!
and let the sequencer take care of the rest.
^ permalink raw reply
* Re: [PATCH 5/6] sequencer: Expose API to cherry-picking machinery
From: Jonathan Nieder @ 2011-08-11 20:16 UTC (permalink / raw)
To: Ramkumar Ramachandra
Cc: Git List, Junio C Hamano, Christian Couder, Daniel Barkalow,
Jeff King
In-Reply-To: <1313088705-32222-6-git-send-email-artagnon@gmail.com>
Ramkumar Ramachandra wrote:
> --- a/sequencer.h
> +++ b/sequencer.h
> @@ -7,7 +7,32 @@
> #define SEQ_TODO_FILE "sequencer/todo"
> #define SEQ_OPTS_FILE "sequencer/opts"
>
> +#define COMMIT_MESSAGE_INIT { NULL, NULL, NULL, NULL, NULL };
I don't think this should be exposed. The rest seems pretty sane,
though I haven't read the patch carefully.
^ permalink raw reply
* Re: [PATCH 4/6] revert: Allow mixed pick and revert instructions
From: Jonathan Nieder @ 2011-08-11 20:12 UTC (permalink / raw)
To: Ramkumar Ramachandra
Cc: Git List, Junio C Hamano, Christian Couder, Daniel Barkalow,
Jeff King
In-Reply-To: <1313088705-32222-5-git-send-email-artagnon@gmail.com>
Ramkumar Ramachandra wrote:
> Change the way the instruction parser works, allowing arbitrary
> (action, operand) pairs to be parsed.
The first part of this sentence is not very satisfying. Maybe it
means something like
Parse the instruction list in .git/sequencer/todo as a list
of (action, operand) pairs, instead of assuming all instructions
use the same action.
[...]
> This patch lays the foundation for extending the parser to support
> more actions so 'git rebase -i' can reuse this machinery in the
> future.
Exciting stuff. :)
[...]
> +++ b/builtin/revert.c
[...]
> @@ -457,7 +456,8 @@ static int run_git_commit(const char *defmsg, struct replay_opts *opts)
> return run_command_v_opt(args, RUN_GIT_CMD);
> }
>
> -static int do_pick_commit(struct commit *commit, struct replay_opts *opts)
> +static int do_pick_commit(struct commit *commit, enum replay_action action,
> + struct replay_opts *opts)
[...]
> @@ -517,7 +517,8 @@ static int do_pick_commit(struct commit *commit, struct replay_opts *opts)
> /* 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));
> + action == REPLAY_REVERT ? "revert" : "cherry-pick",
> + sha1_to_hex(parent->object.sha1));
My first thought was "why stop using the helper function action_name"?
But now I see that it previously came from "opts" (i.e., the command
line) and now comes from the todo file.
The command name there was never really important except when
cherry-pick or revert is being called by a script, and the message
indicates which command was having trouble parsing the commit. If I
am using "git cherry-pick --continue" to continue after a failed
revert, I suspect action_name(opts) ["cherry-pick: "] would actually be
more sensible than the command name corresponding to the particular
pick/revert line.
[...]
> - return NULL;
> + return error(_("Unrecognized action: %s"), start);
Probably should be mentioned in the commit message. Doesn't this
print the problematic line and all lines after it?
Maybe something like
len = strchrnul(p, '\n') - p;
if (len > 255)
len = 255;
return error(_("Unrecognized action: %.*s"), (int) len, p);
would do.
[...]
> -static int parse_insn_buffer(char *buf, struct commit_list **todo_list,
> - struct replay_opts *opts)
> +static int parse_insn_buffer(char *buf, struct replay_insn_list **todo_list)
> {
> - struct commit_list **next = todo_list;
> - struct commit *commit;
> + 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++) {
> - commit = parse_insn_line(p, opts);
> - if (!commit)
> + if (parse_insn_line(p, &item) < 0)
> return error(_("Could not parse line %d."), i);
Could we can make this error message more clearly suggest that it's
giving context to the error above it? For example, something vaguely
like
error: unrecognized action: reset c78a78c9 Going back
error: on line 7
fatal: unusable instruction sheet ".git/sequencer/todo"
hint: to continue after fixing it, use "git cherry-pick --continue"
hint: or to bail out, use "git cherry-pick --abort"
Does a "cherry-pick --continue" in this scenario skip the first commit
in the todo list? Should it?
> --- a/t/t3510-cherry-pick-sequence.sh
> +++ b/t/t3510-cherry-pick-sequence.sh
> @@ -240,4 +240,62 @@ test_expect_success 'missing commit descriptions in instruction sheet' '
> test_cmp expect actual
> '
>
> +test_expect_success 'revert --continue continues after cherry-pick' '
Haven't read the tests yet. The general idea of this patch still
seems very sane.
^ permalink raw reply
* Re: [PATCH 0/2] Add an update=none option for 'loose' submodules
From: Heiko Voigt @ 2011-08-11 20:00 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Jens Lehmann
In-Reply-To: <7v8vqzreeo.fsf@alter.siamese.dyndns.org>
Hi Junio,
On Thu, Aug 11, 2011 at 11:28:31AM -0700, Junio C Hamano wrote:
> Heiko Voigt <hvoigt@hvoigt.net> writes:
>
> > If a submodule is used to seperate some bigger parts of a project into
> > an optional directory it is helpful to not clone/update them by default.
>
> Sorry if I am slow, but I do not get this.
>
> I thought unless you say "submodule init" once, a submodule you are not
> interested in should not be cloned nor updated at all. If that is not the
> case, isn't it a bug to be fixed without a new configuration variable that
> fixes it only when it is set?
What I usually do is say "submodule init" without any extra option once.
That will register all submodules from .gitmodules in the config. Now
when I say "submodule update" all submodules would be cloned. In the
case of recursive submodules actually
git submodule update --init --recursive
is the only command which can get you really everything in one go.
Do you think the "submodule init" behavior is wrong? If so I think its a
bit late to change this since people using submodules (me included)
already have got used to it.
With this config variable all submodules will still be registered to
.git/config on "submodule init" but "submodule update" will skip those
submodules. Since we already have merge and rebase as alternate options
to update a submodule it just sounds logical to me to have an additional
option to disable updating.
> > We have been talking about loose submodules for some time:
>
> Also before introducing a new terminology "loose submodule", please define
> it somewhere. It feels confusing to me that a normal submodule, which
> shouldn't be auto-cloned nor auto-updated without "submodule init", needs
> to be called by a name other than simply a "submodule" but with an
> adjuctive "loose submodule".
Thats why I avoided talking about it in the docs. For the commit message
I thought it would be kind of intuitive but I can update the commit
message so that it becomes more clear.
Cheers Heiko
^ permalink raw reply
* Re: [PATCH 3/6] revert: Parse instruction sheet more cautiously
From: Jonathan Nieder @ 2011-08-11 19:47 UTC (permalink / raw)
To: Ramkumar Ramachandra
Cc: Git List, Junio C Hamano, Christian Couder, Daniel Barkalow,
Jeff King
In-Reply-To: <1313088705-32222-4-git-send-email-artagnon@gmail.com>
Ramkumar Ramachandra wrote:
> Fix a buffer overflow bug by checking that parsed SHA-1 hex will fit
> in the buffer we've created for it.
Nit: it seems best to either describe the behavior or the code change.
So, for example:
Do not overflow a buffer when the second word in a "pick"
or "revert" line in .git/sequencer/todo is very long.
Or:
Check that the commit name argument to a "pick" or "revert"
action is not too long, to avoid overflowing an on-stack
buffer.
It would also be comforting to readers to mention when the bug was
introduced, so they don't start worrying about protecting their
installations against privilege escalation:
This fixes a regression introduced by <...>, which has not
escaped into the wild yet, luckily.
Could we have a test for this?
> Also change the instruction sheet format
How is this "Also" part related to the earlier bit? Does one make the
other easier, or is it more of a "While we're changing this code" kind
of thing?
> subtly so that a description of the commit after the object
> name is optional. So now, an instruction sheet like this is perfectly
> valid:
>
> pick 35b0426
> pick fbd5bbcbc2e
> pick 7362160f
Sounds convenient. :) Thanks.
> --- a/builtin/revert.c
> +++ b/builtin/revert.c
> @@ -697,26 +697,24 @@ static struct commit *parse_insn_line(char *start, struct replay_opts *opts)
> unsigned char commit_sha1[20];
> char sha1_abbrev[40];
> enum replay_action action;
> - int insn_len = 0;
> - char *p, *q;
> + char *p = start, *q, *end = strchrnul(start, '\n');
By the way, why are these non-const? (Not about this patch.)
I don't know why, but I would be more comfortable reading something
like this:
const char *p, *q;
p = start;
if (!prefixcmp(p, "pick ")) {
action = CHERRY_PICK;
p += strlen("pick ");
} else if (...) {
...
} ...
q = p + strcspn(p, " \n");
if (q - p + 1 > sizeof(...))
...
...
Maybe because I can imagine how the pointers move, always forward and
never past the end of the line.
> --- a/t/t3510-cherry-pick-sequence.sh
> +++ b/t/t3510-cherry-pick-sequence.sh
> @@ -211,4 +211,33 @@ test_expect_success 'malformed instruction sheet 2' '
> test_must_fail git cherry-pick --continue
> '
>
> +test_expect_success 'missing commit descriptions in instruction sheet' '
What assertion does this test check? "commit descriptions in insn
sheet are optional"?
> + pristine_detach initial &&
> + test_must_fail git cherry-pick base..anotherpick &&
A failed cherry-pick.
> + echo "c" >foo &&
> + git add foo &&
> + git commit &&
Resolving the conflict.
> + cut -d" " -f1,2 .git/sequencer/todo >new_sheet &&
> + cp new_sheet .git/sequencer/todo &&
Mucking about with the insn sheet.
> + git cherry-pick --continue &&
Continuing.
> + 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
Checking that the cherry-pick succeeded and the resulting list of
commits. How is this expected to potentially fail? Maybe something
like
git rev-list HEAD >commits &&
test_line_count = 4 commits
or
git diff --exit-code <something>
would make what this is intended to check clearer. As hinted above,
some blank lines or comments might make the earlier part easier to
read.
Thanks, this patch does two good things. For what it's worth, with
the changes hinted at above and a split into two patches if that seems
sensible,
Acked-by: Jonathan Nieder <jrnieder@gmail.com>
^ permalink raw reply
* Re: [PATCH 2/6] revert: Free memory after get_message call
From: Jonathan Nieder @ 2011-08-11 19:24 UTC (permalink / raw)
To: Ramkumar Ramachandra
Cc: Git List, Junio C Hamano, Christian Couder, Daniel Barkalow,
Jeff King
In-Reply-To: <1313088705-32222-3-git-send-email-artagnon@gmail.com>
Ramkumar Ramachandra wrote:
> The format_todo function leaks memory because it forgets to call
> free_message after get_message. Fix this.
>
> Suggested-by: Jonathan Nieder <jrnieder@gmail.com>
> Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
That's "Reported-by", I think. :)
Is this a big leak or a small one? Is it one-time or in a loop?
> ---
> builtin/revert.c | 1 +
> 1 files changed, 1 insertions(+), 0 deletions(-)
>
> diff --git a/builtin/revert.c b/builtin/revert.c
> index a548a14..1a4187a 100644
> --- a/builtin/revert.c
> +++ b/builtin/revert.c
> @@ -688,6 +688,7 @@ static int format_todo(struct strbuf *buf, struct commit_list *todo_list,
> return error(_("Cannot get commit message for %s"), sha1_abbrev);
> strbuf_addf(buf, "%s %s %s\n", action_str, sha1_abbrev, msg.subject);
> }
> + free_message(&msg);
> return 0;
> }
I don't see how this could work. Since there an xmalloc() in each
loop iteration, I would have expected the free() to be in the loop
body, too.
^ permalink raw reply
* Re: [PATCH 1/6] revert: Don't remove the sequencer state on error
From: Jonathan Nieder @ 2011-08-11 19:20 UTC (permalink / raw)
To: Ramkumar Ramachandra
Cc: Git List, Junio C Hamano, Christian Couder, Daniel Barkalow,
Jeff King
In-Reply-To: <1313088705-32222-2-git-send-email-artagnon@gmail.com>
Ramkumar Ramachandra wrote:
> The cherry-pick/ revert machinery now removes the sequencer state when
> do_pick_commit returns a non-zero, and when only one instruction is
> left in the todo_list. Since do_pick_commit has a way to distinguish
> errors from conflicts using the signed-ness of the return value,
> utilize this to ensure that the sequencer state is only removed when
> there's a conflict and there is only one instruction left in the
> todo_list.
I'm having trouble parsing this. Is the idea of this one to mitigate
some of the problems with the "remove sequencer state when a conflict
is encountered in the last commit" hack, by suppressing such behavior
when there is an internal error rather than a conflict? Why bother,
when the behavior is suppressed altogether later in the series?
^ permalink raw reply
* Re: [PATCH 0/6] Towards a generalized sequencer
From: Jonathan Nieder @ 2011-08-11 19:03 UTC (permalink / raw)
To: Ramkumar Ramachandra
Cc: Git List, Junio C Hamano, Christian Couder, Daniel Barkalow,
Jeff King
In-Reply-To: <1313088705-32222-1-git-send-email-artagnon@gmail.com>
Ramkumar Ramachandra wrote:
> No new ideas; just the same ideas implemented in a more sane
> way.
Thanks! Will try it out.
> Note: I didn't know what to do with the license header in the fifth
> patch. I just assumed that it was some historical cruft and removed
> it.
Please don't. Technically it's allowed by the license if I understand
correctly (since the copyright notices are not accompanied by a
disclaimer of warranty) but it's almost always the wrong thing to do
to remove a copyright notice without the author's permission.
^ permalink raw reply
* [PATCH 6/6] sequencer: Remove sequencer state after final commit
From: Ramkumar Ramachandra @ 2011-08-11 18:51 UTC (permalink / raw)
To: Git List
Cc: Junio C Hamano, Jonathan Nieder, Christian Couder,
Daniel Barkalow, Jeff King
In-Reply-To: <1313088705-32222-1-git-send-email-artagnon@gmail.com>
Since d3f4628e (revert: Remove sequencer state when no commits are
pending, 2011-07-06), the sequencer removes the sequencer state before
the final commit is actually completed. This design is inherently
flawed, as it will not allow the user to abort the sequencer operation
at that stage. Instead, write and expose a new function to count the
number of commits left in the instruction sheet; use this in
builtin/commit.c to remove the sequencer state when a commit has
successfully completed and there is only one instruction left in the
sheet.
Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
---
builtin/commit.c | 7 ++++++-
sequencer.c | 26 +++++++++++++++-----------
sequencer.h | 1 +
t/t3510-cherry-pick-sequence.sh | 4 ++--
4 files changed, 24 insertions(+), 14 deletions(-)
diff --git a/builtin/commit.c b/builtin/commit.c
index e1af9b1..4a5af9a 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -26,6 +26,7 @@
#include "unpack-trees.h"
#include "quote.h"
#include "submodule.h"
+#include "sequencer.h"
static const char * const builtin_commit_usage[] = {
"git commit [options] [--] <filepattern>...",
@@ -1521,7 +1522,11 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
unlink(git_path("MERGE_MODE"));
unlink(git_path("SQUASH_MSG"));
- if (commit_index_files())
+ /* Remove sequencer state if we just finished the last insn */
+ if (sequencer_count_todo() == 1)
+ remove_sequencer_state(1);
+
+ if (commit_index_files())
die (_("Repository has been updated, but unable to write\n"
"new_index file. Check that disk is not full or quota is\n"
"not exceeded, and then \"git reset HEAD\" to recover."));
diff --git a/sequencer.c b/sequencer.c
index e72618c..783b4a9 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -736,6 +736,20 @@ static void read_populate_todo(struct replay_insn_list **todo_list)
die(_("Unusable instruction sheet: %s"), todo_file);
}
+int sequencer_count_todo(void)
+{
+ struct replay_insn_list *todo_list = NULL;
+ struct replay_insn_list *cur;
+ int insn_count = 0;
+
+ if (!file_exists(git_path(SEQ_TODO_FILE)))
+ return 0;
+ read_populate_todo(&todo_list);
+ for (cur = todo_list; cur; cur = cur->next)
+ insn_count += 1;
+ return insn_count;
+}
+
static int populate_opts_cb(const char *key, const char *value, void *data)
{
struct replay_opts *opts = data;
@@ -901,18 +915,8 @@ static int pick_commits(struct replay_insn_list *todo_list,
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 && res > 0)
- /*
- * A conflict 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);
+ if (res)
return res;
- }
}
/*
diff --git a/sequencer.h b/sequencer.h
index ebf20cb..c64ba91 100644
--- a/sequencer.h
+++ b/sequencer.h
@@ -52,5 +52,6 @@ void remove_sequencer_state(int aggressive);
void sequencer_parse_args(int argc, const char **argv, struct replay_opts *opts);
int sequencer_pick_revisions(struct replay_opts *opts);
+int sequencer_count_todo(void);
#endif
diff --git a/t/t3510-cherry-pick-sequence.sh b/t/t3510-cherry-pick-sequence.sh
index bc7fb13..57e9e7c 100755
--- a/t/t3510-cherry-pick-sequence.sh
+++ b/t/t3510-cherry-pick-sequence.sh
@@ -82,13 +82,13 @@ 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 'final commit cleans up sequencer state' '
pristine_detach initial &&
test_must_fail git cherry-pick base..picked &&
- test_path_is_missing .git/sequencer &&
echo "resolved" >foo &&
git add foo &&
git commit &&
+ test_path_is_missing .git/sequencer &&
{
git rev-list HEAD |
git diff-tree --root --stdin |
--
1.7.6.351.gb35ac.dirty
^ permalink raw reply related
* [PATCH 5/6] sequencer: Expose API to cherry-picking machinery
From: Ramkumar Ramachandra @ 2011-08-11 18:51 UTC (permalink / raw)
To: Git List
Cc: Junio C Hamano, Jonathan Nieder, Christian Couder,
Daniel Barkalow, Jeff King
In-Reply-To: <1313088705-32222-1-git-send-email-artagnon@gmail.com>
Move code from builtin/revert.c to sequencer.c and expose a public API
without making any functional changes. Although it is useful only to
existing callers of cherry-pick and revert now, this patch lays the
foundation for future expansion.
Helped-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
---
builtin/revert.c | 1001 +-----------------------------------------------------
sequencer.c | 958 +++++++++++++++++++++++++++++++++++++++++++++++++++-
sequencer.h | 28 ++
3 files changed, 989 insertions(+), 998 deletions(-)
diff --git a/builtin/revert.c b/builtin/revert.c
index 483c957..fc818fd 100644
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -1,999 +1,6 @@
#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"
-/*
- * This implements the builtins revert and cherry-pick.
- *
- * Copyright (c) 2007 Johannes E. Schindelin
- *
- * Based on git-revert.sh, which is
- *
- * Copyright (c) 2005 Linus Torvalds
- * Copyright (c) 2005 Junio C Hamano
- */
-
-static const char * const revert_usage[] = {
- "git revert [options] <commit-ish>",
- "git revert <subcommand>",
- NULL
-};
-
-static const char * const cherry_pick_usage[] = {
- "git cherry-pick [options] <commit-ish>",
- "git cherry-pick <subcommand>",
- 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;
- int commit_argc;
- const char **commit_argv;
-
- /* Merge strategy */
- const char *strategy;
- const char **xopts;
- size_t xopts_nr, xopts_alloc;
-};
-
-#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;
-}
-
-static int option_parse_x(const struct option *opt,
- const char *arg, int unset)
-{
- struct replay_opts **opts_ptr = opt->value;
- struct replay_opts *opts = *opts_ptr;
-
- if (unset)
- return 0;
-
- ALLOC_GROW(opts->xopts, opts->xopts_nr + 1, opts->xopts_alloc);
- opts->xopts[opts->xopts_nr++] = xstrdup(arg);
- return 0;
-}
-
-static void verify_opt_compatible(const char *me, const char *base_opt, ...)
-{
- const char *this_opt;
- va_list ap;
-
- va_start(ap, base_opt);
- while ((this_opt = va_arg(ap, const char *))) {
- if (va_arg(ap, int))
- break;
- }
- va_end(ap);
-
- if (this_opt)
- die(_("%s: %s cannot be used with %s"), me, this_opt, base_opt);
-}
-
-static void verify_opt_mutually_compatible(const char *me, ...)
-{
- const char *opt1, *opt2;
- va_list ap;
-
- va_start(ap, me);
- while ((opt1 = va_arg(ap, const char *))) {
- if (va_arg(ap, int))
- break;
- }
- if (opt1) {
- while ((opt2 = va_arg(ap, const char *))) {
- if (va_arg(ap, int))
- break;
- }
- }
-
- if (opt1 && opt2)
- die(_("%s: %s cannot be used with %s"), me, opt1, opt2);
-}
-
-static void parse_args(int argc, const char **argv, struct replay_opts *opts)
-{
- const char * const * usage_str = revert_or_cherry_pick_usage(opts);
- const char *me = action_name(opts);
- int noop;
- 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_BOOLEAN('n', "no-commit", &opts->no_commit, "don't automatically commit"),
- OPT_BOOLEAN('e', "edit", &opts->edit, "edit the commit message"),
- { OPTION_BOOLEAN, 'r', NULL, &noop, NULL, "no-op (backward compatibility)",
- PARSE_OPT_NOARG | PARSE_OPT_HIDDEN, NULL, 0 },
- OPT_BOOLEAN('s', "signoff", &opts->signoff, "add Signed-off-by:"),
- OPT_INTEGER('m', "mainline", &opts->mainline, "parent number"),
- OPT_RERERE_AUTOUPDATE(&opts->allow_rerere_auto),
- OPT_STRING(0, "strategy", &opts->strategy, "strategy", "merge strategy"),
- OPT_CALLBACK('X', "strategy-option", &opts, "option",
- "option for merge strategy", option_parse_x),
- OPT_END(),
- OPT_END(),
- OPT_END(),
- };
-
- if (opts->action == REPLAY_PICK) {
- struct option cp_extra[] = {
- OPT_BOOLEAN('x', NULL, &opts->record_origin, "append commit name"),
- OPT_BOOLEAN(0, "ff", &opts->allow_ff, "allow fast-forward"),
- OPT_END(),
- };
- if (parse_options_concat(options, ARRAY_SIZE(options), cp_extra))
- die(_("program error"));
- }
-
- opts->commit_argc = parse_options(argc, argv, NULL, options, usage_str,
- PARSE_OPT_KEEP_ARGV0 |
- PARSE_OPT_KEEP_UNKNOWN);
-
- /* Check for incompatible subcommands */
- verify_opt_mutually_compatible(me,
- "--reset", reset,
- "--continue", contin,
- NULL);
-
- /* Set the subcommand */
- if (reset)
- opts->subcommand = REPLAY_RESET;
- else if (contin)
- opts->subcommand = REPLAY_CONTINUE;
- else
- opts->subcommand = REPLAY_NONE;
-
- /* Check for incompatible command line arguments */
- if (opts->subcommand != REPLAY_NONE) {
- char *this_operation;
- if (opts->subcommand == REPLAY_RESET)
- this_operation = "--reset";
- else
- this_operation = "--continue";
-
- verify_opt_compatible(me, this_operation,
- "--no-commit", opts->no_commit,
- "--signoff", opts->signoff,
- "--mainline", opts->mainline,
- "--strategy", opts->strategy ? 1 : 0,
- "--strategy-option", opts->xopts ? 1 : 0,
- "-x", opts->record_origin,
- "--ff", opts->allow_ff,
- NULL);
- }
-
- else if (opts->commit_argc < 2)
- usage_with_options(usage_str, options);
-
- if (opts->allow_ff)
- verify_opt_compatible(me, "--ff",
- "--signoff", opts->signoff,
- "--no-commit", opts->no_commit,
- "-x", opts->record_origin,
- "--edit", opts->edit,
- NULL);
- opts->commit_argv = argv;
-}
-
-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(void)
-{
- 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;
- }
-
- 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)
-{
- struct tree *tree = xcalloc(1, sizeof(struct tree));
-
- tree->object.parsed = 1;
- tree->object.type = OBJ_TREE;
- pretend_sha1_file(NULL, 0, OBJ_TREE, tree->object.sha1);
- return tree;
-}
-
-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 == REPLAY_REVERT ? "revert" : "cherry-pick",
- 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->no_commit)
- write_cherry_pick_head(commit);
- }
-
- 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 (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();
- 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 rev_info *revs, struct replay_opts *opts)
-{
- int argc;
-
- init_revisions(revs, NULL);
- revs->no_walk = 1;
- if (opts->action != REPLAY_REVERT)
- revs->reverse = 1;
-
- argc = setup_revisions(opts->commit_argc, opts->commit_argv, revs, NULL);
- if (argc > 1)
- usage(*revert_or_cherry_pick_usage(opts));
-
- if (prepare_revision_walk(revs))
- die(_("revision walk setup failed"));
-
- if (!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;
- */
-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;
- struct commit_message msg = { NULL, NULL, NULL, NULL, NULL };
-
- action_str = cur->action == REPLAY_REVERT ? "revert" : "pick";
- sha1_abbrev = find_unique_abbrev(cur->operand->object.sha1, DEFAULT_ABBREV);
- if (get_message(cur->operand, &msg))
- return error(_("Cannot get commit message for %s"), sha1_abbrev);
- strbuf_addf(buf, "%s %s %s\n", action_str, sha1_abbrev, msg.subject);
- free_message(&msg);
- }
- return 0;
-}
-
-static int parse_insn_line(char *start, struct replay_insn_list *item)
-{
- unsigned char commit_sha1[20];
- char sha1_abbrev[40];
- char *p = start, *q, *end = strchrnul(start, '\n');
-
- if (!prefixcmp(start, "pick ")) {
- item->action = REPLAY_PICK;
- p += strlen("pick ");
- } else if (!prefixcmp(start, "revert ")) {
- item->action = REPLAY_REVERT;
- p += strlen("revert ");
- } else
- return error(_("Unrecognized action: %s"), start);
-
- q = strchrnul(p, ' ');
- if (q > end)
- q = end;
- if (q - p + 1 > sizeof(sha1_abbrev))
- return error(_("Object name too large: %s"), p);
- memcpy(sha1_abbrev, p, q - p);
- sha1_abbrev[q - p] = '\0';
-
- if (get_sha1(sha1_abbrev, commit_sha1) < 0)
- return error(_("Malformed object name: %s"), sha1_abbrev);
-
- item->operand = lookup_commit_reference(commit_sha1);
- if (!item->operand)
- return error(_("Not a valid commit: %s"), sha1_abbrev);
-
- 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++) {
- if (parse_insn_line(p, &item) < 0)
- return error(_("Could not parse line %d."), i);
- next = replay_insn_list_append(item.action, item.operand, next);
- p = strchrnul(p, '\n');
- if (*p)
- p++;
- }
- 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 rev_info revs;
- struct commit *commit;
- struct replay_insn_list **next;
-
- prepare_revs(&revs, opts);
-
- next = todo_list;
- while ((commit = get_revision(&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 && res > 0)
- /*
- * A conflict 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];
-
- 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;
@@ -1004,8 +11,8 @@ int cmd_revert(int argc, const char **argv, const char *prefix)
opts.edit = 1;
opts.action = REPLAY_REVERT;
git_config(git_default_config, NULL);
- parse_args(argc, argv, &opts);
- res = pick_revisions(&opts);
+ sequencer_parse_args(argc, argv, &opts);
+ res = sequencer_pick_revisions(&opts);
if (res < 0)
die(_("revert failed"));
return res;
@@ -1019,8 +26,8 @@ int cmd_cherry_pick(int argc, const char **argv, const char *prefix)
memset(&opts, 0, sizeof(opts));
opts.action = REPLAY_PICK;
git_config(git_default_config, NULL);
- parse_args(argc, argv, &opts);
- res = pick_revisions(&opts);
+ sequencer_parse_args(argc, argv, &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..e72618c 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -1,8 +1,809 @@
#include "cache.h"
#include "sequencer.h"
-#include "strbuf.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"
+static const char * const revert_usage[] = {
+ "git revert [options] <commit-ish>",
+ "git revert <subcommand>",
+ NULL
+};
+
+static const char * const cherry_pick_usage[] = {
+ "git cherry-pick [options] <commit-ish>",
+ "git cherry-pick <subcommand>",
+ NULL
+};
+
+#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;
+}
+
+static int option_parse_x(const struct option *opt,
+ const char *arg, int unset)
+{
+ struct replay_opts **opts_ptr = opt->value;
+ struct replay_opts *opts = *opts_ptr;
+
+ if (unset)
+ return 0;
+
+ ALLOC_GROW(opts->xopts, opts->xopts_nr + 1, opts->xopts_alloc);
+ opts->xopts[opts->xopts_nr++] = xstrdup(arg);
+ return 0;
+}
+
+static void verify_opt_compatible(const char *me, const char *base_opt, ...)
+{
+ const char *this_opt;
+ va_list ap;
+
+ va_start(ap, base_opt);
+ while ((this_opt = va_arg(ap, const char *))) {
+ if (va_arg(ap, int))
+ break;
+ }
+ va_end(ap);
+
+ if (this_opt)
+ die(_("%s: %s cannot be used with %s"), me, this_opt, base_opt);
+}
+
+static void verify_opt_mutually_compatible(const char *me, ...)
+{
+ const char *opt1, *opt2;
+ va_list ap;
+
+ va_start(ap, me);
+ while ((opt1 = va_arg(ap, const char *))) {
+ if (va_arg(ap, int))
+ break;
+ }
+ if (opt1) {
+ while ((opt2 = va_arg(ap, const char *))) {
+ if (va_arg(ap, int))
+ break;
+ }
+ }
+
+ if (opt1 && opt2)
+ die(_("%s: %s cannot be used with %s"), me, opt1, opt2);
+}
+
+void sequencer_parse_args(int argc, const char **argv, struct replay_opts *opts)
+{
+ const char * const * usage_str = revert_or_cherry_pick_usage(opts);
+ const char *me = action_name(opts);
+ int noop;
+ 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_BOOLEAN('n', "no-commit", &opts->no_commit, "don't automatically commit"),
+ OPT_BOOLEAN('e', "edit", &opts->edit, "edit the commit message"),
+ { OPTION_BOOLEAN, 'r', NULL, &noop, NULL, "no-op (backward compatibility)",
+ PARSE_OPT_NOARG | PARSE_OPT_HIDDEN, NULL, 0 },
+ OPT_BOOLEAN('s', "signoff", &opts->signoff, "add Signed-off-by:"),
+ OPT_INTEGER('m', "mainline", &opts->mainline, "parent number"),
+ OPT_RERERE_AUTOUPDATE(&opts->allow_rerere_auto),
+ OPT_STRING(0, "strategy", &opts->strategy, "strategy", "merge strategy"),
+ OPT_CALLBACK('X', "strategy-option", &opts, "option",
+ "option for merge strategy", option_parse_x),
+ OPT_END(),
+ OPT_END(),
+ OPT_END(),
+ };
+
+ if (opts->action == REPLAY_PICK) {
+ struct option cp_extra[] = {
+ OPT_BOOLEAN('x', NULL, &opts->record_origin, "append commit name"),
+ OPT_BOOLEAN(0, "ff", &opts->allow_ff, "allow fast-forward"),
+ OPT_END(),
+ };
+ if (parse_options_concat(options, ARRAY_SIZE(options), cp_extra))
+ die(_("program error"));
+ }
+
+ opts->commit_argc = parse_options(argc, argv, NULL, options, usage_str,
+ PARSE_OPT_KEEP_ARGV0 |
+ PARSE_OPT_KEEP_UNKNOWN);
+
+ /* Check for incompatible subcommands */
+ verify_opt_mutually_compatible(me,
+ "--reset", reset,
+ "--continue", contin,
+ NULL);
+
+ /* Set the subcommand */
+ if (reset)
+ opts->subcommand = REPLAY_RESET;
+ else if (contin)
+ opts->subcommand = REPLAY_CONTINUE;
+ else
+ opts->subcommand = REPLAY_NONE;
+
+ /* Check for incompatible command line arguments */
+ if (opts->subcommand != REPLAY_NONE) {
+ char *this_operation;
+ if (opts->subcommand == REPLAY_RESET)
+ this_operation = "--reset";
+ else
+ this_operation = "--continue";
+
+ verify_opt_compatible(me, this_operation,
+ "--no-commit", opts->no_commit,
+ "--signoff", opts->signoff,
+ "--mainline", opts->mainline,
+ "--strategy", opts->strategy ? 1 : 0,
+ "--strategy-option", opts->xopts ? 1 : 0,
+ "-x", opts->record_origin,
+ "--ff", opts->allow_ff,
+ NULL);
+ }
+
+ else if (opts->commit_argc < 2)
+ usage_with_options(usage_str, options);
+
+ if (opts->allow_ff)
+ verify_opt_compatible(me, "--ff",
+ "--signoff", opts->signoff,
+ "--no-commit", opts->no_commit,
+ "-x", opts->record_origin,
+ "--edit", opts->edit,
+ NULL);
+ opts->commit_argv = argv;
+}
+
+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(void)
+{
+ 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;
+ }
+
+ 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)
+{
+ struct tree *tree = xcalloc(1, sizeof(struct tree));
+
+ tree->object.parsed = 1;
+ tree->object.type = OBJ_TREE;
+ pretend_sha1_file(NULL, 0, OBJ_TREE, tree->object.sha1);
+ return tree;
+}
+
+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 = COMMIT_MESSAGE_INIT;
+ 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 == REPLAY_REVERT ? "revert" : "cherry-pick",
+ 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->no_commit)
+ write_cherry_pick_head(commit);
+ }
+
+ 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 (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();
+ 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 rev_info *revs, struct replay_opts *opts)
+{
+ int argc;
+
+ init_revisions(revs, NULL);
+ revs->no_walk = 1;
+ if (opts->action != REPLAY_REVERT)
+ revs->reverse = 1;
+
+ argc = setup_revisions(opts->commit_argc, opts->commit_argv, revs, NULL);
+ if (argc > 1)
+ usage(*revert_or_cherry_pick_usage(opts));
+
+ if (prepare_revision_walk(revs))
+ die(_("revision walk setup failed"));
+
+ if (!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;
+ */
+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;
+ struct commit_message msg = COMMIT_MESSAGE_INIT;
+
+ action_str = cur->action == REPLAY_REVERT ? "revert" : "pick";
+ sha1_abbrev = find_unique_abbrev(cur->operand->object.sha1, DEFAULT_ABBREV);
+ if (get_message(cur->operand, &msg))
+ return error(_("Cannot get commit message for %s"), sha1_abbrev);
+ strbuf_addf(buf, "%s %s %s\n", action_str, sha1_abbrev, msg.subject);
+ free_message(&msg);
+ }
+ return 0;
+}
+
+static int parse_insn_line(char *start, struct replay_insn_list *item)
+{
+ unsigned char commit_sha1[20];
+ char sha1_abbrev[40];
+ char *p = start, *q, *end = strchrnul(start, '\n');
+
+ if (!prefixcmp(start, "pick ")) {
+ item->action = REPLAY_PICK;
+ p += strlen("pick ");
+ } else if (!prefixcmp(start, "revert ")) {
+ item->action = REPLAY_REVERT;
+ p += strlen("revert ");
+ } else
+ return error(_("Unrecognized action: %s"), start);
+
+ q = strchrnul(p, ' ');
+ if (q > end)
+ q = end;
+ if (q - p + 1 > sizeof(sha1_abbrev))
+ return error(_("Object name too large: %s"), p);
+ memcpy(sha1_abbrev, p, q - p);
+ sha1_abbrev[q - p] = '\0';
+
+ if (get_sha1(sha1_abbrev, commit_sha1) < 0)
+ return error(_("Malformed object name: %s"), sha1_abbrev);
+
+ item->operand = lookup_commit_reference(commit_sha1);
+ if (!item->operand)
+ return error(_("Not a valid commit: %s"), sha1_abbrev);
+
+ 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++) {
+ if (parse_insn_line(p, &item) < 0)
+ return error(_("Could not parse line %d."), i);
+ next = replay_insn_list_append(item.action, item.operand, next);
+ p = strchrnul(p, '\n');
+ if (*p)
+ p++;
+ }
+ 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 rev_info revs;
+ struct commit *commit;
+ struct replay_insn_list **next;
+
+ prepare_revs(&revs, opts);
+
+ next = todo_list;
+ while ((commit = get_revision(&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;
+}
+
void remove_sequencer_state(int aggressive)
{
struct strbuf seq_dir = STRBUF_INIT;
@@ -17,3 +818,158 @@ void remove_sequencer_state(int aggressive)
strbuf_release(&seq_dir);
strbuf_release(&seq_old_dir);
}
+
+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 && res > 0)
+ /*
+ * A conflict 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];
+
+ 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..ebf20cb 100644
--- a/sequencer.h
+++ b/sequencer.h
@@ -7,7 +7,32 @@
#define SEQ_TODO_FILE "sequencer/todo"
#define SEQ_OPTS_FILE "sequencer/opts"
+#define COMMIT_MESSAGE_INIT { NULL, NULL, NULL, NULL, NULL };
+
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;
+ int commit_argc;
+ const char **commit_argv;
+
+ /* Merge strategy */
+ const char *strategy;
+ const char **xopts;
+ size_t xopts_nr, xopts_alloc;
+};
struct replay_insn_list {
enum replay_action action;
@@ -25,4 +50,7 @@ struct replay_insn_list {
*/
void remove_sequencer_state(int aggressive);
+void sequencer_parse_args(int argc, const char **argv, struct replay_opts *opts);
+int sequencer_pick_revisions(struct replay_opts *opts);
+
#endif
--
1.7.6.351.gb35ac.dirty
^ permalink raw reply related
* [PATCH 4/6] revert: Allow mixed pick and revert instructions
From: Ramkumar Ramachandra @ 2011-08-11 18:51 UTC (permalink / raw)
To: Git List
Cc: Junio C Hamano, Jonathan Nieder, Christian Couder,
Daniel Barkalow, Jeff King
In-Reply-To: <1313088705-32222-1-git-send-email-artagnon@gmail.com>
Change the way the instruction parser works, allowing arbitrary
(action, operand) pairs to be parsed. So now, you can do:
pick fdc0b12 picked
revert 965fed4 anotherpick
For cherry-pick and revert, this means that a 'git cherry-pick
--continue' can continue an ongoing revert operation and viceversa.
This patch lays the foundation for extending the parser to support
more actions so 'git rebase -i' can reuse this machinery in the
future.
Helped-by: Jonathan Nieder <jrnider@gmail.com>
Acked-by: Jonathan Nieder <jrnider@gmail.com>
Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
---
builtin/revert.c | 128 ++++++++++++++++++--------------------
sequencer.h | 8 +++
t/t3510-cherry-pick-sequence.sh | 58 ++++++++++++++++++
3 files changed, 127 insertions(+), 67 deletions(-)
diff --git a/builtin/revert.c b/builtin/revert.c
index f44f749..483c957 100644
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -39,7 +39,6 @@ static const char * const cherry_pick_usage[] = {
NULL
};
-enum replay_action { REVERT, CHERRY_PICK };
enum replay_subcommand { REPLAY_NONE, REPLAY_RESET, REPLAY_CONTINUE };
struct replay_opts {
@@ -68,14 +67,14 @@ struct replay_opts {
static const char *action_name(const struct replay_opts *opts)
{
- return opts->action == REVERT ? "revert" : "cherry-pick";
+ 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 == REVERT ? revert_usage : cherry_pick_usage;
+ return opts->action == REPLAY_REVERT ? revert_usage : cherry_pick_usage;
}
static int option_parse_x(const struct option *opt,
@@ -154,7 +153,7 @@ static void parse_args(int argc, const char **argv, struct replay_opts *opts)
OPT_END(),
};
- if (opts->action == CHERRY_PICK) {
+ if (opts->action == REPLAY_PICK) {
struct option cp_extra[] = {
OPT_BOOLEAN('x', NULL, &opts->record_origin, "append commit name"),
OPT_BOOLEAN(0, "ff", &opts->allow_ff, "allow fast-forward"),
@@ -353,7 +352,7 @@ static int error_dirty_index(struct replay_opts *opts)
return error_resolve_conflict(action_name(opts));
/* Different translation strings for cherry-pick and revert */
- if (opts->action == CHERRY_PICK)
+ 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."));
@@ -457,7 +456,8 @@ static int run_git_commit(const char *defmsg, struct replay_opts *opts)
return run_command_v_opt(args, RUN_GIT_CMD);
}
-static int do_pick_commit(struct commit *commit, struct replay_opts *opts)
+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;
@@ -517,7 +517,8 @@ static int do_pick_commit(struct commit *commit, struct replay_opts *opts)
/* 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));
+ action == REPLAY_REVERT ? "revert" : "cherry-pick",
+ sha1_to_hex(parent->object.sha1));
if (get_message(commit, &msg) != 0)
return error(_("Cannot get commit message for %s"),
@@ -532,7 +533,7 @@ static int do_pick_commit(struct commit *commit, struct replay_opts *opts)
defmsg = git_pathdup("MERGE_MSG");
- if (opts->action == REVERT) {
+ if (action == REPLAY_REVERT) {
base = commit;
base_label = msg.label;
next = parent;
@@ -575,7 +576,7 @@ static int do_pick_commit(struct commit *commit, struct replay_opts *opts)
write_cherry_pick_head(commit);
}
- if (!opts->strategy || !strcmp(opts->strategy, "recursive") || opts->action == REVERT) {
+ 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);
@@ -594,7 +595,7 @@ static int do_pick_commit(struct commit *commit, struct replay_opts *opts)
}
if (res) {
- error(opts->action == REVERT
+ error(action == REPLAY_REVERT
? _("could not revert %s... %s")
: _("could not apply %s... %s"),
find_unique_abbrev(commit->object.sha1, DEFAULT_ABBREV),
@@ -618,7 +619,7 @@ static void prepare_revs(struct rev_info *revs, struct replay_opts *opts)
init_revisions(revs, NULL);
revs->no_walk = 1;
- if (opts->action != REVERT)
+ if (opts->action != REPLAY_REVERT)
revs->reverse = 1;
argc = setup_revisions(opts->commit_argc, opts->commit_argv, revs, NULL);
@@ -664,88 +665,81 @@ static void read_and_refresh_cache(struct replay_opts *opts)
* assert(commit_list_count(list) == 2);
* return list;
*/
-struct commit_list **commit_list_append(struct commit *commit,
- struct commit_list **next)
+struct replay_insn_list **replay_insn_list_append(enum replay_action action,
+ struct commit *operand,
+ struct replay_insn_list **next)
{
- struct commit_list *new = xmalloc(sizeof(struct commit_list));
- new->item = commit;
+ 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 commit_list *todo_list,
- struct replay_opts *opts)
+static int format_todo(struct strbuf *buf, struct replay_insn_list *todo_list)
{
- struct commit_list *cur = NULL;
- struct commit_message msg = { NULL, NULL, NULL, NULL, NULL };
- const char *sha1_abbrev = NULL;
- const char *action_str = opts->action == REVERT ? "revert" : "pick";
+ struct replay_insn_list *cur;
for (cur = todo_list; cur; cur = cur->next) {
- sha1_abbrev = find_unique_abbrev(cur->item->object.sha1, DEFAULT_ABBREV);
- if (get_message(cur->item, &msg))
+ const char *sha1_abbrev, *action_str;
+ struct commit_message msg = { NULL, NULL, NULL, NULL, NULL };
+
+ action_str = cur->action == REPLAY_REVERT ? "revert" : "pick";
+ sha1_abbrev = find_unique_abbrev(cur->operand->object.sha1, DEFAULT_ABBREV);
+ if (get_message(cur->operand, &msg))
return error(_("Cannot get commit message for %s"), sha1_abbrev);
strbuf_addf(buf, "%s %s %s\n", action_str, sha1_abbrev, msg.subject);
+ free_message(&msg);
}
- free_message(&msg);
return 0;
}
-static struct commit *parse_insn_line(char *start, struct replay_opts *opts)
+static int parse_insn_line(char *start, struct replay_insn_list *item)
{
unsigned char commit_sha1[20];
char sha1_abbrev[40];
- enum replay_action action;
char *p = start, *q, *end = strchrnul(start, '\n');
if (!prefixcmp(start, "pick ")) {
- action = CHERRY_PICK;
+ item->action = REPLAY_PICK;
p += strlen("pick ");
} else if (!prefixcmp(start, "revert ")) {
- action = REVERT;
+ item->action = REPLAY_REVERT;
p += strlen("revert ");
} else
- return NULL;
+ return error(_("Unrecognized action: %s"), start);
q = strchrnul(p, ' ');
if (q > end)
q = end;
if (q - p + 1 > sizeof(sha1_abbrev))
- return NULL;
+ return error(_("Object name too large: %s"), p);
memcpy(sha1_abbrev, p, q - p);
sha1_abbrev[q - p] = '\0';
- /*
- * Verify that the action matches up with the one in
- * opts; we don't support arbitrary instructions
- */
- if (action != opts->action) {
- const char *action_str;
- action_str = action == REVERT ? "revert" : "cherry-pick";
- error(_("Cannot %s during a %s"), action_str, action_name(opts));
- return NULL;
- }
-
if (get_sha1(sha1_abbrev, commit_sha1) < 0)
- return NULL;
+ return error(_("Malformed object name: %s"), sha1_abbrev);
+
+ item->operand = lookup_commit_reference(commit_sha1);
+ if (!item->operand)
+ return error(_("Not a valid commit: %s"), sha1_abbrev);
- return lookup_commit_reference(commit_sha1);
+ item->next = NULL;
+ return 0;
}
-static int parse_insn_buffer(char *buf, struct commit_list **todo_list,
- struct replay_opts *opts)
+static int parse_insn_buffer(char *buf, struct replay_insn_list **todo_list)
{
- struct commit_list **next = todo_list;
- struct commit *commit;
+ 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++) {
- commit = parse_insn_line(p, opts);
- if (!commit)
+ if (parse_insn_line(p, &item) < 0)
return error(_("Could not parse line %d."), i);
- next = commit_list_append(commit, next);
+ next = replay_insn_list_append(item.action, item.operand, next);
p = strchrnul(p, '\n');
if (*p)
p++;
@@ -755,8 +749,7 @@ static int parse_insn_buffer(char *buf, struct commit_list **todo_list,
return 0;
}
-static void read_populate_todo(struct commit_list **todo_list,
- struct replay_opts *opts)
+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;
@@ -772,7 +765,7 @@ static void read_populate_todo(struct commit_list **todo_list,
}
close(fd);
- res = parse_insn_buffer(buf.buf, todo_list, opts);
+ res = parse_insn_buffer(buf.buf, todo_list);
strbuf_release(&buf);
if (res)
die(_("Unusable instruction sheet: %s"), todo_file);
@@ -821,18 +814,18 @@ static void read_populate_opts(struct replay_opts **opts_ptr)
die(_("Malformed options sheet: %s"), opts_file);
}
-static void walk_revs_populate_todo(struct commit_list **todo_list,
+static void walk_revs_populate_todo(struct replay_insn_list **todo_list,
struct replay_opts *opts)
{
struct rev_info revs;
struct commit *commit;
- struct commit_list **next;
+ struct replay_insn_list **next;
prepare_revs(&revs, opts);
next = todo_list;
while ((commit = get_revision(&revs)))
- next = commit_list_append(commit, next);
+ next = replay_insn_list_append(opts->action, commit, next);
}
static int create_seq_dir(void)
@@ -861,7 +854,7 @@ static void save_head(const char *head)
die(_("Error wrapping up %s."), head_file);
}
-static void save_todo(struct commit_list *todo_list, struct replay_opts *opts)
+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;
@@ -869,7 +862,7 @@ static void save_todo(struct commit_list *todo_list, struct replay_opts *opts)
int fd;
fd = hold_lock_file_for_update(&todo_lock, todo_file, LOCK_DIE_ON_ERROR);
- if (format_todo(&buf, todo_list, opts) < 0)
+ 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);
@@ -913,9 +906,10 @@ static void save_opts(struct replay_opts *opts)
}
}
-static int pick_commits(struct commit_list *todo_list, struct replay_opts *opts)
+static int pick_commits(struct replay_insn_list *todo_list,
+ struct replay_opts *opts)
{
- struct commit_list *cur;
+ struct replay_insn_list *cur;
int res;
setenv(GIT_REFLOG_ACTION, action_name(opts), 0);
@@ -925,8 +919,8 @@ static int pick_commits(struct commit_list *todo_list, struct replay_opts *opts)
read_and_refresh_cache(opts);
for (cur = todo_list; cur; cur = cur->next) {
- save_todo(cur, opts);
- res = do_pick_commit(cur->item, opts);
+ save_todo(cur);
+ res = do_pick_commit(cur->operand, cur->action, opts);
if (res) {
if (!cur->next && res > 0)
/*
@@ -951,7 +945,7 @@ static int pick_commits(struct commit_list *todo_list, struct replay_opts *opts)
static int pick_revisions(struct replay_opts *opts)
{
- struct commit_list *todo_list = NULL;
+ struct replay_insn_list *todo_list = NULL;
unsigned char sha1[20];
read_and_refresh_cache(opts);
@@ -968,7 +962,7 @@ static int pick_revisions(struct replay_opts *opts)
if (!file_exists(git_path(SEQ_TODO_FILE)))
goto error;
read_populate_opts(&opts);
- read_populate_todo(&todo_list, opts);
+ read_populate_todo(&todo_list);
/* Verify that the conflict has been resolved */
if (!index_differs_from("HEAD", 0))
@@ -988,7 +982,7 @@ static int pick_revisions(struct replay_opts *opts)
return -1;
}
if (get_sha1("HEAD", sha1)) {
- if (opts->action == REVERT)
+ if (opts->action == REPLAY_REVERT)
return error(_("Can't revert as initial commit"));
return error(_("Can't cherry-pick into empty head"));
}
@@ -1008,7 +1002,7 @@ int cmd_revert(int argc, const char **argv, const char *prefix)
memset(&opts, 0, sizeof(opts));
if (isatty(0))
opts.edit = 1;
- opts.action = REVERT;
+ opts.action = REPLAY_REVERT;
git_config(git_default_config, NULL);
parse_args(argc, argv, &opts);
res = pick_revisions(&opts);
@@ -1023,7 +1017,7 @@ int cmd_cherry_pick(int argc, const char **argv, const char *prefix)
int res;
memset(&opts, 0, sizeof(opts));
- opts.action = CHERRY_PICK;
+ opts.action = REPLAY_PICK;
git_config(git_default_config, NULL);
parse_args(argc, argv, &opts);
res = pick_revisions(&opts);
diff --git a/sequencer.h b/sequencer.h
index 905d295..f4db257 100644
--- a/sequencer.h
+++ b/sequencer.h
@@ -7,6 +7,14 @@
#define SEQ_TODO_FILE "sequencer/todo"
#define SEQ_OPTS_FILE "sequencer/opts"
+enum replay_action { REPLAY_REVERT, REPLAY_PICK };
+
+struct replay_insn_list {
+ enum replay_action action;
+ struct commit *operand;
+ struct replay_insn_list *next;
+};
+
/*
* Removes SEQ_OLD_DIR and renames SEQ_DIR to SEQ_OLD_DIR, ignoring
* any errors. Intended to be used by 'git reset'.
diff --git a/t/t3510-cherry-pick-sequence.sh b/t/t3510-cherry-pick-sequence.sh
index bc5f0b8..bc7fb13 100755
--- a/t/t3510-cherry-pick-sequence.sh
+++ b/t/t3510-cherry-pick-sequence.sh
@@ -240,4 +240,62 @@ test_expect_success 'missing commit descriptions in instruction sheet' '
test_cmp expect actual
'
+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 3/6] revert: Parse instruction sheet more cautiously
From: Ramkumar Ramachandra @ 2011-08-11 18:51 UTC (permalink / raw)
To: Git List
Cc: Junio C Hamano, Jonathan Nieder, Christian Couder,
Daniel Barkalow, Jeff King
In-Reply-To: <1313088705-32222-1-git-send-email-artagnon@gmail.com>
Fix a buffer overflow bug by checking that parsed SHA-1 hex will fit
in the buffer we've created for it. Also change the instruction sheet
format subtly so that a description of the commit after the object
name is optional. So now, an instruction sheet like this is perfectly
valid:
pick 35b0426
pick fbd5bbcbc2e
pick 7362160f
Suggested-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
---
builtin/revert.c | 20 +++++++++-----------
t/t3510-cherry-pick-sequence.sh | 29 +++++++++++++++++++++++++++++
2 files changed, 38 insertions(+), 11 deletions(-)
diff --git a/builtin/revert.c b/builtin/revert.c
index 1a4187a..f44f749 100644
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -697,26 +697,24 @@ static struct commit *parse_insn_line(char *start, struct replay_opts *opts)
unsigned char commit_sha1[20];
char sha1_abbrev[40];
enum replay_action action;
- int insn_len = 0;
- char *p, *q;
+ char *p = start, *q, *end = strchrnul(start, '\n');
if (!prefixcmp(start, "pick ")) {
action = CHERRY_PICK;
- insn_len = strlen("pick");
- p = start + insn_len + 1;
+ p += strlen("pick ");
} else if (!prefixcmp(start, "revert ")) {
action = REVERT;
- insn_len = strlen("revert");
- p = start + insn_len + 1;
+ p += strlen("revert ");
} else
return NULL;
- q = strchr(p, ' ');
- if (!q)
+ q = strchrnul(p, ' ');
+ if (q > end)
+ q = end;
+ if (q - p + 1 > sizeof(sha1_abbrev))
return NULL;
- q++;
-
- strlcpy(sha1_abbrev, p, q - p);
+ memcpy(sha1_abbrev, p, q - p);
+ sha1_abbrev[q - p] = '\0';
/*
* Verify that the action matches up with the one in
diff --git a/t/t3510-cherry-pick-sequence.sh b/t/t3510-cherry-pick-sequence.sh
index 3bca2b3..bc5f0b8 100755
--- a/t/t3510-cherry-pick-sequence.sh
+++ b/t/t3510-cherry-pick-sequence.sh
@@ -211,4 +211,33 @@ test_expect_success 'malformed instruction sheet 2' '
test_must_fail git cherry-pick --continue
'
+test_expect_success 'missing commit descriptions in instruction sheet' '
+ 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 |
+ 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_done
--
1.7.6.351.gb35ac.dirty
^ permalink raw reply related
* [PATCH 2/6] revert: Free memory after get_message call
From: Ramkumar Ramachandra @ 2011-08-11 18:51 UTC (permalink / raw)
To: Git List
Cc: Junio C Hamano, Jonathan Nieder, Christian Couder,
Daniel Barkalow, Jeff King
In-Reply-To: <1313088705-32222-1-git-send-email-artagnon@gmail.com>
The format_todo function leaks memory because it forgets to call
free_message after get_message. Fix this.
Suggested-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
---
builtin/revert.c | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/builtin/revert.c b/builtin/revert.c
index a548a14..1a4187a 100644
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -688,6 +688,7 @@ static int format_todo(struct strbuf *buf, struct commit_list *todo_list,
return error(_("Cannot get commit message for %s"), sha1_abbrev);
strbuf_addf(buf, "%s %s %s\n", action_str, sha1_abbrev, msg.subject);
}
+ free_message(&msg);
return 0;
}
--
1.7.6.351.gb35ac.dirty
^ permalink raw reply related
* [PATCH 1/6] revert: Don't remove the sequencer state on error
From: Ramkumar Ramachandra @ 2011-08-11 18:51 UTC (permalink / raw)
To: Git List
Cc: Junio C Hamano, Jonathan Nieder, Christian Couder,
Daniel Barkalow, Jeff King
In-Reply-To: <1313088705-32222-1-git-send-email-artagnon@gmail.com>
The cherry-pick/ revert machinery now removes the sequencer state when
do_pick_commit returns a non-zero, and when only one instruction is
left in the todo_list. Since do_pick_commit has a way to distinguish
errors from conflicts using the signed-ness of the return value,
utilize this to ensure that the sequencer state is only removed when
there's a conflict and there is only one instruction left in the
todo_list.
Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
---
builtin/revert.c | 6 +++---
1 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/builtin/revert.c b/builtin/revert.c
index 8b452e8..a548a14 100644
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -929,10 +929,10 @@ static int pick_commits(struct commit_list *todo_list, struct replay_opts *opts)
save_todo(cur, opts);
res = do_pick_commit(cur->item, opts);
if (res) {
- if (!cur->next)
+ if (!cur->next && res > 0)
/*
- * An error was encountered while
- * picking the last commit; the
+ * A conflict was encountered while
+ * picking the last commit. The
* sequencer state is useless now --
* the user simply needs to resolve
* the conflict and commit
--
1.7.6.351.gb35ac.dirty
^ permalink raw reply related
* [PATCH 0/6] Towards a generalized sequencer
From: Ramkumar Ramachandra @ 2011-08-11 18:51 UTC (permalink / raw)
To: Git List
Cc: Junio C Hamano, Jonathan Nieder, Christian Couder,
Daniel Barkalow, Jeff King
Hi,
I've prepared a nicer series after Jonathan's feedback on the previous
one. No new ideas; just the same ideas implemented in a more sane
way. The first three patches fix some minor annoyances and don't
dontribute much to the series in general. The fourth patch implements
a very important idea: the ability to parse a general (action,
operand) pair in the instruction sheet. The fifth patch may come as a
real shocker; I was shocked myself, but I'm now convinced that this is
the right way forward. Since it's only code movement, it should be
very easy to review. The final patch solves a long-standing problem
by introducing tighter coupling between 'git commit' and the
sequencer.
Thanks for lending a ear. Enjoy reading the rest!
Note: I didn't know what to do with the license header in the fifth
patch. I just assumed that it was some historical cruft and removed
it.
Ramkumar Ramachandra (6):
revert: Don't remove the sequencer state on error
revert: Free memory after get_message call
revert: Parse instruction sheet more cautiously
revert: Allow mixed pick and revert instructions
sequencer: Expose API to cherry-picking machinery
sequencer: Remove sequencer state after final commit
builtin/commit.c | 7 +-
builtin/revert.c | 1012 +--------------------------------------
sequencer.c | 962 +++++++++++++++++++++++++++++++++++++-
sequencer.h | 37 ++
t/t3510-cherry-pick-sequence.sh | 91 ++++-
5 files changed, 1099 insertions(+), 1010 deletions(-)
--
1.7.6.351.gb35ac.dirty
^ permalink raw reply
* Re: [PATCH 0/2] Add an update=none option for 'loose' submodules
From: Junio C Hamano @ 2011-08-11 18:28 UTC (permalink / raw)
To: Heiko Voigt; +Cc: git, Jens Lehmann
In-Reply-To: <cover.1312923673.git.hvoigt@hvoigt.net>
Heiko Voigt <hvoigt@hvoigt.net> writes:
> If a submodule is used to seperate some bigger parts of a project into
> an optional directory it is helpful to not clone/update them by default.
Sorry if I am slow, but I do not get this.
I thought unless you say "submodule init" once, a submodule you are not
interested in should not be cloned nor updated at all. If that is not the
case, isn't it a bug to be fixed without a new configuration variable that
fixes it only when it is set?
> We have been talking about loose submodules for some time:
Also before introducing a new terminology "loose submodule", please define
it somewhere. It feels confusing to me that a normal submodule, which
shouldn't be auto-cloned nor auto-updated without "submodule init", needs
to be called by a name other than simply a "submodule" but with an
adjuctive "loose submodule".
^ permalink raw reply
* [PATCH 09/11] object: try 4-way cuckoo
From: Junio C Hamano @ 2011-08-11 17:53 UTC (permalink / raw)
To: git
In-Reply-To: <1313085196-13249-1-git-send-email-gitster@pobox.com>
The more we probe alternative slots, the more expensive average
look-up gets, while it helps reduce the load factor of the hash
table.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
../+v/6bb99816f5676ed5ddb6922363b7470a7e8c61f7/git-pack-objects
Counting objects: 2139209, done.
31.09user 2.05system 0:33.25elapsed 99%CPU (0avgtext+0avgdata 3135840maxresident)k
0inputs+0outputs (0major+290849minor)pagefaults 0swaps
Counting objects: 2139209, done.
31.12user 2.14system 0:33.37elapsed 99%CPU (0avgtext+0avgdata 3136128maxresident)k
0inputs+0outputs (0major+290866minor)pagefaults 0swaps
Counting objects: 2139209, done.
31.17user 2.01system 0:33.29elapsed 99%CPU (0avgtext+0avgdata 3136512maxresident)k
0inputs+0outputs (0major+290890minor)pagefaults 0swaps
---
object.c | 15 ++++-----------
1 files changed, 4 insertions(+), 11 deletions(-)
diff --git a/object.c b/object.c
index c777520..caced56 100644
--- a/object.c
+++ b/object.c
@@ -49,12 +49,12 @@ struct object *get_indexed_object(unsigned int idx)
struct object *lookup_object(const unsigned char *sha1)
{
struct object *obj;
- unsigned int hashval[5];
+ unsigned int hashval[4];
if (!obj_hash)
return NULL;
- memcpy(hashval, sha1, 20);
+ memcpy(hashval, sha1, 16);
if ((obj = obj_hash[H(hashval, 0)]) && !hashcmp(sha1, obj->sha1))
return obj;
if ((obj = obj_hash[H(hashval, 1)]) && !hashcmp(sha1, obj->sha1))
@@ -63,8 +63,6 @@ struct object *lookup_object(const unsigned char *sha1)
return obj;
if ((obj = obj_hash[H(hashval, 3)]) && !hashcmp(sha1, obj->sha1))
return obj;
- if ((obj = obj_hash[H(hashval, 4)]) && !hashcmp(sha1, obj->sha1))
- return obj;
return NULL;
}
@@ -84,9 +82,9 @@ static struct object *insert_obj_hash(struct object *obj)
for (loop = obj_hash_size / 2; 0 <= loop; loop--) {
struct object *tmp_obj;
unsigned int ix;
- unsigned int hashval[5];
+ unsigned int hashval[4];
- memcpy(hashval, obj->sha1, 20);
+ memcpy(hashval, obj->sha1, 16);
ix = H(hashval, 0);
if (!obj_hash[ix]) {
obj_hash[ix] = obj;
@@ -103,11 +101,6 @@ static struct object *insert_obj_hash(struct object *obj)
return NULL;
}
ix = H(hashval, 3);
- if (!obj_hash[ix]) {
- obj_hash[ix] = obj;
- return NULL;
- }
- ix = H(hashval, 4);
tmp_obj = obj_hash[ix];
obj_hash[ix] = obj;
if (!tmp_obj)
--
1.7.6.433.g1421f
^ permalink raw reply related
* [PATCH 10/11] object: try 3-way cuckoo
From: Junio C Hamano @ 2011-08-11 17:53 UTC (permalink / raw)
To: git
In-Reply-To: <1313085196-13249-1-git-send-email-gitster@pobox.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
../+v/48b49262cd2d0d64696b489fe5bdfa8efb9180f5/git-pack-objects
Counting objects: 2139209, done.
30.68user 2.26system 0:33.05elapsed 99%CPU (0avgtext+0avgdata 3660832maxresident)k
0inputs+0outputs (0major+421963minor)pagefaults 0swaps
Counting objects: 2139209, done.
30.76user 2.26system 0:33.12elapsed 99%CPU (0avgtext+0avgdata 3660480maxresident)k
0inputs+0outputs (0major+421941minor)pagefaults 0swaps
Counting objects: 2139209, done.
30.74user 2.33system 0:33.18elapsed 99%CPU (0avgtext+0avgdata 3661696maxresident)k
0inputs+0outputs (0major+422017minor)pagefaults 0swaps
---
object.c | 15 ++++-----------
1 files changed, 4 insertions(+), 11 deletions(-)
diff --git a/object.c b/object.c
index caced56..18e6bcf 100644
--- a/object.c
+++ b/object.c
@@ -49,20 +49,18 @@ struct object *get_indexed_object(unsigned int idx)
struct object *lookup_object(const unsigned char *sha1)
{
struct object *obj;
- unsigned int hashval[4];
+ unsigned int hashval[3];
if (!obj_hash)
return NULL;
- memcpy(hashval, sha1, 16);
+ memcpy(hashval, sha1, 12);
if ((obj = obj_hash[H(hashval, 0)]) && !hashcmp(sha1, obj->sha1))
return obj;
if ((obj = obj_hash[H(hashval, 1)]) && !hashcmp(sha1, obj->sha1))
return obj;
if ((obj = obj_hash[H(hashval, 2)]) && !hashcmp(sha1, obj->sha1))
return obj;
- if ((obj = obj_hash[H(hashval, 3)]) && !hashcmp(sha1, obj->sha1))
- return obj;
return NULL;
}
@@ -82,9 +80,9 @@ static struct object *insert_obj_hash(struct object *obj)
for (loop = obj_hash_size / 2; 0 <= loop; loop--) {
struct object *tmp_obj;
unsigned int ix;
- unsigned int hashval[4];
+ unsigned int hashval[3];
- memcpy(hashval, obj->sha1, 16);
+ memcpy(hashval, obj->sha1, 12);
ix = H(hashval, 0);
if (!obj_hash[ix]) {
obj_hash[ix] = obj;
@@ -96,11 +94,6 @@ static struct object *insert_obj_hash(struct object *obj)
return NULL;
}
ix = H(hashval, 2);
- if (!obj_hash[ix]) {
- obj_hash[ix] = obj;
- return NULL;
- }
- ix = H(hashval, 3);
tmp_obj = obj_hash[ix];
obj_hash[ix] = obj;
if (!tmp_obj)
--
1.7.6.433.g1421f
^ permalink raw reply related
* [PATCH 11/11] object: try 2-way cuckoo again
From: Junio C Hamano @ 2011-08-11 17:53 UTC (permalink / raw)
To: git
In-Reply-To: <1313085196-13249-1-git-send-email-gitster@pobox.com>
This should match the first "naive" cuckoo hashing implementation.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
../+v/ddb2c7c09c7af16bf00aa5aef6cdf54ae4611aec/git-pack-objects
Counting objects: 2139209, done.
31.28user 3.15system 0:34.53elapsed 99%CPU (0avgtext+0avgdata 8177056maxresident)k
0inputs+0outputs (0major+1206514minor)pagefaults 0swaps
Counting objects: 2139209, done.
31.06user 3.26system 0:34.43elapsed 99%CPU (0avgtext+0avgdata 8177056maxresident)k
0inputs+0outputs (0major+1206514minor)pagefaults 0swaps
Counting objects: 2139209, done.
31.06user 3.22system 0:34.39elapsed 99%CPU (0avgtext+0avgdata 8176512maxresident)k
0inputs+0outputs (0major+1206542minor)pagefaults 0swaps
---
object.c | 15 ++++-----------
1 files changed, 4 insertions(+), 11 deletions(-)
diff --git a/object.c b/object.c
index 18e6bcf..fc08e5b 100644
--- a/object.c
+++ b/object.c
@@ -49,18 +49,16 @@ struct object *get_indexed_object(unsigned int idx)
struct object *lookup_object(const unsigned char *sha1)
{
struct object *obj;
- unsigned int hashval[3];
+ unsigned int hashval[2];
if (!obj_hash)
return NULL;
- memcpy(hashval, sha1, 12);
+ memcpy(hashval, sha1, 8);
if ((obj = obj_hash[H(hashval, 0)]) && !hashcmp(sha1, obj->sha1))
return obj;
if ((obj = obj_hash[H(hashval, 1)]) && !hashcmp(sha1, obj->sha1))
return obj;
- if ((obj = obj_hash[H(hashval, 2)]) && !hashcmp(sha1, obj->sha1))
- return obj;
return NULL;
}
@@ -80,20 +78,15 @@ static struct object *insert_obj_hash(struct object *obj)
for (loop = obj_hash_size / 2; 0 <= loop; loop--) {
struct object *tmp_obj;
unsigned int ix;
- unsigned int hashval[3];
+ unsigned int hashval[2];
- memcpy(hashval, obj->sha1, 12);
+ memcpy(hashval, obj->sha1, 8);
ix = H(hashval, 0);
if (!obj_hash[ix]) {
obj_hash[ix] = obj;
return NULL;
}
ix = H(hashval, 1);
- if (!obj_hash[ix]) {
- obj_hash[ix] = obj;
- return NULL;
- }
- ix = H(hashval, 2);
tmp_obj = obj_hash[ix];
obj_hash[ix] = obj;
if (!tmp_obj)
--
1.7.6.433.g1421f
^ permalink raw reply related
* [PATCH 08/11] object: try 5-way cuckoo -- use all 20-bytes of SHA-1
From: Junio C Hamano @ 2011-08-11 17:53 UTC (permalink / raw)
To: git
In-Reply-To: <1313085196-13249-1-git-send-email-gitster@pobox.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
../+v/ced878c983ddc9a256b22535aa8af006529ec3d0/git-pack-objects
Counting objects: 2139209, done.
31.66user 2.14system 0:33.91elapsed 99%CPU (0avgtext+0avgdata 2874176maxresident)k
0inputs+0outputs (0major+225342minor)pagefaults 0swaps
Counting objects: 2139209, done.
31.79user 2.03system 0:33.93elapsed 99%CPU (0avgtext+0avgdata 2875808maxresident)k
0inputs+0outputs (0major+225444minor)pagefaults 0swaps
Counting objects: 2139209, done.
31.80user 1.94system 0:33.84elapsed 99%CPU (0avgtext+0avgdata 2875792maxresident)k
0inputs+0outputs (0major+225443minor)pagefaults 0swaps
---
object.c | 43 ++++++++++++++++++++++++++++++-------------
1 files changed, 30 insertions(+), 13 deletions(-)
diff --git a/object.c b/object.c
index 7624c48..c777520 100644
--- a/object.c
+++ b/object.c
@@ -43,27 +43,27 @@ struct object *get_indexed_object(unsigned int idx)
return obj_hash[idx];
}
-static unsigned int hash_val(const unsigned char *sha1)
-{
- unsigned int hash;
- memcpy(&hash, sha1, sizeof(unsigned int));
- return hash;
-}
-
-#define H1(sha1) (hash_val(sha1) % obj_hash_size)
-#define H2(sha1) (hash_val((sha1) + sizeof(unsigned int)) % obj_hash_size)
+#define H(hv,ix) ((hv[ix]) & (obj_hash_size-1))
struct object *lookup_object(const unsigned char *sha1)
{
struct object *obj;
+ unsigned int hashval[5];
if (!obj_hash)
return NULL;
- if ((obj = obj_hash[H1(sha1)]) && !hashcmp(sha1, obj->sha1))
+ memcpy(hashval, sha1, 20);
+ if ((obj = obj_hash[H(hashval, 0)]) && !hashcmp(sha1, obj->sha1))
+ return obj;
+ if ((obj = obj_hash[H(hashval, 1)]) && !hashcmp(sha1, obj->sha1))
+ return obj;
+ if ((obj = obj_hash[H(hashval, 2)]) && !hashcmp(sha1, obj->sha1))
return obj;
- if ((obj = obj_hash[H2(sha1)]) && !hashcmp(sha1, obj->sha1))
+ if ((obj = obj_hash[H(hashval, 3)]) && !hashcmp(sha1, obj->sha1))
+ return obj;
+ if ((obj = obj_hash[H(hashval, 4)]) && !hashcmp(sha1, obj->sha1))
return obj;
return NULL;
}
@@ -84,13 +84,30 @@ static struct object *insert_obj_hash(struct object *obj)
for (loop = obj_hash_size / 2; 0 <= loop; loop--) {
struct object *tmp_obj;
unsigned int ix;
+ unsigned int hashval[5];
- ix = H1(obj->sha1);
+ memcpy(hashval, obj->sha1, 20);
+ ix = H(hashval, 0);
+ if (!obj_hash[ix]) {
+ obj_hash[ix] = obj;
+ return NULL;
+ }
+ ix = H(hashval, 1);
+ if (!obj_hash[ix]) {
+ obj_hash[ix] = obj;
+ return NULL;
+ }
+ ix = H(hashval, 2);
+ if (!obj_hash[ix]) {
+ obj_hash[ix] = obj;
+ return NULL;
+ }
+ ix = H(hashval, 3);
if (!obj_hash[ix]) {
obj_hash[ix] = obj;
return NULL;
}
- ix = H2(obj->sha1);
+ ix = H(hashval, 4);
tmp_obj = obj_hash[ix];
obj_hash[ix] = obj;
if (!tmp_obj)
--
1.7.6.433.g1421f
^ permalink raw reply related
* [PATCH 06/11] object: growing the hash-table more aggressively does not help much
From: Junio C Hamano @ 2011-08-11 17:53 UTC (permalink / raw)
To: git
In-Reply-To: <1313085196-13249-1-git-send-email-gitster@pobox.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
../+v/2555c0d98970098861c97b7746f2b8846c823e6f/git-pack-objects
Counting objects: 2139209, done.
32.07user 1.96system 0:34.15elapsed 99%CPU (0avgtext+0avgdata 2966176maxresident)k
0inputs+0outputs (0major+225410minor)pagefaults 0swaps
Counting objects: 2139209, done.
31.89user 2.16system 0:34.16elapsed 99%CPU (0avgtext+0avgdata 2965264maxresident)k
0inputs+0outputs (0major+225336minor)pagefaults 0swaps
Counting objects: 2139209, done.
32.01user 2.12system 0:34.23elapsed 99%CPU (0avgtext+0avgdata 2964832maxresident)k
0inputs+0outputs (0major+225332minor)pagefaults 0swaps
---
object.c | 3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/object.c b/object.c
index d95d7a6..c2c0a7d 100644
--- a/object.c
+++ b/object.c
@@ -83,7 +83,8 @@ struct object *lookup_object(const unsigned char *sha1)
static int next_size(int sz)
{
- return sz < 32 ? 32 : 2 * sz;
+ return (sz < 32 ? 32 :
+ (sz < 1024 * 1024 ? 8 : 2) * sz);
}
static void grow_object_hash(void)
--
1.7.6.433.g1421f
^ permalink raw reply related
* [PATCH 07/11] object: try naive cuckoo hashing
From: Junio C Hamano @ 2011-08-11 17:53 UTC (permalink / raw)
To: git
In-Reply-To: <1313085196-13249-1-git-send-email-gitster@pobox.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
../+v/65c71a61547aecd8a6830391fda31ed5d18e0529/git-pack-objects
Counting objects: 2139209, done.
32.04user 3.43system 0:35.58elapsed 99%CPU (0avgtext+0avgdata 8178672maxresident)k
0inputs+0outputs (0major+1206546minor)pagefaults 0swaps
Counting objects: 2139209, done.
33.41user 3.15system 0:36.67elapsed 99%CPU (0avgtext+0avgdata 8178688maxresident)k
0inputs+0outputs (0major+1206547minor)pagefaults 0swaps
Counting objects: 2139209, done.
32.24user 3.17system 0:35.51elapsed 99%CPU (0avgtext+0avgdata 8179968maxresident)k
0inputs+0outputs (0major+1206598minor)pagefaults 0swaps
---
object.c | 99 ++++++++++++++++++++++++++++++++++++++++++--------------------
1 files changed, 67 insertions(+), 32 deletions(-)
diff --git a/object.c b/object.c
index c2c0a7d..7624c48 100644
--- a/object.c
+++ b/object.c
@@ -50,33 +50,52 @@ static unsigned int hash_val(const unsigned char *sha1)
return hash;
}
-static void insert_obj_hash(struct object *obj, struct object **hash, unsigned int size)
-{
- unsigned int j = hash_val(obj->sha1) & (size-1);
- while (hash[j]) {
- j++;
- if (j >= size)
- j = 0;
- }
- hash[j] = obj;
-}
+#define H1(sha1) (hash_val(sha1) % obj_hash_size)
+#define H2(sha1) (hash_val((sha1) + sizeof(unsigned int)) % obj_hash_size)
struct object *lookup_object(const unsigned char *sha1)
{
- unsigned int i;
struct object *obj;
if (!obj_hash)
return NULL;
- i = hash_val(sha1) & (obj_hash_size-1);
- while ((obj = obj_hash[i]) != NULL) {
- if (!hashcmp(sha1, obj->sha1))
- break;
- i++;
- if (i == obj_hash_size)
- i = 0;
+ if ((obj = obj_hash[H1(sha1)]) && !hashcmp(sha1, obj->sha1))
+ return obj;
+ if ((obj = obj_hash[H2(sha1)]) && !hashcmp(sha1, obj->sha1))
+ return obj;
+ return NULL;
+}
+
+static void grow_object_hash(void); /* forward */
+
+/*
+ * A naive single-table cuckoo hashing implementation.
+ * Return NULL when "obj" is successfully inserted. Otherwise
+ * return a pointer to the object to be inserted (which may
+ * be different from the original obj). The caller is expected
+ * to grow the hash table and re-insert the returned object.
+ */
+static struct object *insert_obj_hash(struct object *obj)
+{
+ int loop;
+
+ for (loop = obj_hash_size / 2; 0 <= loop; loop--) {
+ struct object *tmp_obj;
+ unsigned int ix;
+
+ ix = H1(obj->sha1);
+ if (!obj_hash[ix]) {
+ obj_hash[ix] = obj;
+ return NULL;
+ }
+ ix = H2(obj->sha1);
+ tmp_obj = obj_hash[ix];
+ obj_hash[ix] = obj;
+ if (!tmp_obj)
+ return NULL;
+ obj = tmp_obj;
}
return obj;
}
@@ -89,25 +108,36 @@ static int next_size(int sz)
static void grow_object_hash(void)
{
- int i;
- int new_hash_size = next_size(obj_hash_size);
- struct object **new_hash;
-
- new_hash = xcalloc(new_hash_size, sizeof(struct object *));
- for (i = 0; i < obj_hash_size; i++) {
- struct object *obj = obj_hash[i];
- if (!obj)
+ struct object **current_hash;
+ int current_size;
+
+ current_hash = obj_hash;
+ current_size = obj_hash_size;
+ while (1) {
+ int i;
+ obj_hash_size = next_size(obj_hash_size);
+ obj_hash = xcalloc(obj_hash_size, sizeof(struct object *));
+
+ for (i = 0; i < current_size; i++) {
+ if (!current_hash[i])
+ continue;
+ if (insert_obj_hash(current_hash[i]))
+ break;
+ }
+ if (i < current_size) {
+ /* too small - grow and retry */
+ free(obj_hash);
continue;
- insert_obj_hash(obj, new_hash, new_hash_size);
+ }
+ free(current_hash);
+ return;
}
- free(obj_hash);
- obj_hash = new_hash;
- obj_hash_size = new_hash_size;
}
void *create_object(const unsigned char *sha1, int type, void *o)
{
struct object *obj = o;
+ struct object *to_insert;
obj->parsed = 0;
obj->used = 0;
@@ -117,8 +147,13 @@ void *create_object(const unsigned char *sha1, int type, void *o)
if (obj_hash_size - 1 <= nr_objs * 2)
grow_object_hash();
-
- insert_obj_hash(obj, obj_hash, obj_hash_size);
+ to_insert = obj;
+ while (1) {
+ to_insert = insert_obj_hash(to_insert);
+ if (!to_insert)
+ break;
+ grow_object_hash();
+ }
nr_objs++;
return obj;
}
--
1.7.6.433.g1421f
^ permalink raw reply related
* [PATCH 03/11] pack-objects --count-only
From: Junio C Hamano @ 2011-08-11 17:53 UTC (permalink / raw)
To: git
In-Reply-To: <1313085196-13249-1-git-send-email-gitster@pobox.com>
This is merely to help debugging the bottleneck of "Counting objects"
phase of the pack-objects process. Use it like this:
git pack-objects --count-only --keep-true-parents --honor-pack-keep \
--non-empty --all --reflog --no-reuse-delta \
--delta-base-offset --stdout </dev/null >/dev/null
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
builtin/pack-objects.c | 7 +++++++
1 files changed, 7 insertions(+), 0 deletions(-)
diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index f402a84..6a208a9 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -2121,6 +2121,7 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix)
int use_internal_rev_list = 0;
int thin = 0;
int all_progress_implied = 0;
+ int count_only = 0;
uint32_t i;
const char **rp_av;
int rp_ac_alloc = 64;
@@ -2291,6 +2292,10 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix)
grafts_replace_parents = 0;
continue;
}
+ if (!strcmp("--count-only", arg)) {
+ count_only = 1;
+ continue;
+ }
usage(pack_usage);
}
@@ -2348,6 +2353,8 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix)
if (non_empty && !nr_result)
return 0;
+ if (count_only)
+ return 0;
if (nr_result)
prepare_pack(window, depth);
write_pack_file();
--
1.7.6.433.g1421f
^ permalink raw reply related
* [PATCH 05/11] object hash: we know the table size is a power of two
From: Junio C Hamano @ 2011-08-11 17:53 UTC (permalink / raw)
To: git
In-Reply-To: <1313085196-13249-1-git-send-email-gitster@pobox.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
object.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/object.c b/object.c
index 259a67e..d95d7a6 100644
--- a/object.c
+++ b/object.c
@@ -52,7 +52,7 @@ static unsigned int hash_val(const unsigned char *sha1)
static void insert_obj_hash(struct object *obj, struct object **hash, unsigned int size)
{
- unsigned int j = hash_val(obj->sha1) % size;
+ unsigned int j = hash_val(obj->sha1) & (size-1);
while (hash[j]) {
j++;
@@ -70,7 +70,7 @@ struct object *lookup_object(const unsigned char *sha1)
if (!obj_hash)
return NULL;
- i = hash_val(sha1) % obj_hash_size;
+ i = hash_val(sha1) & (obj_hash_size-1);
while ((obj = obj_hash[i]) != NULL) {
if (!hashcmp(sha1, obj->sha1))
break;
--
1.7.6.433.g1421f
^ permalink raw reply related
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