* Re: [feature wishlist] add commit subcommand to git add -i
From: Conrad Irwin @ 2011-08-14 8:48 UTC (permalink / raw)
To: Jim Cromie; +Cc: git
In-Reply-To: <CAJfuBxwW8Dyp8FTS13uPOBKZGL9JOEqaSOhGN+zBJ_8BHpJE3g@mail.gmail.com>
On Sun, Aug 14, 2011 at 1:38 AM, Jim Cromie <jim.cromie@gmail.com> wrote:
> going further, if git rebase -i had ability to "back" a fixup patch
> back to where it should have been, and adjust the intervening patches
> where conflict would normally happen, that would be awesome.
> Simplistically, this would just shift the patch 1 step back iteratively,
> until it wouldnt apply properly, and then --abort, stopping at the last
> clean rebase.
>
> apologies if this is too hair-brained, or already done.
It sounds like you're looking for several git commit
(-p|--interactive) --fixup <commit>, followed by a git rebase -i
--autosquash. It's not quite as automatic as you describe, but I think
that automating it would be pretty hard to do correctly.
Conrad
^ permalink raw reply
* Re: What's cooking in git.git (Aug 2011, #03; Thu, 11)
From: Ramkumar Ramachandra @ 2011-08-14 8:44 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vr54rpogf.fsf@alter.siamese.dyndns.org>
Hi Junio,
Junio C Hamano writes:
> I am envisioning that we would declare feature freeze after most of the
> topics that are in 'next' as of today graduate to 'master' for the next
> release, and keep the remainder cooking for the cycle after that. Hopefully
> that would happen around 24th.
> * rr/revert-cherry-pick-continue (2011-08-08) 18 commits
> [...]
> Will merge to "next".
If this won't graduate to 'next' before the 24th, I have plenty of
time to re-roll fixing all the issues that I've addressed in the
beginning of the new series. Depending on what you'd prefer, I can:
0. Do nothing. Get this series merged, and get the fresh series
merged as originally planned.
1. Squash the first few patches in the new series into this series and
post a re-roll. The new series will then be left with just two
commits.
2. Post a new series that combines both. I won't introduce any new
code, so no fresh review will be required. Since both the series have
been reviewed and tested independently, I don't see why this shouldn't
be possible.
Do let me know what you'd like. Thanks.
-- Ram
^ permalink raw reply
* [feature wishlist] add commit subcommand to git add -i
From: Jim Cromie @ 2011-08-14 8:38 UTC (permalink / raw)
To: git
when using git add -i, it would be handy to have a [c]ommit option.
This would save some typing:
q # to quit git add -i
$> commit ....
git add -i <file> # to resume incremental adds
*** Commands ***
1: [s]tatus 2: [u]pdate 3: [r]evert 4: [a]dd untracked
5: [p]atch 6: [d]iff 7: [q]uit 8: [h]elp
9: [c]commit
What now>
[f]ragment would also be handy, which would break each chunk of a diff
into a separate commit, with the summary line provided automatically
<file> @@ -696,7 +692,7 @@ int foo ...
This would help a bit with random cleanups, since rebase -i could then
be used to
reorder and recombine the fragments, and edit the commit messages afterwards.
going further, if git rebase -i had ability to "back" a fixup patch
back to where it should have been, and adjust the intervening patches
where conflict would normally happen, that would be awesome.
Simplistically, this would just shift the patch 1 step back iteratively,
until it wouldnt apply properly, and then --abort, stopping at the last
clean rebase.
apologies if this is too hair-brained, or already done.
^ permalink raw reply
* [PATCH 7/7] sequencer: Remove sequencer state after final commit
From: Ramkumar Ramachandra @ 2011-08-14 8:33 UTC (permalink / raw)
To: Junio C Hamano
Cc: Git List, Jonathan Nieder, Christian Couder, Daniel Barkalow,
Jeff King
In-Reply-To: <1313310789-10216-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, make 'git commit' notify the sequencer after
every successful commit; the sequencer then removes the state if no
more instructions are pending.
Mentored-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
---
builtin/commit.c | 8 ++++++++
sequencer.c | 23 ++++++++++++-----------
sequencer.h | 1 +
t/t3032-merge-recursive-options.sh | 2 ++
t/t3510-cherry-pick-sequence.sh | 4 ++--
5 files changed, 25 insertions(+), 13 deletions(-)
diff --git a/builtin/commit.c b/builtin/commit.c
index e1af9b1..1699371 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,6 +1522,13 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
unlink(git_path("MERGE_MODE"));
unlink(git_path("SQUASH_MSG"));
+ /*
+ * Notify the sequencer that we're committing. The routine
+ * removes the sequencer state if our commit just completed
+ * the last instruction.
+ */
+ sequencer_notify_commit();
+
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"
diff --git a/sequencer.c b/sequencer.c
index 95bce89..caae932 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -580,6 +580,17 @@ static void read_populate_todo(struct replay_insn_list **todo_list)
die(_("Unusable instruction sheet: %s"), todo_file);
}
+void sequencer_notify_commit(void)
+{
+ struct replay_insn_list *todo_list = NULL;
+
+ if (!file_exists(git_path(SEQ_TODO_FILE)))
+ return;
+ read_populate_todo(&todo_list);
+ if (!todo_list->next)
+ remove_sequencer_state(1);
+}
+
static int populate_opts_cb(const char *key, const char *value, void *data)
{
struct replay_opts *opts = data;
@@ -743,18 +754,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)
- /*
- * An error was encountered while
- * picking the last commit; the
- * sequencer state is useless now --
- * the user simply needs to resolve
- * the conflict and commit
- */
- remove_sequencer_state(0);
+ if (res)
return res;
- }
}
/*
diff --git a/sequencer.h b/sequencer.h
index 5e56cd4..3b89d42 100644
--- a/sequencer.h
+++ b/sequencer.h
@@ -51,5 +51,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);
+void sequencer_notify_commit(void);
#endif
diff --git a/t/t3032-merge-recursive-options.sh b/t/t3032-merge-recursive-options.sh
index 2b17311..1da99b7 100755
--- a/t/t3032-merge-recursive-options.sh
+++ b/t/t3032-merge-recursive-options.sh
@@ -114,8 +114,10 @@ test_expect_success 'naive cherry-pick fails' '
git read-tree --reset -u HEAD &&
test_must_fail git cherry-pick --no-commit remote &&
git read-tree --reset -u HEAD &&
+ git cherry-pick --reset &&
test_must_fail git cherry-pick remote &&
test_must_fail git update-index --refresh &&
+ git cherry-pick --reset &&
grep "<<<<<<" text.txt
'
diff --git a/t/t3510-cherry-pick-sequence.sh b/t/t3510-cherry-pick-sequence.sh
index c284c96..abd13fe 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 6/7] sequencer: Expose API to cherry-picking machinery
From: Ramkumar Ramachandra @ 2011-08-14 8:33 UTC (permalink / raw)
To: Junio C Hamano
Cc: Git List, Jonathan Nieder, Christian Couder, Daniel Barkalow,
Jeff King
In-Reply-To: <1313310789-10216-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 | 824 +-----------------------------------------------------
sequencer.c | 805 ++++++++++++++++++++++++++++++++++++++++++++++++++++-
sequencer.h | 27 ++
3 files changed, 832 insertions(+), 824 deletions(-)
diff --git a/builtin/revert.c b/builtin/revert.c
index e8bdeea..7ddfe84 100644
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -1,19 +1,9 @@
#include "cache.h"
#include "builtin.h"
-#include "object.h"
-#include "commit.h"
-#include "tag.h"
-#include "run-command.h"
-#include "exec_cmd.h"
-#include "utf8.h"
#include "parse-options.h"
-#include "cache-tree.h"
#include "diff.h"
#include "revision.h"
#include "rerere.h"
-#include "merge-recursive.h"
-#include "refs.h"
-#include "dir.h"
#include "sequencer.h"
/*
@@ -39,40 +29,11 @@ static const char * const cherry_pick_usage[] = {
NULL
};
-enum replay_subcommand { REPLAY_NONE, REPLAY_RESET, REPLAY_CONTINUE };
-
-struct replay_opts {
- enum replay_action action;
- enum replay_subcommand subcommand;
-
- /* Boolean options */
- int edit;
- int record_origin;
- int no_commit;
- int signoff;
- int allow_ff;
- int allow_rerere_auto;
-
- int mainline;
-
- /* Merge strategy */
- const char *strategy;
- const char **xopts;
- size_t xopts_nr, xopts_alloc;
-
- /* Only used by REPLAY_NONE */
- struct rev_info *revs;
-};
-
-#define GIT_REFLOG_ACTION "GIT_REFLOG_ACTION"
-
static const char *action_name(const struct replay_opts *opts)
{
return opts->action == REPLAY_REVERT ? "revert" : "cherry-pick";
}
-static char *get_encoding(const char *message);
-
static const char * const *revert_or_cherry_pick_usage(struct replay_opts *opts)
{
return opts->action == REPLAY_REVERT ? revert_usage : cherry_pick_usage;
@@ -224,787 +185,6 @@ static void parse_args(int argc, const char **argv, struct replay_opts *opts)
usage_with_options(usage_str, options);
}
-struct commit_message {
- char *parent_label;
- const char *label;
- const char *subject;
- char *reencoded_message;
- const char *message;
-};
-
-static int get_message(struct commit *commit, struct commit_message *out)
-{
- const char *encoding;
- const char *abbrev, *subject;
- int abbrev_len, subject_len;
- char *q;
-
- if (!commit->buffer)
- return -1;
- encoding = get_encoding(commit->buffer);
- if (!encoding)
- encoding = "UTF-8";
- if (!git_commit_encoding)
- git_commit_encoding = "UTF-8";
-
- out->reencoded_message = NULL;
- out->message = commit->buffer;
- if (strcmp(encoding, git_commit_encoding))
- out->reencoded_message = reencode_string(commit->buffer,
- git_commit_encoding, encoding);
- if (out->reencoded_message)
- out->message = out->reencoded_message;
-
- abbrev = find_unique_abbrev(commit->object.sha1, DEFAULT_ABBREV);
- abbrev_len = strlen(abbrev);
-
- subject_len = find_commit_subject(out->message, &subject);
-
- out->parent_label = xmalloc(strlen("parent of ") + abbrev_len +
- strlen("... ") + subject_len + 1);
- q = out->parent_label;
- q = mempcpy(q, "parent of ", strlen("parent of "));
- out->label = q;
- q = mempcpy(q, abbrev, abbrev_len);
- q = mempcpy(q, "... ", strlen("... "));
- out->subject = q;
- q = mempcpy(q, subject, subject_len);
- *q = '\0';
- return 0;
-}
-
-static void free_message(struct commit_message *msg)
-{
- free(msg->parent_label);
- free(msg->reencoded_message);
-}
-
-static char *get_encoding(const char *message)
-{
- const char *p = message, *eol;
-
- while (*p && *p != '\n') {
- for (eol = p + 1; *eol && *eol != '\n'; eol++)
- ; /* do nothing */
- if (!prefixcmp(p, "encoding ")) {
- char *result = xmalloc(eol - 8 - p);
- strlcpy(result, p + 9, eol - 8 - p);
- return result;
- }
- p = eol;
- if (*p == '\n')
- p++;
- }
- return NULL;
-}
-
-static void write_cherry_pick_head(struct commit *commit)
-{
- int fd;
- struct strbuf buf = STRBUF_INIT;
-
- strbuf_addf(&buf, "%s\n", sha1_to_hex(commit->object.sha1));
-
- fd = open(git_path("CHERRY_PICK_HEAD"), O_WRONLY | O_CREAT, 0666);
- if (fd < 0)
- die_errno(_("Could not open '%s' for writing"),
- git_path("CHERRY_PICK_HEAD"));
- if (write_in_full(fd, buf.buf, buf.len) != buf.len || close(fd))
- die_errno(_("Could not write to '%s'"), git_path("CHERRY_PICK_HEAD"));
- strbuf_release(&buf);
-}
-
-static void print_advice(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_name(opts), sha1_to_hex(parent->object.sha1));
-
- if (get_message(commit, &msg) != 0)
- return error(_("Cannot get commit message for %s"),
- sha1_to_hex(commit->object.sha1));
-
- /*
- * "commit" is an existing commit. We would want to apply
- * the difference it introduces since its first parent "prev"
- * on top of the current HEAD if we are cherry-pick. Or the
- * reverse of it if we are revert.
- */
-
- defmsg = git_pathdup("MERGE_MSG");
-
- if (action == REPLAY_REVERT) {
- base = commit;
- base_label = msg.label;
- next = parent;
- next_label = msg.parent_label;
- strbuf_addstr(&msgbuf, "Revert \"");
- strbuf_addstr(&msgbuf, msg.subject);
- strbuf_addstr(&msgbuf, "\"\n\nThis reverts commit ");
- strbuf_addstr(&msgbuf, sha1_to_hex(commit->object.sha1));
-
- if (commit->parents && commit->parents->next) {
- strbuf_addstr(&msgbuf, ", reversing\nchanges made to ");
- strbuf_addstr(&msgbuf, sha1_to_hex(parent->object.sha1));
- }
- strbuf_addstr(&msgbuf, ".\n");
- } else {
- const char *p;
-
- base = parent;
- base_label = msg.parent_label;
- next = commit;
- next_label = msg.label;
-
- /*
- * Append the commit log message to msgbuf; it starts
- * after the tree, parent, author, committer
- * information followed by "\n\n".
- */
- p = strstr(msg.message, "\n\n");
- if (p) {
- p += 2;
- strbuf_addstr(&msgbuf, p);
- }
-
- if (opts->record_origin) {
- strbuf_addstr(&msgbuf, "(cherry picked from commit ");
- strbuf_addstr(&msgbuf, sha1_to_hex(commit->object.sha1));
- strbuf_addstr(&msgbuf, ")\n");
- }
- if (!opts->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 replay_opts *opts)
-{
- if (opts->action != REPLAY_REVERT)
- opts->revs->reverse ^= 1;
-
- if (prepare_revision_walk(opts->revs))
- die(_("revision walk setup failed"));
-
- if (!opts->revs->commits)
- die(_("empty commit set passed"));
-}
-
-static void read_and_refresh_cache(struct replay_opts *opts)
-{
- static struct lock_file index_lock;
- int index_fd = hold_locked_index(&index_lock, 0);
- if (read_index_preload(&the_index, NULL) < 0)
- die(_("git %s: failed to read the index"), action_name(opts));
- refresh_index(&the_index, REFRESH_QUIET|REFRESH_UNMERGED, NULL, NULL, NULL);
- if (the_index.cache_changed) {
- if (write_index(&the_index, index_fd) ||
- commit_locked_index(&index_lock))
- die(_("git %s: failed to refresh the index"), action_name(opts));
- }
- rollback_lock_file(&index_lock);
-}
-
-/*
- * Append a commit to the end of the commit_list.
- *
- * next starts by pointing to the variable that holds the head of an
- * empty commit_list, and is updated to point to the "next" field of
- * the last item on the list as new commits are appended.
- *
- * Usage example:
- *
- * struct commit_list *list;
- * struct commit_list **next = &list;
- *
- * next = commit_list_append(c1, next);
- * next = commit_list_append(c2, next);
- * assert(commit_list_count(list) == 2);
- * return list;
- */
-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];
- const char *p, *q;
-
- p = start;
- if (!prefixcmp(start, "pick ")) {
- item->action = REPLAY_PICK;
- p += strlen("pick ");
- } else if (!prefixcmp(start, "revert ")) {
- item->action = REPLAY_REVERT;
- p += strlen("revert ");
- } else {
- int len = strchrnul(p, '\n') - p;
- if (len > 255)
- len = 255;
- return error(_("Unrecognized action: %.*s"), len, p);
- }
-
- q = p + strcspn(p, " \n");
- if (q - p + 1 > sizeof(sha1_abbrev)) {
- int len = q - p;
- if (len > 255)
- len = 255;
- return error(_("Object name too large: %.*s"), len, 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(_("on 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 commit *commit;
- struct replay_insn_list **next;
-
- prepare_revs(opts);
-
- next = todo_list;
- while ((commit = get_revision(opts->revs)))
- next = replay_insn_list_append(opts->action, commit, next);
-}
-
-static int create_seq_dir(void)
-{
- const char *seq_dir = git_path(SEQ_DIR);
-
- if (file_exists(seq_dir))
- return error(_("%s already exists."), seq_dir);
- else if (mkdir(seq_dir, 0777) < 0)
- die_errno(_("Could not create sequencer directory '%s'."), seq_dir);
- return 0;
-}
-
-static void save_head(const char *head)
-{
- const char *head_file = git_path(SEQ_HEAD_FILE);
- static struct lock_file head_lock;
- struct strbuf buf = STRBUF_INIT;
- int fd;
-
- fd = hold_lock_file_for_update(&head_lock, head_file, LOCK_DIE_ON_ERROR);
- strbuf_addf(&buf, "%s\n", head);
- if (write_in_full(fd, buf.buf, buf.len) < 0)
- die_errno(_("Could not write to %s."), head_file);
- if (commit_lock_file(&head_lock) < 0)
- die(_("Error wrapping up %s."), head_file);
-}
-
-static void save_todo(struct replay_insn_list *todo_list)
-{
- const char *todo_file = git_path(SEQ_TODO_FILE);
- static struct lock_file todo_lock;
- struct strbuf buf = STRBUF_INIT;
- int fd;
-
- fd = hold_lock_file_for_update(&todo_lock, todo_file, LOCK_DIE_ON_ERROR);
- if (format_todo(&buf, todo_list) < 0)
- die(_("Could not format %s."), todo_file);
- if (write_in_full(fd, buf.buf, buf.len) < 0) {
- strbuf_release(&buf);
- die_errno(_("Could not write to %s."), todo_file);
- }
- if (commit_lock_file(&todo_lock) < 0) {
- strbuf_release(&buf);
- die(_("Error wrapping up %s."), todo_file);
- }
- strbuf_release(&buf);
-}
-
-static void save_opts(struct replay_opts *opts)
-{
- const char *opts_file = git_path(SEQ_OPTS_FILE);
-
- if (opts->no_commit)
- git_config_set_in_file(opts_file, "options.no-commit", "true");
- if (opts->edit)
- git_config_set_in_file(opts_file, "options.edit", "true");
- if (opts->signoff)
- git_config_set_in_file(opts_file, "options.signoff", "true");
- if (opts->record_origin)
- git_config_set_in_file(opts_file, "options.record-origin", "true");
- if (opts->allow_ff)
- git_config_set_in_file(opts_file, "options.allow-ff", "true");
- if (opts->mainline) {
- struct strbuf buf = STRBUF_INIT;
- strbuf_addf(&buf, "%d", opts->mainline);
- git_config_set_in_file(opts_file, "options.mainline", buf.buf);
- strbuf_release(&buf);
- }
- if (opts->strategy)
- git_config_set_in_file(opts_file, "options.strategy", opts->strategy);
- if (opts->xopts) {
- int i;
- for (i = 0; i < opts->xopts_nr; i++)
- git_config_set_multivar_in_file(opts_file,
- "options.strategy-option",
- opts->xopts[i], "^$", 0);
- }
-}
-
-static int pick_commits(struct replay_insn_list *todo_list,
- struct replay_opts *opts)
-{
- struct replay_insn_list *cur;
- int res;
-
- setenv(GIT_REFLOG_ACTION, action_name(opts), 0);
- if (opts->allow_ff)
- assert(!(opts->signoff || opts->no_commit ||
- opts->record_origin || opts->edit));
- read_and_refresh_cache(opts);
-
- for (cur = todo_list; cur; cur = cur->next) {
- save_todo(cur);
- res = do_pick_commit(cur->operand, cur->action, opts);
- if (res) {
- if (!cur->next)
- /*
- * An error was encountered while
- * picking the last commit; the
- * sequencer state is useless now --
- * the user simply needs to resolve
- * the conflict and commit
- */
- remove_sequencer_state(0);
- return res;
- }
- }
-
- /*
- * Sequence of picks finished successfully; cleanup by
- * removing the .git/sequencer directory
- */
- remove_sequencer_state(1);
- return 0;
-}
-
-static int pick_revisions(struct replay_opts *opts)
-{
- struct replay_insn_list *todo_list = NULL;
- unsigned char sha1[20];
-
- if (opts->subcommand == REPLAY_NONE)
- assert(opts->revs);
-
- read_and_refresh_cache(opts);
-
- /*
- * Decide what to do depending on the arguments; a fresh
- * cherry-pick should be handled differently from an existing
- * one that is being continued
- */
- if (opts->subcommand == REPLAY_RESET) {
- remove_sequencer_state(1);
- return 0;
- } else if (opts->subcommand == REPLAY_CONTINUE) {
- if (!file_exists(git_path(SEQ_TODO_FILE)))
- goto error;
- read_populate_opts(&opts);
- read_populate_todo(&todo_list);
-
- /* Verify that the conflict has been resolved */
- if (!index_differs_from("HEAD", 0))
- todo_list = todo_list->next;
- } else {
- /*
- * Start a new cherry-pick/ revert sequence; but
- * first, make sure that an existing one isn't in
- * progress
- */
-
- walk_revs_populate_todo(&todo_list, opts);
- if (create_seq_dir() < 0) {
- error(_("A cherry-pick or revert is in progress."));
- advise(_("Use --continue to continue the operation"));
- advise(_("or --reset to forget about it"));
- return -1;
- }
- if (get_sha1("HEAD", sha1)) {
- if (opts->action == REPLAY_REVERT)
- return error(_("Can't revert as initial commit"));
- return error(_("Can't cherry-pick into empty head"));
- }
- save_head(sha1_to_hex(sha1));
- save_opts(opts);
- }
- return pick_commits(todo_list, opts);
-error:
- return error(_("No %s in progress"), action_name(opts));
-}
-
int cmd_revert(int argc, const char **argv, const char *prefix)
{
struct replay_opts opts;
@@ -1016,7 +196,7 @@ int cmd_revert(int argc, const char **argv, const char *prefix)
opts.action = REPLAY_REVERT;
git_config(git_default_config, NULL);
parse_args(argc, argv, &opts);
- res = pick_revisions(&opts);
+ res = sequencer_pick_revisions(&opts);
if (res < 0)
die(_("revert failed"));
return res;
@@ -1031,7 +211,7 @@ int cmd_cherry_pick(int argc, const char **argv, const char *prefix)
opts.action = REPLAY_PICK;
git_config(git_default_config, NULL);
parse_args(argc, argv, &opts);
- res = pick_revisions(&opts);
+ res = sequencer_pick_revisions(&opts);
if (res < 0)
die(_("cherry-pick failed"));
return res;
diff --git a/sequencer.c b/sequencer.c
index bc2c046..95bce89 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -1,13 +1,656 @@
#include "cache.h"
#include "sequencer.h"
-#include "strbuf.h"
+#include "object.h"
+#include "commit.h"
+#include "tag.h"
+#include "run-command.h"
+#include "exec_cmd.h"
+#include "utf8.h"
+#include "cache-tree.h"
+#include "diff.h"
+#include "revision.h"
+#include "rerere.h"
+#include "merge-recursive.h"
+#include "refs.h"
#include "dir.h"
+#define GIT_REFLOG_ACTION "GIT_REFLOG_ACTION"
+#define COMMIT_MESSAGE_INIT { NULL, NULL, NULL, NULL, NULL };
+
+struct commit_message {
+ char *parent_label;
+ const char *label;
+ const char *subject;
+ char *reencoded_message;
+ const char *message;
+};
+
+static const char *action_keyword(const struct replay_opts *opts)
+{
+ return opts->action == REPLAY_REVERT ? "revert" : "cherry-pick";
+}
+
+static char *get_encoding(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_keyword(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_keyword(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_keyword(opts), sha1_to_hex(parent->object.sha1));
+
+ if (get_message(commit, &msg) != 0)
+ return error(_("Cannot get commit message for %s"),
+ sha1_to_hex(commit->object.sha1));
+
+ /*
+ * "commit" is an existing commit. We would want to apply
+ * the difference it introduces since its first parent "prev"
+ * on top of the current HEAD if we are cherry-pick. Or the
+ * reverse of it if we are revert.
+ */
+
+ defmsg = git_pathdup("MERGE_MSG");
+
+ if (action == REPLAY_REVERT) {
+ base = commit;
+ base_label = msg.label;
+ next = parent;
+ next_label = msg.parent_label;
+ strbuf_addstr(&msgbuf, "Revert \"");
+ strbuf_addstr(&msgbuf, msg.subject);
+ strbuf_addstr(&msgbuf, "\"\n\nThis reverts commit ");
+ strbuf_addstr(&msgbuf, sha1_to_hex(commit->object.sha1));
+
+ if (commit->parents && commit->parents->next) {
+ strbuf_addstr(&msgbuf, ", reversing\nchanges made to ");
+ strbuf_addstr(&msgbuf, sha1_to_hex(parent->object.sha1));
+ }
+ strbuf_addstr(&msgbuf, ".\n");
+ } else {
+ const char *p;
+
+ base = parent;
+ base_label = msg.parent_label;
+ next = commit;
+ next_label = msg.label;
+
+ /*
+ * Append the commit log message to msgbuf; it starts
+ * after the tree, parent, author, committer
+ * information followed by "\n\n".
+ */
+ p = strstr(msg.message, "\n\n");
+ if (p) {
+ p += 2;
+ strbuf_addstr(&msgbuf, p);
+ }
+
+ if (opts->record_origin) {
+ strbuf_addstr(&msgbuf, "(cherry picked from commit ");
+ strbuf_addstr(&msgbuf, sha1_to_hex(commit->object.sha1));
+ strbuf_addstr(&msgbuf, ")\n");
+ }
+ if (!opts->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 replay_opts *opts)
+{
+ if (opts->action != REPLAY_REVERT)
+ opts->revs->reverse ^= 1;
+
+ if (prepare_revision_walk(opts->revs))
+ die(_("revision walk setup failed"));
+
+ if (!opts->revs->commits)
+ die(_("empty commit set passed"));
+}
+
+static void read_and_refresh_cache(struct replay_opts *opts)
+{
+ static struct lock_file index_lock;
+ int index_fd = hold_locked_index(&index_lock, 0);
+ if (read_index_preload(&the_index, NULL) < 0)
+ die(_("git %s: failed to read the index"), action_keyword(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_keyword(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];
+ const char *p, *q;
+
+ p = start;
+ if (!prefixcmp(start, "pick ")) {
+ item->action = REPLAY_PICK;
+ p += strlen("pick ");
+ } else if (!prefixcmp(start, "revert ")) {
+ item->action = REPLAY_REVERT;
+ p += strlen("revert ");
+ } else {
+ int len = strchrnul(p, '\n') - p;
+ if (len > 255)
+ len = 255;
+ return error(_("Unrecognized action: %.*s"), len, p);
+ }
+
+ q = p + strcspn(p, " \n");
+ if (q - p + 1 > sizeof(sha1_abbrev)) {
+ int len = q - p;
+ if (len > 255)
+ len = 255;
+ return error(_("Object name too large: %.*s"), len, 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(_("on 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 commit *commit;
+ struct replay_insn_list **next;
+
+ prepare_revs(opts);
+
+ next = todo_list;
+ while ((commit = get_revision(opts->revs)))
+ next = replay_insn_list_append(opts->action, commit, next);
+}
+
+static int create_seq_dir(void)
+{
+ const char *seq_dir = git_path(SEQ_DIR);
+
+ if (file_exists(seq_dir))
+ return error(_("%s already exists."), seq_dir);
+ else if (mkdir(seq_dir, 0777) < 0)
+ die_errno(_("Could not create sequencer directory '%s'."), seq_dir);
+ return 0;
+}
+
void remove_sequencer_state(int aggressive)
{
struct strbuf seq_dir = STRBUF_INIT;
struct strbuf seq_old_dir = STRBUF_INIT;
-
strbuf_addf(&seq_dir, "%s", git_path(SEQ_DIR));
strbuf_addf(&seq_old_dir, "%s", git_path(SEQ_OLD_DIR));
remove_dir_recursively(&seq_old_dir, 0);
@@ -17,3 +660,161 @@ 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_keyword(opts), 0);
+ if (opts->allow_ff)
+ assert(!(opts->signoff || opts->no_commit ||
+ opts->record_origin || opts->edit));
+ read_and_refresh_cache(opts);
+
+ for (cur = todo_list; cur; cur = cur->next) {
+ save_todo(cur);
+ res = do_pick_commit(cur->operand, cur->action, opts);
+ if (res) {
+ if (!cur->next)
+ /*
+ * An error was encountered while
+ * picking the last commit; the
+ * sequencer state is useless now --
+ * the user simply needs to resolve
+ * the conflict and commit
+ */
+ remove_sequencer_state(0);
+ return res;
+ }
+ }
+
+ /*
+ * Sequence of picks finished successfully; cleanup by
+ * removing the .git/sequencer directory
+ */
+ remove_sequencer_state(1);
+ return 0;
+}
+
+int sequencer_pick_revisions(struct replay_opts *opts)
+{
+ struct replay_insn_list *todo_list = NULL;
+ unsigned char sha1[20];
+
+ if (opts->subcommand == REPLAY_NONE)
+ assert(opts->revs);
+
+ read_and_refresh_cache(opts);
+
+ /*
+ * Decide what to do depending on the arguments; a fresh
+ * cherry-pick should be handled differently from an existing
+ * one that is being continued
+ */
+ if (opts->subcommand == REPLAY_RESET) {
+ remove_sequencer_state(1);
+ return 0;
+ } else if (opts->subcommand == REPLAY_CONTINUE) {
+ if (!file_exists(git_path(SEQ_TODO_FILE)))
+ goto error;
+ read_populate_opts(&opts);
+ read_populate_todo(&todo_list);
+
+ /* Verify that the conflict has been resolved */
+ if (!index_differs_from("HEAD", 0))
+ todo_list = todo_list->next;
+ } else {
+ /*
+ * Start a new cherry-pick/ revert sequence; but
+ * first, make sure that an existing one isn't in
+ * progress
+ */
+
+ walk_revs_populate_todo(&todo_list, opts);
+ if (create_seq_dir() < 0) {
+ error(_("A cherry-pick or revert is in progress."));
+ advise(_("Use --continue to continue the operation"));
+ advise(_("or --reset to forget about it"));
+ return -1;
+ }
+ if (get_sha1("HEAD", sha1)) {
+ if (opts->action == REPLAY_REVERT)
+ return error(_("Can't revert as initial commit"));
+ return error(_("Can't cherry-pick into empty head"));
+ }
+ save_head(sha1_to_hex(sha1));
+ save_opts(opts);
+ }
+ return pick_commits(todo_list, opts);
+error:
+ return error(_("No %s in progress"), action_keyword(opts));
+}
diff --git a/sequencer.h b/sequencer.h
index f4db257..5e56cd4 100644
--- a/sequencer.h
+++ b/sequencer.h
@@ -8,6 +8,30 @@
#define SEQ_OPTS_FILE "sequencer/opts"
enum replay_action { REPLAY_REVERT, REPLAY_PICK };
+enum replay_subcommand { REPLAY_NONE, REPLAY_RESET, REPLAY_CONTINUE };
+
+struct replay_opts {
+ enum replay_action action;
+ enum replay_subcommand subcommand;
+
+ /* Boolean options */
+ int edit;
+ int record_origin;
+ int no_commit;
+ int signoff;
+ int allow_ff;
+ int allow_rerere_auto;
+
+ int mainline;
+
+ /* Merge strategy */
+ const char *strategy;
+ const char **xopts;
+ size_t xopts_nr, xopts_alloc;
+
+ /* Only used by REPLAY_NONE */
+ struct rev_info *revs;
+};
struct replay_insn_list {
enum replay_action action;
@@ -25,4 +49,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 5/7] revert: Make the argument parser responsible for setup_revisions
From: Ramkumar Ramachandra @ 2011-08-14 8:33 UTC (permalink / raw)
To: Junio C Hamano
Cc: Git List, Jonathan Nieder, Christian Couder, Daniel Barkalow,
Jeff King
In-Reply-To: <1313310789-10216-1-git-send-email-artagnon@gmail.com>
Currently, prepare_revs calls setup_revisions with the (argc, argv) in
the opts structure filled in by parse_args. As a result, prepare_revs
has to take up the responsibility of erroring out and printing usage
information when (argc, argv) is malformed. Since parse_args is doing
this for other kinds of parse errors anyway, give it the task of
calling setup_revisions as well.
Based-on-patch-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
---
builtin/revert.c | 53 +++++++++++++++++++++++++++++------------------------
1 files changed, 29 insertions(+), 24 deletions(-)
diff --git a/builtin/revert.c b/builtin/revert.c
index 8faca07..e8bdeea 100644
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -54,13 +54,14 @@ struct replay_opts {
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;
+
+ /* Only used by REPLAY_NONE */
+ struct rev_info *revs;
};
#define GIT_REFLOG_ACTION "GIT_REFLOG_ACTION"
@@ -163,9 +164,9 @@ static void parse_args(int argc, const char **argv, struct replay_opts *opts)
die(_("program error"));
}
- opts->commit_argc = parse_options(argc, argv, NULL, options, usage_str,
- PARSE_OPT_KEEP_ARGV0 |
- PARSE_OPT_KEEP_UNKNOWN);
+ 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,
@@ -200,9 +201,6 @@ static void parse_args(int argc, const char **argv, struct replay_opts *opts)
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,
@@ -210,7 +208,20 @@ static void parse_args(int argc, const char **argv, struct replay_opts *opts)
"-x", opts->record_origin,
"--edit", opts->edit,
NULL);
- opts->commit_argv = argv;
+
+ if (opts->subcommand == REPLAY_NONE) {
+ opts->revs = xmalloc(sizeof(*opts->revs));
+ init_revisions(opts->revs, NULL);
+ opts->revs->no_walk = 1;
+ if (argc < 2)
+ usage_with_options(usage_str, options);
+ argc = setup_revisions(argc, argv, opts->revs, NULL);
+ } else
+ opts->revs = NULL;
+
+ /* Forbid stray command-line arguments */
+ if (argc > 1)
+ usage_with_options(usage_str, options);
}
struct commit_message {
@@ -612,23 +623,15 @@ static int do_pick_commit(struct commit *commit, enum replay_action action,
return res;
}
-static void prepare_revs(struct rev_info *revs, struct replay_opts *opts)
+static void prepare_revs(struct replay_opts *opts)
{
- int argc;
-
- init_revisions(revs, NULL);
- revs->no_walk = 1;
if (opts->action != REPLAY_REVERT)
- revs->reverse = 1;
+ opts->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))
+ if (prepare_revision_walk(opts->revs))
die(_("revision walk setup failed"));
- if (!revs->commits)
+ if (!opts->revs->commits)
die(_("empty commit set passed"));
}
@@ -823,14 +826,13 @@ static void read_populate_opts(struct replay_opts **opts_ptr)
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);
+ prepare_revs(opts);
next = todo_list;
- while ((commit = get_revision(&revs)))
+ while ((commit = get_revision(opts->revs)))
next = replay_insn_list_append(opts->action, commit, next);
}
@@ -954,6 +956,9 @@ static int pick_revisions(struct replay_opts *opts)
struct replay_insn_list *todo_list = NULL;
unsigned char sha1[20];
+ if (opts->subcommand == REPLAY_NONE)
+ assert(opts->revs);
+
read_and_refresh_cache(opts);
/*
--
1.7.6.351.gb35ac.dirty
^ permalink raw reply related
* [PATCH 4/7] revert: Allow mixed pick and revert instructions
From: Ramkumar Ramachandra @ 2011-08-14 8:33 UTC (permalink / raw)
To: Junio C Hamano
Cc: Git List, Jonathan Nieder, Christian Couder, Daniel Barkalow,
Jeff King
In-Reply-To: <1313310789-10216-1-git-send-email-artagnon@gmail.com>
Parse the instruction list in '.git/sequencer/todo' as a list of
(action, operand) pairs, instead of assuming all instructions use the
same action. 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. While at it, also improve the error messages reported by the
insn sheet parser.
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 | 137 ++++++++++++++++++++-------------------
sequencer.h | 8 ++
t/t3510-cherry-pick-sequence.sh | 58 ++++++++++++++++
3 files changed, 135 insertions(+), 68 deletions(-)
diff --git a/builtin/revert.c b/builtin/revert.c
index 89b2bba..8faca07 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;
@@ -532,7 +532,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 +575,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 +594,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 +618,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,27 +664,29 @@ 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);
@@ -692,59 +694,58 @@ static int format_todo(struct strbuf *buf, struct commit_list *todo_list,
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;
const char *p, *q;
p = start;
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;
+ } else {
+ int len = strchrnul(p, '\n') - p;
+ if (len > 255)
+ len = 255;
+ return error(_("Unrecognized action: %.*s"), len, p);
+ }
q = p + strcspn(p, " \n");
- if (q - p + 1 > sizeof(sha1_abbrev))
- return NULL;
+ if (q - p + 1 > sizeof(sha1_abbrev)) {
+ int len = q - p;
+ if (len > 255)
+ len = 255;
+ return error(_("Object name too large: %.*s"), len, 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);
- return lookup_commit_reference(commit_sha1);
+ 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 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)
- return error(_("Could not parse line %d."), i);
- next = commit_list_append(commit, next);
+ if (parse_insn_line(p, &item) < 0)
+ return error(_("on line %d."), i);
+ next = replay_insn_list_append(item.action, item.operand, next);
p = strchrnul(p, '\n');
if (*p)
p++;
@@ -754,8 +755,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;
@@ -771,7 +771,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);
@@ -820,18 +820,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)
@@ -860,7 +860,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;
@@ -868,7 +868,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);
@@ -912,9 +912,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);
@@ -924,8 +925,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)
/*
@@ -950,7 +951,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);
@@ -967,7 +968,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))
@@ -987,7 +988,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"));
}
@@ -1007,7 +1008,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);
@@ -1022,7 +1023,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 734ee12..c284c96 100755
--- a/t/t3510-cherry-pick-sequence.sh
+++ b/t/t3510-cherry-pick-sequence.sh
@@ -236,4 +236,62 @@ test_expect_success 'commit descriptions in insn sheet are optional' '
test_line_count = 4 commits
'
+test_expect_success 'revert --continue continues after cherry-pick' '
+ pristine_detach initial &&
+ test_must_fail git cherry-pick base..anotherpick &&
+ echo "c" >foo &&
+ git add foo &&
+ git commit &&
+ git revert --continue &&
+ test_path_is_missing .git/sequencer &&
+ {
+ git rev-list HEAD |
+ git diff-tree --root --stdin |
+ sed "s/$_x40/OBJID/g"
+ } >actual &&
+ cat >expect <<-\EOF &&
+ OBJID
+ :100644 100644 OBJID OBJID M foo
+ OBJID
+ :100644 100644 OBJID OBJID M foo
+ OBJID
+ :100644 100644 OBJID OBJID M unrelated
+ OBJID
+ :000000 100644 OBJID OBJID A foo
+ :000000 100644 OBJID OBJID A unrelated
+ EOF
+ test_cmp expect actual
+'
+
+test_expect_success 'mixed pick and revert instructions' '
+ pristine_detach initial &&
+ test_must_fail git cherry-pick base..anotherpick &&
+ echo "c" >foo &&
+ git add foo &&
+ git commit &&
+ oldsha=`git rev-parse --short HEAD~1` &&
+ echo "revert $oldsha unrelatedpick" >>.git/sequencer/todo &&
+ git cherry-pick --continue &&
+ test_path_is_missing .git/sequencer &&
+ {
+ git rev-list HEAD |
+ git diff-tree --root --stdin |
+ sed "s/$_x40/OBJID/g"
+ } >actual &&
+ cat >expect <<-\EOF &&
+ OBJID
+ :100644 100644 OBJID OBJID M unrelated
+ OBJID
+ :100644 100644 OBJID OBJID M foo
+ OBJID
+ :100644 100644 OBJID OBJID M foo
+ OBJID
+ :100644 100644 OBJID OBJID M unrelated
+ OBJID
+ :000000 100644 OBJID OBJID A foo
+ :000000 100644 OBJID OBJID A unrelated
+ EOF
+ test_cmp expect actual
+'
+
test_done
--
1.7.6.351.gb35ac.dirty
^ permalink raw reply related
* [PATCH 3/7] revert: Make commit descriptions in insn sheet optional
From: Ramkumar Ramachandra @ 2011-08-14 8:33 UTC (permalink / raw)
To: Junio C Hamano
Cc: Git List, Jonathan Nieder, Christian Couder, Daniel Barkalow,
Jeff King
In-Reply-To: <1313310789-10216-1-git-send-email-artagnon@gmail.com>
Change the instruction sheet format subtly so that a description of
the commit after the object name is optional. As a result, an
instruction sheet like this is now perfectly valid:
pick 35b0426
pick fbd5bbcbc2e
pick 7362160f
Suggested-by: Jonathan Nieder <jrnieder@gmail.com>
Acked-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
---
builtin/revert.c | 19 ++++++++-----------
t/t3510-cherry-pick-sequence.sh | 14 ++++++++++++++
2 files changed, 22 insertions(+), 11 deletions(-)
diff --git a/builtin/revert.c b/builtin/revert.c
index 8d29003..89b2bba 100644
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -697,26 +697,23 @@ 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;
+ const char *p, *q;
+ p = start;
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 - p + 1 > sizeof(sha1_abbrev))
+ q = p + strcspn(p, " \n");
+ 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 6882acb..734ee12 100755
--- a/t/t3510-cherry-pick-sequence.sh
+++ b/t/t3510-cherry-pick-sequence.sh
@@ -222,4 +222,18 @@ test_expect_success 'malformed instruction sheet 3' '
test_must_fail git cherry-pick --continue
'
+test_expect_success 'commit descriptions in insn sheet are optional' '
+ pristine_detach initial &&
+ test_must_fail git cherry-pick base..anotherpick &&
+ echo "c" >foo &&
+ git add foo &&
+ git commit &&
+ cut -d" " -f1,2 .git/sequencer/todo >new_sheet &&
+ cp new_sheet .git/sequencer/todo &&
+ git cherry-pick --continue &&
+ test_path_is_missing .git/sequencer &&
+ git rev-list HEAD >commits
+ test_line_count = 4 commits
+'
+
test_done
--
1.7.6.351.gb35ac.dirty
^ permalink raw reply related
* [PATCH 2/7] revert: Fix buffer overflow in insn sheet parser
From: Ramkumar Ramachandra @ 2011-08-14 8:33 UTC (permalink / raw)
To: Junio C Hamano
Cc: Git List, Jonathan Nieder, Christian Couder, Daniel Barkalow,
Jeff King
In-Reply-To: <1313310789-10216-1-git-send-email-artagnon@gmail.com>
Check that the commit name argument to a "pick" or "revert" action in
'.git/sequencer/todo' is not too long, to avoid overflowing an
on-stack buffer. This fixes a regression introduced by a037033b
(revert: Introduce --continue to continue the operation, 2011-07-27).
Reported-by: Jonathan Nieder <jrnieder@gmail.com>
Acked-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
---
builtin/revert.c | 2 +-
t/t3510-cherry-pick-sequence.sh | 11 +++++++++++
2 files changed, 12 insertions(+), 1 deletions(-)
diff --git a/builtin/revert.c b/builtin/revert.c
index 8a35bfd..8d29003 100644
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -712,7 +712,7 @@ static struct commit *parse_insn_line(char *start, struct replay_opts *opts)
return NULL;
q = strchr(p, ' ');
- if (!q)
+ if (!q || q - p + 1 > sizeof(sha1_abbrev))
return NULL;
q++;
diff --git a/t/t3510-cherry-pick-sequence.sh b/t/t3510-cherry-pick-sequence.sh
index 3bca2b3..6882acb 100755
--- a/t/t3510-cherry-pick-sequence.sh
+++ b/t/t3510-cherry-pick-sequence.sh
@@ -211,4 +211,15 @@ test_expect_success 'malformed instruction sheet 2' '
test_must_fail git cherry-pick --continue
'
+test_expect_success 'malformed instruction sheet 3' '
+ pristine_detach initial &&
+ test_must_fail git cherry-pick base..anotherpick &&
+ echo "resolved" >foo &&
+ git add foo &&
+ git commit &&
+ sed "s/pick \([0-9a-f]\+\)\(.*\)/pick \1\1\1\1\1\1\1\1\2/" .git/sequencer/todo >new_sheet &&
+ cp new_sheet .git/sequencer/todo &&
+ test_must_fail git cherry-pick --continue
+'
+
test_done
--
1.7.6.351.gb35ac.dirty
^ permalink raw reply related
* [PATCH 1/7] revert: Free memory after get_message call
From: Ramkumar Ramachandra @ 2011-08-14 8:33 UTC (permalink / raw)
To: Junio C Hamano
Cc: Git List, Jonathan Nieder, Christian Couder, Daniel Barkalow,
Jeff King
In-Reply-To: <1313310789-10216-1-git-send-email-artagnon@gmail.com>
The format_todo function leaks memory because it forgets to call
free_message after get_message. It is a potentially big leak, because
fresh memory is allocated to store the commit message message for each
commit. Fix this.
Reported-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 8b452e8..8a35bfd 100644
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -687,6 +687,7 @@ static int format_todo(struct strbuf *buf, struct commit_list *todo_list,
if (get_message(cur->item, &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;
}
--
1.7.6.351.gb35ac.dirty
^ permalink raw reply related
* [PATCH v2 0/7] Generalized sequencer foundations
From: Ramkumar Ramachandra @ 2011-08-14 8:33 UTC (permalink / raw)
To: Junio C Hamano
Cc: Git List, Jonathan Nieder, Christian Couder, Daniel Barkalow,
Jeff King
Hi,
This is the second iternation prepared mainly in response to
Jonathan's reviews. It's ready for inclusion (or very nearly so) now.
I'll start working on more features as soon as I'm confident that this
series doesn't need to be rewritten.
Thanks.
-- Ram
Ramkumar Ramachandra (7):
revert: Free memory after get_message call
revert: Fix buffer overflow in insn sheet parser
revert: Make commit descriptions in insn sheet optional
revert: Allow mixed pick and revert instructions
revert: Make the argument parser responsible for setup_revisions
sequencer: Expose API to cherry-picking machinery
sequencer: Remove sequencer state after final commit
builtin/commit.c | 8 +
builtin/revert.c | 858 +-----------------------------------
sequencer.c | 806 +++++++++++++++++++++++++++++++++-
sequencer.h | 36 ++
t/t3032-merge-recursive-options.sh | 2 +
t/t3510-cherry-pick-sequence.sh | 87 ++++-
6 files changed, 956 insertions(+), 841 deletions(-)
--
1.7.6.351.gb35ac.dirty
^ permalink raw reply
* Re: git am ~/Documents/mail-[1-9]-11.txt Patch format detection failed.
From: Jim Cromie @ 2011-08-14 8:20 UTC (permalink / raw)
To: Ramkumar Ramachandra; +Cc: git, David Aguilar, David Barr
In-Reply-To: <CALkWK0kmZLiSzSwZ9YqjZMpx3SzZ5gDC4Y6CFtSu3x2EcvE-rQ@mail.gmail.com>
On Sat, Aug 13, 2011 at 9:06 PM, Ramkumar Ramachandra
<artagnon@gmail.com> wrote:
> Hi Jim,
>
> [+CC: David Barr]
>
> Jim Cromie writes:
>> git am is giving me trouble.
>>
>> Im a gmail user, and have saved a patch series by doing
>> show-original, save w rename for each patch
>> [...]
>> Ive attached mail-1-11, which applies cleanly to v3.0, but wont am.
>
> It works quite well for me after David's recent patch: 0e8341f2 (am:
> ignore leading whitespace before patch, 2011-08-08). The patch has
> already made it to 'next', but hasn't hit 'master' yet. Could you
> please try it and tell us if there are any issues?
>
> Thanks.
>
> -- Ram
>
thanks Ram,
I just pulled and built next branch, it worked for me.
[jimc@groucho linux-2.6]$ ../../git/git am ~/Documents/mail-[1-9]-11.txt
Applying: dynamic_debug: Add __dynamic_dev_dbg
Applying: dynamic_debug: Consolidate prefix output to single routine
Applying: dynamic_debug: Remove uses of KERN_CONT in dynamic_emit_prefix
Applying: dynamic_debug: Convert printks to pr_<level>
Applying: dynamic_debug: remove unused control variables
...
thanks
^ permalink raw reply
* Re: git am ~/Documents/mail-[1-9]-11.txt Patch format detection failed.
From: Ramkumar Ramachandra @ 2011-08-14 3:06 UTC (permalink / raw)
To: Jim Cromie; +Cc: git, David Aguilar, David Barr
In-Reply-To: <CAJfuBxx9Ayo8cHga+vK+Ynx5P7dhST7f=p87-qyjEzofSdZ=RQ@mail.gmail.com>
Hi Jim,
[+CC: David Barr]
Jim Cromie writes:
> git am is giving me trouble.
>
> Im a gmail user, and have saved a patch series by doing
> show-original, save w rename for each patch
> [...]
> Ive attached mail-1-11, which applies cleanly to v3.0, but wont am.
It works quite well for me after David's recent patch: 0e8341f2 (am:
ignore leading whitespace before patch, 2011-08-08). The patch has
already made it to 'next', but hasn't hit 'master' yet. Could you
please try it and tell us if there are any issues?
Thanks.
-- Ram
^ permalink raw reply
* Re: git am ~/Documents/mail-[1-9]-11.txt Patch format detection failed.
From: David Aguilar @ 2011-08-14 2:14 UTC (permalink / raw)
To: Jim Cromie; +Cc: git
In-Reply-To: <CAJfuBxx9Ayo8cHga+vK+Ynx5P7dhST7f=p87-qyjEzofSdZ=RQ@mail.gmail.com>
On Sat, Aug 13, 2011 at 01:49:35PM -0600, Jim Cromie wrote:
> git am is giving me trouble.
>
> Im a gmail user, and have saved a patch series by doing
> show-original, save w rename for each patch
> [snip]
My experience with gmail + patches is that you need to use an
email client to download patches, not the web interface.
mutt works well for me. Any client that can talk to gmail
over imaps:// should do the trick.
I couldn't find what was wrong with the patch you
attached to your original message, though.
--
David
^ permalink raw reply
* [PATCH] rev-parse: Allow @{U} as a synonym for @{u}
From: Conrad Irwin @ 2011-08-14 1:57 UTC (permalink / raw)
To: git; +Cc: Sitaram Chamarty, Conrad Irwin
In-Reply-To: <CAMK1S_hZkdXiQb_UTB=snLAXPmo5yrCnFaQFHZqYq5AXqOgWng@mail.gmail.com>
This facilitates specifying the upstream of a branch if you have
difficulty synchronising your shift-key and other characters, if you
just want to SHOUT at git, or if you're expecting consistency with
@{1DAY}.
Signed-off-by: Conrad Irwin <conrad.irwin@gmail.com>
---
Documentation/revisions.txt | 4 ++--
sha1_name.c | 2 +-
t/t1507-rev-parse-upstream.sh | 8 ++++++++
3 files changed, 11 insertions(+), 3 deletions(-)
diff --git a/Documentation/revisions.txt b/Documentation/revisions.txt
index b290b61..c89d74d 100644
--- a/Documentation/revisions.txt
+++ b/Documentation/revisions.txt
@@ -87,8 +87,8 @@ the '$GIT_DIR/refs' directory or from the '$GIT_DIR/packed-refs' file.
before the current one.
'<refname>@\{upstream\}', e.g. 'master@\{upstream\}', '@\{u\}'::
- The suffix '@\{upstream\}' to a ref (short form '<refname>@\{u\}') refers to
- the branch the ref is set to build on top of. A missing ref defaults
+ The suffix '@\{upstream\}' to a ref (short form '<refname>@\{u\}', or '@\{U\}')
+ refers to the branch the ref is set to build on top of. A missing ref defaults
to the current branch.
'<rev>{caret}', e.g. 'HEAD{caret}, v1.5.1{caret}0'::
diff --git a/sha1_name.c b/sha1_name.c
index ff5992a..ef4b77f 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -334,7 +334,7 @@ static inline int upstream_mark(const char *string, int len)
for (i = 0; i < ARRAY_SIZE(suffix); i++) {
int suffix_len = strlen(suffix[i]);
if (suffix_len <= len
- && !memcmp(string, suffix[i], suffix_len))
+ && !strncasecmp(string, suffix[i], suffix_len))
return suffix_len;
}
return 0;
diff --git a/t/t1507-rev-parse-upstream.sh b/t/t1507-rev-parse-upstream.sh
index a455551..5be605f 100755
--- a/t/t1507-rev-parse-upstream.sh
+++ b/t/t1507-rev-parse-upstream.sh
@@ -37,6 +37,14 @@ test_expect_success '@{u} resolves to correct full name' '
test refs/remotes/origin/master = "$(full_name @{u})"
'
+test_expect_success '@{U} resolves to correct full name' '
+ test refs/remotes/origin/master = "$(full_name @{U})"
+'
+
+test_expect_success '@{Upstream} resolves to correct full name' '
+ test refs/remotes/origin/master = "$(full_name @{Upstream})"
+'
+
test_expect_success 'my-side@{upstream} resolves to correct full name' '
test refs/remotes/origin/side = "$(full_name my-side@{u})"
'
--
1.7.6.448.gc83e3.dirty
^ permalink raw reply related
* [RFC] grep should detect binary files like diff
From: Conrad Irwin @ 2011-08-13 23:51 UTC (permalink / raw)
To: git; +Cc: conrad.irwin, gitster, rene.scharfe
Hi all,
The problem I have is a large number of fixture files, in text-based
formats, that pollute the output of various git commands. I can mitigate
this in git-diff using gitattributes mechanism, but there's no similar
configuration for git-grep.
There are two issues with the naive approach to implementing this that
I've attached below. Firstly the configuration variable is namespaced
under "diff", and secondly the attributes mechanism is not thread-safe.
I'm inclined to think that the namespacing issue is not too significant;
it would just require some documentation. If it is an issue then maybe a
new "grep" attribute could be created. Are there any places where you
might want git-grep and git-diff to treat different sets of files as
binary?
The thread-safety of the attributes mechanism is a much bigger problem,
and is the only reason I made the behaviour depend on a configuration
option below. You can either have a multi-threaded grep, or a grep that
detects binary files properly :(. I'm not sure how to even start
resolving an issue like that, though I'm happy to accept pointers. Does
anyone, Junio?, know what it would take to fix?
Conrad
---
Documentation/config.txt | 5 ++++
Documentation/git-grep.txt | 5 ++++
Documentation/gitattributes.txt | 3 ++
builtin/grep.c | 5 ++++
grep.c | 48 +++++++++++++++++++++-----------------
grep.h | 1 +
t/t7008-grep-binary.sh | 18 ++++++++++++++
7 files changed, 63 insertions(+), 22 deletions(-)
diff --git a/Documentation/config.txt b/Documentation/config.txt
index 0658ffb..b7d65b3 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1070,6 +1070,11 @@ grep.lineNumber::
grep.extendedRegexp::
If set to true, enable '--extended-regexp' option by default.
+grep.binaryFiles::
+ If set to true, linkgit:git-grep[1] will treat files as binary under
+ the same circumstances as linkgit:git-diff[1]. See the
+ "Marking files as binary" section of linkgit:gitattributes[5].
+
gui.commitmsgwidth::
Defines how wide the commit message window is in the
linkgit:git-gui[1]. "75" is the default.
diff --git a/Documentation/git-grep.txt b/Documentation/git-grep.txt
index e44a498..fd9ebc4 100644
--- a/Documentation/git-grep.txt
+++ b/Documentation/git-grep.txt
@@ -41,6 +41,11 @@ grep.lineNumber::
grep.extendedRegexp::
If set to true, enable '--extended-regexp' option by default.
+grep.binaryFiles::
+ If set to true, linkgit:git-grep[1] will treat files as binary under
+ the same circumstances as linkgit:git-diff[1]. See the
+ "Marking files as binary" section of linkgit:gitattributes[5].
+
OPTIONS
-------
diff --git a/Documentation/gitattributes.txt b/Documentation/gitattributes.txt
index 2bbe76b..180aa2f 100644
--- a/Documentation/gitattributes.txt
+++ b/Documentation/gitattributes.txt
@@ -644,6 +644,9 @@ attribute in the `.gitattributes` file:
This will cause git to generate `Binary files differ` (or a binary
patch, if binary patches are enabled) instead of a regular diff.
+If the grep.binaryFiles configuration variable is set, linkgit:git-grep[1]
+will also treat such files as binary, by default printing
+`Binary file matches` instead of the matching line.
However, one may also want to specify other diff driver attributes. For
example, you might want to use `textconv` to convert postscript files to
diff --git a/builtin/grep.c b/builtin/grep.c
index 1851797..e9d9003 100644
--- a/builtin/grep.c
+++ b/builtin/grep.c
@@ -324,6 +324,11 @@ static int grep_config(const char *var, const char *value, void *cb)
return 0;
}
+ if (!strcmp(var, "grep.binaryfiles")) {
+ opt->userdiff_binary = git_config_bool(var, value);
+ return 0;
+ }
+
if (!strcmp(var, "color.grep"))
opt->color = git_config_colorbool(var, value, -1);
else if (!strcmp(var, "color.grep.context"))
diff --git a/grep.c b/grep.c
index 26e8d8e..84063eb 100644
--- a/grep.c
+++ b/grep.c
@@ -924,8 +924,8 @@ int grep_threads_ok(const struct grep_opt *opt)
* machinery in grep_buffer_1. The attribute code is not
* thread safe, so we disable the use of threads.
*/
- if (opt->funcname && !opt->unmatch_name_only && !opt->status_only &&
- !opt->name_only)
+ if ((opt->funcname && !opt->unmatch_name_only && !opt->status_only &&
+ !opt->name_only) || opt->userdiff_binary)
return 0;
return 1;
@@ -947,6 +947,7 @@ static int grep_buffer_1(struct grep_opt *opt, const char *name,
unsigned count = 0;
int try_lookahead = 0;
int show_function = 0;
+ int load_userdiff_func;
enum grep_context ctx = GREP_CONTEXT_HEAD;
xdemitconf_t xecfg;
@@ -968,31 +969,34 @@ static int grep_buffer_1(struct grep_opt *opt, const char *name,
}
opt->last_shown = 0;
- switch (opt->binary) {
- case GREP_BINARY_DEFAULT:
- if (buffer_is_binary(buf, size))
- binary_match_only = 1;
- break;
- case GREP_BINARY_NOMATCH:
- if (buffer_is_binary(buf, size))
- return 0; /* Assume unmatch */
- break;
- case GREP_BINARY_TEXT:
- break;
- default:
- die("bug: unknown binary handling mode");
- }
+ load_userdiff_func = opt->funcname && !opt->unmatch_name_only && !opt->status_only &&
+ !opt->name_only && !collect_hits;
memset(&xecfg, 0, sizeof(xecfg));
- if (opt->funcname && !opt->unmatch_name_only && !opt->status_only &&
- !opt->name_only && !binary_match_only && !collect_hits) {
+ if (opt->userdiff_binary || load_userdiff_func) {
+ /* we have to be careful not to call this if we're using threads */
struct userdiff_driver *drv = userdiff_find_by_path(name);
- if (drv && drv->funcname.pattern) {
- const struct userdiff_funcname *pe = &drv->funcname;
- xdiff_set_find_func(&xecfg, pe->pattern, pe->cflags);
- opt->priv = &xecfg;
+
+ if (opt->userdiff_binary && drv && drv->binary != -1)
+ binary_match_only = drv->binary;
+ else if (opt->binary != GREP_BINARY_TEXT)
+ binary_match_only = buffer_is_binary(buf, size);
+
+ if (load_userdiff_func && !binary_match_only) {
+ if (drv && drv->funcname.pattern) {
+ const struct userdiff_funcname *pe = &drv->funcname;
+ xdiff_set_find_func(&xecfg, pe->pattern, pe->cflags);
+ opt->priv = &xecfg;
+ }
}
+ } else if (opt->binary != GREP_BINARY_TEXT) {
+ binary_match_only = buffer_is_binary(buf, size);
+ }
+
+ if (binary_match_only && opt->binary == GREP_BINARY_NOMATCH) {
+ return 0;
}
+
try_lookahead = should_lookahead(opt);
while (left) {
diff --git a/grep.h b/grep.h
index ae50c45..303cb78 100644
--- a/grep.h
+++ b/grep.h
@@ -90,6 +90,7 @@ struct grep_opt {
#define GREP_BINARY_NOMATCH 1
#define GREP_BINARY_TEXT 2
int binary;
+ int userdiff_binary;
int extended;
int pcre;
int relative;
diff --git a/t/t7008-grep-binary.sh b/t/t7008-grep-binary.sh
index e058d18..a88503b 100755
--- a/t/t7008-grep-binary.sh
+++ b/t/t7008-grep-binary.sh
@@ -99,4 +99,22 @@ test_expect_failure 'git grep y<NUL>x a' "
test_must_fail git grep -f f a
"
+test_expect_success 'git -c grep.binaryFiles=1 grep ina a' "
+ echo 'a diff' > .gitattributes &&
+ printf 'binaryQfile' | q_to_nul >a &&
+ echo 'a:binaryQfile' | q_to_nul >expect &&
+ git -c grep.binaryFiles=1 grep ina a > actual &&
+ rm .gitattributes &&
+ test_cmp expect actual
+"
+test_expect_success 'git -c grep.binaryFiles=1 grep tex t' "
+ echo 'text' > t &&
+ git add t &&
+ echo 't -diff' > .gitattributes &&
+ echo Binary file t matches >expect &&
+ git -c grep.binaryFiles=1 grep tex t >actual &&
+ rm .gitattributes &&
+ test_cmp expect actual
+"
+
test_done
--
1.7.6.409.ge7a85
^ permalink raw reply related
* Re: t5800-*.sh: Intermittent test failures
From: Ramsay Jones @ 2011-08-13 20:51 UTC (permalink / raw)
To: Sverre Rabbelier
Cc: GIT Mailing-list, Jeff King, Jonathan Nieder, Junio C Hamano
In-Reply-To: <CAGdFq_jv_T-x7VGqm_j-fDfeW6TsBG95=1TWn91Yk9B3TGZdsQ@mail.gmail.com>
Sverre Rabbelier wrote:
>> 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?
I indulged in a spot of "printf debugging". ;-) see more below.
> Also, you say git remote-test everywhere, but it should be git
> remote-testgit, typo?
Yep. [It was actually caused by a cut/paste/edit of pstree output (pstree
truncates long fields); not that you could guess that! ;-P ]
So ...
I added some additional debug code to transport-helper.c (see below) in
addition to creating debug output files from the git-fast-import/export
commands. (I won't show the code for this debug output; it wouldn't be
hard to imagine! :-)
In addition to the uninteresting "printf debugging" info, I used
gettimeofday() to show the start and end times for the git(fast-export)
process and the start time for git-fast-import. The last hunk below,
for instance, shows the code to output the git(fast-export) end time ...
--- >8 ----
diff --git a/transport-helper.c b/transport-helper.c
index 74c3122..7c9d881 100644
--- a/transport-helper.c
+++ b/transport-helper.c
@@ -132,6 +132,8 @@ static struct child_process *get_helper(struct transport *transport)
snprintf(git_dir_buf, sizeof(git_dir_buf), "%s=%s", GIT_DIR_ENVIRONMENT, get_git_dir());
helper->env = helper_env;
+ if (debug)
+ fprintf(stderr, "Debug: start remote helper: <%s>\n", helper->argv[0]);
code = start_command(helper);
if (code < 0 && errno == ENOENT)
die("Unable to find remote helper for '%s'", data->name);
@@ -376,6 +378,8 @@ static int get_importer(struct transport *transport, struct child_process *fasti
fastimport->argv[1] = "--quiet";
fastimport->git_cmd = 1;
+ if (debug)
+ fprintf(stderr, "Debug: get_importer, start fast-import\n");
return start_command(fastimport);
}
@@ -403,6 +407,8 @@ static int get_exporter(struct transport *transport,
fastexport->argv[argc++] = revlist_args->items[i].string;
fastexport->git_cmd = 1;
+ if (debug)
+ fprintf(stderr, "Debug: get_exporter, start fast-export\n");
return start_command(fastexport);
}
@@ -756,6 +762,11 @@ static int push_refs_with_export(struct transport *transport,
if (finish_command(&exporter))
die("Error while running fast-export");
+ if (debug) {
+ struct timeval tv;
+ gettimeofday(&tv, NULL);
+ fprintf(stderr, "fast-export finished @ %lds %ldus\n", tv.tv_sec, tv.tv_usec);
+ }
push_update_refs_status(data, remote_refs);
return 0;
}
--- >8 ----
The debug output from "./t5800-remote-helpers.sh -v" ends like this:
... [snipped]
Debug: Capabilities complete.
Debug: Remote helper: Waiting...
Got command 'list' with args ''
? refs/heads/new
? refs/heads/master
@refs/heads/master HEAD
Debug: Remote helper: <- ? refs/heads/new
Debug: Remote helper: Waiting...
Debug: Remote helper: <- ? refs/heads/master
Debug: Remote helper: Waiting...
Debug: Remote helper: <- @refs/heads/master HEAD
Debug: Remote helper: Waiting...
Debug: Remote helper: <-
Debug: Read ref listing.
Debug: Remote helper: -> export
Debug: get_exporter, start fast-export
fast-export finished @ 1313178956s 366398us
Debug: Remote helper: Waiting...
Got command 'export' with args ''
The fast-export debug file looks like:
--- >8 ----
fast-export: pid = 11096 (ppid 11090)
started @ 1313178956s 364790us
arg: <fast-export>
arg: <--use-done-feature>
arg: <--export-marks=.git/info/fast-import/a08486a77c5cf1b4aa17fa9e64673e352ebe1a96/testgit.marks>
arg: <--import-marks=.git/info/fast-import/a08486a77c5cf1b4aa17fa9e64673e352ebe1a96/testgit.marks>
arg: <^refs/testgit/origin/master>
arg: <refs/heads/master>
----end args----: <>
handle object: <ab28ce7f215103f3f4bf70fd439541590dccc91b>
handle commit: <refs/heads/master>
main: <done!>
--- >8 ----
The fast-import debug file looks like:
--- >8 ----
fast-import: pid = 11104 (ppid = 11103)
started @ 1313178956s 382392us
main: <start-up>
main: <start-up #1>
main: <before loop>
--- >8 ----
Note that git(fast-export) executes in 1608 micro-seconds and finishes
15994 micro-seconds before git-fast-import starts.
ATB,
Ramsay Jones
^ permalink raw reply related
* Re: [BUG] Git won't commit
From: Jonathan Nieder @ 2011-08-13 21:18 UTC (permalink / raw)
To: Michael Witten; +Cc: Fabrício Nascimento, git
In-Reply-To: <CAMOZ1Bu3AoQb330hkL+8hDTCvKX+amRfPE3PBxDHUZR17N344Q@mail.gmail.com>
Hi,
Michael Witten wrote:
> As for why one version works but the other does not, I'm not sure.
It's my fault, I'm afraid:
$ git log v1.7.4..v1.7.4.4 --grep=template --no-merges
commit 2140b140
Author: Jonathan Nieder <jrnieder@gmail.com>
Date: Fri Feb 25 03:07:57 2011 -0600
commit: error out for missing commit message template
When "git commit" was rewritten in C (v1.5.4-rc0~78^2~30,
2007-11-08), a subtle bug in --template was introduced. If the
file named by a --template parameter is missing, previously git
would error out with a message:
Commit template file does not exist.
but in the C version the --template parameter gets ignored and
the default template is used.
t7500 has two tests for this case which would have caught it, except
that with the default $EDITOR, the commit message template is left
unmodified, causing 'git commit' to error out and the test to
succeed.
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Fabricio, do you remember what you were trying to do when the
"[commit] template" configuration got set? Currently the error
message says
fatal: could not read '<path>'
where <path> is the configured template filename; probably this
should be changed to mention (1) that the file we were expecting to
find is a commit message template and (2) whether we got it from the
commandline or configuration.
What I'm trying to figure out is whether other people were relying on
the accidental "optional template" behavior or if there is some case
where it is more useful. If so, it should be brought back.
^ permalink raw reply
* Re: [PATCH 2/3] fast-import: add a check for tree delta base sha1
From: Jonathan Nieder @ 2011-08-13 21:02 UTC (permalink / raw)
To: Dmitry Ivankov; +Cc: git, Shawn O. Pearce, David Barr
In-Reply-To: <1313145170-24471-3-git-send-email-divanorama@gmail.com>
Hi,
Dmitry Ivankov wrote:
> fast-import is able to write imported tree objects in delta format.
> It holds a tree structure in memory where each tree entry may have
> a delta base sha1 assigned. When delta base data is needed it is
> reconstructed from this in-memory structure. Though sometimes the
> delta base data doesn't match the delta base sha1 so wrong or even
> corrupt pack is produced.
I'm having trouble parsing this; not sure why. Some guesses:
- dropping the word "imported" could help, since it is the
content of trees that comes from the user, not the tree objects
- it's not clear to me what the second sentence is saying. Do you
mean that git looks at the versions[0].sha1 fields of item in
t->entries to construct an in-memory tree object to delta against,
instead of finding the object named by versions[0].sha1, inflating
it, and using it directly?
- the third sentence seems to be describing a problem, but I'm not
sure what the relationship is to this patch: will this patch fix
that problem, or does it just add a test illustrating it?
> To create a small easily reproducible test, add an excessive check
> for delta base sha1. It's not likely that computing sha1 for each
> tree delta base costs us much.
Since the first word in fast-import is "fast", I would be much
happier with some measurements with a typical import (i.e., one that
doesn't use --cat-blob-fd) than the statement "It's not likely". :)
If this tweak will continue to be useful after the fix, perhaps it
could be made optional. I haven't thought carefully about this,
though.
On to the patch.
[...]
> +++ b/fast-import.c
> @@ -1455,12 +1455,22 @@ static void store_tree(struct tree_entry *root)
> store_tree(t->entries[i]);
> }
>
> + if (!is_null_sha1(root->versions[0].sha1)
> + && S_ISDIR(root->versions[0].mode)) {
> + unsigned char old_tree_sha1[20];
> + mktree(t, 0, &old_tree);
> + prepare_object_hash(OBJ_TREE, &old_tree,
> + NULL, NULL, old_tree_sha1);
> +
> + if (hashcmp(old_tree_sha1, root->versions[0].sha1))
> + die("internal tree delta base sha1 mismatch");
This is the heart of the patch; it involves several changes.
1. construct the base object tree whether the base object is in the
current pack or not
2. calculate its hash and compare to ->versions[0].sha1 as a sanity
check.
For large trees, I fear it could be an important slowdown.
> +
> - le = find_object(root->versions[0].sha1);
> - if (S_ISDIR(root->versions[0].mode) && le && le->pack_id == pack_id) {
> - mktree(t, 0, &old_tree);
> + le = find_object(root->versions[0].sha1);
> + if (le && le->pack_id == pack_id) {
> lo.data = old_tree;
> lo.offset = le->idx.offset;
> lo.depth = t->delta_depth;
> }
> + }
[...]
> --- a/t/t9300-fast-import.sh
> +++ b/t/t9300-fast-import.sh
> @@ -734,6 +734,44 @@ test_expect_success \
[...]
> +cat >input2 <<INPUT_END
> +commit refs/heads/L2
> +committer $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE
> +data <<COMMIT
> +update L2
> +COMMIT
> +from refs/heads/L2^0
> +M 040000 @A g
> +M 040000 @E g/b
> +M 040000 @E g/b/h
> +INPUT_END
> +
> +test_expect_failure \
> + 'L: verify internal tree delta base' \
> + 'git fast-import <input &&
> + A=$(git ls-tree L2 a | tr " " "\t" | cut -f 3) &&
> + E=$(git ls-tree L2 a/e | tr " " "\t" | cut -f 3) &&
> + cat input2 | sed -e "s/@A/$A/" -e "s/@E/$E/" >input &&
> + git fast-import <input'
The description ("L: verify internal tree delta base" here) should
describe something we want to work --- a facility or a statement ---
and should leave out words like "verify" unless it is a test of
verification facilities.
In this example, I guess it is testing something like "delta base is
not corrupted when replacing one directory by another"? (That's a
random, wild guess and not meant as an example to be used verbatim.)
I suppose I would be happier if we can find a way to reproduce this
without modifying the behavior in such an invasive way. Which should
be easier while thinking about the fix, so I'll move on to that.
^ permalink raw reply
* Re: [BUG] Git won't commit
From: Michael Witten @ 2011-08-13 20:56 UTC (permalink / raw)
To: Fabrício Nascimento; +Cc: git
In-Reply-To: <CAAC=0R66j43CqpaxXskAwHsitvRFwy7DsjguNPm+sPa4YmTWqw@mail.gmail.com>
2011/8/12 Fabrício Nascimento <fabricio@tailorbirds.com.br>:
> Where git is the 1.7.6 that does not work, and git2 is the 1.7.4 that does
> work.
> [master 09-mapas] git config --get commit.template
> template
> [master 09-mapas] git2 config --get commit.template
> template
Did you get this problem figured out? If not, run:
git config --unset commit.template
and then try making a commit as usual.
As for why one version works but the other does not, I'm not sure.
^ permalink raw reply
* Re: [PATCH 5/6] sequencer: Expose API to cherry-picking machinery
From: Ramkumar Ramachandra @ 2011-08-13 20:54 UTC (permalink / raw)
To: Jonathan Nieder
Cc: Git List, Junio C Hamano, Christian Couder, Daniel Barkalow,
Jeff King
In-Reply-To: <20110813170623.GB1494@elie.gateway.2wire.net>
Hi,
Jonathan Nieder writes:
> builtin/revert.c | 52 ++++++++++++++++++++++++++++++----------------------
> 1 files changed, 30 insertions(+), 22 deletions(-)
Nice! That fixes everything :)
I'll send out a fresh iteration in the morning.
Thanks.
-- Ram
^ permalink raw reply
* [PATCH/RFC] git-svn: remove auto_abbrev (Getopt::Long option)
From: Piotr Krukowiecki @ 2011-08-13 20:33 UTC (permalink / raw)
To: Git Mailing List; +Cc: Eric Wong
RFC because this changes user interface (no option abbreviation),
but I know no other good way to fix the --author problem.
The --author option would be useful, because I usually use
git svn log --oneline
and paste the changes with svn revision numbers to bugzilla etc.
All tests pass with this change.
--------8<--------
From: Piotr Krukowiecki <piotr.krukowiecki@gmail.com>
Date: Sat, 13 Aug 2011 21:00:56 +0200
Subject: [PATCH] git-svn: remove auto_abbrev (Getopt::Long option)
auto_abbrev allows to specify shortest unique option prefix.
For example '--authors-file' can be written also as '--author'
given there is no other option which begins with '--author'.
'git-svn log' passes all unrecognized options to 'git log'. It should
also pass '--author' but due to auto_abbrev it recognizes it as
'--authors-file'. In result it's not possible to use this option.
Fix this by removing auto_abbrev. No other perl script seems to use it,
also other git commands seems to have no option abbreviation feature.
The side effect is that you can not abbreviate options to git-svn, obviously.
In fact only log and blame requires no option abbreviation, because only they
use pass_through option (pass unrecognized options), but I think it would not
be natural if some git-svn commands had options abbreviations and some didn't.
Alternative would be to handle all conflicting blame/log options, but
maintaing it would be a nightmare...
Signed-off-by: Piotr Krukowiecki <piotr.krukowiecki@gmail.com>
---
git-svn.perl | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/git-svn.perl b/git-svn.perl
index 89f83fd..3e2e276 100755
--- a/git-svn.perl
+++ b/git-svn.perl
@@ -56,7 +56,7 @@ use File::Basename qw/dirname basename/;
use File::Path qw/mkpath/;
use File::Spec;
use File::Find;
-use Getopt::Long qw/:config gnu_getopt no_ignore_case auto_abbrev/;
+use Getopt::Long qw/:config gnu_getopt no_ignore_case/;
use IPC::Open3;
use Git;
use Memoize; # core since 5.8.0, Jul 2002
--
1.7.6.GIT
--
Piotr Krukowiecki
^ permalink raw reply related
* [RFC PATCH] test showing EMFILE error with too many packs
From: Pete Wyckoff @ 2011-08-13 20:25 UTC (permalink / raw)
To: Shawn Pearce; +Cc: git
In-Reply-To: <20110812172600.GA15896@padd.com>
pw@padd.com wrote on Fri, 12 Aug 2011 10:26 -0700:
> spearce@spearce.org wrote on Fri, 12 Aug 2011 09:09 -0700:
> > On Fri, Aug 12, 2011 at 08:15, Pete Wyckoff <pw@padd.com> wrote:
> > > Somebody at $work found this problem:
> > >
> > > $ git ls-files -s | wc
> > > error: packfile .git/objects/pack/pack-1627e77da82bbb36118762649c8aa88c05664b1e.pack cannot be accessed
> > > [..lots more similar errors..]
> > >
> > > Turns out his shell's open file descriptor limit was 500. And
> > > there are 1600 pack files in the repo.
> > >
> > > Increasing the descriptor limit to 1024 fixed it. I could
> > > probably get him to repack, which may also fix it.
> > >
> > > Does it seem feasible to look for EMFILE errors and close
> > > some packs? Or at least spit out a more intuitive error?
> >
> > What version of Git? I remember fixing this already.... :-)
>
> Initially 1.7.5.4. Same problem on 1.7.6 and master.
>
> I have your "Limit file descriptors used by packs" (c793430, 28
> feb 2011).
>
> It fails here:
>
> if (!is_pack_valid(p)) {
> error("packfile %s cannot be accessed", p->pack_name);
> goto next;
> }
>
> because p->pack_fd is -1, because an earlier git_open_noatime()
> got EMFILE. The function unuse_one_window() is never able to
> find anything to close.
>
> I'll do some more debugging this weekend. Thanks for pointing
> out that it _should_ be fixed.
Here's a test case (on master). It is easy to repack the
repository to avoid this situation, so I'm not convinced this is
a bug.
But it would be nice at least to report what's going on when we
run out of file descriptors.
The current error is:
$ ../git commit -m fileN
error: packfile .git/objects/pack/pack-7bff59b72a19f371ccd3e4ffefaac6cd1e07c6ed.pack cannot be accessed
error: invalid object 100644 a7d02de3336c72e6d2d24f382b82195dbf625404 for 'file47'
error: Error building trees
-- Pete
--------8<--------
From: Pete Wyckoff <pw@padd.com>
Date: Sat, 13 Aug 2011 15:57:55 -0400
Subject: [RFC PATCH] test showing EMFILE error with too many packs
In a repository with too many pack files, it is
possible to have too many open at once and exceed
the process file descriptor limit.
This test shows one failure with an artificially low limit
of 50. The same failure happens at a "normal" value
like 1024, but takes longer.
Signed-off-by: Pete Wyckoff <pw@padd.com>
---
t/t5308-pack-emfile.sh | 29 +++++++++++++++++++++++++++++
1 files changed, 29 insertions(+), 0 deletions(-)
create mode 100755 t/t5308-pack-emfile.sh
diff --git a/t/t5308-pack-emfile.sh b/t/t5308-pack-emfile.sh
new file mode 100755
index 0000000..66c62a7
--- /dev/null
+++ b/t/t5308-pack-emfile.sh
@@ -0,0 +1,29 @@
+#!/bin/sh
+
+test_description='repos with too many packs should not fail'
+
+. ./test-lib.sh
+
+test_expect_success setup '
+ for i in {1..60}
+ do
+ echo "$i" >"file$i" &&
+ git add "file$i" &&
+ test_tick &&
+ git commit -q -m "$i" &&
+ git repack -q
+ done
+ git prune-packed -q
+'
+
+# run in a subshell since limits cannot be increased
+test_expect_failure 'commit fail with too many packs' '
+ (
+ ulimit -n 50 &&
+ >"fileN" &&
+ git add "fileN" &&
+ git commit -q -m "fileN"
+ )
+'
+
+test_done
--
1.7.5.4
^ permalink raw reply related
* git am ~/Documents/mail-[1-9]-11.txt Patch format detection failed.
From: Jim Cromie @ 2011-08-13 19:49 UTC (permalink / raw)
To: git
[-- Attachment #1: Type: text/plain, Size: 605 bytes --]
git am is giving me trouble.
Im a gmail user, and have saved a patch series by doing
show-original, save w rename for each patch
git apply mail-1-11.txt works,
but it disregards the commit-msg, so commit-ids are lost,
(I could attempt to checkin each with cut-paste commit-msgs,
but this sounds like a losing game)
and patchset isnt trackable as such.
grabbing diffs from lkml.org site doesnt help,
since theyre raw diffs, theyre lacking the commit-msg.
have I missed an option, or is am's format detection less than it should be ?
Ive attached mail-1-11, which applies cleanly to v3.0, but wont am.
[-- Attachment #2: mail-1-11.txt --]
[-- Type: text/plain, Size: 7533 bytes --]
Delivered-To: jim.cromie@gmail.com
Received: by 10.142.178.18 with SMTP id a18cs193734wff;
Thu, 11 Aug 2011 11:36:25 -0700 (PDT)
Received: from mr.google.com ([10.151.86.15])
by 10.151.86.15 with SMTP id o15mr1391877ybl.13.1313087785695 (num_hops = 1);
Thu, 11 Aug 2011 11:36:25 -0700 (PDT)
Received: by 10.151.86.15 with SMTP id o15mr1000356ybl.13.1313087784175;
Thu, 11 Aug 2011 11:36:24 -0700 (PDT)
Return-Path: <jbaron@redhat.com>
Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28])
by mx.google.com with ESMTP id b12si2813718ybi.101.2011.08.11.11.36.23;
Thu, 11 Aug 2011 11:36:24 -0700 (PDT)
Received-SPF: pass (google.com: domain of jbaron@redhat.com designates 209.132.183.28 as permitted sender) client-ip=209.132.183.28;
Authentication-Results: mx.google.com; spf=pass (google.com: domain of jbaron@redhat.com designates 209.132.183.28 as permitted sender) smtp.mail=jbaron@redhat.com
Received: from int-mx02.intmail.prod.int.phx2.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12])
by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id p7BIaLoG008295
(version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK);
Thu, 11 Aug 2011 14:36:21 -0400
Received: from redhat.com (dhcp-100-19-188.bos.redhat.com [10.16.19.188])
by int-mx02.intmail.prod.int.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id p7BIaLal007962;
Thu, 11 Aug 2011 14:36:21 -0400
Date: Thu, 11 Aug 2011 14:36:21 -0400
From: Jason Baron <jbaron@redhat.com>
To: gregkh@suse.de
Cc: joe@perches.com, jim.cromie@gmail.com, bvanassche@acm.org,
linux-kernel@vger.kernel.org, davem@davemloft.net,
aloisio.almeida@openbossa.org, netdev@vger.kernel.org
Message-Id: <b5c13500eac4e5dfa0ee960c6f186eeb1b4585b4.1313085588.git.jbaron@redhat.com>
In-Reply-To: <cover.1313085588.git.jbaron@redhat.com>
References: <cover.1313085588.git.jbaron@redhat.com>
Subject: [PATCH 01/11] dynamic_debug: Add __dynamic_dev_dbg
X-Scanned-By: MIMEDefang 2.67 on 10.5.11.12
From: Joe Perches <joe@perches.com>
Unlike dynamic_pr_debug, dynamic uses of dev_dbg can not
currently add task_pid/KBUILD_MODNAME/__func__/__LINE__
to selected debug output.
Add a new function similar to dynamic_pr_debug to
optionally emit these prefixes.
Cc: Aloisio Almeida <aloisio.almeida@openbossa.org>
Noticed-by: Aloisio Almeida <aloisio.almeida@openbossa.org>
Signed-off-by: Joe Perches <joe@perches.com>
Signed-off-by: Jason Baron <jbaron@redhat.com>
---
drivers/base/core.c | 5 +++--
include/linux/device.h | 5 +++++
include/linux/dynamic_debug.h | 10 ++++++++--
lib/dynamic_debug.c | 38 ++++++++++++++++++++++++++++++++++++++
4 files changed, 54 insertions(+), 4 deletions(-)
diff --git a/drivers/base/core.c b/drivers/base/core.c
index bc8729d..82c8654 100644
--- a/drivers/base/core.c
+++ b/drivers/base/core.c
@@ -1764,8 +1764,8 @@ void device_shutdown(void)
#ifdef CONFIG_PRINTK
-static int __dev_printk(const char *level, const struct device *dev,
- struct va_format *vaf)
+int __dev_printk(const char *level, const struct device *dev,
+ struct va_format *vaf)
{
if (!dev)
return printk("%s(NULL device *): %pV", level, vaf);
@@ -1773,6 +1773,7 @@ static int __dev_printk(const char *level, const struct device *dev,
return printk("%s%s %s: %pV",
level, dev_driver_string(dev), dev_name(dev), vaf);
}
+EXPORT_SYMBOL(__dev_printk);
int dev_printk(const char *level, const struct device *dev,
const char *fmt, ...)
diff --git a/include/linux/device.h b/include/linux/device.h
index c20dfbf..4639419 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -785,6 +785,8 @@ extern const char *dev_driver_string(const struct device *dev);
#ifdef CONFIG_PRINTK
+extern int __dev_printk(const char *level, const struct device *dev,
+ struct va_format *vaf);
extern int dev_printk(const char *level, const struct device *dev,
const char *fmt, ...)
__attribute__ ((format (printf, 3, 4)));
@@ -805,6 +807,9 @@ extern int _dev_info(const struct device *dev, const char *fmt, ...)
#else
+static inline int __dev_printk(const char *level, const struct device *dev,
+ struct va_format *vaf)
+ { return 0; }
static inline int dev_printk(const char *level, const struct device *dev,
const char *fmt, ...)
__attribute__ ((format (printf, 3, 4)));
diff --git a/include/linux/dynamic_debug.h b/include/linux/dynamic_debug.h
index e747ecd..bdf1531 100644
--- a/include/linux/dynamic_debug.h
+++ b/include/linux/dynamic_debug.h
@@ -47,6 +47,13 @@ extern int ddebug_remove_module(const char *mod_name);
extern int __dynamic_pr_debug(struct _ddebug *descriptor, const char *fmt, ...)
__attribute__ ((format (printf, 2, 3)));
+struct device;
+
+extern int __dynamic_dev_dbg(struct _ddebug *descriptor,
+ const struct device *dev,
+ const char *fmt, ...)
+ __attribute__ ((format (printf, 3, 4)));
+
#define dynamic_pr_debug(fmt, ...) do { \
static struct _ddebug descriptor \
__used \
@@ -57,7 +64,6 @@ extern int __dynamic_pr_debug(struct _ddebug *descriptor, const char *fmt, ...)
__dynamic_pr_debug(&descriptor, pr_fmt(fmt), ##__VA_ARGS__); \
} while (0)
-
#define dynamic_dev_dbg(dev, fmt, ...) do { \
static struct _ddebug descriptor \
__used \
@@ -65,7 +71,7 @@ extern int __dynamic_pr_debug(struct _ddebug *descriptor, const char *fmt, ...)
{ KBUILD_MODNAME, __func__, __FILE__, fmt, __LINE__, \
_DPRINTK_FLAGS_DEFAULT }; \
if (unlikely(descriptor.enabled)) \
- dev_printk(KERN_DEBUG, dev, fmt, ##__VA_ARGS__); \
+ __dynamic_dev_dbg(&descriptor, dev, fmt, ##__VA_ARGS__); \
} while (0)
#else
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index 75ca78f..63b6f95 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -30,6 +30,7 @@
#include <linux/jump_label.h>
#include <linux/hardirq.h>
#include <linux/sched.h>
+#include <linux/device.h>
extern struct _ddebug __start___verbose[];
extern struct _ddebug __stop___verbose[];
@@ -456,6 +457,43 @@ int __dynamic_pr_debug(struct _ddebug *descriptor, const char *fmt, ...)
}
EXPORT_SYMBOL(__dynamic_pr_debug);
+int __dynamic_dev_dbg(struct _ddebug *descriptor,
+ const struct device *dev, const char *fmt, ...)
+{
+ struct va_format vaf;
+ va_list args;
+ int res;
+
+ BUG_ON(!descriptor);
+ BUG_ON(!fmt);
+
+ va_start(args, fmt);
+
+ vaf.fmt = fmt;
+ vaf.va = &args;
+
+ res = printk(KERN_DEBUG);
+ if (descriptor->flags & _DPRINTK_FLAGS_INCL_TID) {
+ if (in_interrupt())
+ res += printk(KERN_CONT "<intr> ");
+ else
+ res += printk(KERN_CONT "[%d] ", task_pid_vnr(current));
+ }
+ if (descriptor->flags & _DPRINTK_FLAGS_INCL_MODNAME)
+ res += printk(KERN_CONT "%s:", descriptor->modname);
+ if (descriptor->flags & _DPRINTK_FLAGS_INCL_FUNCNAME)
+ res += printk(KERN_CONT "%s:", descriptor->function);
+ if (descriptor->flags & _DPRINTK_FLAGS_INCL_LINENO)
+ res += printk(KERN_CONT "%d ", descriptor->lineno);
+
+ res += __dev_printk(KERN_CONT, dev, &vaf);
+
+ va_end(args);
+
+ return res;
+}
+EXPORT_SYMBOL(__dynamic_dev_dbg);
+
static __initdata char ddebug_setup_string[1024];
static __init int ddebug_setup_query(char *str)
{
--
1.7.5.4
^ permalink raw reply related
* Re: [PATCH 07/11] object: try naive cuckoo hashing
From: George Spelvin @ 2011-08-13 19:12 UTC (permalink / raw)
To: gitster, linux; +Cc: git
In-Reply-To: <20110813102244.9033.qmail@science.horizon.com>
I've been doing a lot of reading on Cuckoo hashing.
Yes, the single-table variant is described and used. However, the
insertion procedure is not the way you do it.
Also, d-ary Cuckoo hashing (also called d-Cuckoo hashing) where you use
more than 2 hash functions is also used.
The insertion algorithm, however, is not really agreed on.
One algorithm (mostly proposed for hardware) uses d separate tables.
Every entry displaced from table i is displaced to table i+1 (mod d).
http://infoscience.epfl.ch/record/164147/files/cuckoo_dir_hpca2011_camera_ready.pdf
In the single-table case, and in general, however, a displaced entry
has more than one possible new location. This leads to the question of how
to choose.
One proposal is to do a breadth-first search looking for a path to
a free slot. It's provable that this will succeed with high probability
before the exponential growth of the breadth of the search tree
gets too bad.
See "Space Efficient Hash Tables with Worst Case Constant Access Time"
http://www.itu.dk/people/pagh/papers/d-cuckoo-jour.pdf
Another suggested tehcnique is to just pick an alternative at random
and proceed. This is recommended in e.g.
"Efficient Hash Probes on Modern Processors"
http://citeseerx.ist.psu.edu/viewdoc/download?doi=10.1.1.67.1189&rep=rep1&type=pdf
Both of these lead to rather complex implementations. The random number
generator is probably simpler than the breadth-first search, but either way
there's a bunch of auxiliary code.
Sticking with two hash functions, but using multi-entry buckets is
definitely an attractive possibility.
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox