Git development
 help / color / mirror / Atom feed
* Re: Workflow for git dev
From: Thomas Rast @ 2012-02-02 15:16 UTC (permalink / raw)
  To: Tom Michaud; +Cc: git
In-Reply-To: <CALDO3MKFdZ85w5uJEcZ6dkC7SNXxKi7BAb7r78ciFzmNdjo7eg@mail.gmail.com>

Tom Michaud <tom.michaud@gmail.com> writes:

> Hi all,
>
> Would someone kindly point me to a document that describes the
> workflow Junio et al use to develop git?

Documentation/howto/maintain-git.txt

The gitworkflows(7) manpage is also inspired by how development works
around here, and the pretty well-known

  http://nvie.com/posts/a-successful-git-branching-model/

has some pictures of a very similar workflow that you can look at.

-- 
Thomas Rast
trast@{inf,student}.ethz.ch

^ permalink raw reply

* Re: How to find and analyze bad merges?
From: Neal Groothuis @ 2012-02-02 15:09 UTC (permalink / raw)
  To: norbert.nemec; +Cc: git
In-Reply-To: <jgduqg$p9f$1@dough.gmane.org>

>> See the thread [1] for a few relevant side-notes.
>  >
>  > [1] http://thread.gmane.org/gmane.comp.version-control.git/188904
>
> As I understand this thread, the user only requested all commits that
> "modify a file". Our merge-commit strictly speaking did not modify the
> file but simply kept one of the versions, completely swamping all
> modifications from one branch. Exactly the case that is still not
> covered by --full-history.

The thread was prompted by the difficulty I had in figuring out where a
co-worker had accidentally squashed changes in a branch that was being
merged in; I think that's the same issue that you have described.

Re: the merge: it kept one of the versions, but not the other; I would
consider that a change.  This is particularly problematic if you do a "git
log --full-history --simplify-merges".  The simplified history that is
presented will not show the merge, even though in the simplified history
the merge turns into a regular commit that differs from its parent.  It
seems that the history is being simplified to the point of being
inaccurate.

I believe that this is a result checking for TREESAME-ness before the
history simplification occurs, rather than after.  I would love to see
this behavior changed, or at the least, an option added to allow the user
to control it.

^ permalink raw reply

* Workflow for git dev
From: Tom Michaud @ 2012-02-02 14:59 UTC (permalink / raw)
  To: git

Hi all,

Would someone kindly point me to a document that describes the
workflow Junio et al use to develop git?

Much appreciated,
Tom

^ permalink raw reply

* Re: Finding all commits which modify a file
From: Neal Groothuis @ 2012-02-02 14:55 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

> "Neal Groothuis" <ngroot@lo-cal.org> writes:
>
>> Is there a situation where checking for TREESAMEness before
>> simplification
>> is desirable and checking after would not be?
>
> When you do not want to see a side branch that does not contribute to
the end result at all, obviously ;-). Outside that situation, before or
after should not make a difference, I would think.

In that case, you wouldn't be using the --full-history flag at all, yeah?

Right now, we can see where the file gets changed (A1), we just can't see
where it gets changed back (B2).  In fact, if I run git-log --full-history
--simplify-merges foo.txt, it looks like A1 was the last thing to make
changes to foo.txt, which seems misleading to me---history has been
simplified to the point of not being true.

^ permalink raw reply

* Re: [PATCH/RFC v4] grep: Add the option '--exclude'
From: Albert Yale @ 2012-02-02 14:41 UTC (permalink / raw)
  To: Nguyen Thai Ngoc Duy; +Cc: git, gitster
In-Reply-To: <CACsJy8CC+cwV+DEWPZVJqEtMq1Rd_RZtUTpMCeSmem5B_7vDMA@mail.gmail.com>

Thank you for this feedback Nguyen. I'll make the change to
git-grep.txt in my next revision.

Albert

On Thu, Feb 2, 2012 at 9:35 AM, Nguyen Thai Ngoc Duy <pclouds@gmail.com> wrote:
> On Thu, Feb 2, 2012 at 9:25 PM, Albert Yale <surfingalbert@gmail.com> wrote:
>> diff --git a/Documentation/git-grep.txt b/Documentation/git-grep.txt
>> index 6a8b1e3..b45706a 100644
>> --- a/Documentation/git-grep.txt
>> +++ b/Documentation/git-grep.txt
>> @@ -22,6 +22,7 @@ SYNOPSIS
>>           [--color[=<when>] | --no-color]
>>           [-A <post-context>] [-B <pre-context>] [-C <context>]
>>           [-f <file>] [-e] <pattern>
>> +          [-x <pattern>|--exclude <pattern>]
>>           [--and|--or|--not|(|)|-e <pattern>...]
>>           [ [--exclude-standard] [--cached | --no-index | --untracked] | <tree>...]
>>           [--] [<pathspec>...]
>
> I'd like to have a careful look at this but haven't found time/energy
> yet. So just a minor comment. Please use the term <pathspec> instead
> of <pattern>. Exclude pattern has complete different syntax than
> pathspec and --exclude followed by "pattern" may give wrong
> impression.
>
> I'd also avoid reuse "-x" if it's common for exclude patterns, but
> finding a new letter may be difficult. Keep in mind this feature in
> the long run benefits more commands than just git-grep and ideally
> they should all use the same letter for short option name.
> --
> Duy

^ permalink raw reply

* Re: [PATCH] i18n: diff/apply statistics
From: Nguyen Thai Ngoc Duy @ 2012-02-02 14:40 UTC (permalink / raw)
  To: Jiang Xin; +Cc: Git List, avarab
In-Reply-To: <1328116977-61458-1-git-send-email-worldhello.net@gmail.com>

On Thu, Feb 2, 2012 at 12:22 AM, Jiang Xin <worldhello.net@gmail.com> wrote:
> translate oneline statistics of diff/apply.

There's another patch with similar goal:

http://thread.gmane.org/gmane.comp.version-control.git/189453/focus=189509
-- 
Duy

^ permalink raw reply

* Re: [PATCH/RFC v4] grep: Add the option '--exclude'
From: Nguyen Thai Ngoc Duy @ 2012-02-02 14:35 UTC (permalink / raw)
  To: Albert Yale; +Cc: git, gitster
In-Reply-To: <1328192753-29162-1-git-send-email-surfingalbert@gmail.com>

On Thu, Feb 2, 2012 at 9:25 PM, Albert Yale <surfingalbert@gmail.com> wrote:
> diff --git a/Documentation/git-grep.txt b/Documentation/git-grep.txt
> index 6a8b1e3..b45706a 100644
> --- a/Documentation/git-grep.txt
> +++ b/Documentation/git-grep.txt
> @@ -22,6 +22,7 @@ SYNOPSIS
>           [--color[=<when>] | --no-color]
>           [-A <post-context>] [-B <pre-context>] [-C <context>]
>           [-f <file>] [-e] <pattern>
> +          [-x <pattern>|--exclude <pattern>]
>           [--and|--or|--not|(|)|-e <pattern>...]
>           [ [--exclude-standard] [--cached | --no-index | --untracked] | <tree>...]
>           [--] [<pathspec>...]

I'd like to have a careful look at this but haven't found time/energy
yet. So just a minor comment. Please use the term <pathspec> instead
of <pattern>. Exclude pattern has complete different syntax than
pathspec and --exclude followed by "pattern" may give wrong
impression.

I'd also avoid reuse "-x" if it's common for exclude patterns, but
finding a new letter may be difficult. Keep in mind this feature in
the long run benefits more commands than just git-grep and ideally
they should all use the same letter for short option name.
-- 
Duy

^ permalink raw reply

* [PATCH/RFC v4] grep: Add the option '--exclude'
From: Albert Yale @ 2012-02-02 14:25 UTC (permalink / raw)
  To: git; +Cc: gitster, pclouds, Albert Yale

grep: Add the option '--exclude'

Signed-off-by: Albert Yale <surfingalbert@gmail.com>
---
hi Junio,

You're correct that adding an extra parameter to
match_pathspec_depth() was unnecessary.

I added a "struct pathspec_set" as you suggested
in your previous review. It had the side effect
of forcing me to update a few more files than was
previously necessary.

I understand that you want to keep the internals
of a pathspec hidden from its users as much as
possible. But if someone is already accessing its
internals, I don't think it's a problem to change
their usage, for example, from "ps->nr" to
"ps->include.nr". Please correct me if I'm wrong.

There's one thing I did differently. You suggested
moving "recursive" into "struct pathspec_set". I
kept it in "struct pathspec" because it's usually
modified alongside "max_depth".

btw, the last two parameters of
match_pathspec_depth(), namely "prefix" and
"seen", are not used anywhere. I understand their
potential usefulness, but I wanted to point this
out in case you want to further simplify calls to
this function.

Looking forward to your feedback,

Albert Yale

 Documentation/git-grep.txt |    5 ++
 builtin/diff.c             |    6 +-
 builtin/fast-export.c      |    2 +-
 builtin/grep.c             |   31 +++++++++++++-
 builtin/log.c              |    2 +-
 builtin/ls-files.c         |    2 +-
 builtin/ls-tree.c          |    6 +-
 cache.h                    |   15 +++++--
 dir.c                      |  100 +++++++++++++++++++++++++++++++++++---------
 list-objects.c             |    2 +-
 revision.c                 |    4 +-
 tree-diff.c                |    2 +-
 tree-walk.c                |    8 ++--
 13 files changed, 143 insertions(+), 42 deletions(-)

diff --git a/Documentation/git-grep.txt b/Documentation/git-grep.txt
index 6a8b1e3..b45706a 100644
--- a/Documentation/git-grep.txt
+++ b/Documentation/git-grep.txt
@@ -22,6 +22,7 @@ SYNOPSIS
 	   [--color[=<when>] | --no-color]
 	   [-A <post-context>] [-B <pre-context>] [-C <context>]
 	   [-f <file>] [-e] <pattern>
+	   [-x <pattern>|--exclude <pattern>]
 	   [--and|--or|--not|(|)|-e <pattern>...]
 	   [ [--exclude-standard] [--cached | --no-index | --untracked] | <tree>...]
 	   [--] [<pathspec>...]
@@ -124,6 +125,10 @@ OPTIONS
 	Use fixed strings for patterns (don't interpret pattern
 	as a regex).
 
+-x <pattern>::
+--exclude <pattern>::
+	Skip files matching pattern.
+
 -n::
 --line-number::
 	Prefix the line number to matching lines.
diff --git a/builtin/diff.c b/builtin/diff.c
index 387afa7..e910f80 100644
--- a/builtin/diff.c
+++ b/builtin/diff.c
@@ -365,10 +365,10 @@ int cmd_diff(int argc, const char **argv, const char *prefix)
 		}
 		die(_("unhandled object '%s' given."), name);
 	}
-	if (rev.prune_data.nr) {
+	if (rev.prune_data.include.nr) {
 		if (!path)
-			path = rev.prune_data.items[0].match;
-		paths += rev.prune_data.nr;
+			path = rev.prune_data.include.items[0].match;
+		paths += rev.prune_data.include.nr;
 	}
 
 	/*
diff --git a/builtin/fast-export.c b/builtin/fast-export.c
index 08fed98..23043a2 100644
--- a/builtin/fast-export.c
+++ b/builtin/fast-export.c
@@ -674,7 +674,7 @@ int cmd_fast_export(int argc, const char **argv, const char *prefix)
 	if (import_filename)
 		import_marks(import_filename);
 
-	if (import_filename && revs.prune_data.nr)
+	if (import_filename && revs.prune_data.include.nr)
 		full_tree = 1;
 
 	get_tags_and_duplicates(&revs.pending, &extra_refs);
diff --git a/builtin/grep.c b/builtin/grep.c
index 5c2ae94..19d1afd 100644
--- a/builtin/grep.c
+++ b/builtin/grep.c
@@ -566,6 +566,10 @@ static int grep_tree(struct grep_opt *opt, const struct pathspec *pathspec,
 	while (tree_entry(tree, &entry)) {
 		int te_len = tree_entry_len(&entry);
 
+		if (!match_pathspec_depth(pathspec,
+					  entry.path, strlen(entry.path),
+					  0, NULL))
+			continue;
 		if (match != all_entries_interesting) {
 			match = tree_entry_interesting(&entry, base, tn_len, pathspec);
 			if (match == all_entries_not_interesting)
@@ -606,8 +610,16 @@ static int grep_tree(struct grep_opt *opt, const struct pathspec *pathspec,
 static int grep_object(struct grep_opt *opt, const struct pathspec *pathspec,
 		       struct object *obj, const char *name)
 {
-	if (obj->type == OBJ_BLOB)
+	if (obj->type == OBJ_BLOB) {
+		const char *name_without_sha1 = strchr(name, ':') + 1;
+		if (!match_pathspec_depth(pathspec,
+					  name_without_sha1,
+					  strlen(name_without_sha1),
+					  0, NULL))
+			return 0;
+
 		return grep_sha1(opt, obj->sha1, name, 0);
+	}
 	if (obj->type == OBJ_COMMIT || obj->type == OBJ_TREE) {
 		struct tree_desc tree;
 		void *data;
@@ -764,6 +776,14 @@ static int pattern_callback(const struct option *opt, const char *arg,
 	return 0;
 }
 
+static int exclude_cb(const struct option *opt, const char *arg,
+		      int unset)
+{
+	struct string_list *exclude_list = opt->value;
+	string_list_append(exclude_list, arg);
+	return 0;
+}
+
 static int help_callback(const struct option *opt, const char *arg, int unset)
 {
 	return -1;
@@ -792,6 +812,7 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
 		pattern_type_pcre,
 	};
 	int pattern_type = pattern_type_unspecified;
+	struct string_list exclude_list = STRING_LIST_INIT_NODUP;
 
 	struct option options[] = {
 		OPT_BOOLEAN(0, "cached", &cached,
@@ -872,6 +893,8 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
 			"read patterns from file", file_callback),
 		{ OPTION_CALLBACK, 'e', NULL, &opt, "pattern",
 			"match <pattern>", PARSE_OPT_NONEG, pattern_callback },
+		{ OPTION_CALLBACK, 'x', "exclude", &exclude_list, "pattern",
+		  "add <pattern> to ignore rules", PARSE_OPT_NONEG, exclude_cb },
 		{ OPTION_CALLBACK, 0, "and", &opt, NULL,
 		  "combine patterns specified with -e",
 		  PARSE_OPT_NOARG | PARSE_OPT_NONEG, and_callback },
@@ -1054,6 +1077,10 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
 	pathspec.max_depth = opt.max_depth;
 	pathspec.recursive = 1;
 
+	if (exclude_list.nr) {
+		init_pathspec_exclude_from_string_list(&pathspec, &exclude_list);
+	}
+
 	if (show_in_pager && (cached || list.nr))
 		die(_("--open-files-in-pager only works on the worktree"));
 
@@ -1103,5 +1130,7 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
 	if (hit && show_in_pager)
 		run_pager(&opt, prefix);
 	free_grep_patterns(&opt);
+	free_pathspec(&pathspec);
+	string_list_clear(&exclude_list, 0);
 	return !hit;
 }
diff --git a/builtin/log.c b/builtin/log.c
index 7d1f6f8..c837616 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -125,7 +125,7 @@ static void cmd_log_init_finish(int argc, const char **argv, const char *prefix,
 		rev->always_show_header = 0;
 	if (DIFF_OPT_TST(&rev->diffopt, FOLLOW_RENAMES)) {
 		rev->always_show_header = 0;
-		if (rev->diffopt.pathspec.nr != 1)
+		if (rev->diffopt.pathspec.include.nr != 1)
 			usage("git logs can only follow renames on one pathname at a time");
 	}
 
diff --git a/builtin/ls-files.c b/builtin/ls-files.c
index 7cff175..8cae770 100644
--- a/builtin/ls-files.c
+++ b/builtin/ls-files.c
@@ -326,7 +326,7 @@ void overlay_tree_on_cache(const char *tree_name, const char *prefix)
 		matchbuf[0] = prefix;
 		matchbuf[1] = NULL;
 		init_pathspec(&pathspec, matchbuf);
-		pathspec.items[0].use_wildcard = 0;
+		pathspec.include.items[0].use_wildcard = 0;
 	} else
 		init_pathspec(&pathspec, NULL);
 	if (read_tree(tree, 1, &pathspec))
diff --git a/builtin/ls-tree.c b/builtin/ls-tree.c
index 6b666e1..651169c 100644
--- a/builtin/ls-tree.c
+++ b/builtin/ls-tree.c
@@ -167,9 +167,9 @@ int cmd_ls_tree(int argc, const char **argv, const char *prefix)
 		die("Not a valid object name %s", argv[0]);
 
 	init_pathspec(&pathspec, get_pathspec(prefix, argv + 1));
-	for (i = 0; i < pathspec.nr; i++)
-		pathspec.items[i].use_wildcard = 0;
-	pathspec.has_wildcard = 0;
+	for (i = 0; i < pathspec.include.nr; i++)
+		pathspec.include.items[i].use_wildcard = 0;
+	pathspec.include.has_wildcard = 0;
 	tree = parse_tree_indirect(sha1);
 	if (!tree)
 		die("not a tree object");
diff --git a/cache.h b/cache.h
index 9bd8c2d..ea70b92 100644
--- a/cache.h
+++ b/cache.h
@@ -522,12 +522,9 @@ extern int index_name_is_other(const struct index_state *, const char *, int);
 extern int ie_match_stat(const struct index_state *, struct cache_entry *, struct stat *, unsigned int);
 extern int ie_modified(const struct index_state *, struct cache_entry *, struct stat *, unsigned int);
 
-struct pathspec {
-	const char **raw; /* get_pathspec() result, not freed by free_pathspec() */
+struct pathspec_set {
 	int nr;
 	unsigned int has_wildcard:1;
-	unsigned int recursive:1;
-	int max_depth;
 	struct pathspec_item {
 		const char *match;
 		int len;
@@ -535,7 +532,17 @@ struct pathspec {
 	} *items;
 };
 
+struct pathspec {
+	const char **raw; /* get_pathspec() result, not freed by free_pathspec() */
+	unsigned int recursive:1;
+	int max_depth;
+
+	struct pathspec_set include;
+	struct pathspec_set exclude;
+};
+
 extern int init_pathspec(struct pathspec *, const char **);
+extern int init_pathspec_exclude_from_string_list(struct pathspec *, const struct string_list *);
 extern void free_pathspec(struct pathspec *);
 extern int ce_path_match(const struct cache_entry *ce, const struct pathspec *pathspec);
 
diff --git a/dir.c b/dir.c
index 0a78d00..faf2db3 100644
--- a/dir.c
+++ b/dir.c
@@ -9,6 +9,8 @@
 #include "dir.h"
 #include "refs.h"
 
+#include "string-list.h"
+
 struct path_simplify {
 	int len;
 	const char *path;
@@ -251,17 +253,18 @@ static int match_pathspec_item(const struct pathspec_item *item, int prefix,
  * and a mark is left in seen[] array for pathspec element that
  * actually matched anything.
  */
-int match_pathspec_depth(const struct pathspec *ps,
-			 const char *name, int namelen,
-			 int prefix, char *seen)
+static int match_pathspec_set_depth(const struct pathspec_set *ps,
+				    unsigned int recursive, int max_depth,
+				    const char *name, int namelen,
+				    int prefix, char *seen)
 {
 	int i, retval = 0;
 
 	if (!ps->nr) {
-		if (!ps->recursive || ps->max_depth == -1)
+		if (!recursive || max_depth == -1)
 			return MATCHED_RECURSIVELY;
 
-		if (within_depth(name, namelen, 0, ps->max_depth))
+		if (within_depth(name, namelen, 0, max_depth))
 			return MATCHED_EXACTLY;
 		else
 			return 0;
@@ -275,12 +278,12 @@ int match_pathspec_depth(const struct pathspec *ps,
 		if (seen && seen[i] == MATCHED_EXACTLY)
 			continue;
 		how = match_pathspec_item(ps->items+i, prefix, name, namelen);
-		if (ps->recursive && ps->max_depth != -1 &&
+		if (recursive && max_depth != -1 &&
 		    how && how != MATCHED_FNMATCH) {
 			int len = ps->items[i].len;
 			if (name[len] == '/')
 				len++;
-			if (within_depth(name+len, namelen-len, 0, ps->max_depth))
+			if (within_depth(name+len, namelen-len, 0, max_depth))
 				how = MATCHED_EXACTLY;
 			else
 				how = 0;
@@ -295,6 +298,25 @@ int match_pathspec_depth(const struct pathspec *ps,
 	return retval;
 }
 
+int match_pathspec_depth(const struct pathspec *ps,
+			 const char *name, int namelen,
+			 int prefix, char *seen)
+{
+	int retval = match_pathspec_set_depth(&ps->include,
+					      ps->recursive, ps->max_depth,
+					      name, namelen, prefix, seen);
+
+	if (retval && ps->exclude.nr)
+	{
+		if (match_pathspec_set_depth(&ps->exclude,
+					     ps->recursive, ps->max_depth,
+					     name, namelen, prefix, seen))
+			return 0;
+	}
+
+	return retval;
+}
+
 static int no_wildcard(const char *string)
 {
 	return string[strcspn(string, "*?[{\\")] == '\0';
@@ -1259,9 +1281,44 @@ static int pathspec_item_cmp(const void *a_, const void *b_)
 	return strcmp(a->match, b->match);
 }
 
+static int init_pathspec_item(struct pathspec_item *item, const char *path)
+{
+	item->match = path;
+	item->len = strlen(path);
+	item->use_wildcard = !no_wildcard(path);
+
+	return item->use_wildcard;
+}
+
+int init_pathspec_exclude_from_string_list(struct pathspec *pathspec,
+					   const struct string_list *path_list)
+{
+	struct pathspec_set *exclude = &pathspec->exclude;
+	int i;
+
+	if (!path_list->nr)
+		return 0;
+
+	exclude->nr = path_list->nr;
+	exclude->items = xcalloc(path_list->nr, sizeof(struct pathspec_item));
+
+	for (i = 0; i < path_list->nr; i++) {
+		struct pathspec_item *item = exclude->items+i;
+		const char *path = path_list->items[i].string;
+
+		exclude->has_wildcard |= init_pathspec_item(item, path);
+	}
+
+	qsort(exclude->items, exclude->nr,
+	      sizeof(struct pathspec_item), pathspec_item_cmp);
+
+	return 0;
+}
+
 int init_pathspec(struct pathspec *pathspec, const char **paths)
 {
 	const char **p = paths;
+	struct pathspec_set *include = &pathspec->include;
 	int i;
 
 	memset(pathspec, 0, sizeof(*pathspec));
@@ -1270,23 +1327,20 @@ int init_pathspec(struct pathspec *pathspec, const char **paths)
 	while (*p)
 		p++;
 	pathspec->raw = paths;
-	pathspec->nr = p - paths;
-	if (!pathspec->nr)
+
+	include->nr = p - paths;
+	if (!include->nr)
 		return 0;
 
-	pathspec->items = xmalloc(sizeof(struct pathspec_item)*pathspec->nr);
-	for (i = 0; i < pathspec->nr; i++) {
-		struct pathspec_item *item = pathspec->items+i;
+	include->items = xmalloc(sizeof(struct pathspec_item)*include->nr);
+	for (i = 0; i < include->nr; i++) {
+		struct pathspec_item *item = include->items+i;
 		const char *path = paths[i];
 
-		item->match = path;
-		item->len = strlen(path);
-		item->use_wildcard = !no_wildcard(path);
-		if (item->use_wildcard)
-			pathspec->has_wildcard = 1;
+		include->has_wildcard |= init_pathspec_item(item, path);
 	}
 
-	qsort(pathspec->items, pathspec->nr,
+	qsort(include->items, include->nr,
 	      sizeof(struct pathspec_item), pathspec_item_cmp);
 
 	return 0;
@@ -1294,6 +1348,12 @@ int init_pathspec(struct pathspec *pathspec, const char **paths)
 
 void free_pathspec(struct pathspec *pathspec)
 {
-	free(pathspec->items);
-	pathspec->items = NULL;
+	if (pathspec->include.items) {
+		free(pathspec->include.items);
+		pathspec->include.items = NULL;
+	}
+	if (pathspec->exclude.items) {
+		free(pathspec->exclude.items);
+		pathspec->exclude.items = NULL;
+	}
 }
diff --git a/list-objects.c b/list-objects.c
index 3dd4a96..cdac52d 100644
--- a/list-objects.c
+++ b/list-objects.c
@@ -71,7 +71,7 @@ static void process_tree(struct rev_info *revs,
 	struct tree_desc desc;
 	struct name_entry entry;
 	struct name_path me;
-	enum interesting match = revs->diffopt.pathspec.nr == 0 ?
+	enum interesting match = revs->diffopt.pathspec.include.nr == 0 ?
 		all_entries_interesting: entry_not_interesting;
 	int baselen = base->len;
 
diff --git a/revision.c b/revision.c
index c97d834..a516df4 100644
--- a/revision.c
+++ b/revision.c
@@ -389,7 +389,7 @@ static int rev_compare_tree(struct rev_info *revs, struct commit *parent, struct
 		 * tagged commit by specifying both --simplify-by-decoration
 		 * and pathspec.
 		 */
-		if (!revs->prune_data.nr)
+		if (!revs->prune_data.include.nr)
 			return REV_TREE_SAME;
 	}
 
@@ -1835,7 +1835,7 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct s
 	if (revs->topo_order)
 		revs->limited = 1;
 
-	if (revs->prune_data.nr) {
+	if (revs->prune_data.include.nr) {
 		diff_tree_setup_paths(revs->prune_data.raw, &revs->pruning);
 		/* Can't prune commits with rename following: the paths change.. */
 		if (!DIFF_OPT_TST(&revs->diffopt, FOLLOW_RENAMES))
diff --git a/tree-diff.c b/tree-diff.c
index 28ad6db..43aa00f 100644
--- a/tree-diff.c
+++ b/tree-diff.c
@@ -146,7 +146,7 @@ int diff_tree(struct tree_desc *t1, struct tree_desc *t2,
 	for (;;) {
 		if (diff_can_quit_early(opt))
 			break;
-		if (opt->pathspec.nr) {
+		if (opt->pathspec.include.nr) {
 			skip_uninteresting(t1, &base, opt, &t1_match);
 			skip_uninteresting(t2, &base, opt, &t2_match);
 		}
diff --git a/tree-walk.c b/tree-walk.c
index 492c7cd..e95cd6a 100644
--- a/tree-walk.c
+++ b/tree-walk.c
@@ -584,10 +584,10 @@ enum interesting tree_entry_interesting(const struct name_entry *entry,
 {
 	int i;
 	int pathlen, baselen = base->len - base_offset;
-	int never_interesting = ps->has_wildcard ?
+	int never_interesting = ps->include.has_wildcard ?
 		entry_not_interesting : all_entries_not_interesting;
 
-	if (!ps->nr) {
+	if (!ps->include.nr) {
 		if (!ps->recursive || ps->max_depth == -1)
 			return all_entries_interesting;
 		return within_depth(base->buf + base_offset, baselen,
@@ -598,8 +598,8 @@ enum interesting tree_entry_interesting(const struct name_entry *entry,
 
 	pathlen = tree_entry_len(entry);
 
-	for (i = ps->nr - 1; i >= 0; i--) {
-		const struct pathspec_item *item = ps->items+i;
+	for (i = ps->include.nr - 1; i >= 0; i--) {
+		const struct pathspec_item *item = ps->include.items+i;
 		const char *match = item->match;
 		const char *base_str = base->buf + base_offset;
 		int matchlen = item->len;
-- 
1.7.8.3

^ permalink raw reply related

* Re: [PATCH v2] Use correct grammar in diffstat summary line
From: Nguyen Thai Ngoc Duy @ 2012-02-02 14:22 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: git, Jonathan Nieder, Ævar Arnfjörð,
	Frederik Schwarzer, Brandon Casey
In-Reply-To: <7vhaza2qjw.fsf@alter.siamese.dyndns.org>

On Wed, Feb 01, 2012 at 01:26:43PM -0800, Junio C Hamano wrote:
> Nice.  Will queue

Please also squash this in (resend looks ugly and it's hard to point
out changes). It makes the code look less ugly, use Q_() for gettext
poisoning and revert am input text back as Jonathan suggested.

I take it --summary is un-i18n-able, should we introduce.. umm..
--nice-summary or something that can support i18n?

-- 8< --
diff --git a/diff.c b/diff.c
index 5f3ce97..07c94f2 100644
--- a/diff.c
+++ b/diff.c
@@ -1325,6 +1325,7 @@ static void fill_print_name(struct diffstat_file *file)
 int print_stat_summary(FILE *fp, int files, int insertions, int deletions)
 {
 	struct strbuf sb = STRBUF_INIT;
+	const char *fmt;
 	int ret;
 
 	if (!files) {
@@ -1332,10 +1333,8 @@ int print_stat_summary(FILE *fp, int files, int insertions, int deletions)
 		return fputs(_(" 0 files changed\n"), fp);
 	}
 
-	strbuf_addf(&sb,
-		    ngettext(" %d file changed", " %d files changed",
-			     files),
-		    files);
+	fmt = Q_(" %d file changed", " %d files changed", files);
+	strbuf_addf(&sb, fmt, files);
 
 	/*
 	 * For binary diff, the caller may want to print "x files
@@ -1346,25 +1345,17 @@ int print_stat_summary(FILE *fp, int files, int insertions, int deletions)
 	 * but nothing about added/removed lines? Is this a bug in Git?").
 	 */
 	if (insertions || deletions == 0) {
-		strbuf_addf(&sb,
-			    /*
-			     * TRANSLATORS: "+" in (+) is a line addition marker;
-			     * do not translate it.
-			     */
-			    ngettext(", %d insertion(+)", ", %d insertions(+)",
-				     insertions),
-			    insertions);
+		/* TRANSLATORS: "+" in (+) is a line addition marker,
+		   do not translate it */
+		fmt = Q_(", %d insertion(+)", ", %d insertions(+)", insertions);
+		strbuf_addf(&sb, fmt, insertions);
 	}
 
 	if (deletions || insertions == 0) {
-		strbuf_addf(&sb,
-			    /*
-			     * TRANSLATORS: "-" in (-) is a line removal marker;
-			     * do not translate it.
-			     */
-			    ngettext(", %d deletion(-)", ", %d deletions(-)",
-				     deletions),
-			    deletions);
+		/* TRANSLATORS: "-" in (-) is a line removal marker,
+		   do not translate it */
+		fmt = Q_(", %d deletion(-)", ", %d deletions(-)", deletions);
+		strbuf_addf(&sb, fmt, deletions);
 	}
 	strbuf_addch(&sb, '\n');
 	ret = fputs(sb.buf, fp);
diff --git a/t/t0023-crlf-am.sh b/t/t0023-crlf-am.sh
index 18fe27b..12a3d78 100755
--- a/t/t0023-crlf-am.sh
+++ b/t/t0023-crlf-am.sh
@@ -12,7 +12,7 @@ Subject: test1
 
 ---
  foo |    1 +
- 1 file changed, 1 insertion(+)
+ 1 files changed, 1 insertions(+)
  create mode 100644 foo
 
 diff --git a/foo b/foo
-- 8< --

-- 
Duy

^ permalink raw reply related

* [PATCH v4 2/2] find_pack_entry(): do not keep packed_git pointer locally
From: Nguyễn Thái Ngọc Duy @ 2012-02-02 13:53 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Nicolas Pitre,
	Nguyễn Thái Ngọc Duy
In-Reply-To: <alpine.LFD.2.02.1202011100010.2759@xanadu.home>

Commit f7c22cc (always start looking up objects in the last used pack
first - 2007-05-30) introduce a static packed_git* pointer as an
optimization.  The kept pointer however may become invalid if
free_pack_by_name() happens to free that particular pack.

Current code base does not access packs after calling
free_pack_by_name() so it should not be a problem. Anyway, move the
pointer out so that free_pack_by_name() can reset it to avoid running
into troubles in future.

Thanks to Junio for code layout suggestions.

Acked-by: Nicolas Pitre <nico@fluxnic.net>
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 Credit where credit is due. No code changes from pu.

 sha1_file.c |   27 +++++++++++++--------------
 1 files changed, 13 insertions(+), 14 deletions(-)

diff --git a/sha1_file.c b/sha1_file.c
index 61e51ed..6b1b512 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -54,6 +54,8 @@ static struct cached_object empty_tree = {
 	0
 };
 
+static struct packed_git *last_found_pack;
+
 static struct cached_object *find_cached_object(const unsigned char *sha1)
 {
 	int i;
@@ -720,6 +722,8 @@ void free_pack_by_name(const char *pack_name)
 			close_pack_index(p);
 			free(p->bad_object_sha1);
 			*pp = p->next;
+			if (last_found_pack == p)
+				last_found_pack = NULL;
 			free(p);
 			return;
 		}
@@ -2046,27 +2050,22 @@ static int fill_pack_entry(const unsigned char *sha1,
 
 static int find_pack_entry(const unsigned char *sha1, struct pack_entry *e)
 {
-	static struct packed_git *last_found = (void *)1;
 	struct packed_git *p;
 
 	prepare_packed_git();
 	if (!packed_git)
 		return 0;
-	p = (last_found == (void *)1) ? packed_git : last_found;
 
-	do {
-		if (fill_pack_entry(sha1, e, p)) {
-			last_found = p;
-			return 1;
-		}
+	if (last_found_pack && fill_pack_entry(sha1, e, last_found_pack))
+		return 1;
 
-		if (p == last_found)
-			p = packed_git;
-		else
-			p = p->next;
-		if (p == last_found)
-			p = p->next;
-	} while (p);
+	for (p = packed_git; p; p = p->next) {
+		if (p == last_found_pack || !fill_pack_entry(sha1, e, p))
+			continue;
+
+		last_found_pack = p;
+		return 1;
+	}
 	return 0;
 }
 
-- 
1.7.8.36.g69ee2

^ permalink raw reply related

* Re: How best to handle multiple-authorship commits in GIT?
From: Frans Klaver @ 2012-02-02 13:41 UTC (permalink / raw)
  To: David Howells; +Cc: git, valerie.aurora
In-Reply-To: <21056.1328185509@redhat.com>

Hi,

On Thu, Feb 2, 2012 at 1:25 PM, David Howells <dhowells@redhat.com> wrote:
>
> Hi,
>
> I've been assigned a stack of patches to maintain and try and get upstream by
> my employer.  Most of the patches currently have the authorship set to Val,
> but since I'll be maintaining them if they go in upstream and I've changed
> them a lot, I feel I should reassign the author field to myself so people
> pester me rather than Val with questions about them.  However, I don't want to
> deny Val or any other contributor credit for their work on the patches.
>
> I can see a number of ways of doing this, and am wondering which will be best:
>
>  (1) Ascribe multiple authorship directly in the commit.  I suspect this would
>     require a change to GIT and its associated tools.  That way I could put my
>     name in the priority pestering spot, but doing a search on authorship
>     would still credit Val and others.
>
>  (2) Add an extra tag 'Originally-authored-by' (or maybe 'Coauthored-by' as I
>     saw someone recommend) in amongst the 'Signed-off-by' list.  But that
>     doesn't give them credit in a gitweb search without changing gitweb.
>
>  (3) Don't actually modify Val's commits to bring them up to date, but rather
>     create a historical GIT tree with Val's commits committed as-are and then
>     add my changes to the top in a number of large merge commits (there have
>     been multiple major breakages due to different merge windows).
>
>     I dislike this approach because it doesn't produce a nice set of patches I
>     can give to someone to review (which is a must).  Plus, for the most part,
>     it's actually easier to port Val's patches individually.
>
> Can GIT be modified to do (1)?  Gitweb's display need only show one of the
> authors in the single-row-per-patch list mode, but should find a patch by any
> of the authors in an author search and should display all the authors in the
> commit display.

I always thought of the author field as being an indication of who is
ultimately responsible for its implementation (the one in the
pestering spot). (1) may seem desirous, but doesn't (2) seem like a
cleaner and more maintainable solution? Gitweb will show the entire
log message if people are interested in the exact change, right?

Cheers,
Frans

^ permalink raw reply

* Re: [PATCH] i18n: po for zh_cn
From: Nguyen Thai Ngoc Duy @ 2012-02-02 13:29 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Jiang Xin, Git List, Ævar Arnfjörð Bjarmason
In-Reply-To: <7vr4ye15kr.fsf@alter.siamese.dyndns.org>

On Thu, Feb 2, 2012 at 6:45 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Jiang Xin <worldhello.net@gmail.com> writes:
>
>> Git can speak Chinese now.
>>
>> Signed-off-by: Jiang Xin <worldhello.net@gmail.com>
>> ---
>>  po/zh_cn.po | 3568 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>  1 个文件被修改, 3568 处添加(+), 0 处删除(-)
>>  create mode 100644 po/zh_cn.po
>
> I do not mind Chinese in the patch text (i.e. below), but I would have
> preferred the above not to be in Chinese, which I do not read---I can
> guess what 文件, 添加 and 删除 are, and I can also guess that 个 and 处
> are units of counting, but nevertheless...

Such a stat line would be wonderful in an all-Chinese environment
though. I'm thinking perhaps it's a good idea to support
core.officialLocale (or workingLanguage). Commands that produce stuff
for outside like format-patch would prefer core.exchangeLanguage over
$LANG. Commands in blurred zone can learn --official option to ignore
$LANG.

We can then have a shared config with core.officialLocale =
en_US.UTF-8 somewhere in git.git. Developers of multi-nation companies
would be pleased, I think.
-- 
Duy

^ permalink raw reply

* I18N.pm is incompatible with perl < 5.8.3
From: Tom G. Christensen @ 2012-02-02 13:11 UTC (permalink / raw)
  To: git

Hello,

While running the git 1.7.9 testsuite on RHEL 3 with perl 5.8.0 and 
gettext 0.11.4 I got this error in t0202-gettext-perl.sh:
# test_external test Perl Git::I18N API failed: /usr/bin/perl 
/builddir/build/BUILD/git-1.7.9/t/t0202/test.pl
# test_external_without_stderr test no stderr: Perl Git::I18N API 
failed: /usr/bin/perl /builddir/build/BUILD/git-1.7.9/t/t0202/test.pl:

A verbose run gave me this:
# test_external test Perl Git::I18N API failed: /usr/bin/perl 
/builddir/build/BUILD/git-1.7.9/t/t0202/test.pl
# expecting no stderr from previous command
# test_external_without_stderr test no stderr: Perl Git::I18N API 
failed: /usr/bin/perl /builddir/build/BUILD/git-1.7.9/t/t0202/test.pl:
# Stderr is:
"import" is not exported by the Exporter module
Can't continue after import errors at 
/builddir/build/BUILD/git-1.7.9/t/../perl/blib/lib/Git/I18N.pm line 5
BEGIN failed--compilation aborted at 
/builddir/build/BUILD/git-1.7.9/t/../perl/blib/lib/Git/I18N.pm line 5.
Compilation failed in require at 
/builddir/build/BUILD/git-1.7.9/t/t0202/test.pl line 8.
BEGIN failed--compilation aborted at 
/builddir/build/BUILD/git-1.7.9/t/t0202/test.pl line 8.
# Looks like your test died before it could output anything.

I found the cause and the solution here:
http://www.nntp.perl.org/group/perl.module.build/2008/02/msg1214.html

I've changed
  use Exporter 'import'
to
  BEGIN {
   require Exporter;
   *{import} = \&Exporter::import;
   }
in I18N.pm.

The test now passes (GETTEXT_LOCALE=1):
# lib-gettext: No is_IS UTF-8 locale available
# lib-gettext: No is_IS ISO-8859-1 locale available
# run 1: Perl Git::I18N API (/usr/bin/perl 
/builddir/build/BUILD/git-1.7.9/t/t0202/test.pl)
1..8
ok 1 - Testing Git::I18N with NO Perl gettext library
ok 2 - Git::I18N is located at 
/builddir/build/BUILD/git-1.7.9/t/../perl/blib/lib/Git/I18N.pm
ok 3 - sanity: Git::I18N has 1 export(s)
ok 4 - sanity: Git::I18N exports everything by default
ok 5 - sanity: __ has a $ prototype
ok 6 - Passing a string through __() in the C locale works
ok 7 - Without a gettext library + <C> locale <TEST: A Perl test string> 
turns into <TEST: A Perl test string>
ok 8 - Without a gettext library + <is> locale <TEST: A Perl test 
string> turns into <TEST: A Perl test string>
# test_external test Perl Git::I18N API was ok
# expecting no stderr from previous command
# test_external_without_stderr test no stderr: Perl Git::I18N API was ok

-tgc

^ permalink raw reply

* How best to handle multiple-authorship commits in GIT?
From: David Howells @ 2012-02-02 12:25 UTC (permalink / raw)
  To: git; +Cc: dhowells, valerie.aurora


Hi,

I've been assigned a stack of patches to maintain and try and get upstream by
my employer.  Most of the patches currently have the authorship set to Val,
but since I'll be maintaining them if they go in upstream and I've changed
them a lot, I feel I should reassign the author field to myself so people
pester me rather than Val with questions about them.  However, I don't want to
deny Val or any other contributor credit for their work on the patches.

I can see a number of ways of doing this, and am wondering which will be best:

 (1) Ascribe multiple authorship directly in the commit.  I suspect this would
     require a change to GIT and its associated tools.  That way I could put my
     name in the priority pestering spot, but doing a search on authorship
     would still credit Val and others.

 (2) Add an extra tag 'Originally-authored-by' (or maybe 'Coauthored-by' as I
     saw someone recommend) in amongst the 'Signed-off-by' list.  But that
     doesn't give them credit in a gitweb search without changing gitweb.

 (3) Don't actually modify Val's commits to bring them up to date, but rather
     create a historical GIT tree with Val's commits committed as-are and then
     add my changes to the top in a number of large merge commits (there have
     been multiple major breakages due to different merge windows).

     I dislike this approach because it doesn't produce a nice set of patches I
     can give to someone to review (which is a must).  Plus, for the most part,
     it's actually easier to port Val's patches individually.

Can GIT be modified to do (1)?  Gitweb's display need only show one of the
authors in the single-row-per-patch list mode, but should find a patch by any
of the authors in an author search and should display all the authors in the
commit display.

David

^ permalink raw reply

* Re: git-svn branches with revision id's in name
From: Carsten Fuchs @ 2012-02-02 12:24 UTC (permalink / raw)
  To: Stephen Duncan Jr; +Cc: git
In-Reply-To: <CAGYrzvwzrsZdHHnSBaMv-sD9mDGVQ-qFuks+JM6d-NQ9Wz9KwQ@mail.gmail.com>

Hi all,

Am 2012-01-30 20:42, schrieb Stephen Duncan Jr:
> [...]
> $ git branch -a
> * master
>    remotes/develop
>    remotes/develop@29271
>    remotes/develop@32463
>    remotes/develop@34103
>    remotes/feature/xyz
>    remotes/feature/xyz@26438
>    remotes/feature/xyz@27542
>    remotes/feature/xyz@35233
>
> Why have these remote branches been created?  What impact does this
> have on my checkout?  Can I remove safely remove them?  How?  I was
> unable to figure out how to reference this behavior in order to search
> for information on it.

Same questions here.
(I know how to remove them, but I'd love to learn and understand why they have been 
created.)

Any info would much be appreciated!

Many thanks and best regards,
Carsten



-- 
    Cafu - the open-source Game and Graphics Engine
for multiplayer, cross-platform, real-time 3D Action
           Learn more at http://www.cafu.de

^ permalink raw reply

* Re: How to find and analyze bad merges?
From: norbert.nemec @ 2012-02-02 12:16 UTC (permalink / raw)
  To: git
In-Reply-To: <20120202120340.GA25190@burratino>

Am 02.02.12 13:03, schrieb Jonathan Nieder:
> David Barr wrote:
>
>> Do the -c --cc or -m flags for git log help in this case?
>> They alter the way merge diffs are presented, as described under Diff Formatting
>> in the git-log(1)  man page.
>
> I suspect Norbert was running into history simplification, so the --full-history
> flag would be the relevant one.

Not quite.

As far as I understand it, history simplification hides the whole branch 
if its changes did not end up in the current branch.

When I tried it out, the --full-history prevented hiding the 
bugfix-commit itself, but it did not show the critical merge commit in 
the log.

> See the thread [1] for a few relevant side-notes.
 >
 > [1] http://thread.gmane.org/gmane.comp.version-control.git/188904

As I understand this thread, the user only requested all commits that 
"modify a file". Our merge-commit strictly speaking did not modify the 
file but simply kept one of the versions, completely swamping all 
modifications from one branch. Exactly the case that is still not 
covered by --full-history.

^ permalink raw reply

* Breakage in master?
From: Erik Faye-Lund @ 2012-02-02 12:14 UTC (permalink / raw)
  To: Git Mailing List, msysGit, Ævar Arnfjörð Bjarmason

Something strange is going on in Junio's current 'master' branch
(f3fb075). "git show" has started to error out on Windows with a
complaint about our vsnprintf:
---8<---

$ git show
commit f3fb07509c2e0b21b12a598fcd0a19a92fc38a9d
Author: Junio C Hamano <gitster@pobox.com>
Date:   Tue Jan 31 22:31:35 2012 -0800

    Update draft release notes to 1.7.10

    Signed-off-by: Junio C Hamano <gitster@pobox.com>

fatal: BUG: your vsnprintf is broken (returned -1)
---8<---

"git status" is also behaving strange:
---8<---
$ git status
# On branch master
# Untracked files:
#   (use "git add <file>..." to include in what will be committed)
#
#       ←[31mcompat/vcbuild/include/sys/resource.h←[m
#       ←[31mtemp.patch←[m
#       ←[31mtest.c←[m
#       ←[31mtest.patch←[m
nothing added to commit but untracked files present (use "git add" to track)
---8<---

Yeah, the ANSI color codes are being printed verbatim, even though
compat/winansi.c is supposed to convert these. "git -p status" works
fine, as it pipes the ANSI codes directly through less.

But here's the REALLY puzzling part: If I add a simple, unused
function to diff-lib.c, like this:

---8<---
diff --git a/diff-lib.c b/diff-lib.c
index fc0dff3..914a224 100644
--- a/diff-lib.c
+++ b/diff-lib.c
@@ -82,6 +82,11 @@ static int match_stat_with_submodule(struct
diff_options *diffopt,
 	return changed;
 }

+static unsigned int foo(const char *a, unsigned int b)
+{
+	return b;
+}
+
 int run_diff_files(struct rev_info *revs, unsigned int option)
 {
 	int entries, i;
---8<---

"git status" starts to error out with that same vsnprintf complaint!

---8<---
$ git status
# On branch master
# Changes not staged for commit:
#   (use "git add <file>..." to update what will be committed)
fatal: BUG: your vsnprintf is broken (returned -1)
---8<---

Here's the stack-trace:
---8<---
Breakpoint 1, die (err=0x5268ec "BUG: your vsnprintf is broken (returned %d)")
    at usage.c:81
81      void NORETURN die(const char *err, ...)
(gdb) bt
#0  die (err=0x5268ec "BUG: your vsnprintf is broken (returned %d)")
    at usage.c:81
#1  0x00466314 in strbuf_vaddf (sb=0x28fb54,
    fmt=0x533ef4 "  (use \"git checkout -- <file>...\" to discard changes in wor
king directory)", ap=0x28fbac "+\025\026\002") at strbuf.c:221
#2  0x004cbb6f in status_vprintf (s=0x28fc38, at_bol=0,
    color=0x46c4f2 "\205└t\n\203─\020[^╔├\215v",
    fmt=0x533ef4 "  (use \"git checkout -- <file>...\" to discard changes in wor
king directory)", ap=0x28fbac "+\025\026\002", trail=0x533a89 "\n")
    at wt-status.c:44
#3  0x004cbe6b in status_printf_ln (s=0x28fc38, color=0x28fc70 "",
    fmt=0x533ef4 "  (use \"git checkout -- <file>...\" to discard changes in wor
king directory)") at wt-status.c:87
#4  0x004cced1 in wt_status_print (s=0x28fc38) at wt-status.c:176
#5  0x0041922e in cmd_status (argc=1, argv=0x319b4, prefix=0x0)
    at builtin/commit.c:1254
#6  0x004019d6 in handle_internal_command (argc=<value optimized out>,
    argv=<value optimized out>) at git.c:308
#7  0x00401c26 in main (argc=2, argv=0x319b0) at git.c:513
(gdb)
---8<---

This smells a bit like a smashed stack to me. Both issues happens in
roughly the same area of the call stack, and when adding an unused
function changes behavior, something really odd is going on ;)

I've bisected the issues down to 5e9637c (i18n: add infrastructure for
translating Git with gettext). Trying to apply my unused-function
patch on top of this commit starts giving the same "fatal: BUG: your
vsnprintf is broken (returned -1)" error. It's ancestor, bc1bbe0(Git
1.7.8-rc2), does not yield any of the issues.

I'm at a loss here. Does anyone have a hunch about what's going on?

^ permalink raw reply related

* Re: How to find and analyze bad merges?
From: norbert.nemec @ 2012-02-02 12:10 UTC (permalink / raw)
  To: git
In-Reply-To: <CAFfmPPMc1V97OPHyrZp+p4YUek1c6fCncyj0s1YU9xjxQBCsDA@mail.gmail.com>

Am 02.02.12 12:41, schrieb David Barr:
> On Thu, Feb 2, 2012 at 10:17 PM, Norbert Nemec
> <norbert.nemec@native-instruments.de>  wrote:
>> To be yet more precise:
>>
>> My complaint is that you need this kind of sledge-hammer solutions to
>> analyze the situation. I, as an semi-expert with git did manage to find the
>> problem without even having to resort to bisect or manually redoing the
>> merge. My complaint is about the perspective of the medium-experienced user
>> who is completely puzzled by the fact that a
>> "git log<filename>" silently skips the critical merge commit.
>
> Do the -c --cc or -m flags for git log help in this case?
> They alter the way merge diffs are presented, as described under Diff Formatting
> in the git-log(1)  man page.

Indeed, these help somewhat. This way, the changes are not hidden, but 
instead lost in the multitude of trivially-resolved conflicts...

^ permalink raw reply

* Re: How to find and analyze bad merges?
From: Jonathan Nieder @ 2012-02-02 12:03 UTC (permalink / raw)
  To: David Barr; +Cc: Norbert Nemec, Thomas Rast, git
In-Reply-To: <CAFfmPPMc1V97OPHyrZp+p4YUek1c6fCncyj0s1YU9xjxQBCsDA@mail.gmail.com>

David Barr wrote:

> Do the -c --cc or -m flags for git log help in this case?
> They alter the way merge diffs are presented, as described under Diff Formatting
> in the git-log(1)  man page.

I suspect Norbert was running into history simplification, so the --full-history
flag would be the relevant one.

See the thread [1] for a few relevant side-notes.

[1] http://thread.gmane.org/gmane.comp.version-control.git/188904

^ permalink raw reply

* Re: How to find and analyze bad merges?
From: David Barr @ 2012-02-02 11:41 UTC (permalink / raw)
  To: Norbert Nemec; +Cc: Thomas Rast, git
In-Reply-To: <4F2A70DA.6020107@native-instruments.de>

On Thu, Feb 2, 2012 at 10:17 PM, Norbert Nemec
<norbert.nemec@native-instruments.de> wrote:
> To be yet more precise:
>
> My complaint is that you need this kind of sledge-hammer solutions to
> analyze the situation. I, as an semi-expert with git did manage to find the
> problem without even having to resort to bisect or manually redoing the
> merge. My complaint is about the perspective of the medium-experienced user
> who is completely puzzled by the fact that a
> "git log <filename>" silently skips the critical merge commit.

Do the -c --cc or -m flags for git log help in this case?
They alter the way merge diffs are presented, as described under Diff Formatting
in the git-log(1)  man page.
--
David Barr

^ permalink raw reply

* Re: [PATCH 1/3] vcs-svn: rename check_overflow arguments for clarity
From: Jonathan Nieder @ 2012-02-02 11:27 UTC (permalink / raw)
  To: David Barr; +Cc: Dmitry Ivankov, Junio C Hamano, Ramsay Jones, GIT Mailing-list
In-Reply-To: <CAFfmPPOeFk871m_N+nLXgQx3Uj4wVhgR9BNFzM2ggtseop0JaA@mail.gmail.com>

David Barr wrote:

> Maybe rename to check_offset_overflow to make it explicit?

Ok, here's the patch I'll squash in. ;-)

diff --git i/vcs-svn/sliding_window.c w/vcs-svn/sliding_window.c
index 2f4ae60f..ec2707c9 100644
--- i/vcs-svn/sliding_window.c
+++ w/vcs-svn/sliding_window.c
@@ -31,7 +31,7 @@ static int read_to_fill_or_whine(struct line_buffer *file,
 	return 0;
 }
 
-static int check_overflow(off_t offset, uintmax_t len)
+static int check_offset_overflow(off_t offset, uintmax_t len)
 {
 	if (len > maximum_signed_value_of_type(off_t))
 		return error("unrepresentable length in delta: "
@@ -48,9 +48,9 @@ int move_window(struct sliding_view *view, off_t off, size_t width)
 	off_t file_offset;
 	assert(view);
 	assert(view->width <= view->buf.len);
-	assert(!check_overflow(view->off, view->buf.len));
+	assert(!check_offset_overflow(view->off, view->buf.len));
 
-	if (check_overflow(off, width))
+	if (check_offset_overflow(off, width))
 		return -1;
 	if (off < view->off || off + width < view->off + view->width)
 		return error("invalid delta: window slides left");

^ permalink raw reply related

* Re: [PATCH 1/3] vcs-svn: rename check_overflow arguments for clarity
From: David Barr @ 2012-02-02 11:25 UTC (permalink / raw)
  To: Jonathan Nieder
  Cc: Dmitry Ivankov, Junio C Hamano, Ramsay Jones, GIT Mailing-list
In-Reply-To: <20120202111628.GN3823@burratino>

On Thu, Feb 2, 2012 at 10:16 PM, Jonathan Nieder <jrnieder@gmail.com> wrote:
> Dmitry Ivankov wrote:
>> On Thu, Feb 2, 2012 at 4:59 PM, Jonathan Nieder <jrnieder@gmail.com> wrote:
>
>>> From: Ramsay Jones <ramsay@ramsay1.demon.co.uk>
>>>
>>> Code using the argument names a and b just doesn't look right (not
>>> sure why!).  Use more explicit names "offset" and "len" to make their
>>> type and function clearer.
>>
>> Well, it's still not clear. Given off_t a, size_t b, check that a+b
>> fits into type... which type?
>> "offset" and "length" don't imply that it's "type of offset" or maybe
>> "type of length".
>
> Hmm... in vector arithmetic, position (i.e., file offset) + displacement
> (i.e., size of chunk) = position (i.e., new file offset).  Any ideas
> for making this clearer?

Maybe rename to check_offset_overflow to make it explicit?
--
David Barr

^ permalink raw reply

* Re: How to find and analyze bad merges?
From: Norbert Nemec @ 2012-02-02 11:17 UTC (permalink / raw)
  To: Thomas Rast; +Cc: git
In-Reply-To: <87haz97c2k.fsf@thomas.inf.ethz.ch>

To be yet more precise:

My complaint is that you need this kind of sledge-hammer solutions to 
analyze the situation. I, as an semi-expert with git did manage to find 
the problem without even having to resort to bisect or manually redoing 
the merge. My complaint is about the perspective of the 
medium-experienced user who is completely puzzled by the fact that a
"git log <filename>" silently skips the critical merge commit.




Am 02.02.12 11:40, schrieb Thomas Rast:
> "norbert.nemec"<norbert.nemec@native-instruments.de>  writes:
>
>> Thinking about a possible solution:
>>
>> Is there a way to re-do a merge-commit and diff the result against the
>> recorded merge without touching the working tree? This would be the
>> killer-feature to analyze a recorded merge-commit.
>
>    git checkout M^
>    git merge M^2
>    git diff M HEAD
>
> You'd have to resolve conflicts though.  If you want to skip that, I
> think you could still see some information if you said
>
>    git reset
>    git diff M
>
> to see the differences between the (unmerged, with conflict hunks) state
> in the worktree and M.
>
> (Remember to re-attach your HEAD after playing around like this.)
>
>> Am 02.02.12 09:16, schrieb Junio C Hamano:
>>>
>>> Bisect?
>>
>> This is not the point: My colleague knew exactly which commit
>> contained the bugfix. The trouble was finding out why this bugfix
>> disappeared even though everything indicated that it was cleanly
>> merged into the current branch.
>
> But that makes it a prime candidate for bisect: you know the good commit
> (the original bugfix), and you know that the newest version is bad.
> Bonus points if you have an automated test for it, in which case bisect
> can nail the offender while you get coffee.
>
> Or am I missing something?
>

-- 
Dr. Norbert Nemec
Teamleader Software Development

Tel +49-30-611035-1882
norbert.nemec@native-instruments.de

KOMPLETE 8 ULTIMATE - the premium NI producer collection
=>  http://www.native-instruments.com/komplete8

TRAKTOR KONTROL S2 - the professional 2.1 DJ system
=>  http://www.native-instruments.com/s2

->>>>>> NATIVE INSTRUMENTS - The Future of Sound <<<<<<-

Registergericht: Amtsgericht Charlottenburg
Registernummer: HRB 72458
UST.-ID.-Nr. DE 20 374 7747

Geschäftsführung: Daniel Haver (CEO), Mate Galic

^ permalink raw reply

* Re: [PATCH 1/3] vcs-svn: rename check_overflow arguments for clarity
From: Jonathan Nieder @ 2012-02-02 11:16 UTC (permalink / raw)
  To: Dmitry Ivankov; +Cc: Junio C Hamano, Ramsay Jones, David Barr, GIT Mailing-list
In-Reply-To: <CA+gfSn9Exv3T0UCB-bFShmSvRCMgygVuWraiToR6ZjgOA_sZ8A@mail.gmail.com>

Dmitry Ivankov wrote:
> On Thu, Feb 2, 2012 at 4:59 PM, Jonathan Nieder <jrnieder@gmail.com> wrote:

>> From: Ramsay Jones <ramsay@ramsay1.demon.co.uk>
>>
>> Code using the argument names a and b just doesn't look right (not
>> sure why!).  Use more explicit names "offset" and "len" to make their
>> type and function clearer.
>
> Well, it's still not clear. Given off_t a, size_t b, check that a+b
> fits into type... which type?
> "offset" and "length" don't imply that it's "type of offset" or maybe
> "type of length".

Hmm... in vector arithmetic, position (i.e., file offset) + displacement
(i.e., size of chunk) = position (i.e., new file offset).  Any ideas
for making this clearer?

^ permalink raw reply

* Re: [PATCH 0/9] respect binary attribute in grep
From: Jeff King @ 2012-02-02 11:07 UTC (permalink / raw)
  To: Thomas Rast
  Cc: Junio C Hamano, Thomas Rast, Conrad Irwin, git,
	Nguyen Thai Ngoc Duy, Dov Grobgeld
In-Reply-To: <87vcnp5wkg.fsf@thomas.inf.ethz.ch>

On Thu, Feb 02, 2012 at 12:00:47PM +0100, Thomas Rast wrote:

> My original plan was to make use_threads git-global, instead of
> grep-global (and shift responsibility to the subsystems instead of their
> users), but that's just me and the patches aren't ready yet.

Yeah, having just dug into the threading code in grep a bit, I agree
that would be a saner approach. The locking is all bolted-on, so you end
up with these weird contracts between code, like the low-level grep code
asking anybody who might be multi-threading it to initialize the mutexes
to cover access to a totally different subsystem. I'd much rather each
subsystem just take care of itself.

-Peff

^ 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