* [PATCH] fetch-pack: check for valid commit from server
From: Nguyễn Thái Ngọc Duy @ 2011-08-19 15:11 UTC (permalink / raw)
To: git, Shawn Pearce, Junio C Hamano; +Cc: Nguyễn Thái Ngọc Duy
In-Reply-To: <1313674563-12755-1-git-send-email-pclouds@gmail.com>
A malicious server can return ACK with non-existent SHA-1 or not a
commit. lookup_commit() in this case may return NULL. Do not let
fetch-pack crash by accessing NULL address in this case.
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
2011/8/19 Shawn Pearce <spearce@spearce.org>:
> 2011/8/18 Nguyễn Thái Ngọc Duy <pclouds@gmail.com>:
>> However it raises another question, what if the other end returns a
>> valid commit, but not the one in "have" line fetch-pack sent? Are we
>> OK with that?
>
> Not really. The server is not supposed to return a SHA-1 in the ACK
> line unless the client said it first in a have line. So aborting with
> an error is reasonable thing for a client to do.
I assumed I could check result_sha1 against sha1. If it did not match,
fetch-pack would abort. But I was wrong because fetch-pack would send
a few have lines before receiving the first ack (which carries sha1
of some 'have' line in the middle, not the last 'have'). I'd leave it
here if anyone wants to tackle it.
builtin/fetch-pack.c | 3 +++
1 files changed, 3 insertions(+), 0 deletions(-)
diff --git a/builtin/fetch-pack.c b/builtin/fetch-pack.c
index 4367984..561f1a3 100644
--- a/builtin/fetch-pack.c
+++ b/builtin/fetch-pack.c
@@ -395,6 +395,9 @@ static int find_common(int fd[2], unsigned char *result_sha1,
case ACK_continue: {
struct commit *commit =
lookup_commit(result_sha1);
+ if (!commit)
+ die("server ACK contained unknown commit %s",
+ sha1_to_hex(result_sha1));
if (args.stateless_rpc
&& ack == ACK_common
&& !(commit->object.flags & COMMON)) {
--
1.7.4.74.g639db
^ permalink raw reply related
* [PATCH v4 4/4] Accept tags in HEAD or MERGE_HEAD
From: Nguyễn Thái Ngọc Duy @ 2011-08-19 14:50 UTC (permalink / raw)
To: git, Junio C Hamano; +Cc: Nguyễn Thái Ngọc Duy
In-Reply-To: <1313765407-29925-1-git-send-email-pclouds@gmail.com>
HEAD and MERGE_HEAD (among other branch tips) should never hold a
tag. That can only be caused by broken tools and is cumbersome to fix
by an end user with:
$ git update-ref HEAD $(git rev-parse HEAD^{commit})
which may look like a magic to a new person.
Be easy, warn users (so broken tools can be fixed if they bother to
report) and move on.
Be robust, if the given SHA-1 cannot be resolved to a commit object,
die (therefore return value is always valid).
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
Junio's point, that if HEAD holds a tag, then head_sha1 and head->object.sha1
in statement [1] are different, is entirely correct. However, favoring
head->object.sha1 over head_sha1 is not enough. The variable head_sha1 is
still there. Somewhere, some time, people may misuse it.
[1] head = lookup_commit_or_die(head_sha1, "HEAD");
Better update head_sha1 to new value in this case, which is the new change
in lookup_commit_or_die().
Or maybe a better approach is
int get_commit_sha1(const char *ref, unsigned char *sha1);
where it only returns zero if it can resolve to SHA-1 of a commit.
builtin/commit.c | 11 +++++------
builtin/fmt-merge-msg.c | 2 +-
builtin/merge.c | 7 ++-----
commit.c | 19 +++++++++++++++++++
commit.h | 1 +
http-push.c | 8 ++++----
revision.c | 6 ++++--
7 files changed, 36 insertions(+), 18 deletions(-)
diff --git a/builtin/commit.c b/builtin/commit.c
index c9c4ea5..72e2cc5 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -1393,11 +1393,8 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
if (get_sha1("HEAD", head_sha1))
initial_commit = 1;
- else {
- head_commit = lookup_commit(head_sha1);
- if (!head_commit || parse_commit(head_commit))
- die(_("could not parse HEAD commit"));
- }
+ else
+ head_commit = lookup_commit_or_die(head_sha1, "HEAD");
if (s.use_color == -1)
s.use_color = git_use_color_default;
@@ -1433,6 +1430,7 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
pptr = &commit_list_insert(c->item, pptr)->next;
} else if (whence == FROM_MERGE) {
struct strbuf m = STRBUF_INIT;
+ struct commit *commit;
FILE *fp;
if (!reflog_msg)
@@ -1446,7 +1444,8 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
unsigned char sha1[20];
if (get_sha1_hex(m.buf, sha1) < 0)
die(_("Corrupt MERGE_HEAD file (%s)"), m.buf);
- pptr = &commit_list_insert(lookup_commit(sha1), pptr)->next;
+ commit = lookup_commit_or_die(sha1, "MERGE_HEAD");
+ pptr = &commit_list_insert(commit, pptr)->next;
}
fclose(fp);
strbuf_release(&m);
diff --git a/builtin/fmt-merge-msg.c b/builtin/fmt-merge-msg.c
index 7581632..7e2f225 100644
--- a/builtin/fmt-merge-msg.c
+++ b/builtin/fmt-merge-msg.c
@@ -293,7 +293,7 @@ static int do_fmt_merge_msg(int merge_title, struct strbuf *in,
struct commit *head;
struct rev_info rev;
- head = lookup_commit(head_sha1);
+ head = lookup_commit_or_die(head_sha1, "HEAD");
init_revisions(&rev, NULL);
rev.commit_format = CMIT_FMT_ONELINE;
rev.ignore_merges = 1;
diff --git a/builtin/merge.c b/builtin/merge.c
index b7260f5..39d9ac8 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -1034,11 +1034,8 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
branch = resolve_ref("HEAD", head, 0, &flag);
if (branch && !prefixcmp(branch, "refs/heads/"))
branch += 11;
- if (!is_null_sha1(head)) {
- head_commit = lookup_commit(head);
- if (!head_commit)
- die(_("could not parse HEAD"));
- }
+ if (!is_null_sha1(head))
+ head_commit = lookup_commit_or_die(head, "HEAD");
git_config(git_merge_config, NULL);
diff --git a/commit.c b/commit.c
index ac337c7..9e7f7ef 100644
--- a/commit.c
+++ b/commit.c
@@ -39,6 +39,25 @@ struct commit *lookup_commit_reference(const unsigned char *sha1)
return lookup_commit_reference_gently(sha1, 0);
}
+/*
+ * Look sha1 up for a commit, defer if needed. If deference occurs,
+ * update "sha1" for consistency with retval->object.sha1. Also warn
+ * users this case because it is expected that sha1 points directly to
+ * a commit.
+ */
+struct commit *lookup_commit_or_die(unsigned char *sha1, const char *ref_name)
+{
+ struct commit *c = lookup_commit_reference(sha1);
+ if (!c)
+ die(_("could not parse %s"), ref_name);
+ if (hashcmp(sha1, c->object.sha1)) {
+ warning(_("%s %s is not a commit!"),
+ ref_name, sha1_to_hex(sha1));
+ hashcpy(sha1, c->object.sha1);
+ }
+ return c;
+}
+
struct commit *lookup_commit(const unsigned char *sha1)
{
struct object *obj = lookup_object(sha1);
diff --git a/commit.h b/commit.h
index a2d571b..a098b4c 100644
--- a/commit.h
+++ b/commit.h
@@ -37,6 +37,7 @@ struct commit *lookup_commit_reference(const unsigned char *sha1);
struct commit *lookup_commit_reference_gently(const unsigned char *sha1,
int quiet);
struct commit *lookup_commit_reference_by_name(const char *name);
+struct commit *lookup_commit_or_die(unsigned char *sha1, const char *ref_name);
int parse_commit_buffer(struct commit *item, const void *buffer, unsigned long size);
int parse_commit(struct commit *item);
diff --git a/http-push.c b/http-push.c
index 6e8f6d0..7ccff8f 100644
--- a/http-push.c
+++ b/http-push.c
@@ -1606,10 +1606,10 @@ static void fetch_symref(const char *path, char **symref, unsigned char *sha1)
strbuf_release(&buffer);
}
-static int verify_merge_base(unsigned char *head_sha1, unsigned char *branch_sha1)
+static int verify_merge_base(unsigned char *head_sha1, struct ref *remote)
{
- struct commit *head = lookup_commit(head_sha1);
- struct commit *branch = lookup_commit(branch_sha1);
+ struct commit *head = lookup_commit_or_die(head_sha1, "HEAD");
+ struct commit *branch = lookup_commit_or_die(remote->old_sha1, remote->name);
struct commit_list *merge_bases = get_merge_bases(head, branch, 1);
return (merge_bases && !merge_bases->next && merge_bases->item == branch);
@@ -1680,7 +1680,7 @@ static int delete_remote_branch(const char *pattern, int force)
return error("Remote branch %s resolves to object %s\nwhich does not exist locally, perhaps you need to fetch?", remote_ref->name, sha1_to_hex(remote_ref->old_sha1));
/* Remote branch must be an ancestor of remote HEAD */
- if (!verify_merge_base(head_sha1, remote_ref->old_sha1)) {
+ if (!verify_merge_base(head_sha1, remote_ref)) {
return error("The branch '%s' is not an ancestor "
"of your current HEAD.\n"
"If you are sure you want to delete it,"
diff --git a/revision.c b/revision.c
index c46cfaa..5e057a0 100644
--- a/revision.c
+++ b/revision.c
@@ -986,10 +986,12 @@ static void prepare_show_merge(struct rev_info *revs)
const char **prune = NULL;
int i, prune_num = 1; /* counting terminating NULL */
- if (get_sha1("HEAD", sha1) || !(head = lookup_commit(sha1)))
+ if (get_sha1("HEAD", sha1))
die("--merge without HEAD?");
- if (get_sha1("MERGE_HEAD", sha1) || !(other = lookup_commit(sha1)))
+ head = lookup_commit_or_die(sha1, "HEAD");
+ if (get_sha1("MERGE_HEAD", sha1))
die("--merge without MERGE_HEAD?");
+ other = lookup_commit_or_die(sha1, "MERGE_HEAD");
add_pending_object(revs, &head->object, "HEAD");
add_pending_object(revs, &other->object, "MERGE_HEAD");
bases = get_merge_bases(head, other, 1);
--
1.7.4.74.g639db
^ permalink raw reply related
* [PATCH v4 3/4] merge: remove global variable head[]
From: Nguyễn Thái Ngọc Duy @ 2011-08-19 14:50 UTC (permalink / raw)
To: git, Junio C Hamano; +Cc: Nguyễn Thái Ngọc Duy
In-Reply-To: <1313765407-29925-1-git-send-email-pclouds@gmail.com>
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
builtin/merge.c | 80 +++++++++++++++++++++++++++++-------------------------
1 files changed, 43 insertions(+), 37 deletions(-)
diff --git a/builtin/merge.c b/builtin/merge.c
index a068660..b7260f5 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -50,7 +50,6 @@ static int fast_forward_only;
static int allow_trivial = 1, have_message;
static struct strbuf merge_msg;
static struct commit_list *remoteheads;
-static unsigned char head[20];
static struct strategy **use_strategies;
static size_t use_strategies_nr, use_strategies_alloc;
static const char **xopts;
@@ -279,7 +278,8 @@ static void reset_hard(unsigned const char *sha1, int verbose)
die(_("read-tree failed"));
}
-static void restore_state(const unsigned char *stash)
+static void restore_state(const unsigned char *head,
+ const unsigned char *stash)
{
struct strbuf sb = STRBUF_INIT;
const char *args[] = { "stash", "apply", NULL, NULL };
@@ -309,10 +309,9 @@ static void finish_up_to_date(const char *msg)
drop_save();
}
-static void squash_message(void)
+static void squash_message(struct commit *commit)
{
struct rev_info rev;
- struct commit *commit;
struct strbuf out = STRBUF_INIT;
struct commit_list *j;
int fd;
@@ -327,7 +326,6 @@ static void squash_message(void)
rev.ignore_merges = 1;
rev.commit_format = CMIT_FMT_MEDIUM;
- commit = lookup_commit(head);
commit->object.flags |= UNINTERESTING;
add_pending_object(&rev, &commit->object, NULL);
@@ -356,9 +354,11 @@ static void squash_message(void)
strbuf_release(&out);
}
-static void finish(const unsigned char *new_head, const char *msg)
+static void finish(struct commit *head_commit,
+ const unsigned char *new_head, const char *msg)
{
struct strbuf reflog_message = STRBUF_INIT;
+ const unsigned char *head = head_commit->object.sha1;
if (!msg)
strbuf_addstr(&reflog_message, getenv("GIT_REFLOG_ACTION"));
@@ -369,7 +369,7 @@ static void finish(const unsigned char *new_head, const char *msg)
getenv("GIT_REFLOG_ACTION"), msg);
}
if (squash) {
- squash_message();
+ squash_message(head_commit);
} else {
if (verbosity >= 0 && !merge_msg.len)
printf(_("No merge message -- not updating HEAD\n"));
@@ -664,7 +664,7 @@ int try_merge_command(const char *strategy, size_t xopts_nr,
}
static int try_merge_strategy(const char *strategy, struct commit_list *common,
- const char *head_arg)
+ struct commit *head, const char *head_arg)
{
int index_fd;
struct lock_file *lock = xcalloc(1, sizeof(struct lock_file));
@@ -710,7 +710,7 @@ static int try_merge_strategy(const char *strategy, struct commit_list *common,
commit_list_insert(j->item, &reversed);
index_fd = hold_locked_index(lock, 1);
- clean = merge_recursive(&o, lookup_commit(head),
+ clean = merge_recursive(&o, head,
remoteheads->item, reversed, &result);
if (active_cache_changed &&
(write_cache(index_fd, active_cache, active_nr) ||
@@ -861,25 +861,26 @@ static void run_prepare_commit_msg(void)
read_merge_msg();
}
-static int merge_trivial(void)
+static int merge_trivial(struct commit *head)
{
unsigned char result_tree[20], result_commit[20];
struct commit_list *parent = xmalloc(sizeof(*parent));
write_tree_trivial(result_tree);
printf(_("Wonderful.\n"));
- parent->item = lookup_commit(head);
+ parent->item = head;
parent->next = xmalloc(sizeof(*parent->next));
parent->next->item = remoteheads->item;
parent->next->next = NULL;
run_prepare_commit_msg();
commit_tree(merge_msg.buf, result_tree, parent, result_commit, NULL);
- finish(result_commit, "In-index merge");
+ finish(head, result_commit, "In-index merge");
drop_save();
return 0;
}
-static int finish_automerge(struct commit_list *common,
+static int finish_automerge(struct commit *head,
+ struct commit_list *common,
unsigned char *result_tree,
const char *wt_strategy)
{
@@ -890,12 +891,12 @@ static int finish_automerge(struct commit_list *common,
free_commit_list(common);
if (allow_fast_forward) {
parents = remoteheads;
- commit_list_insert(lookup_commit(head), &parents);
+ commit_list_insert(head, &parents);
parents = reduce_heads(parents);
} else {
struct commit_list **pptr = &parents;
- pptr = &commit_list_insert(lookup_commit(head),
+ pptr = &commit_list_insert(head,
pptr)->next;
for (j = remoteheads; j; j = j->next)
pptr = &commit_list_insert(j->item, pptr)->next;
@@ -905,7 +906,7 @@ static int finish_automerge(struct commit_list *common,
run_prepare_commit_msg();
commit_tree(merge_msg.buf, result_tree, parents, result_commit, NULL);
strbuf_addf(&buf, "Merge made by %s.", wt_strategy);
- finish(result_commit, buf.buf);
+ finish(head, result_commit, buf.buf);
strbuf_release(&buf);
drop_save();
return 0;
@@ -939,7 +940,8 @@ static int suggest_conflicts(int renormalizing)
return 1;
}
-static struct commit *is_old_style_invocation(int argc, const char **argv)
+static struct commit *is_old_style_invocation(int argc, const char **argv,
+ const unsigned char *head)
{
struct commit *second_token = NULL;
if (argc > 2) {
@@ -1012,9 +1014,11 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
{
unsigned char result_tree[20];
unsigned char stash[20];
+ unsigned char head[20];
+ struct commit *head_commit = NULL;
struct strbuf buf = STRBUF_INIT;
const char *head_arg;
- int flag, head_invalid = 0, i;
+ int flag, i;
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;
@@ -1030,8 +1034,11 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
branch = resolve_ref("HEAD", head, 0, &flag);
if (branch && !prefixcmp(branch, "refs/heads/"))
branch += 11;
- if (is_null_sha1(head))
- head_invalid = 1;
+ if (!is_null_sha1(head)) {
+ head_commit = lookup_commit(head);
+ if (!head_commit)
+ die(_("could not parse HEAD"));
+ }
git_config(git_merge_config, NULL);
@@ -1112,12 +1119,12 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
* additional safety measure to check for it.
*/
- if (!have_message && is_old_style_invocation(argc, argv)) {
+ if (!have_message && is_old_style_invocation(argc, argv, head)) {
strbuf_addstr(&merge_msg, argv[0]);
head_arg = argv[1];
argv += 2;
argc -= 2;
- } else if (head_invalid) {
+ } else if (!head_commit) {
struct object *remote_head;
/*
* If the merged head is a valid one there is no reason
@@ -1164,7 +1171,7 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
}
}
- if (head_invalid || !argc)
+ if (!head_commit || !argc)
usage_with_options(builtin_merge_usage,
builtin_merge_options);
@@ -1205,11 +1212,10 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
}
if (!remoteheads->next)
- common = get_merge_bases(lookup_commit(head),
- remoteheads->item, 1);
+ common = get_merge_bases(head_commit, remoteheads->item, 1);
else {
struct commit_list *list = remoteheads;
- commit_list_insert(lookup_commit(head), &list);
+ commit_list_insert(head_commit, &list);
common = get_octopus_merge_bases(list);
free(list);
}
@@ -1254,7 +1260,7 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
if (checkout_fast_forward(head, remoteheads->item->object.sha1))
return 1;
- finish(o->sha1, msg.buf);
+ finish(head_commit, o->sha1, msg.buf);
drop_save();
return 0;
} else if (!remoteheads->next && common->next)
@@ -1275,7 +1281,7 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
printf(_("Trying really trivial in-index merge...\n"));
if (!read_tree_trivial(common->item->object.sha1,
head, remoteheads->item->object.sha1))
- return merge_trivial();
+ return merge_trivial(head_commit);
printf(_("Nope.\n"));
}
} else {
@@ -1294,8 +1300,7 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
* merge_bases again, otherwise "git merge HEAD^
* HEAD^^" would be missed.
*/
- common_one = get_merge_bases(lookup_commit(head),
- j->item, 1);
+ common_one = get_merge_bases(head_commit, j->item, 1);
if (hashcmp(common_one->item->object.sha1,
j->item->object.sha1)) {
up_to_date = 0;
@@ -1333,7 +1338,7 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
int ret;
if (i) {
printf(_("Rewinding the tree to pristine...\n"));
- restore_state(stash);
+ restore_state(head, stash);
}
if (use_strategies_nr != 1)
printf(_("Trying merge strategy %s...\n"),
@@ -1345,7 +1350,7 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
wt_strategy = use_strategies[i]->name;
ret = try_merge_strategy(use_strategies[i]->name,
- common, head_arg);
+ common, head_commit, head_arg);
if (!option_commit && !ret) {
merge_was_ok = 1;
/*
@@ -1387,14 +1392,15 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
* auto resolved the merge cleanly.
*/
if (automerge_was_ok)
- return finish_automerge(common, result_tree, wt_strategy);
+ return finish_automerge(head_commit, common, result_tree,
+ wt_strategy);
/*
* Pick the result from the best strategy and have the user fix
* it up.
*/
if (!best_strategy) {
- restore_state(stash);
+ restore_state(head, stash);
if (use_strategies_nr > 1)
fprintf(stderr,
_("No merge strategy handled the merge.\n"));
@@ -1406,14 +1412,14 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
; /* We already have its result in the working tree. */
else {
printf(_("Rewinding the tree to pristine...\n"));
- restore_state(stash);
+ restore_state(head, stash);
printf(_("Using the %s to prepare resolving by hand.\n"),
best_strategy);
- try_merge_strategy(best_strategy, common, head_arg);
+ try_merge_strategy(best_strategy, common, head_commit, head_arg);
}
if (squash)
- finish(NULL, NULL);
+ finish(head_commit, NULL, NULL);
else {
int fd;
struct commit_list *j;
--
1.7.4.74.g639db
^ permalink raw reply related
* [PATCH v4 2/4] merge: keep stash[] a local variable
From: Nguyễn Thái Ngọc Duy @ 2011-08-19 14:50 UTC (permalink / raw)
To: git, Junio C Hamano; +Cc: Nguyễn Thái Ngọc Duy
In-Reply-To: <1313765407-29925-1-git-send-email-pclouds@gmail.com>
A stash is created by save_state() and used by restore_state(). Pass
SHA-1 explicitly for clarity and keep stash[] to cmd_merge().
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
builtin/merge.c | 33 ++++++++++++++++-----------------
1 files changed, 16 insertions(+), 17 deletions(-)
diff --git a/builtin/merge.c b/builtin/merge.c
index 325891e..a068660 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -50,7 +50,7 @@ static int fast_forward_only;
static int allow_trivial = 1, have_message;
static struct strbuf merge_msg;
static struct commit_list *remoteheads;
-static unsigned char head[20], stash[20];
+static unsigned char head[20];
static struct strategy **use_strategies;
static size_t use_strategies_nr, use_strategies_alloc;
static const char **xopts;
@@ -217,7 +217,7 @@ static void drop_save(void)
unlink(git_path("MERGE_MODE"));
}
-static void save_state(void)
+static int save_state(unsigned char *stash)
{
int len;
struct child_process cp;
@@ -236,11 +236,12 @@ static void save_state(void)
if (finish_command(&cp) || len < 0)
die(_("stash failed"));
- else if (!len)
- return;
+ else if (!len) /* no changes */
+ return -1;
strbuf_setlen(&buffer, buffer.len-1);
if (get_sha1(buffer.buf, stash))
die(_("not a valid object: %s"), buffer.buf);
+ return 0;
}
static void read_empty(unsigned const char *sha1, int verbose)
@@ -278,7 +279,7 @@ static void reset_hard(unsigned const char *sha1, int verbose)
die(_("read-tree failed"));
}
-static void restore_state(void)
+static void restore_state(const unsigned char *stash)
{
struct strbuf sb = STRBUF_INIT;
const char *args[] = { "stash", "apply", NULL, NULL };
@@ -1010,6 +1011,7 @@ static int setup_with_upstream(const char ***argv)
int cmd_merge(int argc, const char **argv, const char *prefix)
{
unsigned char result_tree[20];
+ unsigned char stash[20];
struct strbuf buf = STRBUF_INIT;
const char *head_arg;
int flag, head_invalid = 0, i;
@@ -1320,21 +1322,18 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
* sync with the head commit. The strategies are responsible
* to ensure this.
*/
- if (use_strategies_nr != 1) {
- /*
- * Stash away the local changes so that we can try more
- * than one.
- */
- save_state();
- } else {
- memcpy(stash, null_sha1, 20);
- }
+ if (use_strategies_nr == 1 ||
+ /*
+ * Stash away the local changes so that we can try more than one.
+ */
+ save_state(stash))
+ hashcpy(stash, null_sha1);
for (i = 0; i < use_strategies_nr; i++) {
int ret;
if (i) {
printf(_("Rewinding the tree to pristine...\n"));
- restore_state();
+ restore_state(stash);
}
if (use_strategies_nr != 1)
printf(_("Trying merge strategy %s...\n"),
@@ -1395,7 +1394,7 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
* it up.
*/
if (!best_strategy) {
- restore_state();
+ restore_state(stash);
if (use_strategies_nr > 1)
fprintf(stderr,
_("No merge strategy handled the merge.\n"));
@@ -1407,7 +1406,7 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
; /* We already have its result in the working tree. */
else {
printf(_("Rewinding the tree to pristine...\n"));
- restore_state();
+ restore_state(stash);
printf(_("Using the %s to prepare resolving by hand.\n"),
best_strategy);
try_merge_strategy(best_strategy, common, head_arg);
--
1.7.4.74.g639db
^ permalink raw reply related
* [PATCH v4 1/4] commit: remove global variable head_sha1[]
From: Nguyễn Thái Ngọc Duy @ 2011-08-19 14:50 UTC (permalink / raw)
To: git, Junio C Hamano; +Cc: Nguyễn Thái Ngọc Duy
In-Reply-To: <1313674994-22902-1-git-send-email-pclouds@gmail.com>
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
builtin/commit.c | 52 ++++++++++++++++++++++++++--------------------------
1 files changed, 26 insertions(+), 26 deletions(-)
diff --git a/builtin/commit.c b/builtin/commit.c
index cb73857..c9c4ea5 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -62,8 +62,6 @@ N_("The previous cherry-pick is now empty, possibly due to conflict resolution.\
"\n"
"Otherwise, please use 'git reset'\n");
-static unsigned char head_sha1[20];
-
static const char *use_message_buffer;
static const char commit_editmsg[] = "COMMIT_EDITMSG";
static struct lock_file index_lock; /* real index */
@@ -296,7 +294,7 @@ static void add_remove_files(struct string_list *list)
}
}
-static void create_base_index(void)
+static void create_base_index(const unsigned char *head_sha1)
{
struct tree *tree;
struct unpack_trees_options opts;
@@ -334,7 +332,8 @@ static void refresh_cache_or_die(int refresh_flags)
die_resolve_conflict("commit");
}
-static char *prepare_index(int argc, const char **argv, const char *prefix, int is_status)
+static char *prepare_index(int argc, const char **argv, const char *prefix,
+ const unsigned char *head_sha1, int is_status)
{
int fd;
struct string_list partial;
@@ -469,7 +468,7 @@ static char *prepare_index(int argc, const char **argv, const char *prefix, int
(uintmax_t) getpid()),
LOCK_DIE_ON_ERROR);
- create_base_index();
+ create_base_index(head_sha1);
add_remove_files(&partial);
refresh_cache(REFRESH_QUIET);
@@ -518,11 +517,8 @@ static int run_status(FILE *fp, const char *index_file, const char *prefix, int
return s->commitable;
}
-static int is_a_merge(const unsigned char *sha1)
+static int is_a_merge(struct commit *commit)
{
- struct commit *commit = lookup_commit(sha1);
- if (!commit || parse_commit(commit))
- die(_("could not parse HEAD commit"));
return !!(commit->parents && commit->parents->next);
}
@@ -627,7 +623,7 @@ static char *cut_ident_timestamp_part(char *string)
}
static int prepare_to_commit(const char *index_file, const char *prefix,
- struct wt_status *s,
+ struct commit *head_commit, struct wt_status *s,
struct strbuf *author_ident)
{
struct stat statbuf;
@@ -848,7 +844,7 @@ static int prepare_to_commit(const char *index_file, const char *prefix,
* empty due to conflict resolution, which the user should okay.
*/
if (!commitable && whence != FROM_MERGE && !allow_empty &&
- !(amend && is_a_merge(head_sha1))) {
+ !(amend && is_a_merge(head_commit))) {
run_status(stdout, index_file, prefix, 0, s);
if (amend)
fputs(_(empty_amend_advice), stderr);
@@ -1026,9 +1022,6 @@ static int parse_and_validate_options(int argc, const char *argv[],
if (!use_editor)
setenv("GIT_EDITOR", ":", 1);
- if (get_sha1("HEAD", head_sha1))
- initial_commit = 1;
-
/* Sanity check options */
if (amend && initial_commit)
die(_("You have nothing to amend."));
@@ -1102,12 +1095,12 @@ static int parse_and_validate_options(int argc, const char *argv[],
}
static int dry_run_commit(int argc, const char **argv, const char *prefix,
- struct wt_status *s)
+ const unsigned char *head_sha1, struct wt_status *s)
{
int commitable;
const char *index_file;
- index_file = prepare_index(argc, argv, prefix, 1);
+ index_file = prepare_index(argc, argv, prefix, head_sha1, 1);
commitable = run_status(stdout, index_file, prefix, 0, s);
rollback_index_files();
@@ -1383,11 +1376,13 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
const char *index_file, *reflog_msg;
char *nl, *p;
unsigned char commit_sha1[20];
+ unsigned char head_sha1[20];
struct ref_lock *ref_lock;
struct commit_list *parents = NULL, **pptr = &parents;
struct stat statbuf;
int allow_fast_forward = 1;
struct wt_status s;
+ struct commit *head_commit;
if (argc == 2 && !strcmp(argv[1], "-h"))
usage_with_options(builtin_commit_usage, builtin_commit_options);
@@ -1396,6 +1391,14 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
git_config(git_commit_config, &s);
determine_whence(&s);
+ if (get_sha1("HEAD", head_sha1))
+ initial_commit = 1;
+ else {
+ head_commit = lookup_commit(head_sha1);
+ if (!head_commit || parse_commit(head_commit))
+ die(_("could not parse HEAD commit"));
+ }
+
if (s.use_color == -1)
s.use_color = git_use_color_default;
argc = parse_and_validate_options(argc, argv, builtin_commit_usage,
@@ -1403,13 +1406,14 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
if (dry_run) {
if (diff_use_color_default == -1)
diff_use_color_default = git_use_color_default;
- return dry_run_commit(argc, argv, prefix, &s);
+ return dry_run_commit(argc, argv, prefix, head_sha1, &s);
}
- index_file = prepare_index(argc, argv, prefix, 0);
+ index_file = prepare_index(argc, argv, prefix, head_sha1, 0);
/* Set up everything for writing the commit object. This includes
running hooks, writing the trees, and interacting with the user. */
- if (!prepare_to_commit(index_file, prefix, &s, &author_ident)) {
+ if (!prepare_to_commit(index_file, prefix, head_commit,
+ &s, &author_ident)) {
rollback_index_files();
return 1;
}
@@ -1421,15 +1425,11 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
reflog_msg = "commit (initial)";
} else if (amend) {
struct commit_list *c;
- struct commit *commit;
if (!reflog_msg)
reflog_msg = "commit (amend)";
- commit = lookup_commit(head_sha1);
- if (!commit || parse_commit(commit))
- die(_("could not parse HEAD commit"));
- for (c = commit->parents; c; c = c->next)
+ for (c = head_commit->parents; c; c = c->next)
pptr = &commit_list_insert(c->item, pptr)->next;
} else if (whence == FROM_MERGE) {
struct strbuf m = STRBUF_INIT;
@@ -1437,7 +1437,7 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
if (!reflog_msg)
reflog_msg = "commit (merge)";
- pptr = &commit_list_insert(lookup_commit(head_sha1), pptr)->next;
+ pptr = &commit_list_insert(head_commit, pptr)->next;
fp = fopen(git_path("MERGE_HEAD"), "r");
if (fp == NULL)
die_errno(_("could not open '%s' for reading"),
@@ -1463,7 +1463,7 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
reflog_msg = (whence == FROM_CHERRY_PICK)
? "commit (cherry-pick)"
: "commit";
- pptr = &commit_list_insert(lookup_commit(head_sha1), pptr)->next;
+ pptr = &commit_list_insert(head_commit, pptr)->next;
}
/* Finally, get the commit message */
--
1.7.4.74.g639db
^ permalink raw reply related
* Re: [PATCH RFC] gitk: Allow commit editing
From: Michal Sojka @ 2011-08-19 14:40 UTC (permalink / raw)
To: Chris Packham, Jeff King; +Cc: git, paulus
In-Reply-To: <87sjoxv835.fsf@steelpick.2x.cz>
On Fri, 19 Aug 2011, Michal Sojka wrote:
> On Fri, 19 Aug 2011, Chris Packham wrote:
> > One thing I've thought about (but don't know enough TCL to begin to
> > implement) is a graphical rebase front end. I often use git gui to make
> > tweaks to the last commit (message and content) so why not extend that
> > to a rebase operation. I think that might address some of Peffs concerns
> > because the user would be invoking something specifically intended for
> > rebasing and accepts all the caveats that go along with that.
>
> Hi Chris,
>
> the version of the patch below supports not only editing of commit
> message but also of the commit itself. There is an easy way to split
> commits and to remove lines/hunks form the commits. Additionally, with
> the help of editor and "Rescan" button in git gui, you can add things to
> the commits.
>
> I think that in the similar way as in this patch, it would be easy to
> allow gitk doing fixup and squash operations offered by rebase.
Here is another proof of concept, that gitk can also fold commits
together. It applies on top of the previously sent patch.
This operation is probably even more dangerous then simple edit because
it reorders commits. Does anybody have an idea which checks should be
employed in order to warn users about unexpected results?
-Michal
--8<---------------cut here---------------start------------->8---
From 5943dfcd9738f8da9d159f912302cfbb2b4d35b5 Mon Sep 17 00:00:00 2001
From: Michal Sojka <sojka@os.inf.tu-dresden.de>
Date: Fri, 19 Aug 2011 16:33:55 +0200
Subject: [PATCH 2/2] gitk: Learn how to fixup commits
Signed-off-by: Michal Sojka <sojka@os.inf.tu-dresden.de>
---
gitk-git/gitk | 41 ++++++++++++++++++++++++++++++++++++-----
1 files changed, 36 insertions(+), 5 deletions(-)
diff --git a/gitk-git/gitk b/gitk-git/gitk
index 432ca9b..53fa51d 100755
--- a/gitk-git/gitk
+++ b/gitk-git/gitk
@@ -2488,6 +2488,7 @@ proc makewindow {} {
{mc "Diff this -> selected" command {diffvssel 0}}
{mc "Diff selected -> this" command {diffvssel 1}}
{mc "Edit this commit" command edit_commit}
+ {mc "Fold this into selected (fixup)" command fixup_commit}
{mc "Make patch" command mkpatch}
{mc "Create tag" command mktag}
{mc "Write commit to file" command writecommit}
@@ -8448,6 +8449,7 @@ proc rowmenu {x y id} {
$menu entryconfigure [mca "Diff this -> selected"] -state $state
$menu entryconfigure [mca "Diff selected -> this"] -state $state
$menu entryconfigure [mca "Make patch"] -state $state
+ $menu entryconfigure [mca "Fold this into selected (fixup)"] -state $state
tk_popup $menu $x $y
}
@@ -9103,14 +9105,24 @@ proc cherrypick {} {
notbusy cherrypick
}
+proc rebase_ok {id} {
+ if {[exec git branch -r --contains=$id] ne {}} {
+ if { [confirm_popup [mc "The commit you are going to edit is contained in at least\
+ one remote branch. It is a bad idea to change a branch that is\
+ possibly used by other people. See git-rebase(1) for details.\n\n\
+ Do you want to continue?"]]} {
+ return 1
+ } else {
+ return 0
+ }
+ }
+ return 1
+}
+
proc edit_commit {} {
global rowmenuid selectedline
- if {[exec git branch -r --contains=$rowmenuid] ne {}} {
- if {![confirm_popup [mc "The commit you are going to edit is contained in at least\
- one remote branch. It is a bad idea to change a branch that is\
- possibly used by other people. See git-rebase(1) for details.\n\n\
- Do you want to continue?"]]} return }
+ if {![rebase_ok $rowmenuid]} return
nowbusy edit [mc "Editing commit"]
if {[catch {exec sh -c "(GIT_EDITOR='sed -ie 1s/^pick/edit/' git rebase -p -i $rowmenuid^ && git gui citool --amend) 2>&1"} err]} {
@@ -9141,6 +9153,25 @@ proc edit_commit {} {
notbusy edit
}
+proc fixup_commit {} {
+ global rowmenuid selectedline
+ set selectedid [commitonrow $selectedline]
+ set baseid [exec git merge-base $selectedid $rowmenuid]
+
+ if {![rebase_ok $baseid]} return
+
+ set this [string range $rowmenuid 0 6]
+ set selected [string range $selectedid 0 6]
+ nowbusy fixup
+ if {[catch {exec sh -c "GIT_EDITOR=\"sed -i -e '/^pick $selected/a fixup $this' -e '/^pick $this/d'\" git rebase -p -i $baseid^ 2>&1"} err]} {
+ notbusy fixup
+ error_popup $err
+ exec git rebase --abort
+ return
+ }
+ updatecommits
+ notbusy fixup
+}
proc resethead {} {
global mainhead rowmenuid confirm_ok resettype NS
--
1.7.5.4
--8<---------------cut here---------------end--------------->8---
^ permalink raw reply related
* Re: [PATCH v3 0/4] git-p4: Improve branch support
From: Vitor Antunes @ 2011-08-19 14:30 UTC (permalink / raw)
To: Pete Wyckoff; +Cc: git, Tor Arvid Lund
In-Reply-To: <20110819115310.GC15639@padd.com>
On Fri, Aug 19, 2011 at 12:53 PM, Pete Wyckoff <pw@padd.com> wrote:
> Patches 1 and 2 are great. We should have gotten those in way
> back when you first submitted them. I happily ack those.
>
> I'm still a bit hung up on #3, mainly because I don't get branch
> support. Let me play around with your test. Having this
> playbook of how it is supposed to work will help to educate me.
(I am probably going in too much detail here. Please skip any
redundant information.)
In general, you can see branches in P4 as you do in SVN. They are
simple copies from one directory to another. For example:
p4 integrate //depot/big/directory/path/myproj/version_A \
//depot/big/directory/path/myproj/version_B
Now the history of "version_B" is tied to "version_A". If you want to
integrate (merge, in this case) new updates from one of the
directories to the other you can just rerun the above command
(version_A and version_B order may change depending on the direction
of the merge).
P4 also allows you to define "branch specs" that you can use as a
short hand instead of having to type in the full directory paths. So,
assuming that you have a branch named "myproj_B" with the following
view defined:
//depot/big/directory/path/myproj/version_A \
//depot/big/directory/path/myproj/version_B
When you want to integrate it again you can simply type:
p4 integrate -b myproj_B
or, if you want to integrate from version_B into version_A instead:
p4 integrate -b myproj_B -r
git-p4 is using these branch specs to identify branches. Without this
extra information it is nearly impossible for it to identify branches
because an integrate can be done of a single file.
But since P4 does not strictly require branch specs and because most
of the time integrations are done through P4V, then most of the time
no branch specs are created. Now, if this happens git-p4 will not be
able to detect branches! And creating branches in P4 just for git-p4
does not make much sense, right? This is the main reason behind the
third patch... :)
Sorry for the long email.
--
Vitor Antunes
^ permalink raw reply
* Re: [PATCH v2 1/4] git-p4: Allow setting rename/copy detection threshold.
From: Vitor Antunes @ 2011-08-19 13:51 UTC (permalink / raw)
To: Pete Wyckoff; +Cc: Junio C Hamano, git, Tor Arvid Lund
In-Reply-To: <20110819114719.GB15639@padd.com>
On Fri, Aug 19, 2011 at 12:47 PM, Pete Wyckoff <pw@padd.com> wrote:
> I think you have to decide that 1 means 1 in this case.
> Everything else can mean true. That may suggest that using
> --bool or --bool-or-int isn't possible in this case.
But doing that kind of post-processing would require me to call
git-config (at least) twice: first to check if it is a number with a
possible "." in the middle or "%" at the end and a second time with
the --bool option. I have no problem in doing this, but I think it
increases the complexity without bringing major advantages.
I will use --bool for detectCopiesHarder and will send you the new set
of patches tonight, unless we decide to also start using it for
detectCopies and detectRenames.
--
Vitor Antunes
^ permalink raw reply
* Re: [PATCH RFC] gitk: Allow commit editing
From: Michal Sojka @ 2011-08-19 13:34 UTC (permalink / raw)
To: Chris Packham, Jeff King; +Cc: git, paulus
In-Reply-To: <4E4E4C84.4030804@gmail.com>
On Fri, 19 Aug 2011, Chris Packham wrote:
> One thing I've thought about (but don't know enough TCL to begin to
> implement) is a graphical rebase front end. I often use git gui to make
> tweaks to the last commit (message and content) so why not extend that
> to a rebase operation. I think that might address some of Peffs concerns
> because the user would be invoking something specifically intended for
> rebasing and accepts all the caveats that go along with that.
Hi Chris,
the version of the patch below supports not only editing of commit
message but also of the commit itself. There is an easy way to split
commits and to remove lines/hunks form the commits. Additionally, with
the help of editor and "Rescan" button in git gui, you can add things to
the commits.
I think that in the similar way as in this patch, it would be easy to
allow gitk doing fixup and squash operations offered by rebase.
-Michal
--8<---------------cut here---------------start------------->8---
From 1d0f0a778afbaeb928cdecb3f18065757b3aa2fa Mon Sep 17 00:00:00 2001
From: Michal Sojka <sojka@os.inf.tu-dresden.de>
Date: Fri, 19 Aug 2011 15:21:47 +0200
Subject: [PATCH] gitk: Allow commit editing
I often use gitk to review patches before pushing them and would like
to have an easy way of fixing typos in commit messages. The current
approach with copying the commitid, switching to terminal, invoking
git rebase -i, editing the commit and switching back to gitk is a way
too complicated just for changing a single letter in commit message or
remove a debug printf().
This patch adds "Edit this commit" item to gitk's context menu which
invokes interactive rebase in a non-interactive way :-). git gui is
used to actually edit the commit.
Splitting of commits (as described in git-rebase(1)) is also supported.
The user is warned if the commit that is going to be edited is
contained in a remote branch.
Signed-off-by: Michal Sojka <sojka@os.inf.tu-dresden.de>
---
gitk-git/gitk | 40 ++++++++++++++++++++++++++++++++++++++++
1 files changed, 40 insertions(+), 0 deletions(-)
diff --git a/gitk-git/gitk b/gitk-git/gitk
index 4cde0c4..432ca9b 100755
--- a/gitk-git/gitk
+++ b/gitk-git/gitk
@@ -2487,6 +2487,7 @@ proc makewindow {} {
makemenu $rowctxmenu {
{mc "Diff this -> selected" command {diffvssel 0}}
{mc "Diff selected -> this" command {diffvssel 1}}
+ {mc "Edit this commit" command edit_commit}
{mc "Make patch" command mkpatch}
{mc "Create tag" command mktag}
{mc "Write commit to file" command writecommit}
@@ -9102,6 +9103,45 @@ proc cherrypick {} {
notbusy cherrypick
}
+proc edit_commit {} {
+ global rowmenuid selectedline
+
+ if {[exec git branch -r --contains=$rowmenuid] ne {}} {
+ if {![confirm_popup [mc "The commit you are going to edit is contained in at least\
+ one remote branch. It is a bad idea to change a branch that is\
+ possibly used by other people. See git-rebase(1) for details.\n\n\
+ Do you want to continue?"]]} return }
+
+ nowbusy edit [mc "Editing commit"]
+ if {[catch {exec sh -c "(GIT_EDITOR='sed -ie 1s/^pick/edit/' git rebase -p -i $rowmenuid^ && git gui citool --amend) 2>&1"} err]} {
+ notbusy edit
+ error_popup $err
+ exec git rebase --abort
+ return
+ }
+ set newcommit [exec git rev-parse HEAD]
+ while {[catch {exec sh -c "git diff-index --quiet --cached HEAD && git diff-files --quiet"} err]} {
+ if {[confirm_popup [mc "There are uncommited changes in the working tree or in the index.\
+ Do you want to create a new commit (OK) or throw them away (Cancel)?"]]} {
+ catch {exec git gui citool} err;
+ # In case of error (i.e. the user did not commit anything), we just ask him again
+ } else {
+ exec git reset --hard
+ }
+ }
+ if {[catch {exec sh -c "git rebase --continue 2>&1"} err]} {
+ notbusy edit
+ error_popup $err
+ exec git rebase --abort
+ return
+ }
+ updatecommits
+ # XXX How to select the edited commit? This doesn't work.
+ selbyid $newcommit
+ notbusy edit
+}
+
+
proc resethead {} {
global mainhead rowmenuid confirm_ok resettype NS
--
1.7.5.4
--8<---------------cut here---------------end--------------->8---
^ permalink raw reply related
* Re: [PATCH v3] Accept tags in HEAD or MERGE_HEAD
From: Nguyen Thai Ngoc Duy @ 2011-08-19 12:53 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vd3g2a431.fsf@alter.siamese.dyndns.org>
2011/8/19 Junio C Hamano <gitster@pobox.com>:
>> static unsigned char head_sha1[20];
>> +static struct commit *head_commit;
>
> I was not happy with the file-scope global head_sha1[] already, and this
> makes me even less happy. Was it too much trouble to keep them local to
> cmd_commit() and pass them around as arguments where necessary? If you
> pass around head_commit, is_null_sha1(head_sha1) can be replaced with a
> check !head_commit so we may even be able to lose the head_sha1[] global.
>
> And actually removing head_sha1[] is a necessary step from the correctness
> point of view. The repository may have given an object name for a tag in
> head_sha1[] and lookup_expect_commit() may have peeled it to a commit.
> The code may want to add the "HEAD" as one of the parents of a new commit,
> and head_sha1[] is no longer the place to pick that information up after
> your fix. The code needs to look at head_commit->object.sha1[] instead.
> Losing the use of head_sha1[] variable and forcing the code to go to
> head_commit->object.sha1[] reduces the risk of such confusion.
I actually wanted to get rid of head_sha1 but it made changes bigger.
But the patch's become big already. Let me remove head_sha1[] (and
head[] in merge.c) globally, then put this fix on top.
--
Duy
^ permalink raw reply
* [PATCH] log: decorate "replaced" on to replaced commits
From: Nguyễn Thái Ngọc Duy @ 2011-08-19 12:43 UTC (permalink / raw)
To: git, Junio C Hamano; +Cc: Nguyễn Thái Ngọc Duy
In-Reply-To: <7vippua433.fsf@alter.siamese.dyndns.org>
Old code also decorates "new" commits with "refs/replace/SHA1". This
is now gone, but I guess no one will miss it.
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
2011/8/19 Junio C Hamano <gitster@pobox.com>:
> If the argument is "we know only commits and tags are listed and blobs and
> trees are not shown with --decorate option" and "excluding the decoration
> that we know will never be used will avoid bloating the decorate hashtable
> with unused cruft", then add_name_decoration() should be doing the check
> for all of its callers, not just this one, no?
Makes sense. Moreover replacing blobs and trees are generally not safe.
If people do that, they may have more issues to worry about than this.
Let's keep it simple. We can fix add_name_decoration() later if it becomes
a real problem.
log-tree.c | 16 +++++++++++++++-
1 files changed, 15 insertions(+), 1 deletions(-)
diff --git a/log-tree.c b/log-tree.c
index e945701..d73e69c 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -92,8 +92,22 @@ static void add_name_decoration(enum decoration_type type, const char *name, str
static int add_ref_decoration(const char *refname, const unsigned char *sha1, int flags, void *cb_data)
{
- struct object *obj = parse_object(sha1);
+ struct object *obj;
enum decoration_type type = DECORATION_NONE;
+
+ if (!prefixcmp(refname, "refs/replace/")) {
+ unsigned char original_sha1[20];
+ if (get_sha1_hex(refname + 13, original_sha1)) {
+ warning("invalid replace ref %s", refname);
+ return 0;
+ }
+ obj = parse_object(original_sha1);
+ if (obj)
+ add_name_decoration(DECORATION_GRAFTED, "replaced", obj);
+ return 0;
+ }
+
+ obj = parse_object(sha1);
if (!obj)
return 0;
--
1.7.4.74.g639db
^ permalink raw reply related
* Re: [PATCH RFC] gitk: Allow commit editing
From: Michal Sojka @ 2011-08-19 12:23 UTC (permalink / raw)
To: Jeff King; +Cc: git, paulus
In-Reply-To: <20110818223346.GA8481@sigill.intra.peff.net>
On Thu, 18 Aug 2011, Jeff King wrote:
> On Wed, Aug 17, 2011 at 09:56:11PM +0200, Michal Sojka wrote:
>
> > Hi, this is a proof of concept patch that allows editing of commits
> > from gitk. I often review patches before pushing in gitk and if I
> > would like to have an easy way of fixing typos in commit messages etc.
> >
> > So the patch adds "Edit this commit" item to gitk's context menu and
> > the actual editing is done by non-interactively invoking interactive
> > rebase :-) and git gui.
>
> Invoking rebase behind the scenes makes me very nervous. In particular:
>
> 1. There is nothing to indicate to the user that they are rewriting a
> string of commits, which is going to wreak havoc if any of the
> commits have been published elsewhere (either pushed somewhere, or
> even present in another local branch). I.e., rebasing generally
> needs to be a conscious decision of the user.
I added a warning if the edited commit is contained in a remote branch.
Would you consider this sufficient?
> 2. Even if you accept the hazard of rewriting commits, you don't pass
> "-p" to rebase. So it will linearize your history. I don't know how
> robust "-p" is these days, and if it's up to the challenge of
> people using it to rebase potentially large segments of complex
> history.
I added "-p" to rebase. I do not know about the robustness of "-p" as
well, but is anything goes wrong during the rebase, git rebase --abort
hopefully reverts everything.
> So I think your idea is sane, and if you use it appropriately (by
> editing commits in recent-ish linear stretches of history) your patch
> works fine. But I really worry that it is going to be a problem for less
> clueful users to stumble across in the menu.
See the next version of the patch in a follow-up mail.
-Michal
^ permalink raw reply
* [PATCH] gitk: Allow commit editing
From: Michal Sojka @ 2011-08-19 12:25 UTC (permalink / raw)
To: git; +Cc: peff, paulus, Michal Sojka
In-Reply-To: <87obzlwpx0.fsf@steelpick.2x.cz>
I often use gitk to review patches before pushing them and would like
to have an easy way of fixing typos in commit messages. The current
approach if copying the commitid, switching to terminal, invoking git
rebase -i, editing the commit and switching back to gitk is a way too
complicated just for chaning a single letter.
This patch adds "Edit this commit" item to gitk's context menu which
invokes interactive rebase in a non-interactive way :-). git gui is
used to actually edit the commit.
The user is warned if the commit that is going to be edited is
contained in a remote branch.
Signed-off-by: Michal Sojka <sojka@os.inf.tu-dresden.de>
---
gitk-git/gitk | 31 +++++++++++++++++++++++++++++++
1 files changed, 31 insertions(+), 0 deletions(-)
diff --git a/gitk-git/gitk b/gitk-git/gitk
index 4cde0c4..83b3307 100755
--- a/gitk-git/gitk
+++ b/gitk-git/gitk
@@ -2487,6 +2487,7 @@ proc makewindow {} {
makemenu $rowctxmenu {
{mc "Diff this -> selected" command {diffvssel 0}}
{mc "Diff selected -> this" command {diffvssel 1}}
+ {mc "Edit this commit" command edit_commit}
{mc "Make patch" command mkpatch}
{mc "Create tag" command mktag}
{mc "Write commit to file" command writecommit}
@@ -9102,6 +9103,36 @@ proc cherrypick {} {
notbusy cherrypick
}
+proc edit_commit {} {
+ global rowmenuid selectedline
+
+ if {[exec git branch -r --contains=$rowmenuid] ne {}} {
+ if {![confirm_popup [mc "The commit you are going to edit appears in at least one\
+ remote branch. It is a bad idea to change a branch that is\
+ possibly used by other people. See git-rebase(1) for details.\n\n\
+ Do you want to continue?"]]} return }
+
+ nowbusy edit [mc "Editing commit"]
+ if {[catch {exec sh -c "(GIT_EDITOR='sed -ie 1s/^pick/edit/' git rebase -p -i $rowmenuid^ && git gui citool --amend) 2>&1"} err]} {
+ notbusy edit
+ error_popup $err
+ exec git rebase --abort
+ return
+ }
+ set newcommit [exec git rev-parse HEAD]
+ if {[catch {exec sh -c "git rebase --continue 2>&1"} err]} {
+ notbusy edit
+ error_popup $err
+ exec git rebase --abort
+ return
+ }
+ updatecommits
+ # XXX How to select the edited commit? This doesn't work.
+ selbyid $newcommit
+ notbusy edit
+}
+
+
proc resethead {} {
global mainhead rowmenuid confirm_ok resettype NS
--
1.7.5.4
^ permalink raw reply related
* Re: [PATCH 10/14] http: use hostname in credential description
From: Ted Zlatanov @ 2011-08-19 12:01 UTC (permalink / raw)
To: git
In-Reply-To: <20110722221337.GA15753@sigill.intra.peff.net>
I see some info in "What's Cooking" about this patch but it's unclear to
me whether the hostname issue (where it's hard to have multiple
identities on a single server, which I think is all right) is blocking
the inclusion of the patch into the next release, or if it will be
included eventually if no one complains about that issue, or something
else...
Thanks
Ted
^ permalink raw reply
* Re: [PATCH v3 0/4] git-p4: Improve branch support
From: Pete Wyckoff @ 2011-08-19 11:53 UTC (permalink / raw)
To: Vitor Antunes; +Cc: git, Tor Arvid Lund
In-Reply-To: <1313711046-23489-1-git-send-email-vitor.hda@gmail.com>
vitor.hda@gmail.com wrote on Fri, 19 Aug 2011 00:44 +0100:
> Add missing test case from last version of this set of patches.
>
> Vitor Antunes (4):
> git-p4: Correct branch base depot path detection
> git-p4: Allow filtering Perforce branches by user
> git-p4: Allow branch definition with git config
> git-p4: Add simple test case for branch import
>
> contrib/fast-import/git-p4 | 40 ++++++++++++++++++++++--
> contrib/fast-import/git-p4.txt | 13 ++++++++
> t/t9800-git-p4.sh | 64 ++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 113 insertions(+), 4 deletions(-)
Patches 1 and 2 are great. We should have gotten those in way
back when you first submitted them. I happily ack those.
I'm still a bit hung up on #3, mainly because I don't get branch
support. Let me play around with your test. Having this
playbook of how it is supposed to work will help to educate me.
-- Pete
^ permalink raw reply
* Re: [PATCH v2 1/4] git-p4: Allow setting rename/copy detection threshold.
From: Pete Wyckoff @ 2011-08-19 11:47 UTC (permalink / raw)
To: Vitor Antunes; +Cc: Junio C Hamano, git, Tor Arvid Lund
In-Reply-To: <CAOpHH-VsoOSOH6ym2exCH=W4NmuKRw+f53HrwB47YD3u-iDe2w@mail.gmail.com>
vitor.hda@gmail.com wrote on Fri, 19 Aug 2011 00:43 +0100:
> On Fri, Aug 19, 2011 at 12:04 AM, Junio C Hamano <gitster@pobox.com> wrote:
> > This is not a new problem you introduced with this patch, but unless you
> > are invoking "git config --bool" in your gitConfig() (I didn't bother to
> > check), you will misunderstand different ways to say "Yes", e.g.
> >
> > [git-p4]
> > detectRenames
> > detectRenames = on
> > detectRenames = yes
> > detectRenames = 1
> >
> > If you use --bool, you can rely on the values being either true or false,
> > and do not have to do the .lower() thing.
> >
>
> Now that I look at this carefully, Tor added the possibility to add arguments to
> gitConfig() exactly for that purpose. This is helpful for processing the
> detectCopiesHarder option I added.
>
> For detectRenames and detectCopies it is a bit more complex. I think that if I
> use --bool or --bool-or-int then it is possible that certain values will fail to
> be processed. Let me give you some examples:
>
> [git-p4]
> detectRenames = true
> detectRenames = 80%
> detectRenames = 80
> detectRenames = 1%
> detectRenames = 1
>
> It will be difficult for me to, for example, to understand if a 1 represents 1%
> or true. Or am I overcomplicating this? :)
I think you have to decide that 1 means 1 in this case.
Everything else can mean true. That may suggest that using
--bool or --bool-or-int isn't possible in this case.
-- Pete
^ permalink raw reply
* [PATCH] branch.c: use the parsed branch name
From: Michael J Gruber @ 2011-08-19 11:45 UTC (permalink / raw)
To: git
When setting up tracking info, branch.c uses the given branch specifier
("name"). Use the parsed name ("ref.buf") instead so that
git branch --set-upstream @{-1} foo
sets up tracking info for the previous branch rather than for a branch
named "@{-1}".
Signed-off-by: Michael J Gruber <git@drmicha.warpmail.net>
---
That's actually 1 of n in my branch pattern series, but independent,
and ready to go now.
branch.c | 2 +-
t/t6040-tracking-info.sh | 14 ++++++++++++++
2 files changed, 15 insertions(+), 1 deletions(-)
diff --git a/branch.c b/branch.c
index c0c865a..d62cc01 100644
--- a/branch.c
+++ b/branch.c
@@ -210,7 +210,7 @@ void create_branch(const char *head,
start_name);
if (real_ref && track)
- setup_tracking(name, real_ref, track);
+ setup_tracking(ref.buf+11, real_ref, track);
if (!dont_change_ref)
if (write_ref_sha1(lock, sha1, msg) < 0)
diff --git a/t/t6040-tracking-info.sh b/t/t6040-tracking-info.sh
index a9b0ac1..19de5b1 100755
--- a/t/t6040-tracking-info.sh
+++ b/t/t6040-tracking-info.sh
@@ -110,4 +110,18 @@ test_expect_success '--set-upstream does not change branch' '
grep -q "^refs/heads/master$" actual &&
cmp expect2 actual2
'
+
+test_expect_success '--set-upstream @{-1}' '
+ git checkout from-master &&
+ git checkout from-master2 &&
+ git config branch.from-master2.merge > expect2 &&
+ git branch --set-upstream @{-1} follower &&
+ git config branch.from-master.merge > actual &&
+ git config branch.from-master2.merge > actual2 &&
+ git branch --set-upstream from-master follower &&
+ git config branch.from-master.merge > expect &&
+ test_cmp expect2 actual2 &&
+ test_cmp expect actual
+'
+
test_done
--
1.7.6.678.gaaad0
^ permalink raw reply related
* Re: [PATCH RFC] gitk: Allow commit editing
From: Chris Packham @ 2011-08-19 11:44 UTC (permalink / raw)
To: Jeff King; +Cc: Michal Sojka, git, paulus
In-Reply-To: <20110818223346.GA8481@sigill.intra.peff.net>
Hi,
On 19/08/11 10:33, Jeff King wrote:
> On Wed, Aug 17, 2011 at 09:56:11PM +0200, Michal Sojka wrote:
>
>> Hi, this is a proof of concept patch that allows editing of commits
>> from gitk. I often review patches before pushing in gitk and if I
>> would like to have an easy way of fixing typos in commit messages etc.
I've often _thought_ that I wanted something like this. As you say
firing up gitk to review the patch you're about to send (or commit
you're about to push) is probably a fairly common workflow.
>>
>> So the patch adds "Edit this commit" item to gitk's context menu and
>> the actual editing is done by non-interactively invoking interactive
>> rebase :-) and git gui.
>
> Invoking rebase behind the scenes makes me very nervous. In particular:
>
> 1. There is nothing to indicate to the user that they are rewriting a
> string of commits, which is going to wreak havoc if any of the
> commits have been published elsewhere (either pushed somewhere, or
> even present in another local branch). I.e., rebasing generally
> needs to be a conscious decision of the user.
>
> Yes, a veteran user who thinks about it will realize there is no
> way to edit an old commit without rebasing, but I suspect relying
> on that is not enough. There should probably be a prominent
> warning at least the first time somebody uses this feature.
>
> 2. Even if you accept the hazard of rewriting commits, you don't pass
> "-p" to rebase. So it will linearize your history. I don't know how
> robust "-p" is these days, and if it's up to the challenge of
> people using it to rebase potentially large segments of complex
> history.
>
> So I think your idea is sane, and if you use it appropriately (by
> editing commits in recent-ish linear stretches of history) your patch
> works fine. But I really worry that it is going to be a problem for less
> clueful users to stumble across in the menu.
>
> -Peff
And as Peff points out there are some major caveats about how far back
you would want to let people edit. I suspect you could come up with some
sane rules (e.g. only allow editing the last commit, the last N commits,
only commits that are not present in remote X). Whatever rules you come
up with to protect people I think you'll end up frustrating them as well
(why can I edit this commit but not that one).
One thing I've thought about (but don't know enough TCL to begin to
implement) is a graphical rebase front end. I often use git gui to make
tweaks to the last commit (message and content) so why not extend that
to a rebase operation. I think that might address some of Peffs concerns
because the user would be invoking something specifically intended for
rebasing and accepts all the caveats that go along with that.
Anyway that's my 2c
--
Chris
^ permalink raw reply
* Re: update-index --index-info producing spurious submodule commits
From: Greg Troxel @ 2011-08-19 11:00 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Richard Hansen
In-Reply-To: <7vpqk2593g.fsf@alter.siamese.dyndns.org>
[-- Attachment #1: Type: text/plain, Size: 1343 bytes --]
Junio C Hamano <gitster@pobox.com> writes:
> Greg Troxel <gdt@ir.bbn.com> writes:
>
>> If what I'm doing is an abuse of update-index, do you or anyone else
>> have a suggestion to make a directory in the index match a tree object?
>
> "ls-tree -r HEAD foo" is probably what you meant to say.
Thanks very much for the clue - that works. The update-index
documentation should probably say that only blobs (or perhaps commits
intended to be submodules??) are acceptable, and perhaps say "ls-tree
-r" instead of ls-tree. It could easily seem reasonable to someone to
pass in a tree and expect that to result in that tree being logically in
the index and to appear in the resulting commit.
For anyone else who is trying to do something like this, here's a
revised script that (I think) correctly reverts a directory to another commit.
#!/bin/sh
if [ -d .git ]; then
echo "existing .git"
exit 1
fi
set -x
git init
mkdir foo
touch foo/bar
git add foo
git commit -m'initial content'
git tag initial
touch foo/baz
git add foo/baz
git commit -m 'add baz'
git cat-file -p initial
git cat-file -p initial:
git rm --cached -r foo
git ls-tree initial foo
git ls-tree -r initial foo
git ls-tree -r initial foo | git update-index --index-info
git diff
git diff --staged
git commit -m'munged foo'
git cat-file -p HEAD
git cat-file -p HEAD:
[-- Attachment #2: Type: application/pgp-signature, Size: 194 bytes --]
^ permalink raw reply
* Re: [PATCH] bash-completion: Make use of git status
From: Thomas Rast @ 2011-08-19 10:10 UTC (permalink / raw)
To: Ron Panduwana; +Cc: git, Shawn O. Pearce, Junio C Hamano, Lee Marlow
In-Reply-To: <1313474948-13531-1-git-send-email-panduwana@gmail.com>
Ron Panduwana wrote:
> $ git add [TAB]path/to/deeply/located/[TAB][TAB]
> path/to/deeply/located/modified-file path/to/deeply/located/new-file
[...]
> +# __git_files_having_status requires 1 argument
> +__git_files_having_status ()
> +{
> + echo "$(git status -uall --porcelain . 2>/dev/null | egrep "^$1" | cut -c4-)"
> +}
> +
Some thoughts:
* running git-status for . has some issues: it doesn't work in the
case of
cd subdir
git add ../some/file[TAB]
It's also inefficient if you are at the top level and
git add path/to/file/a/few/levels/down[TAB]
since it wouldn't actually have to look for untracked files in the
entire repo.
* -uall is not required unless you are looking for untracked files.
For the other commands you could speed up completion by passing
-uno instead.
--
Thomas Rast
trast@{inf,student}.ethz.ch
^ permalink raw reply
* Re: [PATCH] bash-completion: Make use of git status
From: Ron Panduwana @ 2011-08-19 9:54 UTC (permalink / raw)
To: git; +Cc: Shawn O. Pearce, Junio C Hamano, Lee Marlow, Ron Panduwana
In-Reply-To: <1313474948-13531-1-git-send-email-panduwana@gmail.com>
Sorry,
Bounced for Junio, I put his old address. Fixed now.
Ron Panduwana
--
panduwana@gmail.com
On Tue, Aug 16, 2011 at 1:09 PM, Ron Panduwana <panduwana@gmail.com> wrote:
> When autocompleting git add, rm, checkout --, and reset HEAD; Make it so they use git status to offer suitable files.
>
> Signed-off-by: Ron Panduwana <panduwana@gmail.com>
> ---
> Hello,
>
> I made some little improvements to the bash autocompletion script for git add, rm, checkout --, and reset HEAD.
> It's because sometimes after doing "git status" I need to do one/some of the commands and hope the autocompletion can make use the status info git already knows.
> For example:
>
> $ git status
> # On branch master
> # Changes to be committed:
> # (use "git reset HEAD <file>..." to unstage)
> #
> # new file: path/to/deeply/located/added-file
> # modified: path/to/deeply/located/old-file
> # deleted: path/to/deeply/located/removed-file
> #
> # Changed but not updated:
> # (use "git add/rm <file>..." to update what will be committed)
> # (use "git checkout -- <file>..." to discard changes in working directory)
> #
> # deleted: path/to/deeply/located/deleted-file
> # modified: path/to/deeply/located/modified-file
> # deleted: path/to/deeply/located/old-file
> #
> # Untracked files:
> # (use "git add <file>..." to include in what will be committed)
> #
> # path/to/deeply/located/new-file
>
>
> If I need to do one of the commands, I'd like my autocompletion to work like these:
>
> $ git add [TAB]path/to/deeply/located/[TAB][TAB]
> path/to/deeply/located/modified-file path/to/deeply/located/new-file
>
> $ git rm [TAB]path/to/deeply/located/[TAB][TAB]
> path/to/deeply/located/deleted-file path/to/deeply/located/old-file
>
> $ git checkout -- [TAB]path/to/deeply/located/[TAB][TAB]
> path/to/deeply/located/deleted-file path/to/deeply/located/modified-file path/to/deeply/located/old-file
>
> $ git reset HEAD [TAB]path/to/deeply/located/[TAB][TAB]
> path/to/deeply/located/added-file path/to/deeply/located/old-file path/to/deeply/located/removed-file
>
>
> I think it's a good thing to be included as default, so here I send the diff of my modification of the autocompletion script.
>
> PS: for the modification I've taken into account:
> - has_doubledash
> - when we meant to autocomplete the --switches
> - for git rm, whether --cached presents
> - for git checkout, only if it starts with "git checkout -- "
> - for git reset, only if it starts with "git reset HEAD "
> - (ie. when we meant to autocomplete git_refs)
>
> Hope it'd be useful.
>
> contrib/completion/git-completion.bash | 76 ++++++++++++++++++++------------
> 1 files changed, 48 insertions(+), 28 deletions(-)
>
> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
> index 5a83090..233bdbb 100755
> --- a/contrib/completion/git-completion.bash
> +++ b/contrib/completion/git-completion.bash
> @@ -1010,6 +1010,12 @@ __git_has_doubledash ()
> return 1
> }
>
> +# __git_files_having_status requires 1 argument
> +__git_files_having_status ()
> +{
> + echo "$(git status -uall --porcelain . 2>/dev/null | egrep "^$1" | cut -c4-)"
> +}
> +
> __git_whitespacelist="nowarn warn error error-all fix"
>
> _git_am ()
> @@ -1058,17 +1064,17 @@ _git_apply ()
>
> _git_add ()
> {
> - __git_has_doubledash && return
> -
> - case "$cur" in
> - --*)
> - __gitcomp "
> - --interactive --refresh --patch --update --dry-run
> - --ignore-errors --intent-to-add
> - "
> - return
> - esac
> - COMPREPLY=()
> + if ! __git_has_doubledash; then
> + case "$cur" in
> + --*)
> + __gitcomp "
> + --interactive --refresh --patch --update --dry-run
> + --ignore-errors --intent-to-add
> + "
> + return
> + esac
> + fi
> + __gitcomp "$(__git_files_having_status "(.[MAU]|UD|\?\?)")"
> }
>
> _git_archive ()
> @@ -1171,7 +1177,12 @@ _git_bundle ()
>
> _git_checkout ()
> {
> - __git_has_doubledash && return
> + if __git_has_doubledash; then
> + if [[ ${words[2]} = "--" ]]; then
> + __gitcomp "$(__git_files_having_status ".[MD]")"
> + fi
> + return
> + fi
>
> case "$cur" in
> --conflict=*)
> @@ -2313,14 +2324,18 @@ _git_replace ()
>
> _git_reset ()
> {
> - __git_has_doubledash && return
> -
> - case "$cur" in
> - --*)
> - __gitcomp "--merge --mixed --hard --soft --patch"
> + if ! __git_has_doubledash; then
> + case "$cur" in
> + --*)
> + __gitcomp "--merge --mixed --hard --soft --patch"
> + return
> + ;;
> + esac
> + fi
> + if [[ ${words[2]} = "HEAD" ]]; then
> + __gitcomp "$(__git_files_having_status "[ADM].")"
> return
> - ;;
> - esac
> + fi
> __gitcomp "$(__git_refs)"
> }
>
> @@ -2337,15 +2352,20 @@ _git_revert ()
>
> _git_rm ()
> {
> - __git_has_doubledash && return
> -
> - case "$cur" in
> - --*)
> - __gitcomp "--cached --dry-run --ignore-unmatch --quiet"
> - return
> - ;;
> - esac
> - COMPREPLY=()
> + if ! __git_has_doubledash; then
> + case "$cur" in
> + --*)
> + __gitcomp "--cached --dry-run --ignore-unmatch --quiet"
> + return
> + ;;
> + esac
> + fi
> + # check if --cached was specified
> + if [ "$(__git_find_on_cmdline "--cached")" ]; then
> + COMPREPLY=()
> + else
> + __gitcomp "$(__git_files_having_status "(.D|DU|UA)")"
> + fi
> }
>
> _git_shortlog ()
> --
> 1.7.1
>
>
^ permalink raw reply
* Re: git-svn repo in weird state
From: Michael J Gruber @ 2011-08-19 9:54 UTC (permalink / raw)
To: Dave Zarzycki; +Cc: git
In-Reply-To: <2F128636-95A4-4216-894A-F0295A2D23EB@apple.com>
Dave Zarzycki venit, vidit, dixit 18.08.2011 16:22:
> After introspecting the .git directory, it seems like a stale .git/master was created somehow. Why does git-show-ref not show .git/master as a ref?
Because it's not in .git/refs/?
Michael
^ permalink raw reply
* [PATCH v2 4/4] mergetools/meld: Use '--output' when available
From: David Aguilar @ 2011-08-19 9:14 UTC (permalink / raw)
To: Junio C Hamano
Cc: git, Jonathan Nieder, Tanguy Ortolo, Charles Bailey,
Sebastian Schuberth
In-Reply-To: <1313652227-48545-5-git-send-email-davvid@gmail.com>
meld 1.5.0 and newer allow the output file to be specified
when merging multiple files. Check whether the meld command
supports '--output' and use it when available.
Signed-off-by: David Aguilar <davvid@gmail.com>
---
mergetools/meld | 25 ++++++++++++++++++++++++-
1 files changed, 24 insertions(+), 1 deletions(-)
Changes since v1 with help from Jonathan Nieder:
Use $LOCAL $BASE REMOTE when calling meld.
Avoid shell portability issues by using the return status.
of --output instead of parsing --version.
diff --git a/mergetools/meld b/mergetools/meld
index 73d70ae..eaa115c 100644
--- a/mergetools/meld
+++ b/mergetools/meld
@@ -3,7 +3,30 @@ diff_cmd () {
}
merge_cmd () {
+ if test -z "${meld_has_output_option:+set}"
+ then
+ check_meld_for_output_version
+ fi
touch "$BACKUP"
- "$merge_tool_path" "$LOCAL" "$MERGED" "$REMOTE"
+ if test "$meld_has_output_option" = true
+ then
+ "$merge_tool_path" --output "$MERGED" \
+ "$LOCAL" "$BASE" "$REMOTE"
+ else
+ "$merge_tool_path" "$LOCAL" "$MERGED" "$REMOTE"
+ fi
check_unchanged
}
+
+# Check whether 'meld --output <file>' is supported
+check_meld_for_output_version () {
+ meld_path="$(git config mergetool.meld.path)"
+ meld_path="${meld_path:-meld}"
+
+ if "$meld_path" --output /dev/null --help >/dev/null 2>&1
+ then
+ meld_has_output_option=true
+ else
+ meld_has_output_option=false
+ fi
+}
--
1.7.6.553.g1899
^ permalink raw reply related
* Re: [PATCH 7/7] sequencer: Remove sequencer state after final commit
From: Ramkumar Ramachandra @ 2011-08-19 9:08 UTC (permalink / raw)
To: Junio C Hamano
Cc: Jonathan Nieder, Git List, Christian Couder, Daniel Barkalow,
Jeff King
In-Reply-To: <7vvctu8naf.fsf@alter.siamese.dyndns.org>
Hi Jonathan and Junio,
Jonathan Nieder writes:
> Why can't it become the sequencer's responsibility, FWIW? That's an
> implementation detail.
It can be the sequencer's responsibility if we want that! I'm just
dabbling with the different implementation strategies and trying to
see the advantages/ disadvantages of each one. The "right" thing to
do wasn't obvious to me immediately.
Junio C Hamano writes:
> Ramkumar Ramachandra <artagnon@gmail.com> writes:
>>> Why modify tests? I think "git merge --continue" is a nice idea,
>>> and I don't see how it's inconsistent in any way with continuing to
>>> allow old practice.
>
> I agree. Updating the test will hide a regression if Ram's update breaks
> the existing workflow to conclude a conflicted merge with "git commit".
> If we are to add "git merge --continue" sugarcoat to make it easier to
> teach new people saying that "To any Git operation that stops and asks you
> to help, you can tell it that you are done helping by re-running the same
> command with --continue flag", then _new_ tests should be added to make
> sure that "git merge --continue" does act just like "git commit" in such a
> case.
Right. We have to keep the old tests around -- my bad.
>>> As Junio hinted, it could make a lot of sense for "git cherry-pick
>>> <single commit>" to not create sequencer state in the first place.
>>> "git cherry-pick --continue" does not need it --- it is enough to
>>> commit with the conflict resolved. "git cherry-pick --abort" does not
>>> need it, either --- it is enough to "git reset --merge HEAD".
>>
>> Okay, here's my problem with the idea: it'll essentially require the
>> sequencer to differentiate between one-commit operations and
>> many-commit operations.
>
> I think you are looking at it in a wrong way. It is just about giving
> backward compatibility to historical hacks. cherry-pick and revert may be
> the only ones needed, and a new command Foo that implements its workflow
> in terms of the sequencer can choose to (and should choose to unless there
> is a compelling reason not to, because of the exact reason you stated
> here) do a single-command insn sheet with "git Foo --continue" to conclude
> if that one and only step needed help from the end user.
No, no. I don't _want_ to create an AM_HEAD or FOO_HEAD for every new
command that we write -- I was merely pointing out what would happen
if the sequencer were only built to handle multi-commit-operations,
and every new command would have to handle single-commit-operations on
their own. I was emphasizing that the sequencer will need to handle
single-commit-operations as well; Jonathan has suggested that we have
special hacks to handle cherry-pick and merge commands in the
sequencer.
To conclude, let me list out the various implementation strategies
we've thought about, and the one we seem to have settled on:
0. Remove the sequencer state from "git commit". This is wrong, as
Junio pointed out in the first email.
1. Let commands handle single-commit-operations themselves, and call
into the sequencer only for multi-commit-operations. If they don't
call into the sequencer at all, there's no state to persist or
cleanup. This approach is clearly wrong because each new command
would have to come up with AM_HEAD and FOO_HEAD to persist the
single-commit-operation state.
2. Expose two functions from the sequencer: "earlier-function" and
"later-function". Let cherry-pick and merge handle their own
CHERRY_PICK_HEAD and MERGE_HEAD hacks first (by setting up the
revision walker, counting commits etc), and call into later-function
in the sequencer to do less work from the sequencer's end. All new
commands can directly call into earlier-function directly. I know
it's a little hand-wavy, but I hope it's atleast parse'able this time.
As I pointed out in another email, this is also broken because
cherry-pick/ merge would have to implement their own versions of every
subcommand like "--continue".
3. Let all commands call into the sequencer for everything. Let's
teach the sequencer about the CHERRY_PICK_HEAD and MERGE_HEAD hacks so
that it can deal with them when a "pick", "revert" or "merge"
operation is encountered. We'll handle it by treating
CHERRY_PICK_HEAD and MERGE_HEAD as part of the "sequencer state" (so
that subcommands work just fine on them). For all future sequencer
commands, all data will be persisted inside '.git/sequencer'. This
seems most reasonable now.
In a nutshell, instead of side-stepping historical hacks, we want to
teach the sequencer about them as specific cases to handle carefully.
We want to this without affecting the operation of the sequencer in
the general case. Sounds great! An uncompromising UI at the cost of
little ugliness in the sequencer.
Thanks.
-- Ram
^ permalink raw reply
* Re: [PATCH 11/10] support pager.* for aliases
From: Jeff King @ 2011-08-19 8:30 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Steffen Daode Nurpmeso, Ingo Brückl
In-Reply-To: <7vei0i560a.fsf@alter.siamese.dyndns.org>
On Thu, Aug 18, 2011 at 10:23:33PM -0700, Junio C Hamano wrote:
> > I think of it this way:
> >
> > If the user thinks of the alias as just another form of "log", then we
> > do the right thing: we use log's pager config by default, and respect
> > pager.log. They never set pager.foo, because that is nonsensical in
> > their mental model.
> >
> > If the user thinks of the alias as its own command, then they would
> > expect pager.foo to work. And it does what they expect.
> >
> > But like I said, I don't personally plan on using this. It was just the
> > only semantics that really made sense to me,...
>
> I can see that argument, but once you start paying attention to "*.foo",
> you have to keep supporting that forever, and also more importantly, you
> need to worry about interactions between "*.foo" vs "*.log". Which one
> should win? Should they combine if both are defined? My "looks confusing"
> includes that can of worms.
It seems obvious to me that the more-specific *.foo form would take
precedence over the *.log form, and anything else would be crazy. But
that is just my gut feeling. If you are confused or worried, then that
is enough for me to say it is not worth pursuing. It's not a feature I
really care about; it was more about fixing something that looked
obviously wrong while I was in the area.
-Peff
^ 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