Git development
 help / color / mirror / Atom feed
* [PATCH v6.1 6/8] log -L: add --graph prefix before output
From: Thomas Rast @ 2010-12-14 22:54 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Bo Yang
In-Reply-To: <cover.1292366984.git.trast@student.ethz.ch>

From: Bo Yang <struggleyb.nku@gmail.com>

Makes the line level log output look good when used
with the '--graph' option.

Signed-off-by: Bo Yang <struggleyb.nku@gmail.com>
Signed-off-by: Thomas Rast <trast@student.ethz.ch>
---
 line.c |   66 ++++++++++++++++++++++++++++++++++++++++++++++-----------------
 1 files changed, 48 insertions(+), 18 deletions(-)

diff --git a/line.c b/line.c
index 10870a5..1a9a947 100644
--- a/line.c
+++ b/line.c
@@ -1242,6 +1242,13 @@ static void flush_lines(struct diff_options *opt, const char **ptr, const char *
 	const char *p = *ptr;
 	struct strbuf buf = STRBUF_INIT;
 	const char *reset;
+	char *line_prefix = "";
+	struct strbuf *msgbuf;
+
+	if (opt && opt->output_prefix) {
+		msgbuf = opt->output_prefix(opt, opt->output_prefix_data);
+		line_prefix = msgbuf->buf;
+	}
 
 	if (*color)
 		reset = diff_get_color_opt(opt, DIFF_RESET);
@@ -1264,7 +1271,7 @@ static void flush_lines(struct diff_options *opt, const char **ptr, const char *
 
 	while (*ptr < end && *lno <= elno) {
 		if (**ptr == '\n') {
-			fprintf(opt->file, "%s", buf.buf);
+			fprintf(opt->file, "%s%s", line_prefix, buf.buf);
 			if (*ptr - p)
 				fwrite(p, *ptr - p, 1, opt->file);
 			fprintf(opt->file, "%s\n", reset);
@@ -1274,7 +1281,7 @@ static void flush_lines(struct diff_options *opt, const char **ptr, const char *
 		(*ptr)++;
 	}
 	if (*lno <= elno) {
-		fprintf(opt->file, "%s", buf.buf);
+		fprintf(opt->file, "%s%s", line_prefix, buf.buf);
 		if (*ptr - p)
 			fwrite(p, *ptr - p, 1, opt->file);
 		fprintf(opt->file, "%s\n", reset);
@@ -1316,8 +1323,15 @@ static void diff_flush_chunks(struct diff_options *opt, struct line_chunk *chunk
 	struct diff_line_range *range = chunk->range;
 	const char *set = diff_get_color_opt(opt, DIFF_FRAGINFO);
 	const char *reset = diff_get_color_opt(opt, DIFF_RESET);
+	char *line_prefix = "";
+	struct strbuf *msgbuf;
 	int i;
 
+	if (opt && opt->output_prefix) {
+		msgbuf = opt->output_prefix(opt, opt->output_prefix_data);
+		line_prefix = msgbuf->buf;
+	}
+
 	for (i = 0; i < range->nr; i++) {
 		struct line_range *r = range->ranges + i;
 		long lenp = r->pend - r->pstart + 1, pstart = r->pstart;
@@ -1325,8 +1339,8 @@ static void diff_flush_chunks(struct diff_options *opt, struct line_chunk *chunk
 		if (pstart == 0)
 			lenp = 0;
 
-		fprintf(opt->file, "%s@@ -%ld,%ld +%ld,%ld @@%s\n",
-			set, pstart, lenp, r->start, len, reset);
+		fprintf(opt->file, "%s%s@@ -%ld,%ld +%ld,%ld @@%s\n",
+			line_prefix, set, pstart, lenp, r->start, len, reset);
 
 		diff_flush_range(opt, chunk, r);
 	}
@@ -1345,6 +1359,13 @@ static void diff_flush_filepair(struct rev_info *rev, struct diff_line_range *ra
 	const char *reset = diff_get_color_opt(opt, DIFF_RESET);
 	struct line_chunk chunk;
 	int must_show_header;
+	char *line_prefix = "";
+	struct strbuf *msgbuf;
+
+	if (opt && opt->output_prefix) {
+		msgbuf = opt->output_prefix(opt, opt->output_prefix_data);
+		line_prefix = msgbuf->buf;
+	}
 
 	/*
 	 * the ranges that touch no different file, in this case
@@ -1378,21 +1399,26 @@ static void diff_flush_filepair(struct rev_info *rev, struct diff_line_range *ra
 	b_two = quote_two(b_prefix, name_b + (*name_b == '/'));
 	lbl[0] = DIFF_FILE_VALID(one) ? a_one : "/dev/null";
 	lbl[1] = DIFF_FILE_VALID(two) ? b_two : "/dev/null";
-	strbuf_addf(&header, "%sdiff --git %s %s%s\n", set, a_one, b_two, reset);
+	strbuf_addf(&header, "%s%sdiff --git %s %s%s\n", line_prefix,
+			set, a_one, b_two, reset);
 	if (lbl[0][0] == '/') {
-		strbuf_addf(&header, "%snew file mode %06o%s\n", set, two->mode, reset);
+		strbuf_addf(&header, "%s%snew file mode %06o%s\n",
+			line_prefix, set, two->mode, reset);
 	} else if (lbl[1][0] == '/') {
-		strbuf_addf(&header, "%sdeleted file mode %06o%s\n", set, one->mode, reset);
+		strbuf_addf(&header, "%s%sdeleted file mode %06o%s\n",
+			line_prefix, set, one->mode, reset);
 	} else if (one->mode != two->mode) {
-			strbuf_addf(&header, "%sold mode %06o%s\n", set, one->mode, reset);
-			strbuf_addf(&header, "%snew mode %06o%s\n", set, two->mode, reset);
+			strbuf_addf(&header, "%s%sold mode %06o%s\n",
+				line_prefix, set, one->mode, reset);
+			strbuf_addf(&header, "%s%snew mode %06o%s\n",
+				line_prefix, set, two->mode, reset);
 	}
 
 	fprintf(opt->file, "%s%s", header.buf, meta.buf);
 	strbuf_release(&meta);
 	strbuf_release(&header);
-	fprintf(opt->file, "%s--- %s%s\n", set, lbl[0], reset);
-	fprintf(opt->file, "%s+++ %s%s\n", set, lbl[1], reset);
+	fprintf(opt->file, "%s%s--- %s%s\n", line_prefix, set, lbl[0], reset);
+	fprintf(opt->file, "%s%s+++ %s%s\n", line_prefix, set, lbl[1], reset);
 	free((void *)a_one);
 	free((void *)b_two);
 
@@ -1446,12 +1472,13 @@ static void flush_nontrivial_merge(struct rev_info *rev,
 				meta, range->spec->path, reset);
 			for (; i < range->nr; i++) {
 				struct line_range *r = range->ranges + i;
-				fprintf(opt->file, "%s@@ %ld,%ld @@%s\n", frag, r->start,
+				fprintf(opt->file, "%s%s@@ %ld,%ld @@%s\n",
+					line_prefix, frag, r->start,
 					r->end - r->start + 1, reset);
 				flush_lines(opt, &ptr, end, r->start, r->end,
 					&lno, new, ' ');
 			}
-			fprintf(opt->file, "\n");
+			fprintf(opt->file, "%s\n", line_prefix);
 		}
 		range = range->next;
 	}
@@ -1464,6 +1491,8 @@ static void line_log_flush(struct rev_info *rev, struct commit *c)
 							&c->object);
 	struct log_info log;
 	struct diff_options *opt = &rev->diffopt;
+	char *line_prefix = "";
+	struct strbuf *msgbuf;
 
 	if (!range || !(c->object.flags & NONTRIVIAL_MERGE ||
 			c->object.flags & NEED_PRINT))
@@ -1476,11 +1505,12 @@ static void line_log_flush(struct rev_info *rev, struct commit *c)
 	rev->loginfo = &log;
 	show_log(rev);
 	rev->loginfo = NULL;
-	/*
-	 * Add a new line after each commit message, of course we should
-	 * add --graph alignment later when the patches comes to master.
-	 */
-	fprintf(rev->diffopt.file, "\n");
+
+	if (opt && opt->output_prefix) {
+		msgbuf = opt->output_prefix(opt, opt->output_prefix_data);
+		line_prefix = msgbuf->buf;
+	}
+	fprintf(rev->diffopt.file, "%s\n", line_prefix);
 
 	if (c->object.flags & NONTRIVIAL_MERGE)
 		flush_nontrivial_merge(rev, nontrivial);
-- 
1.7.3.3.807.g6ee1f

^ permalink raw reply related

* [PATCH v6.1 5/8] log -L: support parent rewriting
From: Thomas Rast @ 2010-12-14 22:54 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Bo Yang
In-Reply-To: <cover.1292366984.git.trast@student.ethz.ch>

From: Bo Yang <struggleyb.nku@gmail.com>

Walking forward through history (i.e., topologically earliest
commits first), we filter the parent list of every commit as
follows. Consider a parent P:
 - If P touches any of the interesting line ranges, we keep it.
 - If P is a merge and it takes all the interesting line ranges
   from one of its parents, P is rewritten to this parent, else
   we keep P.
 - Otherwise, P is rewritten to its (only) parent P^.

Signed-off-by: Bo Yang <struggleyb.nku@gmail.com>
Signed-off-by: Thomas Rast <trast@student.ethz.ch>
---
 line.c                            |  208 +++++++++++++++++++++++++++++++-----
 revision.c                        |    1 +
 revision.h                        |    5 +-
 t/t4302-log-line-merge-history.sh |  174 +++++++++++++++++++++++++++++++
 4 files changed, 357 insertions(+), 31 deletions(-)
 create mode 100755 t/t4302-log-line-merge-history.sh

diff --git a/line.c b/line.c
index 1ffcaba..10870a5 100644
--- a/line.c
+++ b/line.c
@@ -10,6 +10,7 @@
 #include "xdiff-interface.h"
 #include "strbuf.h"
 #include "log-tree.h"
+#include "graph.h"
 #include "line.h"
 
 struct print_range {
@@ -33,12 +34,6 @@ struct line_range {
 };
 
 /*
- * Eek-a-global
- */
-
-static int limited;
-
-/*
  * These could be in line.h but we put them here so the functions can
  * be static.
  */
@@ -596,14 +591,14 @@ static void add_line_range(struct rev_info *revs, struct commit *commit,
 {
 	struct diff_line_range *ret = NULL;
 
-	if (r) {
-		ret = lookup_decoration(&revs->line_range, &commit->object);
-		if (ret)
-			diff_line_range_merge(ret, r);
-		else
-			add_decoration(&revs->line_range, &commit->object, r);
+	ret = lookup_decoration(&revs->line_range, &commit->object);
+	if (ret && r)
+		diff_line_range_merge(ret, r);
+	else
+		add_decoration(&revs->line_range, &commit->object, r);
+
+	if (r)
 		commit->object.flags |= RANGE_UPDATE;
-	}
 }
 
 static void clear_commit_line_range(struct rev_info *revs, struct commit *commit)
@@ -687,6 +682,22 @@ void map_lines(long p_start, long p_end, long t_start, long t_end,
 		return;
 	}
 
+	if (start == t_start) {
+		*o_start = p_start;
+		*o_end = p_start + (end - start);
+		if (*o_end > p_end)
+			*o_end = p_end;
+		return;
+	}
+
+	if (end == t_end) {
+		*o_start = p_end - (end - start);
+		if (*o_start < p_start)
+			*o_start = p_start;
+		*o_end = p_end;
+		return;
+	}
+
 	/*
 	 * A heuristic for lines mapping:
 	 *
@@ -920,7 +931,7 @@ static void load_tree_desc(struct tree_desc *desc, void **tree,
  * to parents.
  * map_range_cb and map_range are used to map line ranges to the parent.
  */
-static void assign_range_to_parent(struct rev_info *rev, struct commit *commit,
+static int assign_range_to_parent(struct rev_info *rev, struct commit *commit,
 		struct commit *parent, struct diff_line_range *range,
 		struct diff_options *opt, int map)
 {
@@ -1065,10 +1076,22 @@ static void assign_range_to_parent(struct rev_info *rev, struct commit *commit,
 			prev_range->next = NULL;
 	}
 
+	if (!map)
+		goto out;
+
 	if (final_range) {
 		assert(parent);
 		assert(final_range->spec);
 		add_line_range(rev, parent, final_range);
+	} else {
+		/*
+		 * If there is no new ranges assigned to the parent,
+		 * we should mark it as a 'root' commit.
+		 */
+		if (commit->parents && !commit->parents->next) {
+			free(commit->parents);
+			commit->parents = NULL;
+		}
 	}
 
 	/* and the ranges of current commit is updated */
@@ -1076,10 +1099,13 @@ static void assign_range_to_parent(struct rev_info *rev, struct commit *commit,
 	if (diff)
 		commit->object.flags |= NEED_PRINT;
 
+out:
 	if (tree1)
 		free(tree1);
 	if (tree2)
 		free(tree2);
+
+	return diff;
 }
 
 static void diff_update_parent_range(struct rev_info *rev,
@@ -1096,13 +1122,21 @@ static void diff_update_parent_range(struct rev_info *rev,
 	assign_range_to_parent(rev, commit, c, r, &rev->diffopt, 1);
 }
 
+struct commit_state {
+	struct diff_line_range *range;
+	struct object obj;
+};
+
 static void assign_parents_range(struct rev_info *rev, struct commit *commit)
 {
 	struct commit_list *parents = commit->parents;
 	struct diff_line_range *r = lookup_line_range(rev, commit);
 	struct diff_line_range *evil = NULL, *range = NULL;
+	struct decoration parents_state;
+	struct commit_state *state = NULL;
 	int nontrivial = 0;
 
+	memset(&parents_state, 0, sizeof(parents_state));
 	/*
 	 * If we are in linear history, update range and flush the patch if
 	 * necessary
@@ -1119,23 +1153,76 @@ static void assign_parents_range(struct rev_info *rev, struct commit *commit)
 	parents = commit->parents;
 	while (parents) {
 		struct commit *p = parents->item;
-		assign_range_to_parent(rev, commit, p, r, &rev->diffopt, 1);
+		int diff = 0;
+		struct diff_line_range *origin_range = lookup_line_range(rev, p);
+		if (origin_range)
+			origin_range = diff_line_range_clone_deeply(origin_range);
+
+		state = xmalloc(sizeof(*state));
+		state->range = origin_range;
+		state->obj = p->object;
+		add_decoration(&parents_state, &p->object, state);
+		diff = assign_range_to_parent(rev, commit, p, r, &rev->diffopt, 1);
+		/* Since all the ranges comes from this parent, we can ignore others */
+		if (diff == 0) {
+			/* restore the state of parents before this one */
+			parents = commit->parents;
+			while (parents->item != p) {
+				struct commit_list *list = parents;
+				parents = parents->next;
+				clear_commit_line_range(rev, list->item);
+				state = lookup_decoration(&parents_state, &list->item->object);
+				add_decoration(&parents_state, &list->item->object, NULL);
+				add_line_range(rev, list->item, state->range);
+				list->item->object = state->obj;
+				free(state);
+				free(list);
+			}
+
+			commit->parents = parents;
+			parents = parents->next;
+			commit->parents->next = NULL;
+
+			/* free the non-use commit_list */
+			while (parents) {
+				struct commit_list *list = parents;
+				parents = parents->next;
+				free(list);
+			}
+			goto out;
+		}
+		/* take the ranges from 'commit', try to detect nontrivial merge */
 		assign_range_to_parent(rev, commit, p, evil, &rev->diffopt, 0);
 		parents = parents->next;
 	}
 
+	commit->object.flags |= NONTRIVIAL_MERGE;
 	/*
 	 * yes, this must be an evil merge.
 	 */
 	range = evil;
 	while (range) {
 		if (range->nr) {
-			commit->object.flags |= NEED_PRINT | EVIL_MERGE;
+			commit->object.flags |= EVIL_MERGE;
 			nontrivial = 1;
 		}
 		range = range->next;
 	}
 
+out:
+	/* Never print out any diff for a merge commit */
+	commit->object.flags &= ~NEED_PRINT;
+
+	parents = commit->parents;
+	while (parents) {
+		state = lookup_decoration(&parents_state, &parents->item->object);
+		if (state) {
+			free_diff_line_ranges(state->range);
+			free(state);
+		}
+		parents = parents->next;
+	}
+
 	if (nontrivial)
 		add_decoration(&rev->nontrivial_merge, &commit->object, evil);
 	else
@@ -1327,16 +1414,36 @@ static void flush_nontrivial_merge(struct rev_info *rev,
 	const char *frag = diff_get_color_opt(opt, DIFF_FRAGINFO);
 	const char *meta = diff_get_color_opt(opt, DIFF_METAINFO);
 	const char *new = diff_get_color_opt(opt, DIFF_FILE_NEW);
+	char *line_prefix = "";
+	struct strbuf *msgbuf;
+	int evil = 0;
+	struct diff_line_range *r = range;
+
+	if (opt && opt->output_prefix) {
+		msgbuf = opt->output_prefix(opt, opt->output_prefix_data);
+		line_prefix = msgbuf->buf;
+	}
+
+	while (r) {
+		if (r->nr)
+			evil = 1;
+		r = r->next;
+	}
+
+	if (!evil)
+		return;
 
-	fprintf(opt->file, "%s%s%s\n", meta, "nontrivial merge found", reset);
+	fprintf(opt->file, "%s%s%s%s\n", line_prefix, meta,
+		"nontrivial merge found", reset);
 
 	while (range) {
 		if (range->nr) {
 			int lno = 1;
 			const char *ptr = range->spec->data;
-			const char *end = range->spec->data + range->spec->size;
+			const char *end = (const char *)range->spec->data + range->spec->size;
 			int i = 0;
-			fprintf(opt->file, "%s%s%s\n\n", meta, range->spec->path, reset);
+			fprintf(opt->file, "%s%s%s%s\n", line_prefix,
+				meta, range->spec->path, reset);
 			for (; i < range->nr; i++) {
 				struct line_range *r = range->ranges + i;
 				fprintf(opt->file, "%s@@ %ld,%ld @@%s\n", frag, r->start,
@@ -1353,12 +1460,17 @@ static void flush_nontrivial_merge(struct rev_info *rev,
 static void line_log_flush(struct rev_info *rev, struct commit *c)
 {
 	struct diff_line_range *range = lookup_line_range(rev, c);
-	struct diff_line_range *nontrivial = lookup_decoration(&rev->nontrivial_merge, &c->object);
+	struct diff_line_range *nontrivial = lookup_decoration(&rev->nontrivial_merge,
+							&c->object);
 	struct log_info log;
+	struct diff_options *opt = &rev->diffopt;
 
-	if (!range)
+	if (!range || !(c->object.flags & NONTRIVIAL_MERGE ||
+			c->object.flags & NEED_PRINT))
 		return;
 
+	if (rev->graph)
+		graph_update(rev->graph, c);
 	log.commit = c;
 	log.parent = NULL;
 	rev->loginfo = &log;
@@ -1370,13 +1482,42 @@ static void line_log_flush(struct rev_info *rev, struct commit *c)
 	 */
 	fprintf(rev->diffopt.file, "\n");
 
-	if (c->object.flags & EVIL_MERGE)
-		return flush_nontrivial_merge(rev, nontrivial);
+	if (c->object.flags & NONTRIVIAL_MERGE)
+		flush_nontrivial_merge(rev, nontrivial);
+	else {
+		while (range) {
+			if (range->diff)
+				diff_flush_filepair(rev, range);
+			range = range->next;
+		}
+	}
 
-	while (range) {
-		if (range->diff)
-			diff_flush_filepair(rev, range);
-		range = range->next;
+	while (rev->graph && !graph_is_commit_finished(rev->graph)) {
+		struct strbuf sb;
+		strbuf_init(&sb, 0);
+		graph_next_line(rev->graph, &sb);
+		fputs(sb.buf, opt->file);
+	}
+}
+
+static enum rewrite_result rewrite_one(struct rev_info *rev, struct commit **pp)
+{
+	struct diff_line_range *r = NULL;
+	struct commit *p;
+	while (1) {
+		p = *pp;
+		if (p->object.flags & RANGE_UPDATE)
+			assign_parents_range(rev, p);
+		if (p->object.flags & NEED_PRINT ||
+		    p->object.flags & NONTRIVIAL_MERGE)
+			return rewrite_one_ok;
+		if (!p->parents)
+			return rewrite_one_noparents;
+
+		r = lookup_line_range(rev, p);
+		if (!r)
+			return rewrite_one_noparents;
+		*pp = p->parents->item;
 	}
 }
 
@@ -1401,7 +1542,8 @@ int line_log_walk(struct rev_info *rev)
 	}
 	/* Clear the flags */
 	while (list) {
-		list->item->object.flags &= ~(RANGE_UPDATE | EVIL_MERGE | NEED_PRINT);
+		list->item->object.flags &= ~(RANGE_UPDATE | NONTRIVIAL_MERGE |
+						NEED_PRINT | EVIL_MERGE);
 		list = list->next;
 	}
 
@@ -1413,7 +1555,15 @@ int line_log_walk(struct rev_info *rev)
 		if (commit->object.flags & RANGE_UPDATE)
 			assign_parents_range(rev, commit);
 
-		if (commit->object.flags & NEED_PRINT)
+		if (commit->object.flags & NEED_PRINT ||
+		    commit->object.flags & NONTRIVIAL_MERGE) {
+			if (rewrite_parents(rev, commit, rewrite_one))
+				die("Can't rewrite parent for commit %s",
+					sha1_to_hex(commit->object.sha1));
+		}
+
+		if (commit->object.flags & NEED_PRINT ||
+		    commit->object.flags & NONTRIVIAL_MERGE)
 			line_log_flush(rev, commit);
 
 		clear_commit_line_range(rev, commit);
diff --git a/revision.c b/revision.c
index 369ec56..fbebf2f 100644
--- a/revision.c
+++ b/revision.c
@@ -13,6 +13,7 @@
 #include "decorate.h"
 #include "log-tree.h"
 #include "string-list.h"
+#include "line.h"
 
 volatile show_early_output_fn_t show_early_output;
 
diff --git a/revision.h b/revision.h
index 585b15f..6100904 100644
--- a/revision.h
+++ b/revision.h
@@ -16,8 +16,9 @@
 #define SYMMETRIC_LEFT	(1u<<8)
 #define RANGE_UPDATE	(1u<<9) /* for line level traverse */
 #define NEED_PRINT	(1u<<10)
-#define EVIL_MERGE	(1u<<11)
-#define ALL_REV_FLAGS	((1u<<12)-1)
+#define NONTRIVIAL_MERGE	(1u<<11)
+#define EVIL_MERGE	(1u<<12)
+#define ALL_REV_FLAGS	((1u<<13)-1)
 
 #define DECORATE_SHORT_REFS	1
 #define DECORATE_FULL_REFS	2
diff --git a/t/t4302-log-line-merge-history.sh b/t/t4302-log-line-merge-history.sh
new file mode 100755
index 0000000..0c8d023
--- /dev/null
+++ b/t/t4302-log-line-merge-history.sh
@@ -0,0 +1,174 @@
+#!/bin/sh
+#
+# Copyright (c) 2010 Bo Yang
+#
+
+test_description='Test git log -L with merge commit'
+
+. ./test-lib.sh
+. "$TEST_DIRECTORY"/diff-lib.sh
+
+cat >path0 <<\EOF
+void func()
+{
+	printf("hello");
+}
+EOF
+
+test_expect_success 'Add path0 and commit.' '
+	git add path0 &&
+	git commit -m "Base commit"
+'
+
+cat >path0 <<\EOF
+void func()
+{
+	printf("hello earth");
+}
+EOF
+
+test_expect_success 'Change path0 in master.' '
+	git add path0 &&
+	git commit -m "Change path0 in master"
+'
+
+test_expect_success 'Make a new branch from the base commit' '
+	git checkout -b feature master^
+'
+
+cat >path0 <<\EOF
+void func()
+{
+	print("hello moon");
+}
+EOF
+
+test_expect_success 'Change path0 in feature.' '
+	git add path0 &&
+	git commit -m "Change path0 in feature"
+'
+
+test_expect_success 'Merge the master to feature' '
+	! git merge master
+'
+
+cat >path0 <<\EOF
+void func()
+{
+	printf("hello earth and moon");
+}
+EOF
+
+test_expect_success 'Resolve the conflict' '
+	git add path0 &&
+	git commit -m "Merge two branches"
+'
+
+test_expect_success 'Show the line level log of path0' '
+	git log --pretty=format:%s%n%b -L /func/,/^}/:path0 > current
+'
+
+sed 's/Q/ /g' >expected <<\EOF
+Merge two branches
+
+nontrivial merge found
+path0
+@@ 3,1 @@
+Q	printf("hello earth and moon");
+
+
+Change path0 in master
+
+diff --git a/path0 b/path0
+index 56aeee5..11e66c5 100644
+--- a/path0
++++ b/path0
+@@ -1,4 +1,4 @@
+ void func()
+ {
+-	printf("hello");
++	printf("hello earth");
+ }
+
+Change path0 in feature
+
+diff --git a/path0 b/path0
+index 56aeee5..258fced 100644
+--- a/path0
++++ b/path0
+@@ -1,4 +1,4 @@
+ void func()
+ {
+-	printf("hello");
++	print("hello moon");
+ }
+
+Base commit
+
+diff --git a/path0 b/path0
+new file mode 100644
+index 0000000..56aeee5
+--- /dev/null
++++ b/path0
+@@ -0,0 +1,4 @@
++void func()
++{
++	printf("hello");
++}
+EOF
+
+sed -e 's/Q/ /g' -e 's/#$//' > expected-graph <<\EOF
+*   Merge two branches
+|\  #
+| | #
+| | nontrivial merge found
+| | path0
+| | @@ 3,1 @@
+| |QQ	printf("hello earth and moon");
+| | #
+| |   #
+| * Change path0 in master
+| | #
+| | diff --git a/path0 b/path0
+| | index 56aeee5..11e66c5 100644
+| | --- a/path0
+| | +++ b/path0
+| | @@ -3,1 +3,1 @@
+| | -	printf("hello");
+| | +	printf("hello earth");
+| |   #
+* | Change path0 in feature
+|/  #
+|   #
+|   diff --git a/path0 b/path0
+|   index 56aeee5..258fced 100644
+|   --- a/path0
+|   +++ b/path0
+|   @@ -3,1 +3,1 @@
+|   -	printf("hello");
+|   +	print("hello moon");
+|  #
+* Base commit
+  #
+  diff --git a/path0 b/path0
+  new file mode 100644
+  index 0000000..56aeee5
+  --- /dev/null
+  +++ b/path0
+  @@ -0,0 +3,1 @@
+  +	printf("hello");
+EOF
+
+test_expect_success 'Show the line log of the 2 line of path0 with graph' '
+	git log --pretty=format:%s%n%b --graph -L 3,+1:path0 > current-graph
+'
+
+test_expect_success 'validate the output.' '
+	test_cmp current expected
+'
+
+test_expect_success 'validate the graph output.' '
+	test_cmp current-graph expected-graph
+'
+
+test_done
-- 
1.7.3.3.807.g6ee1f

^ permalink raw reply related

* [PATCH v6.1 0/8] git log -L, cleaned up and (hopefully) fixed
From: Thomas Rast @ 2010-12-14 22:54 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano
In-Reply-To: <7vhbegroj2.fsf@alter.siamese.dyndns.org>

Junio C Hamano wrote:
> 
> This patch set has too many whitespace errors, and worse yet, cannot be
> applied with "git am --whitespace=fix" because it has test vectors that
> depend on having "SP followed by HT" (context lines of diff output) and
> traling SP (graph output).
> 
> Could you please straighten that out first?
> 
> My preference is to protect offending whitespaces like this:
> 
> 	sed -e 's/Q/ /g' -e 's/Z$//' >expected-no-M <<\EOF
[...]
> which has an added benefit of making them more visible.

Sorry for the mess.  I stuck to the s/#$// style I already had in some
places for now, but used the Qs and ran the rest through
whitespace=fix.  It now applies cleanly to master.


Bo Yang (8):
  Refactor parse_loc
  Export three functions from diff.c
  Export rewrite_parents() for 'log -L'
  Implement line-history search (git log -L)
  log -L: support parent rewriting
  log -L: add --graph prefix before output
  log -L: add --full-line-diff option
  log -L: implement move/copy detection (-M/-C)

 Documentation/blame-options.txt     |   19 +-
 Documentation/git-log.txt           |   22 +
 Documentation/line-range-format.txt |   18 +
 Makefile                            |    2 +
 builtin/blame.c                     |   99 +--
 builtin/log.c                       |   79 ++-
 diff.c                              |    6 +-
 diff.h                              |   17 +
 line.c                              | 2153 +++++++++++++++++++++++++++++++++++
 line.h                              |   72 ++
 revision.c                          |   22 +-
 revision.h                          |   23 +-
 t/t4301-log-line-single-history.sh  |  685 +++++++++++
 t/t4302-log-line-merge-history.sh   |  174 +++
 t/t4303-log-line-move-detect.sh     |  238 ++++
 t/t4304-log-line-copy-detect.sh     |  220 ++++
 t/t8003-blame-corner-cases.sh       |    6 +
 17 files changed, 3730 insertions(+), 125 deletions(-)
 create mode 100644 Documentation/line-range-format.txt
 create mode 100644 line.c
 create mode 100644 line.h
 create mode 100755 t/t4301-log-line-single-history.sh
 create mode 100755 t/t4302-log-line-merge-history.sh
 create mode 100755 t/t4303-log-line-move-detect.sh
 create mode 100755 t/t4304-log-line-copy-detect.sh

-- 
1.7.3.3.807.g6ee1f

^ permalink raw reply

* Re: [PATCH v3 5/5] mingw_rmdir: set errno=ENOTEMPTY when appropriate
From: Erik Faye-Lund @ 2010-12-14 22:49 UTC (permalink / raw)
  To: Heiko Voigt
  Cc: Johannes Sixt, Pat Thoyts, msysgit, git, Junio C Hamano,
	Johannes Schindelin
In-Reply-To: <20101214222830.GF4084@sandbox>

On Tue, Dec 14, 2010 at 11:28 PM, Heiko Voigt <hvoigt@hvoigt.net> wrote:
> From: Johannes Schindelin <johannes.schindelin@gmx.de>
>
> On Windows, EACCES overrules ENOTEMPTY when calling rmdir(). But if the
> directory is busy, we only want to retry deleting the directory if it
> is empty, so test specifically for that case and set ENOTEMPTY rather
> than EACCES.
>

Hmm... According to MSDN, rmdir(*) should already handle ENOTEMPTY.
Isn't the problem rather the structure of that loop? Shouldn't it be
sufficient to do something like this (note: untested, but the concept
should work, no)?

---8<---
#undef rmdir
int mingw_rmdir(const char *pathname)
{
       int ret, tries;

       for (tries = 0; tries < ARRAY_SIZE(delay); ++tries) {
               if (!(ret = rmdir(pathname)) || errno == ENOTEMPTY ||
                   !is_file_in_use_error(GetLastError()))
                      return ret;

               /*
                * We assume that some other process had the source or
                * destination file open at the wrong moment and retry.
                * In order to give the other process a higher chance to
                * complete its operation, we give up our time slice now.
                * If we have to retry again, we do sleep a bit.
                */
               Sleep(delay[tries]);
       }
       while (is_file_in_use_error(GetLastError()) &&
              ask_user_yes_no("Deletion of directory '%s' failed. "
                       "Should I try again?", pathname))
              ret = rmdir(pathname);
       return ret;
}
---8<---

(*) http://msdn.microsoft.com/en-us/library/wt8es881(v=VS.80).aspx

^ permalink raw reply

* [PATCH v4 2/5] mingw: work around irregular failures of unlink on windows
From: Heiko Voigt @ 2010-12-14 22:44 UTC (permalink / raw)
  To: Erik Faye-Lund; +Cc: Johannes Sixt, Pat Thoyts, msysgit, git, Junio C Hamano
In-Reply-To: <20101214223145.GG4084@sandbox>

If a file is opened by another process (e.g. indexing of an IDE) for
reading it is not allowed to be deleted. So in case unlink fails retry
after waiting for some time. This extends the workaround from 6ac6f878.

Signed-off-by: Heiko Voigt <hvoigt@hvoigt.net>
---
Here is a replacement for the previous patch.

 compat/mingw.c |   31 +++++++++++++++++++++++++++++--
 1 files changed, 29 insertions(+), 2 deletions(-)

diff --git a/compat/mingw.c b/compat/mingw.c
index a7e1c6b..4a1c218 100644
--- a/compat/mingw.c
+++ b/compat/mingw.c
@@ -3,6 +3,8 @@
 #include <conio.h>
 #include "../strbuf.h"
 
+static const int delay[] = { 0, 1, 10, 20, 40 };
+
 int err_win_to_posix(DWORD winerr)
 {
 	int error = ENOSYS;
@@ -116,12 +118,38 @@ int err_win_to_posix(DWORD winerr)
 	return error;
 }
 
+static inline int is_file_in_use_error(DWORD errcode)
+{
+	switch(errcode) {
+	case ERROR_SHARING_VIOLATION:
+	case ERROR_ACCESS_DENIED:
+		return 1;
+	}
+
+	return 0;
+}
+
 #undef unlink
 int mingw_unlink(const char *pathname)
 {
+	int ret, tries = 0;
+
 	/* read-only files cannot be removed */
 	chmod(pathname, 0666);
-	return unlink(pathname);
+	while ((ret = unlink(pathname)) == -1 && tries < ARRAY_SIZE(delay)) {
+		if (!is_file_in_use_error(GetLastError()))
+			break;
+		/*
+		 * We assume that some other process had the source or
+		 * destination file open at the wrong moment and retry.
+		 * In order to give the other process a higher chance to
+		 * complete its operation, we give up our time slice now.
+		 * If we have to retry again, we do sleep a bit.
+		 */
+		Sleep(delay[tries]);
+		tries++;
+	}
+	return ret;
 }
 
 #undef open
@@ -1257,7 +1285,6 @@ int mingw_rename(const char *pold, const char *pnew)
 {
 	DWORD attrs, gle;
 	int tries = 0;
-	static const int delay[] = { 0, 1, 10, 20, 40 };
 
 	/*
 	 * Try native rename() first to get errno right.
-- 
1.7.3.3.566.g98577

^ permalink raw reply related

* subdirectory-filter does not delete files before the directory came into existence?
From: Jan Wielemaker @ 2010-12-14 22:21 UTC (permalink / raw)
  To: git

Hi,

There is a lot of information about extracting a directory from a git
project.  One thing I failed to find though is the following:

I try to extract a directory.  The result is fine, but there is a lot
of history in the result from *before* the directory was added to the
project.  Why?  How can I get rid of this?

If you want to see yourself, I did:

	git clone git://www.swi-prolog.org/home/pl/git/pl-devel.git
	git clone pl-devel odbc
	cd odbc
	git filter-branch --subdirectory-filter packages/odbc --prune-empty
--tag-name-filter cat -- --all
	
Now use e.g. qgit to look at the history.  As from 03/07/2002, when
the packages/odbc directory was created, all looks just fine.  Before
though ...

	Thanks for any hints

		--- Jan

^ permalink raw reply

* Re: [PATCH v3 3/8] mingw: make failures to unlink or move raise a question
From: Erik Faye-Lund @ 2010-12-14 22:35 UTC (permalink / raw)
  To: Heiko Voigt
  Cc: Johannes Sixt, Pat Thoyts, msysgit, git, Junio C Hamano,
	Albert Dvornik, Johannes Schindelin
In-Reply-To: <20101214222122.GD4084@sandbox>

On Tue, Dec 14, 2010 at 11:21 PM, Heiko Voigt <hvoigt@hvoigt.net> wrote:
> On Windows in case a program is accessing a file unlink or
> move operations may fail. To give the user a chance to correct
> this we simply wait until the user asks us to retry or fail.
>
> This is useful because of the following use case which seem
> to happen rarely but when it does it is a mess:
>
> After making some changes the user realizes that he was on the
> incorrect branch. When trying to change the branch some file
> is still in use by some other process and git stops in the
> middle of changing branches. Now the user has lots of files
> with changes mixed with his own. This is especially confusing
> on repositories that contain lots of files.
>
> Although the recent implementation of automatic retry makes
> this scenario much more unlikely lets provide a fallback as
> a last resort.
>
> Thanks to Albert Dvornik for disabling the question if users can't see it.
>
> If the stdout of the command is connected to a terminal but the stderr
> has been redirected, the odds are good that the user can't see any
> question we print out to stderr.  This will result in a "mysterious
> hang" while the app is waiting for user input.
>
> It seems better to be conservative, and avoid asking for input
> whenever the stderr is not a terminal, just like we do for stdin.
>
> Signed-off-by: Heiko Voigt <hvoigt@hvoigt.net>
> Signed-off-by: Albert Dvornik <dvornik+git@gmail.com>
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
> I have added the sign-off from the squashed commit of Albert and
> Johannes. I hope its ok this way.
>
>  compat/mingw.c |   82 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 files changed, 82 insertions(+), 0 deletions(-)
>
> diff --git a/compat/mingw.c b/compat/mingw.c
> index 52183a7..ac9fb4a 100644
> --- a/compat/mingw.c
> +++ b/compat/mingw.c
> @@ -2,6 +2,7 @@
>  #include "win32.h"
>  #include <conio.h>
>  #include "../strbuf.h"
> +#include "../run-command.h"
>
>  static const int delay[] = { 0, 1, 10, 20, 40 };
>
> @@ -129,6 +130,78 @@ static inline int is_file_in_use_error(DWORD errcode)
>        return 0;
>  }
>
> +static int read_yes_no_answer()

Perhaps "static int read_yes_no_answer(void)" for portability?

> +{
> +       char answer[1024];
> +
> +       if (fgets(answer, sizeof(answer), stdin)) {
> +               size_t answer_len = strlen(answer);
> +               int got_full_line = 0, c;
> +
> +               /* remove the newline */
> +               if (answer_len >= 2 && answer[answer_len-2] == '\r') {
> +                       answer[answer_len-2] = '\0';
> +                       got_full_line = 1;
> +               }
> +               else if (answer_len >= 1 && answer[answer_len-1] == '\n') {
> +                       answer[answer_len-1] = '\0';
> +                       got_full_line = 1;
> +               }
> +               /* flush the buffer in case we did not get the full line */
> +               if (!got_full_line)
> +                       while((c = getchar()) != EOF && c != '\n');
> +       } else
> +               /* we could not read, return the
> +                * default answer which is no */
> +               return 0;
> +
> +       if (answer[0] == 'y' && strlen(answer) == 1)
> +               return 1;
> +       if (!strncasecmp(answer, "yes", sizeof(answer)))
> +               return 1;
> +       if (answer[0] == 'n' && strlen(answer) == 1)
> +               return 0;
> +       if (!strncasecmp(answer, "no", sizeof(answer)))
> +               return 0;

Since you're doing case insensitive checks for "yes" and "no", perhaps
it'd make sense to allow upper case 'Y' and 'N' also? Something like:

-       if (answer[0] == 'n' && strlen(answer) == 1)
+       if (tolower(answer[0]) == 'n' && strlen(answer) == 1)

hm?


> +static int ask_user_yes_no(const char *format, ...)
> +{
> +       char question[4096];
> +       const char *retry_hook[] = { NULL, NULL, NULL };
> +       va_list args;
> +
> +       if ((retry_hook[0] = getenv("GIT_ASK_YESNO"))) {
> +
> +               va_start(args, format);
> +               vsnprintf(question, sizeof(question), format, args);
> +               va_end(args);
> +
> +               retry_hook[1] = question;
> +               return !run_command_v_opt(retry_hook, 0);
> +       }
> +
> +       if (!isatty(_fileno(stdin)) || !isatty(_fileno(stderr)))
> +               return 0;

I'm wondering, doesn't this make the semantics a bit wrong? The
function is called "ask_user_yes_no", but it might end up not asking
after all. Perhaps it should be called something that reflects this?
"maybe_ask_yes_no", "ask_yes_no_if_tty", "should_retry"? I don't have
a non-ugly suggestion, but I suspect something like that might leave
other people less puzzled when reading the code.

^ permalink raw reply

* Re: Re: [PATCH v3 2/8] mingw: work around irregular failures of unlink on windows
From: Heiko Voigt @ 2010-12-14 22:31 UTC (permalink / raw)
  To: Erik Faye-Lund; +Cc: Johannes Sixt, Pat Thoyts, msysgit, git, Junio C Hamano
In-Reply-To: <AANLkTiksyVETuR8ftTojf=T2AhKGFtUAmqyj5u-00kW0@mail.gmail.com>

On Tue, Dec 14, 2010 at 11:14:13PM +0100, Erik Faye-Lund wrote:
> On Tue, Dec 14, 2010 at 11:11 PM, Heiko Voigt <hvoigt@hvoigt.net> wrote:
> > +static inline int is_file_in_use_error(DWORD errcode)
> > +{
> > +       switch(GetLastError()) {
> 
> Why pass the error code in, just to ignore it? Shouldn't this switch
> on "errcode" instead?

Definitely. Thanks for spotting that.

Cheers Heiko

^ permalink raw reply

* [PATCH v3 5/5] mingw_rmdir: set errno=ENOTEMPTY when appropriate
From: Heiko Voigt @ 2010-12-14 22:28 UTC (permalink / raw)
  To: Johannes Sixt
  Cc: Pat Thoyts, msysgit, git, Junio C Hamano, Johannes Schindelin
In-Reply-To: <20101214220604.GA4084@sandbox>

From: Johannes Schindelin <johannes.schindelin@gmx.de>

On Windows, EACCES overrules ENOTEMPTY when calling rmdir(). But if the
directory is busy, we only want to retry deleting the directory if it
is empty, so test specifically for that case and set ENOTEMPTY rather
than EACCES.

Noticed by Greg Hazel.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Heiko Voigt <hvoigt@hvoigt.net>
---
 compat/mingw.c |   28 ++++++++++++++++++++++++++++
 1 files changed, 28 insertions(+), 0 deletions(-)

diff --git a/compat/mingw.c b/compat/mingw.c
index b920644..3e013c6 100644
--- a/compat/mingw.c
+++ b/compat/mingw.c
@@ -229,6 +229,30 @@ int mingw_unlink(const char *pathname)
 	return ret;
 }
 
+static int is_dir_empty(const char *path)
+{
+	struct strbuf buf = STRBUF_INIT;
+	WIN32_FIND_DATAA findbuf;
+	HANDLE handle;
+
+	strbuf_addf(&buf, "%s\\*", path);
+	handle = FindFirstFileA(buf.buf, &findbuf);
+	if (handle == INVALID_HANDLE_VALUE) {
+		strbuf_release(&buf);
+		return GetLastError() == ERROR_NO_MORE_FILES;
+	}
+
+	while (!strcmp(findbuf.cFileName, ".") ||
+			!strcmp(findbuf.cFileName, ".."))
+		if (!FindNextFile(handle, &findbuf)) {
+			strbuf_release(&buf);
+			return GetLastError() == ERROR_NO_MORE_FILES;
+		}
+	FindClose(handle);
+	strbuf_release(&buf);
+	return 0;
+}
+
 #undef rmdir
 int mingw_rmdir(const char *pathname)
 {
@@ -237,6 +261,10 @@ int mingw_rmdir(const char *pathname)
 	while ((ret = rmdir(pathname)) == -1 && tries < ARRAY_SIZE(delay)) {
 		if (!is_file_in_use_error(GetLastError()))
 			break;
+		if (!is_dir_empty(pathname)) {
+			errno = ENOTEMPTY;
+			break;
+		}
 		/*
 		 * We assume that some other process had the source or
 		 * destination file open at the wrong moment and retry.
-- 
1.7.3.3.566.gf422f

^ permalink raw reply related

* [PATCH v3 4/5] mingw: add fallback for rmdir in case directory is in use
From: Heiko Voigt @ 2010-12-14 22:25 UTC (permalink / raw)
  To: Johannes Sixt; +Cc: Pat Thoyts, msysgit, git, Junio C Hamano
In-Reply-To: <20101214220604.GA4084@sandbox>

The same logic as for unlink and rename also applies to rmdir. For
example in case you have a shell open in a git controlled folder. This
will easily fail. So lets be nice for such cases as well.

Signed-off-by: Heiko Voigt <heiko.voigt@mahr.de>
---
I just realized that there was a wrong patch count in the subject. It
should have been 5 instead of 8 all the time.

 compat/mingw.c |   25 +++++++++++++++++++++++++
 compat/mingw.h |    3 +++
 2 files changed, 28 insertions(+), 0 deletions(-)

diff --git a/compat/mingw.c b/compat/mingw.c
index ac9fb4a..b920644 100644
--- a/compat/mingw.c
+++ b/compat/mingw.c
@@ -229,6 +229,31 @@ int mingw_unlink(const char *pathname)
 	return ret;
 }
 
+#undef rmdir
+int mingw_rmdir(const char *pathname)
+{
+	int ret, tries = 0;
+
+	while ((ret = rmdir(pathname)) == -1 && tries < ARRAY_SIZE(delay)) {
+		if (!is_file_in_use_error(GetLastError()))
+			break;
+		/*
+		 * We assume that some other process had the source or
+		 * destination file open at the wrong moment and retry.
+		 * In order to give the other process a higher chance to
+		 * complete its operation, we give up our time slice now.
+		 * If we have to retry again, we do sleep a bit.
+		 */
+		Sleep(delay[tries]);
+		tries++;
+	}
+	while (ret == -1 && is_file_in_use_error(GetLastError()) &&
+	       ask_user_yes_no("Deletion of directory '%s' failed. "
+			"Should I try again?", pathname))
+	       ret = rmdir(pathname);
+	return ret;
+}
+
 #undef open
 int mingw_open (const char *filename, int oflags, ...)
 {
diff --git a/compat/mingw.h b/compat/mingw.h
index 8316938..8b159c4 100644
--- a/compat/mingw.h
+++ b/compat/mingw.h
@@ -175,6 +175,9 @@ int link(const char *oldpath, const char *newpath);
 int mingw_unlink(const char *pathname);
 #define unlink mingw_unlink
 
+int mingw_rmdir(const char *path);
+#define rmdir mingw_rmdir
+
 int mingw_open (const char *filename, int oflags, ...);
 #define open mingw_open
 
-- 
1.7.3.3.566.gf422f

^ permalink raw reply related

* Re: Can `add --patch` display the diff in my difftool instead of just printing it on the command line?
From: Dun Peal @ 2010-12-14 22:22 UTC (permalink / raw)
  To: Jeff King; +Cc: Git ML
In-Reply-To: <20101214213810.GB2216@sigill.intra.peff.net>

On Tue, Dec 14, 2010 at 3:38 PM, Jeff King <peff@peff.net> wrote:
> You can't do this with "git add -p" now, but I suspect the patch would
> be relatively straightforward. See patch_update_file in
> git-add--interactive.perl. You just need to replace:
>
>  for (@{$head->{DISPLAY}}) {
>    print;
>  }
>
> with code to dump the diff in @{$head->{TEXT}} either to stdin of your
> display program or to a tempfile that your program operates on.
> Conditional on having some config option to specify your program, of
> course.

That's exactly what I want, thanks. Seems like there should be a flag
/ config that tells `add --interactive` to render the diffs through a
difftool. I know several Git users who always use their difftools for
large, complex patches, which are exactly the kind of patches you'd
manage with --interactive / --patch. Hope one of you Git bigwigs takes
the time to add that feature :-)

.D

^ permalink raw reply

* [PATCH v3 3/8] mingw: make failures to unlink or move raise a question
From: Heiko Voigt @ 2010-12-14 22:21 UTC (permalink / raw)
  To: Johannes Sixt
  Cc: Pat Thoyts, msysgit, git, Junio C Hamano, Albert Dvornik,
	Johannes Schindelin
In-Reply-To: <20101214220604.GA4084@sandbox>

On Windows in case a program is accessing a file unlink or
move operations may fail. To give the user a chance to correct
this we simply wait until the user asks us to retry or fail.

This is useful because of the following use case which seem
to happen rarely but when it does it is a mess:

After making some changes the user realizes that he was on the
incorrect branch. When trying to change the branch some file
is still in use by some other process and git stops in the
middle of changing branches. Now the user has lots of files
with changes mixed with his own. This is especially confusing
on repositories that contain lots of files.

Although the recent implementation of automatic retry makes
this scenario much more unlikely lets provide a fallback as
a last resort.

Thanks to Albert Dvornik for disabling the question if users can't see it.

If the stdout of the command is connected to a terminal but the stderr
has been redirected, the odds are good that the user can't see any
question we print out to stderr.  This will result in a "mysterious
hang" while the app is waiting for user input.

It seems better to be conservative, and avoid asking for input
whenever the stderr is not a terminal, just like we do for stdin.

Signed-off-by: Heiko Voigt <hvoigt@hvoigt.net>
Signed-off-by: Albert Dvornik <dvornik+git@gmail.com>
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
I have added the sign-off from the squashed commit of Albert and
Johannes. I hope its ok this way.

 compat/mingw.c |   82 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 82 insertions(+), 0 deletions(-)

diff --git a/compat/mingw.c b/compat/mingw.c
index 52183a7..ac9fb4a 100644
--- a/compat/mingw.c
+++ b/compat/mingw.c
@@ -2,6 +2,7 @@
 #include "win32.h"
 #include <conio.h>
 #include "../strbuf.h"
+#include "../run-command.h"
 
 static const int delay[] = { 0, 1, 10, 20, 40 };
 
@@ -129,6 +130,78 @@ static inline int is_file_in_use_error(DWORD errcode)
 	return 0;
 }
 
+static int read_yes_no_answer()
+{
+	char answer[1024];
+
+	if (fgets(answer, sizeof(answer), stdin)) {
+		size_t answer_len = strlen(answer);
+		int got_full_line = 0, c;
+
+		/* remove the newline */
+		if (answer_len >= 2 && answer[answer_len-2] == '\r') {
+			answer[answer_len-2] = '\0';
+			got_full_line = 1;
+		}
+		else if (answer_len >= 1 && answer[answer_len-1] == '\n') {
+			answer[answer_len-1] = '\0';
+			got_full_line = 1;
+		}
+		/* flush the buffer in case we did not get the full line */
+		if (!got_full_line)
+			while((c = getchar()) != EOF && c != '\n');
+	} else
+		/* we could not read, return the
+		 * default answer which is no */
+		return 0;
+
+	if (answer[0] == 'y' && strlen(answer) == 1)
+		return 1;
+	if (!strncasecmp(answer, "yes", sizeof(answer)))
+		return 1;
+	if (answer[0] == 'n' && strlen(answer) == 1)
+		return 0;
+	if (!strncasecmp(answer, "no", sizeof(answer)))
+		return 0;
+
+	/* did not find an answer we understand */
+	return -1;
+}
+
+static int ask_user_yes_no(const char *format, ...)
+{
+	char question[4096];
+	const char *retry_hook[] = { NULL, NULL, NULL };
+	va_list args;
+
+	if ((retry_hook[0] = getenv("GIT_ASK_YESNO"))) {
+
+		va_start(args, format);
+		vsnprintf(question, sizeof(question), format, args);
+		va_end(args);
+
+		retry_hook[1] = question;
+		return !run_command_v_opt(retry_hook, 0);
+	}
+
+	if (!isatty(_fileno(stdin)) || !isatty(_fileno(stderr)))
+		return 0;
+
+	while (1) {
+		int answer;
+		va_start(args, format);
+		vfprintf(stderr, format, args);
+		va_end(args);
+		fprintf(stderr, " (y/n)? ");
+
+		if ((answer = read_yes_no_answer()) >= 0)
+			return answer;
+
+		fprintf(stderr, "Sorry, I did not understand your answer. "
+				"Please type 'y' or 'n'\n");
+	}
+}
+
 #undef unlink
 int mingw_unlink(const char *pathname)
 {
@@ -149,6 +222,10 @@ int mingw_unlink(const char *pathname)
 		Sleep(delay[tries]);
 		tries++;
 	}
+	while (ret == -1 && is_file_in_use_error(GetLastError()) &&
+	       ask_user_yes_no("Unlink of file '%s' failed. "
+			"Should I try again?", pathname))
+	       ret = unlink(pathname);
 	return ret;
 }
 
@@ -1326,6 +1403,11 @@ repeat:
 		tries++;
 		goto repeat;
 	}
+	if (gle == ERROR_ACCESS_DENIED &&
+	       ask_user_yes_no("Rename from '%s' to '%s' failed. "
+		       "Should I try again?", pold, pnew))
+		goto repeat;
+
 	errno = EACCES;
 	return -1;
 }
-- 
1.7.3.3.566.gf422f

^ permalink raw reply related

* Re: [PATCH v3 2/8] mingw: work around irregular failures of unlink on windows
From: Erik Faye-Lund @ 2010-12-14 22:14 UTC (permalink / raw)
  To: Heiko Voigt; +Cc: Johannes Sixt, Pat Thoyts, msysgit, git, Junio C Hamano
In-Reply-To: <20101214221134.GC4084@sandbox>

On Tue, Dec 14, 2010 at 11:11 PM, Heiko Voigt <hvoigt@hvoigt.net> wrote:
> If a file is opened by another process (e.g. indexing of an IDE) for
> reading it is not allowed to be deleted. So in case unlink fails retry
> after waiting for some time. This extends the workaround from 6ac6f878.
>
> Signed-off-by: Heiko Voigt <hvoigt@hvoigt.net>
> ---
>  compat/mingw.c |   31 +++++++++++++++++++++++++++++--
>  1 files changed, 29 insertions(+), 2 deletions(-)
>
> diff --git a/compat/mingw.c b/compat/mingw.c
> index a7e1c6b..52183a7 100644
> --- a/compat/mingw.c
> +++ b/compat/mingw.c
> @@ -3,6 +3,8 @@
>  #include <conio.h>
>  #include "../strbuf.h"
>
> +static const int delay[] = { 0, 1, 10, 20, 40 };
> +
>  int err_win_to_posix(DWORD winerr)
>  {
>        int error = ENOSYS;
> @@ -116,12 +118,38 @@ int err_win_to_posix(DWORD winerr)
>        return error;
>  }
>
> +static inline int is_file_in_use_error(DWORD errcode)
> +{
> +       switch(GetLastError()) {

Why pass the error code in, just to ignore it? Shouldn't this switch
on "errcode" instead?

^ permalink raw reply

* [PATCH v3 2/8] mingw: work around irregular failures of unlink on windows
From: Heiko Voigt @ 2010-12-14 22:11 UTC (permalink / raw)
  To: Johannes Sixt; +Cc: Pat Thoyts, msysgit, git, Junio C Hamano
In-Reply-To: <20101214220604.GA4084@sandbox>

If a file is opened by another process (e.g. indexing of an IDE) for
reading it is not allowed to be deleted. So in case unlink fails retry
after waiting for some time. This extends the workaround from 6ac6f878.

Signed-off-by: Heiko Voigt <hvoigt@hvoigt.net>
---
 compat/mingw.c |   31 +++++++++++++++++++++++++++++--
 1 files changed, 29 insertions(+), 2 deletions(-)

diff --git a/compat/mingw.c b/compat/mingw.c
index a7e1c6b..52183a7 100644
--- a/compat/mingw.c
+++ b/compat/mingw.c
@@ -3,6 +3,8 @@
 #include <conio.h>
 #include "../strbuf.h"
 
+static const int delay[] = { 0, 1, 10, 20, 40 };
+
 int err_win_to_posix(DWORD winerr)
 {
 	int error = ENOSYS;
@@ -116,12 +118,38 @@ int err_win_to_posix(DWORD winerr)
 	return error;
 }
 
+static inline int is_file_in_use_error(DWORD errcode)
+{
+	switch(GetLastError()) {
+	case ERROR_SHARING_VIOLATION:
+	case ERROR_ACCESS_DENIED:
+		return 1;
+	}
+
+	return 0;
+}
+
 #undef unlink
 int mingw_unlink(const char *pathname)
 {
+	int ret, tries = 0;
+
 	/* read-only files cannot be removed */
 	chmod(pathname, 0666);
-	return unlink(pathname);
+	while ((ret = unlink(pathname)) == -1 && tries < ARRAY_SIZE(delay)) {
+		if (!is_file_in_use_error(GetLastError()))
+			break;
+		/*
+		 * We assume that some other process had the source or
+		 * destination file open at the wrong moment and retry.
+		 * In order to give the other process a higher chance to
+		 * complete its operation, we give up our time slice now.
+		 * If we have to retry again, we do sleep a bit.
+		 */
+		Sleep(delay[tries]);
+		tries++;
+	}
+	return ret;
 }
 
 #undef open
@@ -1257,7 +1285,6 @@ int mingw_rename(const char *pold, const char *pnew)
 {
 	DWORD attrs, gle;
 	int tries = 0;
-	static const int delay[] = { 0, 1, 10, 20, 40 };
 
 	/*
 	 * Try native rename() first to get errno right.
-- 
1.7.3.3.566.gf422f

^ permalink raw reply related

* [PATCH v3 1/8] mingw: move unlink wrapper to mingw.c
From: Heiko Voigt @ 2010-12-14 22:09 UTC (permalink / raw)
  To: Johannes Sixt; +Cc: Pat Thoyts, msysgit, git, Junio C Hamano
In-Reply-To: <20101214220604.GA4084@sandbox>

The next patch implements a workaround in case unlink fails on Windows.

Signed-off-by: Heiko Voigt <hvoigt@hvoigt.net>
---
 compat/mingw.c |    8 ++++++++
 compat/mingw.h |   11 +++--------
 2 files changed, 11 insertions(+), 8 deletions(-)

diff --git a/compat/mingw.c b/compat/mingw.c
index bee6054..a7e1c6b 100644
--- a/compat/mingw.c
+++ b/compat/mingw.c
@@ -116,6 +116,14 @@ int err_win_to_posix(DWORD winerr)
 	return error;
 }
 
+#undef unlink
+int mingw_unlink(const char *pathname)
+{
+	/* read-only files cannot be removed */
+	chmod(pathname, 0666);
+	return unlink(pathname);
+}
+
 #undef open
 int mingw_open (const char *filename, int oflags, ...)
 {
diff --git a/compat/mingw.h b/compat/mingw.h
index 2283071..8316938 100644
--- a/compat/mingw.h
+++ b/compat/mingw.h
@@ -125,14 +125,6 @@ static inline int mingw_mkdir(const char *path, int mode)
 }
 #define mkdir mingw_mkdir
 
-static inline int mingw_unlink(const char *pathname)
-{
-	/* read-only files cannot be removed */
-	chmod(pathname, 0666);
-	return unlink(pathname);
-}
-#define unlink mingw_unlink
-
 #define WNOHANG 1
 pid_t waitpid(pid_t pid, int *status, unsigned options);
 
@@ -180,6 +172,9 @@ int link(const char *oldpath, const char *newpath);
  * replacements of existing functions
  */
 
+int mingw_unlink(const char *pathname);
+#define unlink mingw_unlink
+
 int mingw_open (const char *filename, int oflags, ...);
 #define open mingw_open
 
-- 
1.7.3.3.566.gf422f

^ permalink raw reply related

* [PATCH v3 0/5] make open/unlink failures user friendly on windows using retry/abort
From: Heiko Voigt @ 2010-12-14 22:06 UTC (permalink / raw)
  To: Johannes Sixt; +Cc: Pat Thoyts, msysgit, git, Junio C Hamano

Hi,

On Wed, Nov 10, 2010 at 09:32:00PM +0100, Johannes Sixt wrote:
> On Dienstag, 9. November 2010, Heiko Voigt wrote:
> > So it seems that unlink also has the problem of getting an
> > ERROR_ACCESS_DENIED (Code 5) sometimes. Johannes would you agree that
> > including this code into the is_file_in_use_error() function and thus
> > having the potential risk of a 71ms delay for real access denials for
> > these calls makes sense?
> 
> Of course, it matches my own observations.

Sorry for the delay. Here is finally a new iteration of the patch
series. I hope I have addressed all raised issues. An extra pair of eyes
is appreciated.

This series has been ported to Junios tree.

I also added one patch from Johannes which I think should be part of
this series.

Cheers Heiko

Heiko Voigt (4):
  mingw: move unlink wrapper to mingw.c
  mingw: work around irregular failures of unlink on windows
  mingw: make failures to unlink or move raise a question
  mingw: add fallback for rmdir in case directory is in use

Johannes Schindelin (1):
  mingw_rmdir: set errno=ENOTEMPTY when appropriate

 compat/mingw.c |  172 +++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 compat/mingw.h |   14 ++---
 2 files changed, 177 insertions(+), 9 deletions(-)

-- 
1.7.3.3.566.gf422f

^ permalink raw reply

* Re: Can `add --patch` display the diff in my difftool instead of just printing it on the command line?
From: Jeff King @ 2010-12-14 21:38 UTC (permalink / raw)
  To: Dun Peal; +Cc: Git ML
In-Reply-To: <AANLkTi=_VyUAL+qjWLEcThX-zC5n0v0WB-W7hTSZn0ae@mail.gmail.com>

On Tue, Dec 14, 2010 at 03:17:09PM -0600, Dun Peal wrote:

> I'd like to display the diff of each modified working copy file, and
> have the option of staging (or reversing) each patch after it is
> displayed. This is exactly the same thing `add --patch` does, except I
> want the diff to be displayed in my difftool (happens to be vimdiff)
> rather than just being printed.

You can't do this with "git add -p" now, but I suspect the patch would
be relatively straightforward. See patch_update_file in
git-add--interactive.perl. You just need to replace:

  for (@{$head->{DISPLAY}}) {
    print;
  }

with code to dump the diff in @{$head->{TEXT}} either to stdin of your
display program or to a tempfile that your program operates on.
Conditional on having some config option to specify your program, of
course.

Or did you want something more integrated with vimdiff? If you want to
primarily see the diff in vim and then have mappings set up to stage or
revert changes, that is pretty straightforward to do. Just have vim dump
the relevant hunk to "git apply --cached" or "git apply -R". I would be
surprised if the vim integration packages didn't have something like
this already.

-Peff

^ permalink raw reply

* Can `add --patch` display the diff in my difftool instead of just printing it on the command line?
From: Dun Peal @ 2010-12-14 21:17 UTC (permalink / raw)
  To: Git ML

Hi.

I'd like to display the diff of each modified working copy file, and
have the option of staging (or reversing) each patch after it is
displayed. This is exactly the same thing `add --patch` does, except I
want the diff to be displayed in my difftool (happens to be vimdiff)
rather than just being printed.

Thanks, D.

^ permalink raw reply

* [PATCH] git-p4: Fix 'p4 opened' in git-p4 for names with spaces
From: Jerzy Kozera @ 2010-12-14 20:56 UTC (permalink / raw)
  To: git; +Cc: Jerzy Kozera
In-Reply-To: <1292360165-26771-1-git-send-email-jerzy.kozera@gmail.com>

Signed-off-by: Jerzy Kozera <jerzy.kozera@gmail.com>
---
 contrib/fast-import/git-p4 |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/contrib/fast-import/git-p4 b/contrib/fast-import/git-p4
index 04ce7e3..a5297e7 100755
--- a/contrib/fast-import/git-p4
+++ b/contrib/fast-import/git-p4
@@ -144,7 +144,7 @@ def setP4ExecBit(file, mode):
 def getP4OpenedType(file):
     # Returns the perforce file type for the given file.
 
-    result = p4_read_pipe("opened %s" % file)
+    result = p4_read_pipe("opened \"%s\"" % file)
     match = re.match(".*\((.+)\)\r?$", result)
     if match:
         return match.group(1)
-- 
1.6.5.2

^ permalink raw reply related

* [PATCH] git-p4: Fix 'p4 opened' in git-p4 for names with spaces
From: Jerzy Kozera @ 2010-12-14 20:56 UTC (permalink / raw)
  To: git; +Cc: Jerzy Kozera

There is problem with git-p4 when trying to submit changes to file containing spaces in name - submit fails with "Command failed: p4 opened [name with spaces here]"

It's caused by not quoting name for p4 opened, and the patch attached fixes it.

Jerzy Kozera (1):
  Fix 'p4 opened' in git-p4 for names with spaces

 contrib/fast-import/git-p4 |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

^ permalink raw reply

* Re: [PATCH 13/14] t4135-*.sh: Skip the "backslash" tests on cygwin
From: Johannes Sixt @ 2010-12-14 20:49 UTC (permalink / raw)
  To: Ramsay Jones; +Cc: Junio C Hamano, GIT Mailing-list, jrnieder
In-Reply-To: <4D07B977.9010502@ramsay1.demon.co.uk>

On Dienstag, 14. Dezember 2010, Ramsay Jones wrote:
> Note t3700-*.sh has a test protected by BSLASHSPEC which
> previously passed on cygwin and will now be (unnecessarily)
> skipped. This test needs to be skipped on MinGW, given how
> it is written; if you remove the single quotes around the
> filename, however, it will pass even on MinGW.

That is suspicious. It would mean that git add does not do file globbing 
anymore. Should it or should it not do file globbing?

-- Hannes

^ permalink raw reply

* Re: [PATCH 08/14] help.c: Fix detection of custom merge strategy on cygwin
From: Johannes Sixt @ 2010-12-14 20:38 UTC (permalink / raw)
  To: Ramsay Jones; +Cc: Junio C Hamano, GIT Mailing-list, jrnieder, vmiklos
In-Reply-To: <4D07B786.2060602@ramsay1.demon.co.uk>

On Dienstag, 14. Dezember 2010, Ramsay Jones wrote:
> @@ -126,7 +126,10 @@ static int is_executable(const char *name)
>  	    !S_ISREG(st.st_mode))
>  		return 0;
>
> -#ifdef WIN32
> +#if defined(WIN32) || defined(__CYGWIN__)
> +#if defined(__CYGWIN__)
> +if ((st.st_mode & S_IXUSR) == 0)
> +#endif
>  {	/* cannot trust the executable bit, peek into the file instead */
>  	char buf[3] = { 0 };
>  	int n;

Do you gain a lot by this extra condition? Wouldn't

-#ifdef WIN32
+#if defined(WIN32) || defined(__CYGWIN__)

be sufficient?

-- Hannes

^ permalink raw reply

* Re: [PATCH 08/14] help.c: Fix detection of custom merge strategy on cygwin
From: Erik Faye-Lund @ 2010-12-14 20:07 UTC (permalink / raw)
  To: Ramsay Jones; +Cc: Junio C Hamano, GIT Mailing-list, j6t, jrnieder, vmiklos
In-Reply-To: <4D07B786.2060602@ramsay1.demon.co.uk>

On Tue, Dec 14, 2010 at 7:29 PM, Ramsay Jones
<ramsay@ramsay1.demon.co.uk> wrote:
>
> Test t7606-merge-custom.sh fails on cygwin when git-merge fails
> with an "Could not find merge strategy 'theirs'" error, despite
> the test correctly preparing an (executable) git-merge-theirs
> script.
>
> The cause of the failure is the mis-detection of the executable
> status of the script, by the is_executable() function, while the
> load_command_list() function is searching the path for additional
> merge strategy programs.
>
> Note that the l/stat() "functions" on cygwin are somewhat
> schizophrenic (see commits adbc0b6, 7faee6b and 7974843), and
> their behaviour depends on the timing of various git setup and
> config function calls. In particular, until the "git_dir" has
> been set (have_git_dir() returns true), the real cygwin (POSIX
> emulating) l/stat() functions are called. Once "git_dir" has
> been set, the "native Win32 API" implementations of l/stat()
> may, or may not, be called depending on the setting of the
> core.filemode and core.ignorecygwinfstricks config variables.
>
> We also note that, since commit c869753, core.filemode is forced
> to false, even on NTFS, by git-init and git-clone. A user (or a
> test) can, of course, reset core.filemode to true explicitly if
> the filesystem supports it (and he doesn't use any problematic
> windows software). The test-suite currently runs all tests on
> cygwin with core.filemode set to false.
>
> Given the above, we see that the built-in merge strategies are
> correctly detected as executable, since they are checked for
> before "git_dir" is set, whereas all custom merge strategies are
> not, since they are checked for after "git_dir" is set.
>
> In order to fix the mis-detection problem, we change the code in
> is_executable() to re-use the conditional WIN32 code section,
> which actually looks at the content of the file to determine if
> the file is executable. On cygwin we also make the additional
> code conditional on the executable bit of the file mode returned
> by the initial stat() call. (only the real cygwin function would
> set the executable bit in the file mode.)
>
> Signed-off-by: Ramsay Jones <ramsay@ramsay1.demon.co.uk>
> ---
>  help.c |    5 ++++-
>  1 files changed, 4 insertions(+), 1 deletions(-)
>
> diff --git a/help.c b/help.c
> index 7f4928e..eabadc9 100644
> --- a/help.c
> +++ b/help.c
> @@ -126,7 +126,10 @@ static int is_executable(const char *name)
>            !S_ISREG(st.st_mode))
>                return 0;
>
> -#ifdef WIN32
> +#if defined(WIN32) || defined(__CYGWIN__)
> +#if defined(__CYGWIN__)
> +if ((st.st_mode & S_IXUSR) == 0)
> +#endif

Perhaps the first check should simply be changed to check for _WIN32
instead of WIN32? IIRC _WIN32 is set on Cygwin, but I could be
mistaken...

^ permalink raw reply

* Re: [PATCH 12/14] t3032-*.sh: Do not strip CR from line-endings while grepping on MinGW
From: Junio C Hamano @ 2010-12-14 19:44 UTC (permalink / raw)
  To: Ramsay Jones; +Cc: GIT Mailing-list, patthoyts
In-Reply-To: <4D07B904.8040206@ramsay1.demon.co.uk>

Ramsay Jones <ramsay@ramsay1.demon.co.uk> writes:

> By default grep reads in text mode and converts CRLF into LF line
> endings, which causes tests 4, 6 and 8 to fail. In a similar manner
> to commit a94114ad  (Do not strip CR when grepping HTTP headers,
> 2010-09-12), we set (and export) the GREP_OPTIONS variable to -U so
> that grep will use binary mode.
>
> Signed-off-by: Ramsay Jones <ramsay@ramsay1.demon.co.uk>

Adding these GNU specific options leaves a bad aftertaste in my mouth, but
I'd pass them, as their use is limited to certain platforms with "prereq".

Thanks.

^ permalink raw reply

* Re: [PATCH 0/14] misc test-suite patches
From: Junio C Hamano @ 2010-12-14 19:44 UTC (permalink / raw)
  To: Ramsay Jones; +Cc: GIT Mailing-list
In-Reply-To: <4D07B4C1.2050007@ramsay1.demon.co.uk>

Ramsay Jones <ramsay@ramsay1.demon.co.uk> writes:

> I've had many of these test-suite related patches lying around for
> a while, so I'm passing these along now in the hope I can remove
> some old branches...

Thanks.

> [PATCH 08/14] help.c: Fix detection of custom merge strategy on cygwin
> [PATCH 09/14] t1301-*.sh: Fix the 'forced modes' test on cygwin

I sense that there is something fundamentally wrong with the tricky l/stat
emulation set-up code there, but I lack an expertise to judge if these two
patches is going in the right direction.  Help from people savvier on
Windows issues than I is very much appreciated.

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox