* 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/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/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
* 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: 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: 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
* 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: 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
* 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: Workflow for git dev
From: Frans Klaver @ 2012-02-02 15:25 UTC (permalink / raw)
To: Tom Michaud; +Cc: git
In-Reply-To: <CALDO3MKFdZ85w5uJEcZ6dkC7SNXxKi7BAb7r78ciFzmNdjo7eg@mail.gmail.com>
On Thu, Feb 2, 2012 at 3:59 PM, Tom Michaud <tom.michaud@gmail.com> wrote:
> Would someone kindly point me to a document that describes the
> workflow Junio et al use to develop git?
Two notable documents in git.git are Documentation/SubmittingPatches
and Documentation/CodingGuidelines.
Hope that helps.
Frans
^ permalink raw reply
* Re: rebase -i reword converts to pick on pre-commit non-zero exit
From: Neal Kreitzinger @ 2012-02-02 16:21 UTC (permalink / raw)
To: Andrew Wong; +Cc: git
In-Reply-To: <4F2A0D4B.6000001@sohovfx.com>
On 2/1/2012 10:12 PM, Andrew Wong wrote:
> On 12-02-01 4:28 PM, Neal Kreitzinger wrote:
>> Instead of picking commit (a) when the pre-commit hook exits
>> non-zero on the reword command, shouldn't interactive rebase learn
>> to edit commit (a) and tell the user that because the pre-commit
>> hook exited non-zero they need to either remedy the pre-commit hook
>> violations and run git commit --amend or run git commit --amend
>> --no-verify to bypass the pre-commit hook?
>
> Yup, I've submitted a patch to address this issue a while ago. This
> new behavior should be in v.1.7.8.2 and later.
I now see that this 1.7.8.2 release note applies:
* When a "reword" action in "git rebase -i" failed to run "commit
--amend",
we did not give the control back to the user to resolve the
situation, and
instead kept the original commit log message.
thanks!
v/r,
neal
^ permalink raw reply
* Re: rebase -i reword runs pre-commit hook with curious results
From: Neal Kreitzinger @ 2012-02-02 16:39 UTC (permalink / raw)
To: Andrew Wong; +Cc: Neal Kreitzinger, git
In-Reply-To: <4F2A2286.3090808@sohovfx.com>
On 2/1/2012 11:43 PM, Andrew Wong wrote:
> On 12-02-01 4:50 PM, Neal Kreitzinger wrote:
>> I'm confused on why and/or how interactive rebase runs the pre-commit
>> hook
>> when doing the reword command for commit (a).
> When you do a "reword" in "rebase -i", it basically does a "cherry-pick"
> of that commit first, then it does a "commit --amend". And your
> pre-commit hook should've been run during the amend.
>> IOW, the pre-commit hook does not get the same results as if I were
>> doing a
>> commandline git-commit of a modified index.
> Does your pre-commit hook work when doing a "commit --amend"? I'm not
> sure if you can actually modify the author (or committer) date from
> inside a pre-commit hook.
(We have a comment on "line 1" in our source with $User:$ $Date:$
keywords that the pre-commit hooks expands to insert "whoami" and "date"
values to effect a user-datestamp at commit time. We do this to enforce
conflicts on same-file edits.) Now that I understand that the
cherry-pick takes place first to effect the transfer of the tree content
and then a subsequent git-commit --amend of "no changes" takes place to
effect the reword opportunity, the behavior makes sense now. (We use
git-commit --amend to reword commit messages also.) The pre-commit hook
runs prior to commit message editor just like commandline git-commit
--amend (and plain git-commit).
thanks!
v/r,
neal
^ permalink raw reply
* Re: [PATCH] i18n: po for zh_cn
From: Ævar Arnfjörð Bjarmason @ 2012-02-02 16:42 UTC (permalink / raw)
To: Nguyen Thai Ngoc Duy; +Cc: Junio C Hamano, Jiang Xin, Git List
In-Reply-To: <CACsJy8BPTuS+u8Grx0ojhdX-5+Vn6=DuojSWO0or7fJE1dbAEQ@mail.gmail.com>
On Thu, Feb 2, 2012 at 14:29, Nguyen Thai Ngoc Duy <pclouds@gmail.com> wrote:
> 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.
For now we can also avoid this whole issue and just declare that
anything that's used for interchange or permanently stored
(git-format-patch output, reflogs, merge commit messages etc.) will be
in English.
^ permalink raw reply
* how to determine oldest supported version of git
From: Neal Kreitzinger @ 2012-02-02 16:46 UTC (permalink / raw)
To: git
What is the best way for me (a git user) to determine what is currently the
oldest supported version of git (the oldest version still getting bugfixes)?
IOW, when can I tell that my version of git is no longer supported?
v/r,
neal
^ permalink raw reply
* Re: I18N.pm is incompatible with perl < 5.8.3
From: Ævar Arnfjörð Bjarmason @ 2012-02-02 16:48 UTC (permalink / raw)
To: Tom G. Christensen; +Cc: git
In-Reply-To: <4F2A8B78.6090902@statsbiblioteket.dk>
On Thu, Feb 2, 2012 at 14:11, Tom G. Christensen
<tgc@statsbiblioteket.dk> wrote:
> Hello,
>
Thanks Tom, I'll submit a patch for that. Does this work, i.e. does
5.8.3 need *{import} = instead of *import = ?
diff --git a/perl/Git/I18N.pm b/perl/Git/I18N.pm
index 07597dc..5bcfed5 100644
--- a/perl/Git/I18N.pm
+++ b/perl/Git/I18N.pm
@@ -2,7 +2,16 @@ package Git::I18N;
use 5.008;
use strict;
use warnings;
-use Exporter 'import';
+BEGIN {
+ require Exporter;
+ if ($] < 5.008003) {
+ *import = \&Exporter::import;
+ } else {
+ # Exporter 5.57 supporting this invocation was released with
+ # 5.8.3
+ Exporter->import('import');
+ }
+}
our @EXPORT = qw(__);
our @EXPORT_OK = @EXPORT;
But actually it might be better to check $Exporter::VERSION
^ permalink raw reply
* how to tell when git release changes porcelain stdout/stderr
From: Neal Kreitzinger @ 2012-02-02 16:50 UTC (permalink / raw)
To: git
What is the best way for me (a git user) to tell when a new git release
changes the stdout/stderr formatting of a porcelain command?
v/r,
neal
^ permalink raw reply
* Re: Breakage in master?
From: Jeff King @ 2012-02-02 17:46 UTC (permalink / raw)
To: Erik Faye-Lund
Cc: Git Mailing List, msysGit, Ævar Arnfjörð Bjarmason
In-Reply-To: <CABPQNSbWu0r_gKGvCHk567pUtQiyDOCO8vFfrzPMFW1eUaj1nw@mail.gmail.com>
On Thu, Feb 02, 2012 at 01:14:19PM +0100, Erik Faye-Lund wrote:
> But here's the REALLY puzzling part: If I add a simple, unused
> function to diff-lib.c, like this:
> [...]
> "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<---
OK, that's definitely odd.
At the moment of the die() in strbuf_vaddf, what does errno say?
vsnprintf should generally never be returning -1 (it should return the
number of characters that would have been written). Since you're on
Windows, I assume you're using the replacement version in
compat/snprintf.c.
That one will return -1 if realloc fails. So I'm curious if that is what
is happening (you might also instrument the call to realloc in
snprintf.c to see if it is failing, and if so, at what maxsize). And/or
check errno in git_vsnprintf after calling the native vsnprintf and
getting -1.
Here's one possible sequence of events that seems plausible to me (and
remember that this is a wild guess):
1. gettext somehow munges the format string in a way that Windows
vsnprintf doesn't like, and it returns -1.
2. Our git_vsnprintf wrapper interprets this -1 as "you didn't give me
enough space to store the result", and we grow our test-buffer to
try again
3. Eventually the test buffer gets unreasonably large, and realloc
fails. We have no choice but to return -1 from our wrapper.
4. strbuf_vaddf sees the -1 and thinks you are using a broken
vsnprintf.
All of that would make sense to me, _except_ for your weird "if I add a
random function, the problem is more reproducible" bit. Which does seem
like something is invoking undefined behavior (of course, it could be
that undefined behavior or stack-smashing that is causing vsnprintf to
report an error). Lacking any better leads, it might be worth pursuing.
> 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've looked at 5e9637c, and it really doesn't do anything that looks
bad. I wonder if your gettext library is buggy. Does compiling with
NO_GETTEXT help?
-Peff
^ permalink raw reply
* Re: How best to handle multiple-authorship commits in GIT?
From: Valerie Aurora @ 2012-02-02 18:00 UTC (permalink / raw)
To: David Howells; +Cc: git@vger.kernel.org, dhowells@redhat.com
In-Reply-To: <21056.1328185509@redhat.com>
On Feb 2, 2012, at 4:25, 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.
>
> David
Thanks, David! I had the same trouble with my set: while I entirely rewrote some patches, I still felt Jan Blunck deserved primary credit. I don't recall my solution, but I'm fine with mentioning my name in the commit message (and I think Jan should get credit too).
In general, this is a big problem for motivating contributors in other cases. Some maintainers have a habit of trivially rewriting patches so that, technically, no line is the same, then taking authorship and giving the actual author an ambiguous Signed-off-by. David hasn't done this here, of course - these are major rewrites - but when someone does all the hard work of finding and fixing a problem, the credit shouldn't go to the person who prettied it up. There is a line in the kernel doc saying how this should be handled, suggested by Rusty, but it's not being followed.
First class support for multiple authorship would be a big way to motivate contributors.
-VAL
^ permalink raw reply
* Re: [PATCH v2] Use correct grammar in diffstat summary line
From: Junio C Hamano @ 2012-02-02 18:24 UTC (permalink / raw)
To: Nguyen Thai Ngoc Duy
Cc: git, Jonathan Nieder, Ævar Arnfjörð,
Frederik Schwarzer, Brandon Casey
In-Reply-To: <20120202142255.GA25871@do>
Nguyen Thai Ngoc Duy <pclouds@gmail.com> writes:
> 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,...
... because?
^ permalink raw reply
* Re: [PATCH 6/9] grep: cache userdiff_driver in grep_source
From: Junio C Hamano @ 2012-02-02 18:34 UTC (permalink / raw)
To: Jeff King
Cc: Thomas Rast, Conrad Irwin, git, Nguyen Thai Ngoc Duy,
Dov Grobgeld
In-Reply-To: <20120202082043.GF6786@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> - grep_attr_lock();
> - drv = userdiff_find_by_path(gs->name);
> - grep_attr_unlock();
> - if (drv && drv->funcname.pattern) {
> - const struct userdiff_funcname *pe = &drv->funcname;
> + grep_source_load_driver(gs);
> + if (gs->driver->funcname.pattern) {
> + const struct userdiff_funcname *pe = &gs->driver->funcname;
When we load driver, gs->driver gets at least "default" driver, so we no
longer need to check for drv != NULL as we used to? Is that the reason
for the slight difference here?
> @@ -1237,6 +1234,7 @@ void grep_source_init(struct grep_source *gs, enum grep_source_type type,
> gs->name = name ? xstrdup(name) : NULL;
> gs->buf = NULL;
> gs->size = 0;
> + gs->driver = NULL;
>
> switch (type) {
> case GREP_SOURCE_FILE:
^ permalink raw reply
* Re: How best to handle multiple-authorship commits in GIT?
From: David Howells @ 2012-02-02 18:36 UTC (permalink / raw)
To: Valerie Aurora; +Cc: dhowells, git@vger.kernel.org
In-Reply-To: <9B990DDC-858D-43BA-BF9E-E0C3435354AF@gmail.com>
Valerie Aurora <valerie.aurora@gmail.com> wrote:
> There is a line in the kernel doc saying how this should be handled,
> suggested by Rusty, but it's not being followed.
Do you know where?
David
^ permalink raw reply
* Re: [PATCH] vcs-svn: Fix some compiler warnings
From: Ramsay Jones @ 2012-02-02 18:24 UTC (permalink / raw)
To: Jonathan Nieder; +Cc: David Barr, Junio C Hamano, GIT Mailing-list
In-Reply-To: <20120131192053.GC12443@burratino>
Jonathan Nieder wrote:
> Ramsay Jones wrote:
>
>> In particular, some versions of gcc complains as follows:
>>
>> CC vcs-svn/sliding_window.o
>> vcs-svn/sliding_window.c: In function `check_overflow':
>> vcs-svn/sliding_window.c:36: warning: comparison is always false \
>> due to limited range of data type
>
> Yuck. Suppressing this warning would presumably also suppress the
> optimization that notices the comparison is always false.
I didn't check, but I would assume so. I would prefer not to loose any
chance at optimizing the code, but in this case, given that we are talking
about a fatal error path, I don't think it is much of a loss.
> The -Wtype-limits warning also triggers in some other perfectly
> reasonable situations: see <http://gcc.gnu.org/PR51712>.
[...]
>> Note that the "some versions of gcc" which complain includes 3.4.4 and
>> 4.1.2, whereas gcc version 4.4.0 compiles the code without complaint.
>
> Thanks for tracking this down. Interesting. -Wtype-limits was split
> out from the default set of warnings (!) in gcc 4.3 to address
> <http://gcc.gnu.org/PR12963>, among other bugs (r124875, 2007-05-20).
Thanks for the above references, and for taking the time to track them
down.
> Is there some less ugly way to write the condition "if this value is
> not representable in this type"?
>
> I guess I could live with something like the following (please don't
> take the names too seriously):
>
> static inline off_t off_t_or_die(uintmax_t val, const char *msg_if_bad)
> {
> if (val > maximum_signed_value_of_type(off_t))
> die("%s", msg_if_bad);
> return (off_t) val;
> }
>
> ...
>
> off_t delta_len = off_t_or_die(len, "enormous delta");
> postimage_len = apply_delta(delta_len, input, ...);
>
> What do you think?
An static inline function was actually my first thought (although I had
something more like Junio's suggestion [elsewhere in this thread] in mind),
but I didn't want to place it in git-compat-util.h and could not find a
suitable place in the vcs-svn directory.
Hmm, I will send a v2 patch along these lines ...
ATB,
Ramsay Jones
^ permalink raw reply
* Re: [PATCH] Fix an "variable might be used uninitialized" gcc warning
From: Ramsay Jones @ 2012-02-02 18:25 UTC (permalink / raw)
To: Jonathan Nieder; +Cc: Junio C Hamano, GIT Mailing-list
In-Reply-To: <20120131194302.GD12443@burratino>
Jonathan Nieder wrote:
> Sure, when the control flow grows too complicated, that's probably worth
> fixing anyway, for the sake of humans especially.
>
> Sometimes gcc is the only crazy one, though. ;-)
Indeed. :-D
ATB,
Ramsay Jones
^ permalink raw reply
* [PATCH v2] vcs-svn: Fix some compiler warnings
From: Ramsay Jones @ 2012-02-02 18:36 UTC (permalink / raw)
To: Jonathan Nieder; +Cc: Junio C Hamano, GIT Mailing-list
In particular, some versions of gcc complains as follows:
CC vcs-svn/sliding_window.o
vcs-svn/sliding_window.c: In function `check_overflow':
vcs-svn/sliding_window.c:36: warning: comparison is always false \
due to limited range of data type
CC vcs-svn/fast_export.o
vcs-svn/fast_export.c: In function `fast_export_blob_delta':
vcs-svn/fast_export.c:303: warning: comparison is always false due \
to limited range of data type
Simply casting the (limited range unsigned) variable in the comparison
to an uintmax_t does not suppress the warning, however, since gcc is
"smart" enough to know that the cast does not change anything regarding
the value of the casted expression. In order to suppress the warning, we
introduce a static inline function to hide the (implicit) cast of the
original variable to an uintmax_t type prior to using it in the overflow
check comparison expression.
Note that the "some versions of gcc" which complain includes 3.4.4 and
4.1.2, whereas gcc version 4.4.0 compiles the code without complaint.
Signed-off-by: Ramsay Jones <ramsay@ramsay1.demon.co.uk>
---
Hi Jonathan,
Any suggestions for a better name than "value_too_large_for_off_t" will
be greatly accepted! :)
Also, I suppose you could make a similar change to the other two sites
in vcs-svn with similar range checks (fast_export.c:174 and svndiff.c:150).
ATB,
Ramsay Jones
git-compat-util.h | 5 +++++
vcs-svn/fast_export.c | 2 +-
vcs-svn/sliding_window.c | 2 +-
3 files changed, 7 insertions(+), 2 deletions(-)
diff --git a/git-compat-util.h b/git-compat-util.h
index 8f3972c..6a6a25a 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -450,6 +450,11 @@ static inline size_t xsize_t(off_t len)
return (size_t)len;
}
+static inline int value_too_large_for_off_t(uintmax_t val)
+{
+ return val > maximum_signed_value_of_type(off_t);
+}
+
static inline int has_extension(const char *filename, const char *ext)
{
size_t len = strlen(filename);
diff --git a/vcs-svn/fast_export.c b/vcs-svn/fast_export.c
index 19d7c34..2e0dcb0 100644
--- a/vcs-svn/fast_export.c
+++ b/vcs-svn/fast_export.c
@@ -300,7 +300,7 @@ void fast_export_blob_delta(uint32_t mode,
uint32_t len, struct line_buffer *input)
{
long postimage_len;
- if (len > maximum_signed_value_of_type(off_t))
+ if (value_too_large_for_off_t(len))
die("enormous delta");
postimage_len = apply_delta((off_t) len, input, old_data, old_mode);
if (mode == REPO_MODE_LNK) {
diff --git a/vcs-svn/sliding_window.c b/vcs-svn/sliding_window.c
index 1bac7a4..0d38482 100644
--- a/vcs-svn/sliding_window.c
+++ b/vcs-svn/sliding_window.c
@@ -33,7 +33,7 @@ static int read_to_fill_or_whine(struct line_buffer *file,
static int check_overflow(off_t a, size_t b)
{
- if (b > maximum_signed_value_of_type(off_t))
+ if (value_too_large_for_off_t(b))
return error("unrepresentable length in delta: "
"%"PRIuMAX" > OFF_MAX", (uintmax_t) b);
if (signed_add_overflows(a, (off_t) b))
--
1.7.9
^ permalink raw reply related
* Re: [PATCH 0/9] respect binary attribute in grep
From: Junio C Hamano @ 2012-02-02 18:39 UTC (permalink / raw)
To: Jeff King
Cc: Thomas Rast, Conrad Irwin, git, Nguyen Thai Ngoc Duy,
Dov Grobgeld
In-Reply-To: <20120202081747.GA10271@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> ... The result turned out much easier to read
> (and explain in the commit messages, as it was simple to break into
> smaller commits)....
Indeed the series is very nicely done ;-)
Thanks.
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox