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